Received-SPF: pass (google.com: domain of linux-kernel+bounces-181317-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Sender: Tejun Heo <htejun@gmail.com>
Date: Thu, 16 May 2024 06:29:36 -1000
From: Tejun Heo <tj@kernel.org>
To: Haakon Bugge <haakon.bugge@oracle.com>
Cc: OFED mailing list <linux-rdma@vger.kernel.org>,
	open list <linux-kernel@vger.kernel.org>,
	netdev <netdev@vger.kernel.org>,
	"rds-devel@oss.oracle.com" <rds-devel@oss.oracle.com>,
	Jason Gunthorpe <jgg@ziepe.ca>, Leon Romanovsky <leon@kernel.org>,
	Saeed Mahameed <saeedm@nvidia.com>,
	Tariq Toukan <tariqt@nvidia.com>,
	"David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Lai Jiangshan <jiangshanlai@gmail.com>,
	Allison Henderson <allison.henderson@oracle.com>,
	Manjunath Patil <manjunath.b.patil@oracle.com>,
	Mark Zhang <markzhang@nvidia.com>,
	Chuck Lever III <chuck.lever@oracle.com>,
	Shiraz Saleem <shiraz.saleem@intel.com>,
	Yang Li <yang.lee@linux.alibaba.com>
Subject: Re: [PATCH v2 1/6] workqueue: Inherit NOIO and NOFS alloc flags
Message-ID: <ZkY0cIiFOmkwzn5G@slm.duckdns.org>
References: <20240515125342.1069999-1-haakon.bugge@oracle.com>
 <20240515125342.1069999-2-haakon.bugge@oracle.com>
 <ZkTos2YXowEFS2fR@slm.duckdns.org>
 <D9786636-CACE-47E1-B4B6-26AB2C4244C3@oracle.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <D9786636-CACE-47E1-B4B6-26AB2C4244C3@oracle.com>

Hello,

On Thu, May 16, 2024 at 03:27:15PM +0000, Haakon Bugge wrote:
> > 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().

This is too subtle and the default behavior doesn't seem great either - in
most cases, the code path which sets up workqueues would be in GFP_KERNEL
context as init paths usually are, so it's not like this would make things
work automatically in most cases. In addition, now, the memory allocations
for workqueues themselves have to be subject to the same GFP restrictions
even when alloc_workqueue() is called from GFP_KERNEL context. It just
doesn't seem well thought out.

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

It being a purely convenience feature, I don't think there's hard
requirement on where this should go although I don't know where you'd carry
this information if you tied it to each work item. And, please don't single
out specific GFP flags. Please make the feature generic so that users who
may need different GFP masking can also use it too. The underlying GFP
feature is already like that. There's no reason to restrict it from
workqueue side.

Thanks.

-- 
tejun