Linux-Fsdevel Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Avi Kivity <avi@scylladb.com>
To: Goldwyn Rodrigues <rgoldwyn@suse.de>
Cc: linux-fsdevel@vger.kernel.org, linux-aio@kvack.org
Subject: Re: [PATCH] fs: Return EOPNOTSUPP if block layer does not support REQ_NOWAIT
Date: Tue, 28 Jul 2020 16:47:45 +0300	[thread overview]
Message-ID: <20a90311-0c3a-7055-d227-cd394dbb590b@scylladb.com> (raw)
In-Reply-To: <20200728133817.lurap7lucjx7q7bw@fiona>


On 28/07/2020 16.38, Goldwyn Rodrigues wrote:
> On 19:08 22/07, Avi Kivity wrote:
>> On 13/12/2018 13.53, Goldwyn Rodrigues wrote:
>>> For AIO+DIO with RWF_NOWAIT, if the block layer does not support REQ_NOWAIT,
>>> it returns EIO. Return EOPNOTSUPP to represent the correct error code.
>>>
>>> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
>>> ---
>>>    fs/direct-io.c | 11 +++++++----
>>>    1 file changed, 7 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/fs/direct-io.c b/fs/direct-io.c
>>> index 41a0e97252ae..77adf33916b8 100644
>>> --- a/fs/direct-io.c
>>> +++ b/fs/direct-io.c
>>> @@ -542,10 +542,13 @@ static blk_status_t dio_bio_complete(struct dio *dio, struct bio *bio)
>>>    	blk_status_t err = bio->bi_status;
>>>    	if (err) {
>>> -		if (err == BLK_STS_AGAIN && (bio->bi_opf & REQ_NOWAIT))
>>> -			dio->io_error = -EAGAIN;
>>> -		else
>>> -			dio->io_error = -EIO;
>>> +		dio->io_error = -EIO;
>>> +		if (bio->bi_opf & REQ_NOWAIT) {
>>> +			if (err == BLK_STS_AGAIN)
>>> +				dio->io_error = -EAGAIN;
>>> +			else if (err == BLK_STS_NOTSUPP)
>>> +				dio->io_error = -EOPNOTSUPP;
>>> +		}
>>>    	}
>>>    	if (dio->is_async && dio->op == REQ_OP_READ && dio->should_dirty) {
>>
>> In the end, did this or some alternative get applied? I'd like to enable
>> RWF_NOWAIT support, but EIO scares me and my application.
>>
> No, it was not. There were lot of objections to return error from the
> block layer for a filesystem nowait request.
>

I see. For me, it makes RWF_NOWAIT unusable, since I have no way to 
distinguish between real EIO and EIO due to this bug.


Maybe the filesystem should ask the block device if it supports nowait 
ahead of time (during mounting), and not pass REQ_NOWAIT at all in those 
cases.


  reply	other threads:[~2020-07-28 13:47 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-13 11:53 Goldwyn Rodrigues
2018-12-13 12:04 ` Avi Kivity
2018-12-13 14:24   ` Christoph Hellwig
2018-12-13 15:44     ` Goldwyn Rodrigues
2018-12-16 10:45     ` Avi Kivity
2018-12-17 17:38       ` Christoph Hellwig
2018-12-18 11:55         ` Goldwyn Rodrigues
2018-12-20 15:32           ` Avi Kivity
2018-12-13 16:27 ` Matthew Wilcox
2018-12-13 19:04   ` Goldwyn Rodrigues
2018-12-13 22:43 ` Dave Chinner
2018-12-14 17:09   ` Goldwyn Rodrigues
2018-12-16 21:35     ` Dave Chinner
2018-12-18 11:53       ` Goldwyn Rodrigues
2020-07-22 16:08 ` Avi Kivity
2020-07-28 13:38   ` Goldwyn Rodrigues
2020-07-28 13:47     ` Avi Kivity [this message]
2020-07-31 13:11   ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20a90311-0c3a-7055-d227-cd394dbb590b@scylladb.com \
    --to=avi@scylladb.com \
    --cc=linux-aio@kvack.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=rgoldwyn@suse.de \
    --subject='Re: [PATCH] fs: Return EOPNOTSUPP if block layer does not support REQ_NOWAIT' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).