From mboxrd@z Thu Jan 1 00:00:00 1970 From: Fernando Luis Vazquez Cao Subject: Re: [PATCH 3/8] fsfreeze: freeze_super and thaw_bdev don't play well together Date: Thu, 09 Aug 2012 18:00:44 +0900 Message-ID: <50237C3C.30604@lab.ntt.co.jp> References: <1342083464.7033.6.camel@nexus.lab.ntt.co.jp> <1342083943.7033.12.camel@nexus.lab.ntt.co.jp> <20120713134537.GF20361@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]:49682 "EHLO tama50.ecl.ntt.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757220Ab2HIJBK (ORCPT ); Thu, 9 Aug 2012 05:01:10 -0400 In-Reply-To: <20120713134537.GF20361@quack.suse.cz> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On 2012/07/13 22:45, Jan Kara wrote: > On Thu 12-07-12 18:05:43, Fernando Luis V=E1zquez Cao wrote: >> -/** >> - * thaw_bdev -- unlock filesystem >> - * @bdev: blockdevice to unlock >> - * @sb: associated superblock >> - * >> - * Unlocks the filesystem and marks it writeable again after freeze= _bdev(). >> - */ >> int thaw_bdev(struct block_device *bdev, struct super_block *sb) >> { >> return __thaw_bdev(bdev, sb, 0); > It's a bit confusing (for reviewer) to remove the documentation he= re when > you remove the whole function in a later patch... Ok. I will get rid of the function and its documentation together in a later patch. >> @@ -1233,29 +1240,33 @@ static int __thaw_super(struct super_blo >> { >> int error =3D 0; >> =20 >> - if (sb->s_frozen =3D=3D SB_UNFROZEN) { >> + mutex_lock(&sb->s_freeze_mutex); >> + if (!sb->s_freeze_count) { >> error =3D -EINVAL; >> - goto out; >> + goto out_unlock; >> } >> + sb->s_freeze_count =3D emergency ? 1 : sb->s_freeze_count; > It would be cleaner to do this somewhere in do_thaw_one() and not = here. > Also you won't have to pass the emergency parameter then... Good point. In the next iteration sb->s_freeze_count is updated by the caller when appropriate. I will remove the emergency parameter too. > Also it might > be more logical for __thaw_super() to expect also s_freeze_mutex lock= ed and > handle the locking in thaw_super(). I think I will keep this as is so that I do not need to add explicit locking to the emergency thaw code, which calls __thaw_super directly. Thank you for your feedback! - Fernando -- 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