From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kara Subject: Re: [PATCH 1/9] vfs: add __iterate_supers() and helpers around it Date: Tue, 25 Sep 2012 11:11:19 +0200 Message-ID: <20120925091119.GA8049@quack.suse.cz> References: <1347605006.6868.2.camel@nexus.lab.ntt.co.jp> <1347605104.6868.3.camel@nexus.lab.ntt.co.jp> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Al Viro , Josef Bacik , Eric Sandeen , Dave Chinner , Christoph Hellwig , Jan Kara , linux-fsdevel@vger.kernel.org, fernando@intellilink.co.jp To: Fernando Luis =?iso-8859-1?Q?V=E1zquez?= Cao Return-path: Received: from cantor2.suse.de ([195.135.220.15]:44678 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751552Ab2IYJLZ (ORCPT ); Tue, 25 Sep 2012 05:11:25 -0400 Content-Disposition: inline In-Reply-To: <1347605104.6868.3.camel@nexus.lab.ntt.co.jp> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Fri 14-09-12 15:45:04, Fernando Luis V=E1zquez Cao wrote: > iterate_supers() calls a function provided by the caller with the s_u= mount > semaphore taken in read mode. However, there may be cases where write= mode > is preferable, so we add __iterate_supers(), which lets one > specify the mode of the lock, and replace iterate_supers with two hel= pers > around __iterate_supers(), iterate_supers_read() and iterate_supers_w= rite(). >=20 > This will be used to fix the emergency thaw (filesystem unfreeze) cod= e, which > iterates over the list of superblocks but needs to hold the s_umount = semaphore > in _write_ mode bebore carrying out the actual thaw operation. >=20 > This patch introduces no semantic changes since iterate_supers() user= s become > iterate_supers_read() which is equivalent. >=20 > Cc: Josef Bacik > Cc: Eric Sandeen > Cc: Christoph Hellwig > Cc: Jan Kara > Cc: Dave Chinner > Signed-off-by: Fernando Luis Vazquez Cao > --- =2E.. > diff -urNp linux-3.6-rc5-orig/fs/super.c linux-3.6-rc5/fs/super.c > --- linux-3.6-rc5-orig/fs/super.c 2012-09-14 11:53:43.416703312 +0900 > +++ linux-3.6-rc5/fs/super.c 2012-09-14 12:30:52.188833193 +0900 > @@ -537,14 +537,22 @@ void drop_super(struct super_block *sb) > EXPORT_SYMBOL(drop_super); > =20 > /** > - * iterate_supers - call function for all active superblocks > + * __iterate_supers - call function for all active superblocks > * @f: function to call > * @arg: argument to pass to it > + * @wlock: mode of superblock lock (false->read lock, true->write lo= ck) > * > * Scans the superblock list and calls given function, passing it > * locked superblock and given argument. > + * > + * When the caller asks for the superblock lock (s_umount semaphore)= to be > + * taken in write mode, the lock is taken but not released because t= he > + * function provided by the caller may deactivate the superblock its= elf. > + * It is that function's job to unlock the superblock as needed in s= uch a > + * case. > */ > -void iterate_supers(void (*f)(struct super_block *, void *), void *a= rg) > +static void __iterate_supers(void (*f)(struct super_block *, void *)= , void *arg, > + bool wlock) > { > struct super_block *sb, *p =3D NULL; > =20 > @@ -555,10 +563,19 @@ void iterate_supers(void (*f)(struct sup > sb->s_count++; > spin_unlock(&sb_lock); > =20 > - down_read(&sb->s_umount); > + if (wlock) > + down_write(&sb->s_umount); > + else > + down_read(&sb->s_umount); > + > if (sb->s_root && (sb->s_flags & MS_BORN)) > f(sb, arg); > - up_read(&sb->s_umount); > + > + /* When the semaphore was taken in write mode the function > + * provided by the caller takes care of unlocking it as > + * needed. See explanation above for details. */ > + if (!wlock) > + up_read(&sb->s_umount); > =20 > spin_lock(&sb_lock); > if (p) These locking rules are ugly and counterintuitive. People will easily get them wrong and create bugs. I'd rather see emergency thaw retake th= e s_umount semaphore so that iterate_supers() can drop it... Honza --=20 Jan Kara SUSE Labs, CR -- 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