2018-12-09 11:21:51

by Avi Kivity

[permalink] [raw]
Subject: Spurious EIO on AIO+DIO+RWF_NOWAIT

I have an application that receives spurious EIO when running with
RWF_NOWAIT enabled. Removing RWF_NOWAIT causes those EIOs to disappear.
The application uses AIO+DIO, and errors were seen on both xfs and ext4.


I suspect the following code:


/*
 * Process one completed BIO.  No locks are held.
 */
static blk_status_t dio_bio_complete(struct dio *dio, struct bio *bio)
{
        struct bio_vec *bvec;
        unsigned i;
        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;
        }

Could it be that REQ_NOWAIT was dropped from bio->bi_opf? or that
bio->bi_status got changed along the way?


Reducing the test case may be a lot of work, but I can package a binary
using docker if a reproducer is needed. Seen on 4.18.19 and 4.19.something.



2018-12-10 13:24:48

by Goldwyn Rodrigues

[permalink] [raw]
Subject: Re: Spurious EIO on AIO+DIO+RWF_NOWAIT

On 13:19 09/12, Avi Kivity wrote:
> I have an application that receives spurious EIO when running with
> RWF_NOWAIT enabled. Removing RWF_NOWAIT causes those EIOs to disappear. The
> application uses AIO+DIO, and errors were seen on both xfs and ext4.
>
>
> I suspect the following code:
>
>
> /*
> ?* Process one completed BIO.? No locks are held.
> ?*/
> static blk_status_t dio_bio_complete(struct dio *dio, struct bio *bio)
> {
> ??????? struct bio_vec *bvec;
> ??????? unsigned i;
> ??????? 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;
> ??????? }
>
> Could it be that REQ_NOWAIT was dropped from bio->bi_opf? or that
> bio->bi_status got changed along the way?
>

I don't think REQ_NOWAIT is dropped. I am assuming bio->bi_status error
is set differently. Is the blk queue being stopped? Is it possible to
instrument the kernel in your testcase?

--
Goldwyn

2018-12-10 13:25:10

by Avi Kivity

[permalink] [raw]
Subject: Re: Spurious EIO on AIO+DIO+RWF_NOWAIT


On 10/12/2018 14.48, Goldwyn Rodrigues wrote:
> On 13:19 09/12, Avi Kivity wrote:
>> I have an application that receives spurious EIO when running with
>> RWF_NOWAIT enabled. Removing RWF_NOWAIT causes those EIOs to disappear. The
>> application uses AIO+DIO, and errors were seen on both xfs and ext4.
>>
>>
>> I suspect the following code:
>>
>>
>> /*
>>  * Process one completed BIO.  No locks are held.
>>  */
>> static blk_status_t dio_bio_complete(struct dio *dio, struct bio *bio)
>> {
>>         struct bio_vec *bvec;
>>         unsigned i;
>>         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;
>>         }
>>
>> Could it be that REQ_NOWAIT was dropped from bio->bi_opf? or that
>> bio->bi_status got changed along the way?
>>
> I don't think REQ_NOWAIT is dropped. I am assuming bio->bi_status error
> is set differently. Is the blk queue being stopped?


Not that I know of.


> Is it possible to
> instrument the kernel in your testcase?
>

I'm happy to apply patches, or run systemtap (or similar) scripts.



2018-12-12 12:08:39

by Avi Kivity

[permalink] [raw]
Subject: Re: Spurious EIO on AIO+DIO+RWF_NOWAIT

On 12/10/18 2:48 PM, Goldwyn Rodrigues wrote:
> On 13:19 09/12, Avi Kivity wrote:
>> I have an application that receives spurious EIO when running with
>> RWF_NOWAIT enabled. Removing RWF_NOWAIT causes those EIOs to disappear. The
>> application uses AIO+DIO, and errors were seen on both xfs and ext4.
>>
>>
>> I suspect the following code:
>>
>>
>> /*
>>  * Process one completed BIO.  No locks are held.
>>  */
>> static blk_status_t dio_bio_complete(struct dio *dio, struct bio *bio)
>> {
>>         struct bio_vec *bvec;
>>         unsigned i;
>>         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;
>>         }
>>
>> Could it be that REQ_NOWAIT was dropped from bio->bi_opf? or that
>> bio->bi_status got changed along the way?
>>
> I don't think REQ_NOWAIT is dropped. I am assuming bio->bi_status error
> is set differently. Is the blk queue being stopped? Is it possible to
> instrument the kernel in your testcase?
>

I traced the function, and I see bio->bi_status == BLK_STS_NOTSUPP and
bio->bi_opf == REQ_OP_WRITE|REQ_SYNC|REQ_NOMERGE|REQ_FUA|REQ_NOWAIT.
Presumably the NOTSUPP is the result of NOWAIT not being supported down
the stack, but shouldn't it be detected earlier? And not converted to EIO?


2018-12-12 14:42:49

by Goldwyn Rodrigues

[permalink] [raw]
Subject: Re: Spurious EIO on AIO+DIO+RWF_NOWAIT

On 14:05 12/12, Avi Kivity wrote:
> On 12/10/18 2:48 PM, Goldwyn Rodrigues wrote:
> > On 13:19 09/12, Avi Kivity wrote:
> > > I have an application that receives spurious EIO when running with
> > > RWF_NOWAIT enabled. Removing RWF_NOWAIT causes those EIOs to disappear. The
> > > application uses AIO+DIO, and errors were seen on both xfs and ext4.
> > >
> > >
> > > I suspect the following code:
> > >
> > >
> > > /*
> > > ?* Process one completed BIO.? No locks are held.
> > > ?*/
> > > static blk_status_t dio_bio_complete(struct dio *dio, struct bio *bio)
> > > {
> > > ??????? struct bio_vec *bvec;
> > > ??????? unsigned i;
> > > ??????? 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;
> > > ??????? }
> > >
> > > Could it be that REQ_NOWAIT was dropped from bio->bi_opf? or that
> > > bio->bi_status got changed along the way?
> > >
> > I don't think REQ_NOWAIT is dropped. I am assuming bio->bi_status error
> > is set differently. Is the blk queue being stopped? Is it possible to
> > instrument the kernel in your testcase?
> >
>
> I traced the function, and I see bio->bi_status == BLK_STS_NOTSUPP and
> bio->bi_opf == REQ_OP_WRITE|REQ_SYNC|REQ_NOMERGE|REQ_FUA|REQ_NOWAIT.
> Presumably the NOTSUPP is the result of NOWAIT not being supported down the
> stack, but shouldn't it be detected earlier? And not converted to EIO?
>

I don't think there is a way to detect it earlier. However, I think we should
return -EOPNOTSUPP if the lower layers do not support REQ_NOWAIT. I will write
a patch to modify this.

--
Goldwyn