* Re: [PATCH 5.4 103/142] Revert "block: end bio with BLK_STS_AGAIN in case of non-mq devs and REQ_NOWAIT"
[not found] ` <[email protected]>
@ 2021-02-03 12:37 ` Andres Freund
2021-02-03 13:03 ` Greg Kroah-Hartman
0 siblings, 1 reply; 8+ messages in thread
From: Andres Freund @ 2021-02-03 12:37 UTC (permalink / raw)
To: Greg Kroah-Hartman, io-uring
Cc: linux-kernel, stable, Bijan Mottahedeh, Sasha Levin
Hi,
On 2020-06-01 19:54:21 +0200, Greg Kroah-Hartman wrote:
> From: Jens Axboe <[email protected]>
>
> [ Upstream commit b0beb28097fa04177b3769f4bb7a0d0d9c4ae76e ]
>
> This reverts commit c58c1f83436b501d45d4050fd1296d71a9760bcb.
>
> io_uring does do the right thing for this case, and we're still returning
> -EAGAIN to userspace for the cases we don't support. Revert this change
> to avoid doing endless spins of resubmits.
>
> Cc: [email protected] # v5.6
> Reported-by: Bijan Mottahedeh <[email protected]>
> Signed-off-by: Jens Axboe <[email protected]>
> Signed-off-by: Sasha Levin <[email protected]>
> ---
> block/blk-core.c | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)
>
This broke io_uring direct-io on ext4 over md.
fallocate -l $((1024*1024*1024)) /srv/part1
fallocate -l $((1024*1024*1024)) /srv/part2
losetup -f /srv/part1
losetup -f /srv/part2
losetup -a # assuming these were loop0/1
mdadm --create -n2 -l stripe -N fast-striped /dev/md/fast-striped /dev/loop0 /dev/loop1
mkfs.ext4 /dev/md/fast-striped
mount /dev/md/fast-striped /mnt/t2
fio --directory=/mnt/t2 --ioengine io_uring --rw write --filesize 1MB --overwrite=1 --name=test --direct=1 --bs=4k
On v5.4.43-101-gbba91cdba612 this fails with
fio: io_u error on file /mnt/t2/test.0.0: Input/output error: write offset=0, buflen=4096
fio: pid=734, err=5/file:io_u.c:1834, func=io_u error, error=Input/output error
whereas previously it worked. libaio still works...
I haven't checked which major kernel version fixed this again, but I did
verify that it's still broken in 5.4.94 and that 5.10.9 works.
I would suspect it's
commit 4503b7676a2e0abe69c2f2c0d8b03aec53f2f048
Author: Jens Axboe <[email protected]>
Date: 2020-06-01 10:00:27 -0600
io_uring: catch -EIO from buffered issue request failure
-EIO bubbles up like -EAGAIN if we fail to allocate a request at the
lower level. Play it safe and treat it like -EAGAIN in terms of sync
retry, to avoid passing back an errant -EIO.
Catch some of these early for block based file, as non-mq devices
generally do not support NOWAIT. That saves us some overhead by
not first trying, then retrying from async context. We can go straight
to async punt instead.
Signed-off-by: Jens Axboe <[email protected]>
which isn't in stable/linux-5.4.y
Greetings,
Andres Freund
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 5.4 103/142] Revert "block: end bio with BLK_STS_AGAIN in case of non-mq devs and REQ_NOWAIT"
2021-02-03 12:37 ` [PATCH 5.4 103/142] Revert "block: end bio with BLK_STS_AGAIN in case of non-mq devs and REQ_NOWAIT" Andres Freund
@ 2021-02-03 13:03 ` Greg Kroah-Hartman
2021-02-03 21:28 ` Andres Freund
0 siblings, 1 reply; 8+ messages in thread
From: Greg Kroah-Hartman @ 2021-02-03 13:03 UTC (permalink / raw)
To: Andres Freund
Cc: io-uring, linux-kernel, stable, Bijan Mottahedeh, Sasha Levin
On Wed, Feb 03, 2021 at 04:37:29AM -0800, Andres Freund wrote:
> Hi,
>
> On 2020-06-01 19:54:21 +0200, Greg Kroah-Hartman wrote:
> > From: Jens Axboe <[email protected]>
> >
> > [ Upstream commit b0beb28097fa04177b3769f4bb7a0d0d9c4ae76e ]
> >
> > This reverts commit c58c1f83436b501d45d4050fd1296d71a9760bcb.
> >
> > io_uring does do the right thing for this case, and we're still returning
> > -EAGAIN to userspace for the cases we don't support. Revert this change
> > to avoid doing endless spins of resubmits.
> >
> > Cc: [email protected] # v5.6
> > Reported-by: Bijan Mottahedeh <[email protected]>
> > Signed-off-by: Jens Axboe <[email protected]>
> > Signed-off-by: Sasha Levin <[email protected]>
> > ---
> > block/blk-core.c | 11 ++++-------
> > 1 file changed, 4 insertions(+), 7 deletions(-)
> >
>
> This broke io_uring direct-io on ext4 over md.
>
> fallocate -l $((1024*1024*1024)) /srv/part1
> fallocate -l $((1024*1024*1024)) /srv/part2
> losetup -f /srv/part1
> losetup -f /srv/part2
> losetup -a # assuming these were loop0/1
> mdadm --create -n2 -l stripe -N fast-striped /dev/md/fast-striped /dev/loop0 /dev/loop1
> mkfs.ext4 /dev/md/fast-striped
> mount /dev/md/fast-striped /mnt/t2
> fio --directory=/mnt/t2 --ioengine io_uring --rw write --filesize 1MB --overwrite=1 --name=test --direct=1 --bs=4k
>
> On v5.4.43-101-gbba91cdba612 this fails with
> fio: io_u error on file /mnt/t2/test.0.0: Input/output error: write offset=0, buflen=4096
> fio: pid=734, err=5/file:io_u.c:1834, func=io_u error, error=Input/output error
>
> whereas previously it worked. libaio still works...
>
> I haven't checked which major kernel version fixed this again, but I did
> verify that it's still broken in 5.4.94 and that 5.10.9 works.
>
> I would suspect it's
>
> commit 4503b7676a2e0abe69c2f2c0d8b03aec53f2f048
> Author: Jens Axboe <[email protected]>
> Date: 2020-06-01 10:00:27 -0600
>
> io_uring: catch -EIO from buffered issue request failure
>
> -EIO bubbles up like -EAGAIN if we fail to allocate a request at the
> lower level. Play it safe and treat it like -EAGAIN in terms of sync
> retry, to avoid passing back an errant -EIO.
>
> Catch some of these early for block based file, as non-mq devices
> generally do not support NOWAIT. That saves us some overhead by
> not first trying, then retrying from async context. We can go straight
> to async punt instead.
>
> Signed-off-by: Jens Axboe <[email protected]>
>
>
> which isn't in stable/linux-5.4.y
Can you test that if the above commit is added, all works well again?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 5.4 103/142] Revert "block: end bio with BLK_STS_AGAIN in case of non-mq devs and REQ_NOWAIT"
2021-02-03 13:03 ` Greg Kroah-Hartman
@ 2021-02-03 21:28 ` Andres Freund
2021-02-03 22:06 ` Greg Kroah-Hartman
0 siblings, 1 reply; 8+ messages in thread
From: Andres Freund @ 2021-02-03 21:28 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: io-uring, linux-kernel, stable, Bijan Mottahedeh, Sasha Levin
Hi,
On 2021-02-03 14:03:09 +0100, Greg Kroah-Hartman wrote:
> > On v5.4.43-101-gbba91cdba612 this fails with
> > fio: io_u error on file /mnt/t2/test.0.0: Input/output error: write offset=0, buflen=4096
> > fio: pid=734, err=5/file:io_u.c:1834, func=io_u error, error=Input/output error
> >
> > whereas previously it worked. libaio still works...
> >
> > I haven't checked which major kernel version fixed this again, but I did
> > verify that it's still broken in 5.4.94 and that 5.10.9 works.
> >
> > I would suspect it's
> >
> > commit 4503b7676a2e0abe69c2f2c0d8b03aec53f2f048
> > Author: Jens Axboe <[email protected]>
> > Date: 2020-06-01 10:00:27 -0600
> >
> > io_uring: catch -EIO from buffered issue request failure
> >
> > -EIO bubbles up like -EAGAIN if we fail to allocate a request at the
> > lower level. Play it safe and treat it like -EAGAIN in terms of sync
> > retry, to avoid passing back an errant -EIO.
> >
> > Catch some of these early for block based file, as non-mq devices
> > generally do not support NOWAIT. That saves us some overhead by
> > not first trying, then retrying from async context. We can go straight
> > to async punt instead.
> >
> > Signed-off-by: Jens Axboe <[email protected]>
> >
> >
> > which isn't in stable/linux-5.4.y
>
> Can you test that if the above commit is added, all works well again?
It doesn't apply cleanly, I'll try to resolve the conflict. However, I
assume that the revert was for a concrete reason - but I can't quite
figure out what b0beb28097fa04177b3769f4bb7a0d0d9c4ae76e was concretely
solving, and whether reverting the revert in 5.4 would re-introduce a
different problem.
commit b0beb28097fa04177b3769f4bb7a0d0d9c4ae76e (tag: block-5.7-2020-05-29, linux-block/block-5.7)
Author: Jens Axboe <[email protected]>
Date: 2020-05-28 13:19:29 -0600
Revert "block: end bio with BLK_STS_AGAIN in case of non-mq devs and REQ_NOWAIT"
This reverts commit c58c1f83436b501d45d4050fd1296d71a9760bcb.
io_uring does do the right thing for this case, and we're still returning
-EAGAIN to userspace for the cases we don't support. Revert this change
to avoid doing endless spins of resubmits.
Cc: [email protected] # v5.6
Reported-by: Bijan Mottahedeh <[email protected]>
Signed-off-by: Jens Axboe <[email protected]>
I suspect it just wasn't aimed at 5.4, and that's that, but I'm not
sure. In which case presumably reverting
bba91cdba612fbce4f8575c5d94d2b146fb83ea3 would be the right fix, not
backporting 4503b7676a2e0abe69c2f2c0d8b03aec53f2f048 et al.
Greetings,
Andres Freund
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 5.4 103/142] Revert "block: end bio with BLK_STS_AGAIN in case of non-mq devs and REQ_NOWAIT"
2021-02-03 21:28 ` Andres Freund
@ 2021-02-03 22:06 ` Greg Kroah-Hartman
2021-02-03 22:58 ` Jens Axboe
0 siblings, 1 reply; 8+ messages in thread
From: Greg Kroah-Hartman @ 2021-02-03 22:06 UTC (permalink / raw)
To: Andres Freund
Cc: io-uring, linux-kernel, stable, Bijan Mottahedeh, Sasha Levin
On Wed, Feb 03, 2021 at 01:28:26PM -0800, Andres Freund wrote:
> Hi,
>
> On 2021-02-03 14:03:09 +0100, Greg Kroah-Hartman wrote:
> > > On v5.4.43-101-gbba91cdba612 this fails with
> > > fio: io_u error on file /mnt/t2/test.0.0: Input/output error: write offset=0, buflen=4096
> > > fio: pid=734, err=5/file:io_u.c:1834, func=io_u error, error=Input/output error
> > >
> > > whereas previously it worked. libaio still works...
> > >
> > > I haven't checked which major kernel version fixed this again, but I did
> > > verify that it's still broken in 5.4.94 and that 5.10.9 works.
> > >
> > > I would suspect it's
> > >
> > > commit 4503b7676a2e0abe69c2f2c0d8b03aec53f2f048
> > > Author: Jens Axboe <[email protected]>
> > > Date: 2020-06-01 10:00:27 -0600
> > >
> > > io_uring: catch -EIO from buffered issue request failure
> > >
> > > -EIO bubbles up like -EAGAIN if we fail to allocate a request at the
> > > lower level. Play it safe and treat it like -EAGAIN in terms of sync
> > > retry, to avoid passing back an errant -EIO.
> > >
> > > Catch some of these early for block based file, as non-mq devices
> > > generally do not support NOWAIT. That saves us some overhead by
> > > not first trying, then retrying from async context. We can go straight
> > > to async punt instead.
> > >
> > > Signed-off-by: Jens Axboe <[email protected]>
> > >
> > >
> > > which isn't in stable/linux-5.4.y
> >
> > Can you test that if the above commit is added, all works well again?
>
> It doesn't apply cleanly, I'll try to resolve the conflict. However, I
> assume that the revert was for a concrete reason - but I can't quite
> figure out what b0beb28097fa04177b3769f4bb7a0d0d9c4ae76e was concretely
> solving, and whether reverting the revert in 5.4 would re-introduce a
> different problem.
>
> commit b0beb28097fa04177b3769f4bb7a0d0d9c4ae76e (tag: block-5.7-2020-05-29, linux-block/block-5.7)
> Author: Jens Axboe <[email protected]>
> Date: 2020-05-28 13:19:29 -0600
>
> Revert "block: end bio with BLK_STS_AGAIN in case of non-mq devs and REQ_NOWAIT"
>
> This reverts commit c58c1f83436b501d45d4050fd1296d71a9760bcb.
>
> io_uring does do the right thing for this case, and we're still returning
> -EAGAIN to userspace for the cases we don't support. Revert this change
> to avoid doing endless spins of resubmits.
>
> Cc: [email protected] # v5.6
> Reported-by: Bijan Mottahedeh <[email protected]>
> Signed-off-by: Jens Axboe <[email protected]>
>
> I suspect it just wasn't aimed at 5.4, and that's that, but I'm not
> sure. In which case presumably reverting
> bba91cdba612fbce4f8575c5d94d2b146fb83ea3 would be the right fix, not
> backporting 4503b7676a2e0abe69c2f2c0d8b03aec53f2f048 et al.
Ok, can you send a revert patch for this?
But it would be good to get Jens to weigh in on this...
thanks,
greg k-h
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 5.4 103/142] Revert "block: end bio with BLK_STS_AGAIN in case of non-mq devs and REQ_NOWAIT"
2021-02-03 22:06 ` Greg Kroah-Hartman
@ 2021-02-03 22:58 ` Jens Axboe
2021-02-03 23:59 ` Andres Freund
0 siblings, 1 reply; 8+ messages in thread
From: Jens Axboe @ 2021-02-03 22:58 UTC (permalink / raw)
To: Greg Kroah-Hartman, Andres Freund
Cc: io-uring, linux-kernel, stable, Bijan Mottahedeh, Sasha Levin
On 2/3/21 3:06 PM, Greg Kroah-Hartman wrote:
> On Wed, Feb 03, 2021 at 01:28:26PM -0800, Andres Freund wrote:
>> Hi,
>>
>> On 2021-02-03 14:03:09 +0100, Greg Kroah-Hartman wrote:
>>>> On v5.4.43-101-gbba91cdba612 this fails with
>>>> fio: io_u error on file /mnt/t2/test.0.0: Input/output error: write offset=0, buflen=4096
>>>> fio: pid=734, err=5/file:io_u.c:1834, func=io_u error, error=Input/output error
>>>>
>>>> whereas previously it worked. libaio still works...
>>>>
>>>> I haven't checked which major kernel version fixed this again, but I did
>>>> verify that it's still broken in 5.4.94 and that 5.10.9 works.
>>>>
>>>> I would suspect it's
>>>>
>>>> commit 4503b7676a2e0abe69c2f2c0d8b03aec53f2f048
>>>> Author: Jens Axboe <[email protected]>
>>>> Date: 2020-06-01 10:00:27 -0600
>>>>
>>>> io_uring: catch -EIO from buffered issue request failure
>>>>
>>>> -EIO bubbles up like -EAGAIN if we fail to allocate a request at the
>>>> lower level. Play it safe and treat it like -EAGAIN in terms of sync
>>>> retry, to avoid passing back an errant -EIO.
>>>>
>>>> Catch some of these early for block based file, as non-mq devices
>>>> generally do not support NOWAIT. That saves us some overhead by
>>>> not first trying, then retrying from async context. We can go straight
>>>> to async punt instead.
>>>>
>>>> Signed-off-by: Jens Axboe <[email protected]>
>>>>
>>>>
>>>> which isn't in stable/linux-5.4.y
>>>
>>> Can you test that if the above commit is added, all works well again?
>>
>> It doesn't apply cleanly, I'll try to resolve the conflict. However, I
>> assume that the revert was for a concrete reason - but I can't quite
>> figure out what b0beb28097fa04177b3769f4bb7a0d0d9c4ae76e was concretely
>> solving, and whether reverting the revert in 5.4 would re-introduce a
>> different problem.
>>
>> commit b0beb28097fa04177b3769f4bb7a0d0d9c4ae76e (tag: block-5.7-2020-05-29, linux-block/block-5.7)
>> Author: Jens Axboe <[email protected]>
>> Date: 2020-05-28 13:19:29 -0600
>>
>> Revert "block: end bio with BLK_STS_AGAIN in case of non-mq devs and REQ_NOWAIT"
>>
>> This reverts commit c58c1f83436b501d45d4050fd1296d71a9760bcb.
>>
>> io_uring does do the right thing for this case, and we're still returning
>> -EAGAIN to userspace for the cases we don't support. Revert this change
>> to avoid doing endless spins of resubmits.
>>
>> Cc: [email protected] # v5.6
>> Reported-by: Bijan Mottahedeh <[email protected]>
>> Signed-off-by: Jens Axboe <[email protected]>
>>
>> I suspect it just wasn't aimed at 5.4, and that's that, but I'm not
>> sure. In which case presumably reverting
>> bba91cdba612fbce4f8575c5d94d2b146fb83ea3 would be the right fix, not
>> backporting 4503b7676a2e0abe69c2f2c0d8b03aec53f2f048 et al.
>
> Ok, can you send a revert patch for this?
>
> But it would be good to get Jens to weigh in on this...
I'll take a look at this.
--
Jens Axboe
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 5.4 103/142] Revert "block: end bio with BLK_STS_AGAIN in case of non-mq devs and REQ_NOWAIT"
2021-02-03 22:58 ` Jens Axboe
@ 2021-02-03 23:59 ` Andres Freund
2021-02-04 14:36 ` Jens Axboe
0 siblings, 1 reply; 8+ messages in thread
From: Andres Freund @ 2021-02-03 23:59 UTC (permalink / raw)
To: Jens Axboe
Cc: Greg Kroah-Hartman, io-uring, linux-kernel, stable,
Bijan Mottahedeh, Sasha Levin
Hi,
On 2021-02-03 15:58:33 -0700, Jens Axboe wrote:
> On 2/3/21 3:06 PM, Greg Kroah-Hartman wrote:
> > On Wed, Feb 03, 2021 at 01:28:26PM -0800, Andres Freund wrote:
> >> On 2021-02-03 14:03:09 +0100, Greg Kroah-Hartman wrote:
> >>>> On v5.4.43-101-gbba91cdba612 this fails with
> >>>> fio: io_u error on file /mnt/t2/test.0.0: Input/output error: write offset=0, buflen=4096
> >>>> fio: pid=734, err=5/file:io_u.c:1834, func=io_u error, error=Input/output error
> >>>>
> >>>> whereas previously it worked. libaio still works...
> >>>>
> >>>> I haven't checked which major kernel version fixed this again, but I did
> >>>> verify that it's still broken in 5.4.94 and that 5.10.9 works.
> >>>>
> >>>> I would suspect it's
> >>>>
> >>>> commit 4503b7676a2e0abe69c2f2c0d8b03aec53f2f048
> >>>> Author: Jens Axboe <[email protected]>
> >>>> Date: 2020-06-01 10:00:27 -0600
> >>>>
> >>>> io_uring: catch -EIO from buffered issue request failure
> >>>>
> >>>> -EIO bubbles up like -EAGAIN if we fail to allocate a request at the
> >>>> lower level. Play it safe and treat it like -EAGAIN in terms of sync
> >>>> retry, to avoid passing back an errant -EIO.
> >>>>
> >>>> Catch some of these early for block based file, as non-mq devices
> >>>> generally do not support NOWAIT. That saves us some overhead by
> >>>> not first trying, then retrying from async context. We can go straight
> >>>> to async punt instead.
> >>>>
> >>>> Signed-off-by: Jens Axboe <[email protected]>
> >>>>
> >>>>
> >>>> which isn't in stable/linux-5.4.y
> >>>
> >>> Can you test that if the above commit is added, all works well again?
> >>
> >> It doesn't apply cleanly, I'll try to resolve the conflict. However, I
> >> assume that the revert was for a concrete reason - but I can't quite
> >> figure out what b0beb28097fa04177b3769f4bb7a0d0d9c4ae76e was concretely
> >> solving, and whether reverting the revert in 5.4 would re-introduce a
> >> different problem.
> >>
> >> commit b0beb28097fa04177b3769f4bb7a0d0d9c4ae76e (tag: block-5.7-2020-05-29, linux-block/block-5.7)
> >> Author: Jens Axboe <[email protected]>
> >> Date: 2020-05-28 13:19:29 -0600
> >>
> >> Revert "block: end bio with BLK_STS_AGAIN in case of non-mq devs and REQ_NOWAIT"
> >>
> >> This reverts commit c58c1f83436b501d45d4050fd1296d71a9760bcb.
> >>
> >> io_uring does do the right thing for this case, and we're still returning
> >> -EAGAIN to userspace for the cases we don't support. Revert this change
> >> to avoid doing endless spins of resubmits.
> >>
> >> Cc: [email protected] # v5.6
> >> Reported-by: Bijan Mottahedeh <[email protected]>
> >> Signed-off-by: Jens Axboe <[email protected]>
> >>
> >> I suspect it just wasn't aimed at 5.4, and that's that, but I'm not
> >> sure. In which case presumably reverting
> >> bba91cdba612fbce4f8575c5d94d2b146fb83ea3 would be the right fix, not
> >> backporting 4503b7676a2e0abe69c2f2c0d8b03aec53f2f048 et al.
Having looked a bit more through the history, I suspect that the reason
5.6 doesn't need c58c1f83436b501d45d4050fd1296d71a9760bcb - which I have
confirmed - is that ext4 was converted to the iomap infrastructure in
5.5, but not in 5.4.
I've confirmed that the repro I shared upthread triggers in
378f32bab3714f04c4e0c3aee4129f6703805550^ but not in
378f32bab3714f04c4e0c3aee4129f6703805550.
> > Ok, can you send a revert patch for this?
> >
> > But it would be good to get Jens to weigh in on this...
>
> I'll take a look at this.
Thanks.
Greetings,
Andres Freund
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 5.4 103/142] Revert "block: end bio with BLK_STS_AGAIN in case of non-mq devs and REQ_NOWAIT"
2021-02-03 23:59 ` Andres Freund
@ 2021-02-04 14:36 ` Jens Axboe
2021-02-04 15:04 ` Greg Kroah-Hartman
0 siblings, 1 reply; 8+ messages in thread
From: Jens Axboe @ 2021-02-04 14:36 UTC (permalink / raw)
To: Andres Freund
Cc: Greg Kroah-Hartman, io-uring, linux-kernel, stable,
Bijan Mottahedeh, Sasha Levin
On 2/3/21 4:59 PM, Andres Freund wrote:
> Hi,
>
> On 2021-02-03 15:58:33 -0700, Jens Axboe wrote:
>> On 2/3/21 3:06 PM, Greg Kroah-Hartman wrote:
>>> On Wed, Feb 03, 2021 at 01:28:26PM -0800, Andres Freund wrote:
>>>> On 2021-02-03 14:03:09 +0100, Greg Kroah-Hartman wrote:
>>>>>> On v5.4.43-101-gbba91cdba612 this fails with
>>>>>> fio: io_u error on file /mnt/t2/test.0.0: Input/output error: write offset=0, buflen=4096
>>>>>> fio: pid=734, err=5/file:io_u.c:1834, func=io_u error, error=Input/output error
>>>>>>
>>>>>> whereas previously it worked. libaio still works...
>>>>>>
>>>>>> I haven't checked which major kernel version fixed this again, but I did
>>>>>> verify that it's still broken in 5.4.94 and that 5.10.9 works.
>>>>>>
>>>>>> I would suspect it's
>>>>>>
>>>>>> commit 4503b7676a2e0abe69c2f2c0d8b03aec53f2f048
>>>>>> Author: Jens Axboe <[email protected]>
>>>>>> Date: 2020-06-01 10:00:27 -0600
>>>>>>
>>>>>> io_uring: catch -EIO from buffered issue request failure
>>>>>>
>>>>>> -EIO bubbles up like -EAGAIN if we fail to allocate a request at the
>>>>>> lower level. Play it safe and treat it like -EAGAIN in terms of sync
>>>>>> retry, to avoid passing back an errant -EIO.
>>>>>>
>>>>>> Catch some of these early for block based file, as non-mq devices
>>>>>> generally do not support NOWAIT. That saves us some overhead by
>>>>>> not first trying, then retrying from async context. We can go straight
>>>>>> to async punt instead.
>>>>>>
>>>>>> Signed-off-by: Jens Axboe <[email protected]>
>>>>>>
>>>>>>
>>>>>> which isn't in stable/linux-5.4.y
>>>>>
>>>>> Can you test that if the above commit is added, all works well again?
>>>>
>>>> It doesn't apply cleanly, I'll try to resolve the conflict. However, I
>>>> assume that the revert was for a concrete reason - but I can't quite
>>>> figure out what b0beb28097fa04177b3769f4bb7a0d0d9c4ae76e was concretely
>>>> solving, and whether reverting the revert in 5.4 would re-introduce a
>>>> different problem.
>>>>
>>>> commit b0beb28097fa04177b3769f4bb7a0d0d9c4ae76e (tag: block-5.7-2020-05-29, linux-block/block-5.7)
>>>> Author: Jens Axboe <[email protected]>
>>>> Date: 2020-05-28 13:19:29 -0600
>>>>
>>>> Revert "block: end bio with BLK_STS_AGAIN in case of non-mq devs and REQ_NOWAIT"
>>>>
>>>> This reverts commit c58c1f83436b501d45d4050fd1296d71a9760bcb.
>>>>
>>>> io_uring does do the right thing for this case, and we're still returning
>>>> -EAGAIN to userspace for the cases we don't support. Revert this change
>>>> to avoid doing endless spins of resubmits.
>>>>
>>>> Cc: [email protected] # v5.6
>>>> Reported-by: Bijan Mottahedeh <[email protected]>
>>>> Signed-off-by: Jens Axboe <[email protected]>
>>>>
>>>> I suspect it just wasn't aimed at 5.4, and that's that, but I'm not
>>>> sure. In which case presumably reverting
>>>> bba91cdba612fbce4f8575c5d94d2b146fb83ea3 would be the right fix, not
>>>> backporting 4503b7676a2e0abe69c2f2c0d8b03aec53f2f048 et al.
>
> Having looked a bit more through the history, I suspect that the reason
> 5.6 doesn't need c58c1f83436b501d45d4050fd1296d71a9760bcb - which I have
> confirmed - is that ext4 was converted to the iomap infrastructure in
> 5.5, but not in 5.4.
>
> I've confirmed that the repro I shared upthread triggers in
> 378f32bab3714f04c4e0c3aee4129f6703805550^ but not in
> 378f32bab3714f04c4e0c3aee4129f6703805550.
I checked up on this, and I do see the issue as well. As far as
io_uring is concerned, we don't need that revert in 5.4. So I think
the right solution here would be to... revert the revert :-)
--
Jens Axboe
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 5.4 103/142] Revert "block: end bio with BLK_STS_AGAIN in case of non-mq devs and REQ_NOWAIT"
2021-02-04 14:36 ` Jens Axboe
@ 2021-02-04 15:04 ` Greg Kroah-Hartman
0 siblings, 0 replies; 8+ messages in thread
From: Greg Kroah-Hartman @ 2021-02-04 15:04 UTC (permalink / raw)
To: Jens Axboe
Cc: Andres Freund, io-uring, linux-kernel, stable, Bijan Mottahedeh,
Sasha Levin
On Thu, Feb 04, 2021 at 07:36:18AM -0700, Jens Axboe wrote:
> On 2/3/21 4:59 PM, Andres Freund wrote:
> > Hi,
> >
> > On 2021-02-03 15:58:33 -0700, Jens Axboe wrote:
> >> On 2/3/21 3:06 PM, Greg Kroah-Hartman wrote:
> >>> On Wed, Feb 03, 2021 at 01:28:26PM -0800, Andres Freund wrote:
> >>>> On 2021-02-03 14:03:09 +0100, Greg Kroah-Hartman wrote:
> >>>>>> On v5.4.43-101-gbba91cdba612 this fails with
> >>>>>> fio: io_u error on file /mnt/t2/test.0.0: Input/output error: write offset=0, buflen=4096
> >>>>>> fio: pid=734, err=5/file:io_u.c:1834, func=io_u error, error=Input/output error
> >>>>>>
> >>>>>> whereas previously it worked. libaio still works...
> >>>>>>
> >>>>>> I haven't checked which major kernel version fixed this again, but I did
> >>>>>> verify that it's still broken in 5.4.94 and that 5.10.9 works.
> >>>>>>
> >>>>>> I would suspect it's
> >>>>>>
> >>>>>> commit 4503b7676a2e0abe69c2f2c0d8b03aec53f2f048
> >>>>>> Author: Jens Axboe <[email protected]>
> >>>>>> Date: 2020-06-01 10:00:27 -0600
> >>>>>>
> >>>>>> io_uring: catch -EIO from buffered issue request failure
> >>>>>>
> >>>>>> -EIO bubbles up like -EAGAIN if we fail to allocate a request at the
> >>>>>> lower level. Play it safe and treat it like -EAGAIN in terms of sync
> >>>>>> retry, to avoid passing back an errant -EIO.
> >>>>>>
> >>>>>> Catch some of these early for block based file, as non-mq devices
> >>>>>> generally do not support NOWAIT. That saves us some overhead by
> >>>>>> not first trying, then retrying from async context. We can go straight
> >>>>>> to async punt instead.
> >>>>>>
> >>>>>> Signed-off-by: Jens Axboe <[email protected]>
> >>>>>>
> >>>>>>
> >>>>>> which isn't in stable/linux-5.4.y
> >>>>>
> >>>>> Can you test that if the above commit is added, all works well again?
> >>>>
> >>>> It doesn't apply cleanly, I'll try to resolve the conflict. However, I
> >>>> assume that the revert was for a concrete reason - but I can't quite
> >>>> figure out what b0beb28097fa04177b3769f4bb7a0d0d9c4ae76e was concretely
> >>>> solving, and whether reverting the revert in 5.4 would re-introduce a
> >>>> different problem.
> >>>>
> >>>> commit b0beb28097fa04177b3769f4bb7a0d0d9c4ae76e (tag: block-5.7-2020-05-29, linux-block/block-5.7)
> >>>> Author: Jens Axboe <[email protected]>
> >>>> Date: 2020-05-28 13:19:29 -0600
> >>>>
> >>>> Revert "block: end bio with BLK_STS_AGAIN in case of non-mq devs and REQ_NOWAIT"
> >>>>
> >>>> This reverts commit c58c1f83436b501d45d4050fd1296d71a9760bcb.
> >>>>
> >>>> io_uring does do the right thing for this case, and we're still returning
> >>>> -EAGAIN to userspace for the cases we don't support. Revert this change
> >>>> to avoid doing endless spins of resubmits.
> >>>>
> >>>> Cc: [email protected] # v5.6
> >>>> Reported-by: Bijan Mottahedeh <[email protected]>
> >>>> Signed-off-by: Jens Axboe <[email protected]>
> >>>>
> >>>> I suspect it just wasn't aimed at 5.4, and that's that, but I'm not
> >>>> sure. In which case presumably reverting
> >>>> bba91cdba612fbce4f8575c5d94d2b146fb83ea3 would be the right fix, not
> >>>> backporting 4503b7676a2e0abe69c2f2c0d8b03aec53f2f048 et al.
> >
> > Having looked a bit more through the history, I suspect that the reason
> > 5.6 doesn't need c58c1f83436b501d45d4050fd1296d71a9760bcb - which I have
> > confirmed - is that ext4 was converted to the iomap infrastructure in
> > 5.5, but not in 5.4.
> >
> > I've confirmed that the repro I shared upthread triggers in
> > 378f32bab3714f04c4e0c3aee4129f6703805550^ but not in
> > 378f32bab3714f04c4e0c3aee4129f6703805550.
>
> I checked up on this, and I do see the issue as well. As far as
> io_uring is concerned, we don't need that revert in 5.4. So I think
> the right solution here would be to... revert the revert :-)
Thanks for looking at this, I'll go revert the revert now.
greg k-h
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-02-04 16:25 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <[email protected]>
[not found] ` <[email protected]>
2021-02-03 12:37 ` [PATCH 5.4 103/142] Revert "block: end bio with BLK_STS_AGAIN in case of non-mq devs and REQ_NOWAIT" Andres Freund
2021-02-03 13:03 ` Greg Kroah-Hartman
2021-02-03 21:28 ` Andres Freund
2021-02-03 22:06 ` Greg Kroah-Hartman
2021-02-03 22:58 ` Jens Axboe
2021-02-03 23:59 ` Andres Freund
2021-02-04 14:36 ` Jens Axboe
2021-02-04 15:04 ` Greg Kroah-Hartman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox