Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751984AbbD0BUw (ORCPT ); Sun, 26 Apr 2015 21:20:52 -0400 Received: from shadbolt.e.decadent.org.uk ([88.96.1.126]:44563 "EHLO shadbolt.e.decadent.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751887AbbD0BUu (ORCPT ); Sun, 26 Apr 2015 21:20:50 -0400 Message-ID: <1430097632.4063.156.camel@decadent.org.uk> Subject: Re: [PATCH 3.10 27/31] deal with deadlock in d_walk() From: Ben Hutchings To: Greg Kroah-Hartman Cc: linux-kernel@vger.kernel.org, stable@vger.kernel.org, Al Viro , hujianyang Date: Mon, 27 Apr 2015 02:20:32 +0100 In-Reply-To: <20150426134210.529222061@linuxfoundation.org> References: <20150426134209.255099785@linuxfoundation.org> <20150426134210.529222061@linuxfoundation.org> Content-Type: multipart/signed; micalg="pgp-sha512"; protocol="application/pgp-signature"; boundary="=-C+A13xXHRANLFt1rUnY3" X-Mailer: Evolution 3.12.9-1+b1 Mime-Version: 1.0 X-SA-Exim-Connect-IP: 192.168.4.249 X-SA-Exim-Mail-From: ben@decadent.org.uk X-SA-Exim-Scanned: No (on shadbolt.decadent.org.uk); SAEximRunCond expanded to false Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8268 Lines: 263 --=-C+A13xXHRANLFt1rUnY3 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Sun, 2015-04-26 at 15:49 +0200, Greg Kroah-Hartman wrote: > 3.10-stable review patch. If anyone has any objections, please let me kn= ow. >=20 > ------------------ >=20 > From: Al Viro >=20 > commit ca5358ef75fc69fee5322a38a340f5739d997c10 upstream. >=20 > ... by not hitting rename_retry for reasons other than rename having > happened. In other words, do _not_ restart when finding that > between unlocking the child and locking the parent the former got > into __dentry_kill(). Skip the killed siblings instead... >=20 > Signed-off-by: Al Viro > Cc: Ben Hutchings > [hujianyang: Backported to 3.10 refer to the work of Ben Hutchings in 3.2= : > - As we only have try_to_ascend() and not d_walk(), apply this > change to all callers of try_to_ascend() > - Adjust context to make __dentry_kill() apply to d_kill()] > Signed-off-by: hujianyang > Signed-off-by: Greg Kroah-Hartman This is broken; you need to fold in commit 20defcec264c from 3.2.y ("dcache: Fix locking bugs in backported "deal with deadlock in d_walk()""). Ben. > --- > fs/dcache.c | 102 ++++++++++++++++++++++++++++++++++++-----------------= ------- > 1 file changed, 62 insertions(+), 40 deletions(-) >=20 > --- a/fs/dcache.c > +++ b/fs/dcache.c > @@ -364,9 +364,9 @@ static struct dentry *d_kill(struct dent > __releases(parent->d_lock) > __releases(dentry->d_inode->i_lock) > { > - list_del(&dentry->d_child); > + __list_del_entry(&dentry->d_child); > /* > - * Inform try_to_ascend() that we are no longer attached to the > + * Inform ascending readers that we are no longer attached to the > * dentry tree > */ > dentry->d_flags |=3D DCACHE_DENTRY_KILLED; > @@ -988,35 +988,6 @@ void shrink_dcache_for_umount(struct sup > } > =20 > /* > - * This tries to ascend one level of parenthood, but > - * we can race with renaming, so we need to re-check > - * the parenthood after dropping the lock and check > - * that the sequence number still matches. > - */ > -static struct dentry *try_to_ascend(struct dentry *old, int locked, unsi= gned seq) > -{ > - struct dentry *new =3D old->d_parent; > - > - rcu_read_lock(); > - spin_unlock(&old->d_lock); > - spin_lock(&new->d_lock); > - > - /* > - * might go back up the wrong parent if we have had a rename > - * or deletion > - */ > - if (new !=3D old->d_parent || > - (old->d_flags & DCACHE_DENTRY_KILLED) || > - (!locked && read_seqretry(&rename_lock, seq))) { > - spin_unlock(&new->d_lock); > - new =3D NULL; > - } > - rcu_read_unlock(); > - return new; > -} > - > - > -/* > * Search for at least 1 mount point in the dentry's subdirs. > * We descend to the next level whenever the d_subdirs > * list is non-empty and continue searching. > @@ -1070,17 +1041,32 @@ resume: > /* > * All done at this level ... ascend and resume the search. > */ > + rcu_read_lock(); > +ascend: > if (this_parent !=3D parent) { > struct dentry *child =3D this_parent; > - this_parent =3D try_to_ascend(this_parent, locked, seq); > - if (!this_parent) > + this_parent =3D child->d_parent; > + > + spin_unlock(&child->d_lock); > + spin_lock(&this_parent->d_lock); > + > + /* might go back up the wrong parent if we have had a rename. */ > + if (!locked && read_seqretry(&rename_lock, seq)) > goto rename_retry; > next =3D child->d_child.next; > + while (unlikely(child->d_flags & DCACHE_DENTRY_KILLED)) { > + if (next =3D=3D &this_parent->d_subdirs) > + goto ascend; > + child =3D list_entry(next, struct dentry, d_child); > + next =3D next->next; > + } > + rcu_read_unlock(); > goto resume; > } > - spin_unlock(&this_parent->d_lock); > if (!locked && read_seqretry(&rename_lock, seq)) > goto rename_retry; > + spin_unlock(&this_parent->d_lock); > + rcu_read_unlock(); > if (locked) > write_sequnlock(&rename_lock); > return 0; /* No mount points found in tree */ > @@ -1092,6 +1078,8 @@ positive: > return 1; > =20 > rename_retry: > + spin_unlock(&this_parent->d_lock); > + rcu_read_unlock(); > if (locked) > goto again; > locked =3D 1; > @@ -1177,23 +1165,40 @@ resume: > /* > * All done at this level ... ascend and resume the search. > */ > + rcu_read_lock(); > +ascend: > if (this_parent !=3D parent) { > struct dentry *child =3D this_parent; > - this_parent =3D try_to_ascend(this_parent, locked, seq); > - if (!this_parent) > + this_parent =3D child->d_parent; > + > + spin_unlock(&child->d_lock); > + spin_lock(&this_parent->d_lock); > + > + /* might go back up the wrong parent if we have had a rename. */ > + if (!locked && read_seqretry(&rename_lock, seq)) > goto rename_retry; > next =3D child->d_child.next; > + while (unlikely(child->d_flags & DCACHE_DENTRY_KILLED)) { > + if (next =3D=3D &this_parent->d_subdirs) > + goto ascend; > + child =3D list_entry(next, struct dentry, d_child); > + next =3D next->next; > + } > + rcu_read_unlock(); > goto resume; > } > out: > - spin_unlock(&this_parent->d_lock); > if (!locked && read_seqretry(&rename_lock, seq)) > goto rename_retry; > + spin_unlock(&this_parent->d_lock); > + rcu_read_unlock(); > if (locked) > write_sequnlock(&rename_lock); > return found; > =20 > rename_retry: > + spin_unlock(&this_parent->d_lock); > + rcu_read_unlock(); > if (found) > return found; > if (locked) > @@ -2954,26 +2959,43 @@ resume: > } > spin_unlock(&dentry->d_lock); > } > + rcu_read_lock(); > +ascend: > if (this_parent !=3D root) { > struct dentry *child =3D this_parent; > if (!(this_parent->d_flags & DCACHE_GENOCIDE)) { > this_parent->d_flags |=3D DCACHE_GENOCIDE; > this_parent->d_count--; > } > - this_parent =3D try_to_ascend(this_parent, locked, seq); > - if (!this_parent) > + this_parent =3D child->d_parent; > + > + spin_unlock(&child->d_lock); > + spin_lock(&this_parent->d_lock); > + > + /* might go back up the wrong parent if we have had a rename. */ > + if (!locked && read_seqretry(&rename_lock, seq)) > goto rename_retry; > next =3D child->d_child.next; > + while (unlikely(child->d_flags & DCACHE_DENTRY_KILLED)) { > + if (next =3D=3D &this_parent->d_subdirs) > + goto ascend; > + child =3D list_entry(next, struct dentry, d_child); > + next =3D next->next; > + } > + rcu_read_unlock(); > goto resume; > } > - spin_unlock(&this_parent->d_lock); > if (!locked && read_seqretry(&rename_lock, seq)) > goto rename_retry; > + spin_unlock(&this_parent->d_lock); > + rcu_read_unlock(); > if (locked) > write_sequnlock(&rename_lock); > return; > =20 > rename_retry: > + spin_unlock(&this_parent->d_lock); > + rcu_read_unlock(); > if (locked) > goto again; > locked =3D 1; >=20 >=20 --=20 Ben Hutchings I'm not a reverse psychological virus. Please don't copy me into your sig. --=-C+A13xXHRANLFt1rUnY3 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIVAwUAVT2O6Oe/yOyVhhEJAQqVmw//ZVCM7lBw+5+HqhyJz5qLDyFq1PMiOrV7 CnoMf7qNFV4OSsDu9uGkLLUfO6XbPWLwJ8v8000/pPmyAaU6SW8rpcMd2YlFqOU/ DOTAxuJYhZKZWsvlP5OXElUvRNcFaZClWLGd2dLpxSJQV6N5ZMJQnXWxW/vqJkk5 qGRqJBOX1zV1fZ5k5ff1LL0S/akloc8OwkDElrDCbCApzWXEg9qalEjQnGVBQ3Ep SDEjujkSHvqSD49c8nIdWIuSiybd9mBjv/baQ6GABCVr0AZ38Xd73151nKEiewTX fcj+C4GNSpYr4DAdf5MNar8AxyAKMJPDJDo+NN24hgTrcc4jnxOc9SwxnFICQBBW CMWr3b7WO/qvx2AdzbRb3pnvopRWQP7x97VNT9WFl0c2Q5C1hVsXMqQYMaWqzfFN Y2CsOisV/9bEUhXVWWApIUEG6jhDbi8TA+YM0mrg/yLa8fENHl4zj6We/McWlY27 LalfY82DBVtHkD9QmQAeDvcyMMwLMxK5zFZyiGLsb6Ygme3Rn+XesP1mkFTA2toB UiS3jf2MmH2zPEGx71g6egaVtuSC1W4b4ef23LPE/oCtkpuuvl7sGK1Sy6X3XTar MLlY6dHEjkxz4yU8la+RER0NIi3635asbrcLO1E4B8tERUdBWgb0/MIyXccwd83g E2c5E0YCvwM= =Ygdc -----END PGP SIGNATURE----- --=-C+A13xXHRANLFt1rUnY3-- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/