From mboxrd@z Thu Jan 1 00:00:00 1970 From: Josef Bacik Subject: Re: [RFC 6/9] fsfreeze: move emergency thaw code to fs/super.c Date: Thu, 13 Sep 2012 15:00:01 -0400 Message-ID: <20120913190001.GH12994@localhost.localdomain> References: <1347533862.5646.2.camel@nexus.lab.ntt.co.jp> <1347534499.5646.15.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 mx1.fusionio.com ([66.114.96.30]:53361 "EHLO mx1.fusionio.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751715Ab2IMTAF (ORCPT ); Thu, 13 Sep 2012 15:00:05 -0400 Content-Disposition: inline In-Reply-To: <1347534499.5646.15.camel@nexus.lab.ntt.co.jp> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Thu, Sep 13, 2012 at 05:08:19AM -0600, Fernando Luis V=E1zquez Cao w= rote: > It makes no sense having the emergency thaw code in fs/buffer.c when = all of > it's operations are one superblocks and the code it executes is all i= n > fs/super.c. Move the code there and clean it up. >=20 > Cc: Josef Bacik > Cc: Eric Sandeen > Cc: Christoph Hellwig > Reviewed-by: Jan Kara > Signed-off-by: Dave Chinner > Signed-off-by: Fernando Luis Vazquez Cao > --- >=20 > diff -urNp linux-3.6-rc5-orig/fs/buffer.c linux-3.6-rc5/fs/buffer.c > --- linux-3.6-rc5-orig/fs/buffer.c 2012-09-12 20:44:13.226112590 +090= 0 > +++ linux-3.6-rc5/fs/buffer.c 2012-09-12 20:50:25.406058417 +0900 > @@ -511,52 +511,6 @@ repeat: > return err; > } > =20 > -static int thaw_super_emergency(struct super_block *sb) > -{ > - int res; > - /* We were called from __iterate_supers with superblock lock taken > - * so we do not need to do it here. */ > - res =3D __thaw_super(sb); > - if (!res) > - deactivate_locked_super(sb); > - else > - up_write(&sb->s_umount); > - return res; > -} > - > -static void do_thaw_one(struct super_block *sb, void *unused) > -{ > - if (sb->s_bdev) { > - char b[BDEVNAME_SIZE]; > - printk(KERN_WARNING "Emergency Thaw on %s.\n", > - bdevname(sb->s_bdev, b)); > - } > - while (!thaw_super_emergency(sb)); > -} > - > -static void do_thaw_all(struct work_struct *work) > -{ > - __iterate_supers(do_thaw_one, NULL, true); > - kfree(work); > - printk(KERN_WARNING "Emergency Thaw complete\n"); > -} > - > -/** > - * emergency_thaw_all -- forcibly thaw every frozen filesystem > - * > - * Used for emergency unfreeze of all filesystems via SysRq > - */ > -void emergency_thaw_all(void) > -{ > - struct work_struct *work; > - > - work =3D kmalloc(sizeof(*work), GFP_ATOMIC); > - if (work) { > - INIT_WORK(work, do_thaw_all); > - schedule_work(work); > - } > -} > - > /** > * sync_mapping_buffers - write out & wait upon a mapping's "associa= ted" buffers > * @mapping: the mapping which wants those buffers written > 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-12 20:24:10.474041390 +0900 > +++ linux-3.6-rc5/fs/super.c 2012-09-12 20:50:42.546044906 +0900 > @@ -1475,3 +1475,49 @@ int thaw_super(struct super_block *sb) > return res; > } > EXPORT_SYMBOL(thaw_super); > + > +static int thaw_super_emergency(struct super_block *sb) > +{ > + int res; > + /* We were called from __iterate_supers with superblock lock taken > + * so we do not need to do it here. */ > + res =3D __thaw_super(sb); > + if (!res) > + deactivate_locked_super(sb); > + else > + up_write(&sb->s_umount); > + return res; > +} So unless I'm missing something this is wrong. We do __iterate_supers(= ) which does down_write(sb) and then call into this. Lets imagine a perfect wo= rld where the sb was only frozen once. So we go into __thaw_super() and return 0= because we were successfull and do deactivate_locked_super() which does up_write(s_umount), and then we loop because we want to get an -EINVAL = to know we completely unfroze, so we call into __thaw_super(sb) without s_umoun= t held and then we get our error and do up_write(s_umount) _again_. So this n= eeds to be reworked to be correct ;). Thanks, Josef -- 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