From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751996AbbBKDQA (ORCPT ); Tue, 10 Feb 2015 22:16:00 -0500 Received: from shadbolt.e.decadent.org.uk ([88.96.1.126]:38432 "EHLO shadbolt.e.decadent.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751615AbbBKDP6 (ORCPT ); Tue, 10 Feb 2015 22:15:58 -0500 Message-ID: <1423624537.2349.164.camel@decadent.org.uk> Subject: Re: [PATCH][v3.2 stable tree] dcache: Balance rcu_read_lock in have_submounts() From: Ben Hutchings To: Steven Rostedt Cc: LKML , stable , Al Viro Date: Wed, 11 Feb 2015 03:15:37 +0000 In-Reply-To: <20150117103806.0a7d3898@gandalf.local.home> References: <20150117103806.0a7d3898@gandalf.local.home> Content-Type: multipart/signed; micalg="pgp-sha512"; protocol="application/pgp-signature"; boundary="=-khBwHEKTkznQDYHmpjbS" X-Mailer: Evolution 3.12.9-1+b1 Mime-Version: 1.0 X-SA-Exim-Connect-IP: 2001:470:1f08:1539:f8a3:8a99:e55b:943d 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 --=-khBwHEKTkznQDYHmpjbS Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Sat, 2015-01-17 at 10:38 -0500, Steven Rostedt wrote: > Porting -rt to the latest 3.2 stable tree I triggered this bug: >=20 > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > [ BUG: bad unlock balance detected! ] > ------------------------------------- > rm/1638 is trying to release lock (rcu_read_lock) at: > [] rcu_read_unlock+0x0/0x23 > but there are no more locks to release! >=20 > other info that might help us debug this: > 2 locks held by rm/1638: > #0: (&sb->s_type->i_mutex_key#9/1){+.+.+.}, at: [] do_rmdir+0= x5f/0xd2 > #1: (&sb->s_type->i_mutex_key#9){+.+.+.}, at: [] vfs_rmdir+0x= 49/0xac >=20 > stack backtrace: > Pid: 1638, comm: rm Not tainted 3.2.66-test-rt96+ #2 > Call Trace: > [] ? printk+0x1d/0x1f > [] print_unlock_inbalance_bug+0xc3/0xcd > [] lock_release_non_nested+0x98/0x1ec > [] ? trace_hardirqs_off_caller+0x18/0x90 > [] ? local_clock+0x2d/0x50 > [] ? d_hash+0x2f/0x2f > [] ? d_hash+0x2f/0x2f > [] lock_release+0x192/0x1ad > [] rcu_read_unlock+0x17/0x23 > [] shrink_dcache_parent+0x227/0x270 > [] vfs_rmdir+0x68/0xac > [] do_rmdir+0x98/0xd2 > [] ? fput+0x1a3/0x1ab > [] ? sysenter_exit+0xf/0x1a > [] ? trace_hardirqs_on_caller+0x118/0x149 > [] sys_unlinkat+0x2b/0x35 > [] sysenter_do_call+0x12/0x12 >=20 >=20 >=20 >=20 > There's a path to calling rcu_read_unlock() without calling > rcu_read_lock() in have_submounts(). >=20 > goto positive; >=20 > positive: > if (!locked && read_seqretry(&rename_lock, seq)) > goto rename_retry; >=20 > rename_retry: > rcu_read_unlock(); >=20 > in the above path, rcu_read_lock() is never done before calling > rcu_read_unlock(); I've reviewed locking contexts in all three functions that I changed when backporting "deal with deadlock in d_walk()". It's actually worse than you say: - We don't hold this_parent->d_lock at the 'positive' label in have_submounts(), but it is unlocked after 'rename_retry'. - There is an rcu_read_unlock() after the 'out' label in select_parent(), but it's not held at the 'goto out'. Does the following patch work for you? Ben. --- a/fs/dcache.c +++ b/fs/dcache.c @@ -1035,7 +1035,7 @@ ascend: return 0; /* No mount points found in tree */ positive: if (!locked && read_seqretry(&rename_lock, seq)) - goto rename_retry; + goto rename_retry_unlocked; if (locked) write_sequnlock(&rename_lock); return 1; @@ -1045,6 +1045,7 @@ rename_retry: rcu_read_unlock(); if (locked) goto again; +rename_retry_unlocked: locked =3D 1; write_seqlock(&rename_lock); goto again; @@ -1109,6 +1110,7 @@ resume: */ if (found && need_resched()) { spin_unlock(&dentry->d_lock); + rcu_read_lock(); goto out; } =20 --- END --- --=20 Ben Hutchings When in doubt, use brute force. - Ken Thompson --=-khBwHEKTkznQDYHmpjbS Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIVAwUAVNrJXue/yOyVhhEJAQolZA//fkVJG4GO43IBEMTQGIt+hHTlCdKgxmW4 oIIdYNJaBPSQJSZfOkDlaTei73tXnF8hguKD543YFynVCmW9lUxtaiPlYo+0+NYU 8dHPe7e/h1veTZMHuXtS3u+jeu72/Ryw0fdv8f/LujPMppuGgv8Gq1zpMHA6VZM5 t8EqzeOQWtqgMGjaPkXW3oaa1PF/RV6xobrFqP2Ip+ayLrN9ft8yQ2SB5DO345ti fTEKwLOTAwWJMfR1JzpQl8AYDZXuU9PHRdBgn+j2g/V0aIT94izc88tPwWrUmgEO uO2LxUd6U95i2rLjrcvIzVqZ7Py+mAq9E+D9muYPhQO9nHjR8MurtJJaCwRXyZtY o5XfdIfUsVMkTKEgE6876ilyjH+ah3MyDtgLjCTdSJumaNypWeOXtAJmiiowSRrc J9ZsJVgYI7bwSgRnilna8Xm7B2uKusm5KX43GgNXO31/Eo1f0u1SlpTypAKW1KR8 ZYB2p/qD6VXG1P3mnYA/nh1CrGyh2SK/3OdkzIA+tZ1noYDR0CfuGjX6ktkvtPDf 2x18zhq0Jw7L+8L7C74IqH0fj4V0+rml5UaeBjyt864Gbjp9/N1FgQ5cEzjxz0QL 9RxHuQgHrLmyTAsY/hXUh9dC1Kgi7qHsQ3Hp08wMw3J1cdvKHd73MKVmw4R80MTu zCUJq8ioH04= =1G0L -----END PGP SIGNATURE----- --=-khBwHEKTkznQDYHmpjbS--