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: Mon, 8 Oct 2012 15:48:16 +0200 Message-ID: <20121008134816.GB9243@quack.suse.cz> References: <1349414653.7347.2.camel@nexus.lab.ntt.co.jp> <1349415106.7347.4.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 To: Fernando Luis =?iso-8859-1?Q?V=E1zquez?= Cao Return-path: Received: from cantor2.suse.de ([195.135.220.15]:56747 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751026Ab2JHNsU (ORCPT ); Mon, 8 Oct 2012 09:48:20 -0400 Content-Disposition: inline In-Reply-To: <1349415106.7347.4.camel@nexus.lab.ntt.co.jp> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Fri 05-10-12 14:31:46, 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 current iterate_super= s() > callers are just updated to use iterate_supers_read() instead. The patch looks good. You can add: Reviewed-by: Jan Kara Honza >=20 > Cc: Josef Bacik > Cc: Eric Sandeen > Cc: Christoph Hellwig > Cc: Jan Kara > Cc: Dave Chinner > Signed-off-by: Fernando Luis Vazquez Cao > --- >=20 > diff -urNp linux-3.6.0-rc7-orig/fs/buffer.c linux-3.6.0-rc7/fs/buffer= =2Ec > --- linux-3.6.0-rc7-orig/fs/buffer.c 2012-09-24 13:45:05.569520902 +0= 900 > +++ linux-3.6.0-rc7/fs/buffer.c 2012-09-26 13:06:06.578342989 +0900 > @@ -521,7 +521,7 @@ static void do_thaw_one(struct super_blo > =20 > static void do_thaw_all(struct work_struct *work) > { > - iterate_supers(do_thaw_one, NULL); > + iterate_supers_read(do_thaw_one, NULL); > kfree(work); > printk(KERN_WARNING "Emergency Thaw complete\n"); > } > diff -urNp linux-3.6.0-rc7-orig/fs/drop_caches.c linux-3.6.0-rc7/fs/d= rop_caches.c > --- linux-3.6.0-rc7-orig/fs/drop_caches.c 2012-07-22 05:58:29.0000000= 00 +0900 > +++ linux-3.6.0-rc7/fs/drop_caches.c 2012-09-26 13:06:06.582342990 +0= 900 > @@ -59,7 +59,7 @@ int drop_caches_sysctl_handler(ctl_table > return ret; > if (write) { > if (sysctl_drop_caches & 1) > - iterate_supers(drop_pagecache_sb, NULL); > + iterate_supers_read(drop_pagecache_sb, NULL); > if (sysctl_drop_caches & 2) > drop_slab(); > } > diff -urNp linux-3.6.0-rc7-orig/fs/quota/quota.c linux-3.6.0-rc7/fs/q= uota/quota.c > --- linux-3.6.0-rc7-orig/fs/quota/quota.c 2012-09-24 13:45:06.1055209= 90 +0900 > +++ linux-3.6.0-rc7/fs/quota/quota.c 2012-09-26 13:06:06.594342992 +0= 900 > @@ -58,7 +58,7 @@ static int quota_sync_all(int type) > return -EINVAL; > ret =3D security_quotactl(Q_SYNC, type, 0, NULL); > if (!ret) > - iterate_supers(quota_sync_one, &type); > + iterate_supers_read(quota_sync_one, &type); > return ret; > } > =20 > diff -urNp linux-3.6.0-rc7-orig/fs/super.c linux-3.6.0-rc7/fs/super.c > --- linux-3.6.0-rc7-orig/fs/super.c 2012-09-24 13:45:06.129520994 +09= 00 > +++ linux-3.6.0-rc7/fs/super.c 2012-09-26 13:14:39.462346665 +0900 > @@ -537,14 +537,16 @@ 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. > */ > -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 +557,18 @@ 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); > + > + if (wlock) > + up_write(&sb->s_umount); > + else > + up_read(&sb->s_umount); > =20 > spin_lock(&sb_lock); > if (p) > @@ -571,6 +581,34 @@ void iterate_supers(void (*f)(struct sup > } > =20 > /** > + * iterate_supers_read - call function for all active superblocks > + * @f: function to call > + * @arg: argument to pass to it > + * > + * Scans the superblock list and calls given function, passing it > + * a superblock locked in _read_ mode and given argument. The lock > + * is automatically relased after the function returns. > + */ > +void iterate_supers_read(void (*f)(struct super_block *, void *), vo= id *arg) > +{ > + __iterate_supers(f, arg , false); > +} > + > +/** > + * iterate_supers_write - call function for all active superblocks > + * @f: function to call > + * @arg: argument to pass to it > + * > + * Scans the superblock list and calls given function, passing it > + * a superblock locked in _write_ mode and given argument. The lock > + * is automatically relased after the function returns. > + */ > +void iterate_supers_write(void (*f)(struct super_block *, void *), v= oid *arg) > +{ > + __iterate_supers(f, arg , true); > +} > + > +/** > * iterate_supers_type - call function for superblocks of given type > * @type: fs type > * @f: function to call > diff -urNp linux-3.6.0-rc7-orig/fs/sync.c linux-3.6.0-rc7/fs/sync.c > --- linux-3.6.0-rc7-orig/fs/sync.c 2012-09-24 13:45:06.129520994 +090= 0 > +++ linux-3.6.0-rc7/fs/sync.c 2012-09-26 13:06:06.594342992 +0900 > @@ -104,9 +104,9 @@ SYSCALL_DEFINE0(sync) > int nowait =3D 0, wait =3D 1; > =20 > wakeup_flusher_threads(0, WB_REASON_SYNC); > - iterate_supers(sync_inodes_one_sb, NULL); > - iterate_supers(sync_fs_one_sb, &nowait); > - iterate_supers(sync_fs_one_sb, &wait); > + iterate_supers_read(sync_inodes_one_sb, NULL); > + iterate_supers_read(sync_fs_one_sb, &nowait); > + iterate_supers_read(sync_fs_one_sb, &wait); > iterate_bdevs(fdatawrite_one_bdev, NULL); > iterate_bdevs(fdatawait_one_bdev, NULL); > if (unlikely(laptop_mode)) > @@ -122,11 +122,11 @@ static void do_sync_work(struct work_str > * Sync twice to reduce the possibility we skipped some inodes / pa= ges > * because they were temporarily locked > */ > - iterate_supers(sync_inodes_one_sb, &nowait); > - iterate_supers(sync_fs_one_sb, &nowait); > + iterate_supers_read(sync_inodes_one_sb, &nowait); > + iterate_supers_read(sync_fs_one_sb, &nowait); > iterate_bdevs(fdatawrite_one_bdev, NULL); > - iterate_supers(sync_inodes_one_sb, &nowait); > - iterate_supers(sync_fs_one_sb, &nowait); > + iterate_supers_read(sync_inodes_one_sb, &nowait); > + iterate_supers_read(sync_fs_one_sb, &nowait); > iterate_bdevs(fdatawrite_one_bdev, NULL); > printk("Emergency Sync complete\n"); > kfree(work); > diff -urNp linux-3.6.0-rc7-orig/include/linux/fs.h linux-3.6.0-rc7/in= clude/linux/fs.h > --- linux-3.6.0-rc7-orig/include/linux/fs.h 2012-09-24 13:45:06.30152= 1033 +0900 > +++ linux-3.6.0-rc7/include/linux/fs.h 2012-09-26 13:06:06.598342993 = +0900 > @@ -2684,7 +2684,8 @@ extern struct super_block *get_super(str > extern struct super_block *get_super_thawed(struct block_device *); > extern struct super_block *get_active_super(struct block_device *bde= v); > extern void drop_super(struct super_block *sb); > -extern void iterate_supers(void (*)(struct super_block *, void *), v= oid *); > +extern void iterate_supers_read(void (*)(struct super_block *, void = *), void *); > +extern void iterate_supers_write(void (*)(struct super_block *, void= *), void *); > extern void iterate_supers_type(struct file_system_type *, > void (*)(struct super_block *, void *), void *); > =20 > diff -urNp linux-3.6.0-rc7-orig/security/selinux/hooks.c linux-3.6.0-= rc7/security/selinux/hooks.c > --- linux-3.6.0-rc7-orig/security/selinux/hooks.c 2012-09-24 13:45:08= =2E121521275 +0900 > +++ linux-3.6.0-rc7/security/selinux/hooks.c 2012-09-26 13:06:06.6103= 42995 +0900 > @@ -5755,7 +5755,7 @@ void selinux_complete_init(void) > =20 > /* Set up any superblocks initialized prior to the policy load. */ > printk(KERN_DEBUG "SELinux: Setting up existing superblocks.\n"); > - iterate_supers(delayed_superblock_init, NULL); > + iterate_supers_read(delayed_superblock_init, NULL); > } > =20 > /* SELinux requires early initialization in order to label >=20 >=20 --=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