Re: [PATCH V4 6/6] loop: add hint for handling aio via IOCB_NOWAIT

Re: [PATCH V4 6/6] loop: add hint for handling aio via IOCB_NOWAIT

[Christoph Hellwig]

On Mon, Oct 06, 2025 at 10:18:12PM +0800, Ming Lei wrote:
> On Fri, Oct 03, 2025 at 12:06:44AM -0700, Christoph Hellwig wrote:
> > On Sun, Sep 28, 2025 at 09:29:25PM +0800, Ming Lei wrote:
> > > - there isn't any queued blocking async WRITEs, because NOWAIT won't cause
> > > contention with blocking WRITE, which often implies exclusive lock
> > 
> > Isn't this a generic thing we should be doing in core code so that
> > it applies to io_uring I/O as well?
> 
> No.
> 
> It is just policy of using NOWAIT or not, so far:
> 
> - RWF_NOWAIT can be set from preadv/pwritev
> 
> - used for handling io_uring FS read/write
> 
> Even though loop's situation is similar with io-uring, however, both two are
> different subsystem, and there is nothing `core code` for both, more importantly
> it is just one policy: use it or not use it, each subsystem can make its
> own decision based on subsystem internal.

I fail to parse what you say here.  You are encoding special magic
about what underlying file systems do in an upper layer.  I'd much
rather have a flag similar FOP_DIO_PARALLEL_WRITE that makes this
limitation clear rather then opencoding it in the loop driver while
leabing the primary user of RWF_NOWAIT out in the cold.

Re: [PATCH V4 6/6] loop: add hint for handling aio via IOCB_NOWAIT

[Ming Lei]

On Mon, Oct 06, 2025 at 11:33:17PM -0700, Christoph Hellwig wrote:
> On Mon, Oct 06, 2025 at 10:18:12PM +0800, Ming Lei wrote:
> > On Fri, Oct 03, 2025 at 12:06:44AM -0700, Christoph Hellwig wrote:
> > > On Sun, Sep 28, 2025 at 09:29:25PM +0800, Ming Lei wrote:
> > > > - there isn't any queued blocking async WRITEs, because NOWAIT won't cause
> > > > contention with blocking WRITE, which often implies exclusive lock
> > > 
> > > Isn't this a generic thing we should be doing in core code so that
> > > it applies to io_uring I/O as well?
> > 
> > No.
> > 
> > It is just policy of using NOWAIT or not, so far:
> > 
> > - RWF_NOWAIT can be set from preadv/pwritev
> > 
> > - used for handling io_uring FS read/write
> > 
> > Even though loop's situation is similar with io-uring, however, both two are
> > different subsystem, and there is nothing `core code` for both, more importantly
> > it is just one policy: use it or not use it, each subsystem can make its
> > own decision based on subsystem internal.
> 
> I fail to parse what you say here.  You are encoding special magic
> about what underlying file systems do in an upper layer.  I'd much

NOWAIT is obviously interface provided by FS, here loop just wants to try
NOWAIT first in block layer dispatch context for avoiding the extra wq
schedule latency.

But for write on sparse file, trying NOWAIT first may bring extra retry
cost, that is why the hint is added. It is very coarse, but potential
regression can be avoided.

> rather have a flag similar FOP_DIO_PARALLEL_WRITE that makes this
> limitation clear rather then opencoding it in the loop driver while

What is the limitation?

> leabing the primary user of RWF_NOWAIT out in the cold.

FOP_DIO_PARALLEL_WRITE is one static FS feature, but here it is FS
runtime behavior, such as if the write can be blocked because of space
allocation, so it can't be done by one static flag.

io-uring shares nothing with loop in this area, it is just one policy wrt.
use NOWAIT or not. I don't understand why you insist on covering both
from FS internal...



Thanks,
Ming

Re: [PATCH V4 6/6] loop: add hint for handling aio via IOCB_NOWAIT

[Christoph Hellwig]

On Tue, Oct 07, 2025 at 08:15:05PM +0800, Ming Lei wrote:
> NOWAIT is obviously interface provided by FS, here loop just wants to try
> NOWAIT first in block layer dispatch context for avoiding the extra wq
> schedule latency.

Yes.

> But for write on sparse file, trying NOWAIT first may bring extra retry
> cost, that is why the hint is added. It is very coarse, but potential
> regression can be avoided.

And that is absolutely not a property of loop, and loop should not have
to know about.  So this logic needs to be in common code, preferably
triggered by a fs flag.  Note that this isn't about holes - it is about
allocating blocks.  For most file systems filling holes or extending
past i_size is what requires allocating blocks.  But for a out of place
write file systems like btrfs, or zoned xfs we always need to allocate
blocks for now.  But I have work that I need to finish off that allows
for non-blocking block allocation in zoned XFS, at which point you
don't need this.  I think some of this might be true for network file
systems already.

> 
> > rather have a flag similar FOP_DIO_PARALLEL_WRITE that makes this
> > limitation clear rather then opencoding it in the loop driver while
> 
> What is the limitation?

See above.

> > leabing the primary user of RWF_NOWAIT out in the cold.
> 
> FOP_DIO_PARALLEL_WRITE is one static FS feature,

It actually isn't :( I need to move it to be a bit more dynamic on a
per-file basis.

> but here it is FS
> runtime behavior, such as if the write can be blocked because of space
> allocation, so it can't be done by one static flag.

Yes, that's why you want a flag to indicate that a file, or maybe file
operations instance can do non-blocking fill of blocks.  But that's
for the future, for now I just want your logic lifted to common code
and shared with io_uring so that we don't have weird hardcoded
assumptions about file system behavior inside the loop driver.

> io-uring shares nothing with loop in this area, it is just one policy wrt.
> use NOWAIT or not. I don't understand why you insist on covering both
> from FS internal...

It's really about all IOCB_NOWAIT users, io_uring being the prime one,
and the one that we can actually easily write tests for.

Re: [PATCH V4 6/6] loop: add hint for handling aio via IOCB_NOWAIT

[Ming Lei]

On Tue, Oct 07, 2025 at 10:56:01PM -0700, Christoph Hellwig wrote:
> On Tue, Oct 07, 2025 at 08:15:05PM +0800, Ming Lei wrote:
> > NOWAIT is obviously interface provided by FS, here loop just wants to try
> > NOWAIT first in block layer dispatch context for avoiding the extra wq
> > schedule latency.
> 
> Yes.
> 
> > But for write on sparse file, trying NOWAIT first may bring extra retry
> > cost, that is why the hint is added. It is very coarse, but potential
> > regression can be avoided.
> 
> And that is absolutely not a property of loop, and loop should not have
> to know about.  So this logic needs to be in common code, preferably
> triggered by a fs flag.  Note that this isn't about holes - it is about
> allocating blocks.  For most file systems filling holes or extending
> past i_size is what requires allocating blocks.  But for a out of place
> write file systems like btrfs, or zoned xfs we always need to allocate
> blocks for now.  But I have work that I need to finish off that allows
> for non-blocking block allocation in zoned XFS, at which point you
> don't need this.  I think some of this might be true for network file
> systems already.

Firstly this FS flag isn't available, if it is added, we may take it into
account, and it is just one check, which shouldn't be blocker for this
loop perf improvement.

Secondly it isn't enough to replace nowait decision from user side, one
case is overwrite, which is a nice usecase for nowait.

> 
> > 
> > > rather have a flag similar FOP_DIO_PARALLEL_WRITE that makes this
> > > limitation clear rather then opencoding it in the loop driver while
> > 
> > What is the limitation?
> 
> See above.
> 
> > > leabing the primary user of RWF_NOWAIT out in the cold.
> > 
> > FOP_DIO_PARALLEL_WRITE is one static FS feature,
> 
> It actually isn't :( I need to move it to be a bit more dynamic on a
> per-file basis.
> 
> > but here it is FS
> > runtime behavior, such as if the write can be blocked because of space
> > allocation, so it can't be done by one static flag.
> 
> Yes, that's why you want a flag to indicate that a file, or maybe file
> operations instance can do non-blocking fill of blocks.  But that's
> for the future, for now I just want your logic lifted to common code
> and shared with io_uring so that we don't have weird hardcoded
> assumptions about file system behavior inside the loop driver.

As I mentioned the hint in this patch is very loop specific for avoiding
potential write perf regression, which just works for loop's case.

It can't be applied on io-uring, otherwise perf regression can be caused on
overwrite from io-uring application.

So I don't know what is the exact common code or logic for both loop and
io-uring.


Thanks,
Ming

Re: [PATCH V4 6/6] loop: add hint for handling aio via IOCB_NOWAIT

[Christoph Hellwig]

On Thu, Oct 09, 2025 at 09:25:47AM +0800, Ming Lei wrote:
> Firstly this FS flag isn't available, if it is added, we may take it into
> account, and it is just one check, which shouldn't be blocker for this
> loop perf improvement.
> 
> Secondly it isn't enough to replace nowait decision from user side, one
> case is overwrite, which is a nice usecase for nowait.

Yes.  But right now you are hardcoding heuristics which is overall a
very minor user of RWF_NOWAIT instead of sorting this out properly.

Re: [PATCH V4 6/6] loop: add hint for handling aio via IOCB_NOWAIT

[Ming Lei]

On Mon, Oct 13, 2025 at 2:28 PM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Thu, Oct 09, 2025 at 09:25:47AM +0800, Ming Lei wrote:
> > Firstly this FS flag isn't available, if it is added, we may take it into
> > account, and it is just one check, which shouldn't be blocker for this
> > loop perf improvement.
> >
> > Secondly it isn't enough to replace nowait decision from user side, one
> > case is overwrite, which is a nice usecase for nowait.
>
> Yes.  But right now you are hardcoding heuristics which is overall a
> very minor user of RWF_NOWAIT instead of sorting this out properly.

Yes, that is why I call the hint as loop specific, it isn't perfect, just for
avoiding potential regression by taking nowait.

Given the improvement is big, and the perf issue has been
reported several times, I'd suggest taking it this way first, and
document it can be improved in future.

Thanks,