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

[Hao Xu <hao.xu@linux.dev>]

On 8/1/23 08:28, Jens Axboe wrote:
> 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

Hi Jens,
To make sure, I'd like to ask, for memory allocation, GFP_NOIO semantics
is all we need in NOWAIT issue, GFP_NOWAIT is not necessary, do I
understand it right?

Thanks,
Hao

> 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.
>