From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tyler Hicks Subject: Re: [PATCH][BUGFIX][RESEND] vfs: Correctly set the dir i_mutex lockdep class Date: Fri, 20 Jan 2012 12:39:55 -0600 Message-ID: <20120120183954.GA9156@boyd> References: <1323705750-28536-1-git-send-email-tyhicks@canonical.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="yrj/dFKFPuw6o+aM" Cc: Mimi Zohar , John Johansen , Jan Kara , Josh Boyer , Peter Zijlstra , Ingo Molnar , linux-fsdevel@vger.kernel.org, Andrew Morton , Christoph Hellwig To: Al Viro Return-path: Received: from youngberry.canonical.com ([91.189.89.112]:33258 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753855Ab2ATSkH (ORCPT ); Fri, 20 Jan 2012 13:40:07 -0500 Content-Disposition: inline In-Reply-To: <1323705750-28536-1-git-send-email-tyhicks@canonical.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: --yrj/dFKFPuw6o+aM Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On 2011-12-12 10:02:30, Tyler Hicks wrote: > 9a7aa12f3911853a introduced additional logic around setting the i_mutex > lockdep class for directory inodes. The idea was that some filesystems > may want their own special lockdep class for different directory > inodes and calling unlock_new_inode() should not clobber one of > those special classes. >=20 > I believe that the added conditional, around the *negated* return value > of lockdep_match_class(), caused directory inodes to be placed in the > wrong lockdep class. >=20 > inode_init_always() sets the i_mutex lockdep class with i_mutex_key for > all inodes. If the filesystem did not change the class during inode > initialization, then the conditional mentioned above was false and the > directory inode was incorrectly left in the non-directory lockdep class. > If the filesystem did set a special lockdep class, then the conditional > mentioned above was true and that class was clobbered with > i_mutex_dir_key. >=20 > This patch removes the negation from the conditional so that the i_mutex > lockdep class is properly set for directory inodes. Special classes are > preserved and directory inodes with unmodified classes are set with > i_mutex_dir_key. >=20 > Signed-off-by: Tyler Hicks > Reviewed-by: Jan Kara > --- >=20 > Hi Al - I thought I'd resend this fix after seeing Mimi's RFC for more > filesystems to use lockdep_annotate_inode_mutex_key(). Currently, that fu= nction > doesn't do what it is supposed to do. Hi Al - Just following up again to see if I can get any comments on this bug fix in hopes of getting it picked up soon. Searching back through list archives, I realize there have been debates about the locking order of i_mutex vs mmap_sem among directory and regular inodes, which I imagine is the reason for this bit of code. But either way, the lock class setting of directory inodes is broken here. Adding hch and akpm to cc for extra eyes/comments. Tyler >=20 > fs/inode.c | 3 +-- > 1 files changed, 1 insertions(+), 2 deletions(-) >=20 > diff --git a/fs/inode.c b/fs/inode.c > index ee4e66b..9d01a0d 100644 > --- a/fs/inode.c > +++ b/fs/inode.c > @@ -855,8 +855,7 @@ void lockdep_annotate_inode_mutex_key(struct inode *i= node) > struct file_system_type *type =3D inode->i_sb->s_type; > =20 > /* Set new key only if filesystem hasn't already changed it */ > - if (!lockdep_match_class(&inode->i_mutex, > - &type->i_mutex_key)) { > + if (lockdep_match_class(&inode->i_mutex, &type->i_mutex_key)) { > /* > * ensure nobody is actually holding i_mutex > */ > --=20 > 1.7.5.4 >=20 > -- > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" = in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html --yrj/dFKFPuw6o+aM Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) iQIcBAEBCgAGBQJPGbT6AAoJENaSAD2qAscKudgP/RhXrG3EVEbBwIXeqa3T0R/t 7qTDrUhEhYmwv1zEerbJ5Zjw0HjgiglvxrNEsWTLzPxmbgLY7zEkHkdv2SXuLJnh wZ+F0DGMrFtXthoL6fH3BU4SOyV8CtNIQp/V0JybtfUsKms4Av1XsjhXoa0nPEl8 NCIK0mlDouKbM563mQq79KpzW/9t/ws/g/vZez4TPQU9GIpTMqCaFwe2Wz6BSTwD pUle/uUoBCa4ORwhdt5njh/aPPDhQXcwxMwyEr1KSmIf11hF9F+L0yZUiIjd/pVS +chj4aEB4frCZdV38NkMFWHd0xU44y3VB2h/4n1sJ/YsZgn40FyAy7rI2oFdk5zp KDmWZIACtJW32i6synWlxPFp8IKW72xDxxSXnKhxz/YP5fw6sBYbk4wm1xlJyDR3 CTfnqD8uH8KlpL8XIxJ8xfYT8xEJRfbffVTM43kFu1UPyt4Itkf2YkJ3gQlI955L rsUYIIYrnNwFAP5wNHbKfIe2yAYXWvAIRGqFwjIimIew8SUPPgrdAloZhDRY52YH pi1itAZquE/fq2N3gaXupyDBvr0hyZrXGOZcKbfLS170ziPIl24melieUE8trky7 VsysQ+aMyUmP7aKlef6rraHZNbw/E++/ENsuQVsxoIJYm/SAAuitvY8VJvOs636F wQc5GfM5zD/JFbQtLK+/ =4rhv -----END PGP SIGNATURE----- --yrj/dFKFPuw6o+aM--