From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754124AbdJIMjI (ORCPT ); Mon, 9 Oct 2017 08:39:08 -0400 Received: from mail-wm0-f66.google.com ([74.125.82.66]:37484 "EHLO mail-wm0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751550AbdJIMjG (ORCPT ); Mon, 9 Oct 2017 08:39:06 -0400 X-Google-Smtp-Source: AOwi7QBpnmOWnYryqg55LkRGyT1oD9GX1tte0iE6z8TyDzvvF4XyvBaVc9Fqnr3xuoIs/Ai5o+MhkQ== X-ME-Sender: X-Sasl-enc: p6/1kVTH8zDtCfPHeuggaW8oRGk6buqBVX1CBLaK3XQp 1507552741 Date: Mon, 9 Oct 2017 20:40:09 +0800 From: Boqun Feng To: Lai Jiangshan Cc: LKML , Josef Bacik , Peter Zijlstra , Tejun Heo Subject: Re: [RFC] workqueue: Fix irq inversion deadlock in manage_workers() Message-ID: <20171009124009.o2b6s2et6itquwyd@tardis> References: <20171008090225.27034-1-boqun.feng@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="gkirq32z6wycgxxh" Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20170912 (1.9.0) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --gkirq32z6wycgxxh Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Oct 09, 2017 at 09:40:43AM +0000, Lai Jiangshan wrote: [...] > > Reported-by: Josef Bacik > > Signed-off-by: Boqun Feng > > Cc: Peter Zijlstra > > --- > > kernel/workqueue.c | 35 ++++++++++++++++++++++++++++++++++- > > 1 file changed, 34 insertions(+), 1 deletion(-) > > > > diff --git a/kernel/workqueue.c b/kernel/workqueue.c > > index 64d0edf428f8..2ea7b04cc48b 100644 > > --- a/kernel/workqueue.c > > +++ b/kernel/workqueue.c > > @@ -1997,7 +1997,40 @@ static bool manage_workers(struct worker *worker) > > maybe_create_worker(pool); > > > > pool->manager =3D NULL; > > + > > + /* > > + * Put the manager back to ->idle_list, this allows us to drop = the > > + * pool->lock safely without racing with put_unbound_pool() > > + * > > + * > > + * worker_thread(): > > + * spin_lock_irq= (&pool->lock); > > + * worker_leave_= idle(); > > + * manage_worker= s(): // return true > > + * mutex_trylo= ck(&pool->manager_arb); > > + * > > + * spin_unlock= _irq(&pool->lock); > > + * mutex_unloc= k(&pool->manager_arb); > > + * > > + * put_unbound_pool(): > > + * mutex_lock(&pool->manager_arb); > > + * spin_lock_irq(&pool->lock); > > + * > > + * > > + * ... > > + * wait_for_completion(&pool->detach_completion); > > + * workers is not = empty> > > + * > > + * spin_lock_irq= (&pool->lock); > > + * workli= st is empty, go to sleep> > > + * > > + * No one is going to wake up the manager worker, even so, it w= on't > > + * complete(->detach_completion), since it's not a DIE worker. > > + */ > > + worker_enter_idle(worker); > > + spin_unlock_irq(&pool->lock); > > mutex_unlock(&pool->manager_arb); > > + spin_lock_irq(&pool->lock); > > return true; > > } > > > > @@ -2202,6 +2235,7 @@ static int worker_thread(void *__worker) > > woke_up: > > spin_lock_irq(&pool->lock); > > > > +recheck: > > /* am I supposed to die? */ > > if (unlikely(worker->flags & WORKER_DIE)) { > > spin_unlock_irq(&pool->lock); > > @@ -2216,7 +2250,6 @@ static int worker_thread(void *__worker) > > } > > > > worker_leave_idle(worker); >=20 > I think worker_leave_idle() might be called multiple times, "Multiple times" is not a problem, the problem is unbalanced enter/leave, right? > which might cause bugs, since recheck is moved up. >=20 And that's impossible, as we only goto recheck if manage_workers() return true, and that means the worker got the manager_arb and set itself idle again. So it's fine. Regards, Boqun > > -recheck: > > /* no more worker necessary? */ > > if (!need_more_worker(pool)) > > goto sleep; > > -- > > 2.14.1 > > --gkirq32z6wycgxxh Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAABCAAdFiEEj5IosQTPz8XU1wRHSXnow7UH+rgFAlnbbiYACgkQSXnow7UH +rhE/AgAoeVwyfUV0onQ5MUPk39GArnzAaA7tC0IEAzveEKPmO8uADKYNPI0+dZh dKF+bkszy1ELIG/Mrf6qRZKSu3wY+5pVo6EJ8mXAhzuWkQbi5uVhUw6ET6pXfLsL Tl+HOaKEDRCsK8ybIu0hW1UG89LY4Pi2Aw8ZmH7eU1rAr+0/yjw0YgnDZMf33M17 e/YBqDgYTRLQW80Pd7aumBbg86SPNOAa0aRKzBIdEPBbScy0a7yuiXiX1MDWwtV1 b70a3r6PVoj3SinRU8S86yM2VY9QzRpC1VYw8VwikFC0z00ERrFD/ioFK1qIV/mT DUrvrPSc/kUjHJzCUVvgN923yS2BVA== =CG+k -----END PGP SIGNATURE----- --gkirq32z6wycgxxh--