From mboxrd@z Thu Jan 1 00:00:00 1970 From: Fernando Luis Vazquez Cao Subject: Re: [PATCH 1/7] fsfreeze: Prevent emergency thaw from looping infinitely Date: Wed, 11 Jul 2012 11:25:56 +0900 Message-ID: <4FFCE434.601@lab.ntt.co.jp> References: <1341908216.11559.6.camel@nexus.lab.ntt.co.jp> <1341908457.11559.9.camel@nexus.lab.ntt.co.jp> <20120710120858.GD13539@quack.suse.cz> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Al Viro , Dave Chinner , Christoph Hellwig , linux-fsdevel@vger.kernel.org To: Jan Kara Return-path: Received: from tama50.ecl.ntt.co.jp ([129.60.39.147]:37868 "EHLO tama50.ecl.ntt.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754170Ab2GKC1R (ORCPT ); Tue, 10 Jul 2012 22:27:17 -0400 In-Reply-To: <20120710120858.GD13539@quack.suse.cz> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On 2012/07/10 21:08, Jan Kara wrote: > On Tue 10-07-12 17:20:57, Fernando Luis V=E1zquez Cao wrote: >> The thawing of a filesystem through sysrq-j loops infinitely as it >> incorrectly detects a thawed filesytsem as frozen and tries to >> unfreeze repeatedly. This is a regression caused by >> 4504230a71566785a05d3e6b53fa1ee071b864eb ("freeze_bdev: grab active >> reference to frozen superblocks") in that it no longer returned >> -EINVAL for superblocks that were not frozen. > Umm, I don't think above mentioned commit is really guilty. Well, it is after that commit that - if (!bdev->bd_fsfreeze_count) { - mutex_unlock(&bdev->bd_fsfreeze_mutex); - return -EINVAL; - } became + if (!bdev->bd_fsfreeze_count) + goto out_unlock; [...] +out_unlock: mutex_unlock(&bdev->bd_fsfreeze_mutex); return 0; which breaks emergency thaw. > Also I think your patch breaks thawing of a block device without moun= ted > filesystem - you end up returning EINVAL for that... It returns EINVAL only after the last thaw and the block device is still properly thawed (i.e. bd_fsfreeze_count becomes 0). Things work because the only place where the kernel checks the return value of thaw_bdev is emergency thaw (the freeze ioctls use the sb level API). That said for the sake of readability I could change the code to return 0 when sb =3D=3D NULL (either as a follow-up patch or as part of a rebase to your tree or Al's). Thanks, =46ernando -- 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