From mboxrd@z Thu Jan 1 00:00:00 1970 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757598AbeAHWwE (ORCPT + 1 other); Mon, 8 Jan 2018 17:52:04 -0500 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:41268 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757004AbeAHWwC (ORCPT ); Mon, 8 Jan 2018 17:52:02 -0500 Date: Mon, 8 Jan 2018 14:52:38 -0800 From: "Paul E. McKenney" To: Tejun Heo Cc: Prateek Sood , Peter Zijlstra , avagin@gmail.com, mingo@kernel.org, linux-kernel@vger.kernel.org, cgroups@vger.kernel.org, sramana@codeaurora.org Subject: Re: [PATCH] cgroup/cpuset: fix circular locking dependency Reply-To: paulmck@linux.vnet.ibm.com References: <20171204225825.GP2421075@devbig577.frc2.facebook.com> <20171204230117.GF20227@worktop.programming.kicks-ass.net> <20171211152059.GH2421075@devbig577.frc2.facebook.com> <20171213160617.GQ3919388@devbig577.frc2.facebook.com> <9843d982-d201-8702-2e4e-0541a4d96b53@codeaurora.org> <20180102161656.GD3668920@devbig577.frc2.facebook.com> <20180102174408.GM7829@linux.vnet.ibm.com> <20180102180119.GA1355@linux.vnet.ibm.com> <20180108122823.GL3668920@devbig577.frc2.facebook.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180108122823.GL3668920@devbig577.frc2.facebook.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-TM-AS-GCONF: 00 x-cbid: 18010822-0040-0000-0000-000003DE0B12 X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00008343; HX=3.00000241; KW=3.00000007; PH=3.00000004; SC=3.00000245; SDB=6.00972207; UDB=6.00492504; IPR=6.00752158; BA=6.00005767; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00018932; XFM=3.00000015; UTC=2018-01-08 22:51:59 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 18010822-0041-0000-0000-000007D363A0 Message-Id: <20180108225238.GN9671@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2018-01-08_13:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 malwarescore=0 suspectscore=0 phishscore=0 bulkscore=0 spamscore=0 clxscore=1011 lowpriorityscore=0 impostorscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1709140000 definitions=main-1801080321 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: On Mon, Jan 08, 2018 at 04:28:23AM -0800, Tejun Heo wrote: > Hello, Paul. > > Sorry about the delay. Travel followed by cold. :( > > On Tue, Jan 02, 2018 at 10:01:19AM -0800, Paul E. McKenney wrote: > > Actually, after taking a quick look, could you please supply me with > > a way of mark a statically allocated workqueue as WQ_MEM_RECLAIM after > > the fact? Otherwise, I end up having to check for the workqueue having > > Hmmm... there is no statically allocated workqueue tho. If you're > referring to the system-wide workqueues (system*_wq), they're just > created dynamically early during boot. Good point, I was confused. But yes, they are conveniently allocated just before the call to rcu_init(), which does work out well. ;-) > > been allocated pretty much each time I use it, which is going to be an > > open invitation for bugs. Plus it looks like there are ways that RCU's > > workqueue wakeups can be executed during very early boot, which can be > > handled, but again in a rather messy fashion. > > > > In contrast, given a way of mark a statically allocated workqueue > > as WQ_MEM_RECLAIM after the fact, I simply continue initializing the > > workqueue at early boot, and then add the WQ_MEM_RECLAIM marking some > > arbitrarily chosen time after the scheduler has been initialized. > > > > The required change to workqueues looks easy, just move the body of > > the "if (flags & WQ_MEM_RECLAIM) {" statement in __alloc_workqueue_key() > > to a separate function, right? > > Ah, okay, yes, currently, workqueue init is kinda silly in that while > it allows init of non-mem-reclaiming workqueues way before workqueue > is actually online, it doesn't allow the same for mem-reclaiming ones. > As you pointed out, it's just an oversight on my part as the init path > split was done initially to accomodate early init of system > workqueues. > > I'll update the code so that rescuers can be added later too; however, > please note that while the work items may be queued, they won't be > executed until workqueue_init() is run (the same as now) as there > can't be worker threads anyway before that point. Thank you! I added the following patch to allow RCU access to the init_rescuer() function. Does that work for you, or did you have some other arrangement in mind? Thanx, Paul ------------------------------------------------------------------------ commit 66683a07503d71e5d5cceac72caf772e6e59c787 Author: Paul E. McKenney Date: Mon Jan 8 14:27:46 2018 -0800 workqueue: Allow init_rescuer() to be invoked from other files This commit exports init_rescuer() so that RCU can invoke it. Signed-off-by: Paul E. McKenney Cc: Tejun Heo Cc: Lai Jiangshan diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h index 4a54ef96aff5..31ce9343b4a9 100644 --- a/include/linux/workqueue.h +++ b/include/linux/workqueue.h @@ -433,6 +433,8 @@ __alloc_workqueue_key(const char *fmt, unsigned int flags, int max_active, #define create_singlethread_workqueue(name) \ alloc_ordered_workqueue("%s", __WQ_LEGACY | WQ_MEM_RECLAIM, name) +int init_rescuer(struct workqueue_struct *wq); + extern void destroy_workqueue(struct workqueue_struct *wq); struct workqueue_attrs *alloc_workqueue_attrs(gfp_t gfp_mask); diff --git a/kernel/workqueue.c b/kernel/workqueue.c index c86cc1ed678b..7440c61c6213 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -3943,7 +3943,7 @@ static int wq_clamp_max_active(int max_active, unsigned int flags, * Workqueues which may be used during memory reclaim should have a rescuer * to guarantee forward progress. */ -static int init_rescuer(struct workqueue_struct *wq) +int init_rescuer(struct workqueue_struct *wq) { struct worker *rescuer; int ret;