From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kara Subject: Re: [PATCH 10/17] fsfreeze: automatically thaw on umount Date: Wed, 9 Jan 2013 18:20:10 +0100 Message-ID: <20130109172010.GH17353@quack.suse.cz> References: <1357557492.8183.1.camel@nexus.lab.ntt.co.jp> <1357558514.8183.16.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 , Luiz Capitulino , 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]:57797 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932156Ab3AIRUP (ORCPT ); Wed, 9 Jan 2013 12:20:15 -0500 Content-Disposition: inline In-Reply-To: <1357558514.8183.16.camel@nexus.lab.ntt.co.jp> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Mon 07-01-13 20:35:14, Fernando Luis V=E1zquez Cao wrote: > The fsfreeze userspace API is a sb level API, which means that to tha= w a frozen > filesystem we need to have access to it. This means that after an unm= ount the > filesystem cannot be thawed, because it is not part of the filesystem > namespace anymore. >=20 > A possible solution to the problem above is to mount the filesystem a= gain and > execute the thaw operation. However, a subsequent mount is not guaran= teed to > succeed and even if it does succeeded it may generate I/O in the proc= ess, > which is not supposed to happen since the filesystem is frozen. >=20 > Another approach is adding a bdev level API through which the unmount= ed > filesystem can be reached. The problem with this is that it only work= s for > filesystems for block devices. We could also return EBUSY when the us= er tries > to ummount an frozen filesystem, but this wold break lazy unmount. >=20 > Automatically freezing on sys_umount fixes all the problems mentioned= above. I have to say I'm not convinced this is the right way to go. For exam= ple if dm-snapshot is run on a filesystem being unmounted, thawing during unmount isn't really desirable (although I find this scenario unlikely = to happen in practice). And the way you implemented the idea is definitely wrong - the filesystem can be mounted several times and you would thaw = it on any umount which can definitely surprise processes using freeze. So what if we changed propagate_mount_busy() to return true if the superblock is frozen. That will *not* break lazy umount because that doesn't care about propagate_mount_busy() returns but all other attempt= s to umount will return EBUSY. Sure our problem with unreachable filesystem being frozen won't be completely solved since frozen filesystem can sti= ll be made unreachable by lazy umount but that does not seem to be that common. What do you think? Honza =20 > Cc: linux-fsdevel@vger.kernel.org > Cc: Josef Bacik > Cc: Eric Sandeen > Cc: Christoph Hellwig > Cc: Dave Chinner > Cc: Jan Kara > Cc: Luiz Capitulino > Signed-off-by: Fernando Luis Vazquez Cao > --- >=20 > diff -urNp linux-3.8-rc1-orig/fs/namespace.c linux-3.8-rc1/fs/namespa= ce.c > --- linux-3.8-rc1-orig/fs/namespace.c 2012-12-25 10:27:41.322737000 += 0900 > +++ linux-3.8-rc1/fs/namespace.c 2012-12-25 16:23:56.904018000 +0900 > @@ -1091,6 +1091,27 @@ int may_umount(struct vfsmount *mnt) > =20 > EXPORT_SYMBOL(may_umount); > =20 > +static void thaw_mount(struct mount *mnt) > +{ > + struct super_block *sb =3D mnt->mnt.mnt_sb; > + > + down_write(&sb->s_umount); > + if (sb->s_writers.frozen =3D=3D SB_UNFROZEN) { > + up_write(&sb->s_umount); > + return; > + } > + /* > + * The superblock may be in the process of being detached from the > + * namespace which means we have to make sure the thaw of the super= block > + * succeeds (once it has been detached the fsfreeze ioctls become > + * unusable). Thus, Force-thaw sb so that all tasks in fsfreeze wai= t > + * queue are woken up. > + */ > + sb->s_freeze_count =3D 1; > + __thaw_super(sb, true); > + deactivate_locked_super(sb); > +} > + > void release_mounts(struct list_head *head) > { > struct mount *mnt; > @@ -1111,6 +1132,7 @@ void release_mounts(struct list_head *he > dput(dentry); > mntput(&m->mnt); > } > + thaw_mount(mnt); > mntput(&mnt->mnt); > } > } > diff -urNp linux-3.8-rc1-orig/fs/super.c linux-3.8-rc1/fs/super.c > --- linux-3.8-rc1-orig/fs/super.c 2012-12-25 16:22:48.272018000 +0900 > +++ linux-3.8-rc1/fs/super.c 2012-12-25 16:23:56.904018000 +0900 > @@ -1414,6 +1414,7 @@ EXPORT_SYMBOL(freeze_super); > /** > * __thaw_super -- unlock filesystem > * @sb: the super to thaw > + * @force: whether or not to force the thaw (read details below befo= re using) > * > * Unlocks the filesystem and marks it writeable again. > * > @@ -1421,11 +1422,17 @@ EXPORT_SYMBOL(freeze_super); > * after this thaw if it succeeded or the corresponding error code o= therwise. > * If the unfreeze fails, @sb is left in the frozen state. > * > + * If the force flag is set the kernel will proceeed with the thaw e= ven if the > + * call to the filesystem specific thaw function (->unfreeze_fs()) f= ails. This > + * feature should be used only in situations where there is no entit= y we can > + * return an error to so that it has a chance to clean things up and= retry, i.e. > + * this is the last oportunity to wake the tasks in the fsfreeze wai= t queue up. > + * > * This is the unlocked version of thaw_super, so it is the caller's= job to > * to protect the superblock by grabbing the s_umount semaphore in w= rite mode > * and release it again on return. See thaw_super() for an example. > */ > -static int __thaw_super(struct super_block *sb) > +int __thaw_super(struct super_block *sb, bool force) > { > int error =3D 0; > =20 > @@ -1442,11 +1449,9 @@ static int __thaw_super(struct super_blo > if (sb->s_flags & MS_RDONLY) > goto out_thaw; > =20 > - if (sb->s_op->unfreeze_fs) { > - error =3D sb->s_op->unfreeze_fs(sb); > - if (error) { > - printk(KERN_ERR > - "VFS:Filesystem thaw failed\n"); > + if (sb->s_op->unfreeze_fs && (error =3D sb->s_op->unfreeze_fs(sb)))= { > + printk(KERN_ERR "VFS: Filesystem thaw failed\n"); > + if (!force) { > sb->s_freeze_count++; > goto out; > } > @@ -1470,7 +1475,7 @@ int thaw_super(struct super_block *sb) > { > int res; > down_write(&sb->s_umount); > - res =3D __thaw_super(sb); > + res =3D __thaw_super(sb, false); > if (!res) /* Active reference released after last thaw. */ > deactivate_locked_super(sb); > else > @@ -1497,7 +1502,7 @@ static void do_thaw_one(struct super_blo > * so we can call the lockless version of thaw_super() safely. > */ > sb->s_freeze_count =3D 1; /* avoid multiple calls to __thaw_super *= / > - res =3D __thaw_super(sb); > + res =3D __thaw_super(sb, false); > if (!res) { > deactivate_locked_super(sb); > /* > diff -urNp linux-3.8-rc1-orig/include/linux/fs.h linux-3.8-rc1/includ= e/linux/fs.h > --- linux-3.8-rc1-orig/include/linux/fs.h 2012-12-25 16:22:48.2720180= 00 +0900 > +++ linux-3.8-rc1/include/linux/fs.h 2012-12-25 16:23:56.904018000 +0= 900 > @@ -1882,6 +1882,7 @@ extern int user_statfs(const char __user > extern int fd_statfs(int, struct kstatfs *); > extern int vfs_ustat(dev_t, struct kstatfs *); > extern int freeze_super(struct super_block *super); > +extern int __thaw_super(struct super_block *super, bool force); > extern int thaw_super(struct super_block *super); > extern void emergency_thaw_all(void); > extern bool our_mnt(struct vfsmount *mnt); >=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