Re: [PATCH v2 1/6] workqueue: Inherit NOIO and NOFS alloc flags

[Haakon Bugge <haakon.bugge@oracle.com>]

> On 15 May 2024, at 18:54, Tejun Heo <tj@kernel.org> wrote:
> 
>> @@ -5583,6 +5600,10 @@ struct workqueue_struct *alloc_workqueue(const char *fmt,
>> 
>> /* init wq */
>> wq->flags = flags;
>> + if (current->flags & PF_MEMALLOC_NOIO)
>> + wq->flags |= __WQ_NOIO;
>> + if (current->flags & PF_MEMALLOC_NOFS)
>> + wq->flags |= __WQ_NOFS;
> 
> So, yeah, please don't do this. What if a NOIO callers wants to scheduler a
> work item so that it can user GFP_KERNEL allocations.

If one work function want to use GPF_KERNEL and another using GFP_NOIO, queued on the same workqueue, one could create two workqueues. Create one that is surrounded by memalloc_noio_{save,restore}, another surrounded by memalloc_flags_save() + current->flags &= ~PF_MEMALLOC_NOIO and memalloc_flags_restore().

If you imply a work functions that performs combinations of GFP_KERNEL and GFP_NOIO, that sounds a little bit peculiar to me, but if needed, it must be open-coded. But wouldn't that be the same case as a WQ created with WQ_MEM_RECLAIM?

> I don't mind a
> convenience feature to workqueue for this but this doesn't seem like the
> right way. Also, memalloc_noio_save() and memalloc_nofs_save() are
> convenience wrappers around memalloc_flags_save(), so it'd probably be
> better to deal with gfp flags directly rather than singling out these two
> flags.

Actually, based on https://lore.kernel.org/linux-fsdevel/ZZcgXI46AinlcBDP@casper.infradead.org, I am inclided to skip GFP_NOFS. Also because the use-case for this series does not need GFP_NOFS.

When you say "deal with gfp flags directly", do you imply during WQ creation or queuing work on one? I am OK with adding the other per-process memory allocation flags, but that doesn's solve your initial issue ("if a NOIO callers wants to scheduler a work item so that it can user GFP_KERNEL").


Thxs, Håkon