From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kara Subject: Re: [PATCH 4/9] fsfreeze: emergency thaw will deadlock on s_umount Date: Tue, 25 Sep 2012 11:24:23 +0200 Message-ID: <20120925092423.GC8049@quack.suse.cz> References: <1347605006.6868.2.camel@nexus.lab.ntt.co.jp> <1347605336.6868.6.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]:45426 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751129Ab2IYJY0 (ORCPT ); Tue, 25 Sep 2012 05:24:26 -0400 Content-Disposition: inline In-Reply-To: <1347605336.6868.6.camel@nexus.lab.ntt.co.jp> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Fri 14-09-12 15:48:56, Fernando Luis V=E1zquez Cao wrote: > The emergency thaw process uses iterate_super() which holds the > sb->s_umount lock in read mode. The current thaw_super() code takes > the sb->s_umount lock in write mode, hence leading to an instant > deadlock. >=20 > Use the unlocked version of thaw_super() to do the thawing and replac= e > iterate_supers() with __iterate_supers() so that the unfreeze operati= on can > be performed with s_umount held as the locking rules for fsfreeze ind= icate. >=20 > As a bonus, by using thaw_super(), which does not nest, instead of th= aw_bdev() > when can get rid of the ugly while loop. Hum, but that will leave block devices frozen? Isn't that a bug? It definitely creates an inconsistent state. I think emergency thaw should also iterate over all block devices (via iterate_bdevs()) and thaw all = of them (and no, this does not allow us to avoid the iterate_super() chang= es because there are filesystems without block devices...). Honza > 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-rc5-orig/fs/buffer.c linux-3.6-rc5/fs/buffer.c > --- linux-3.6-rc5-orig/fs/buffer.c 2012-09-14 12:46:22.988871880 +090= 0 > +++ linux-3.6-rc5/fs/buffer.c 2012-09-14 13:05:47.812919103 +0900 > @@ -513,15 +513,27 @@ repeat: > =20 > static void do_thaw_one(struct super_block *sb, void *unused) > { > - char b[BDEVNAME_SIZE]; > - while (sb->s_bdev && !thaw_bdev(sb->s_bdev, sb)) > - printk(KERN_WARNING "Emergency Thaw on %s\n", > + int res; > + > + if (sb->s_bdev) { > + char b[BDEVNAME_SIZE]; > + printk(KERN_WARNING "Emergency Thaw on %s.\n", > bdevname(sb->s_bdev, b)); > + } > + > + /* 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; > } > =20 > static void do_thaw_all(struct work_struct *work) > { > - iterate_supers_read(do_thaw_one, NULL); > + iterate_supers_write(do_thaw_one, NULL); > kfree(work); > printk(KERN_WARNING "Emergency Thaw complete\n"); > } >=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