Re: [PATCH 3/5] io_uring: add support for getdents

[Jens Axboe <axboe@kernel.dk>]

On 7/31/23 9:26?AM, Darrick J. Wong wrote:
> I've watched quite a bit of NOWAIT whackamole going on over the past few
> years (i_rwsem, the ILOCK, the IO layer, memory allocations...).  IIRC
> these filesystem ios all have to run in process context, right?  If so,
> why don't we capture the NOWAIT state in a PF flag?  We already do that
> for NOFS/NOIO memory allocations to make sure that /all/ reclaim
> attempts cannot recurse into the fs/io stacks.

I would greatly prefer passing down the context rather than capitulating
and adding a task_struct flag for this. I think it _kind of_ makes sense
for things like allocations, as you cannot easily track that all the way
down, but it's a really ugly solution. It certainly creates more churn
passing it down, but it also reveals the parts that need to check it.
WHen new code is added, it's much more likely you'll spot the fact that
there's passed in context. For allocation, you end up in the allocator
anyway, which can augment the gfp mask with whatever is set in the task.
The same is not true for locking and other bits, as they don't return a
value to begin with. When we know they are sane, we can flag the fs as
supporting it (like we've done for async buffered reads, for example).

It's also not an absolute thing, like memory allocations are. It's
perfectly fine to grab a mutex under NOWAIT issue. What you should not
do is grab a mutex that someone else can grab while waiting on IO. This
kind of extra context is only available in the code in question, not
generically for eg mutex locking.

I'm not a huge fan of the "let's add a bool nowait". Most/all use cases
pass down state anyway, putting it in a suitable type struct seems much
cleaner to me than the out-of-band approach of just adding a
current->please_nowait.

-- 
Jens Axboe