From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Sandeen Subject: Re: [RFC 6/9] fsfreeze: move emergency thaw code to fs/super.c Date: Thu, 13 Sep 2012 16:10:35 -0500 Message-ID: <50524BCB.8090500@redhat.com> References: <1347533862.5646.2.camel@nexus.lab.ntt.co.jp> <1347534499.5646.15.camel@nexus.lab.ntt.co.jp> <20120913190001.GH12994@localhost.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: =?ISO-8859-1?Q?Fernando_Luis_V=E1zquez_Cao?= , Al Viro , Dave Chinner , Christoph Hellwig , Jan Kara , "linux-fsdevel@vger.kernel.org" To: Josef Bacik Return-path: Received: from mx1.redhat.com ([209.132.183.28]:20161 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752075Ab2IMVKq (ORCPT ); Thu, 13 Sep 2012 17:10:46 -0400 In-Reply-To: <20120913190001.GH12994@localhost.localdomain> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On 9/13/12 2:00 PM, Josef Bacik wrote: > On Thu, Sep 13, 2012 at 05:08:19AM -0600, Fernando Luis V=E1zquez Cao= wrote: >> 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 = in >> fs/super.c. Move the code there and clean it up. >> >> 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 >> --- >> >> 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 +09= 00 >> +++ 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 "associ= ated" 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 +090= 0 >> +++ 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; >> +} > =09 > So unless I'm missing something this is wrong. We do __iterate_super= s() which > does down_write(sb) and then call into this. Lets imagine a perfect = world 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 -EINVA= L to know > we completely unfroze, so we call into __thaw_super(sb) without s_umo= unt held > and then we get our error and do up_write(s_umount) _again_. So this= needs to > be reworked to be correct ;). Thanks, The stupid emergency sysrq thing was my fault (at someone else's sugges= tion) ;) It's caused a lot of woe, and hasn't worked for two years. Should we k= eep it? -Eric > Josef >=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