Re: [PATCH 3/8] fsfreeze: freeze_super and thaw_bdev don't play well together

[Fernando Luis Vazquez Cao <fernando_b1@lab.ntt.co.jp>]

On 2012/07/13 22:45, Jan Kara wrote:
> On Thu 12-07-12 18:05:43, Fernando Luis Vázquez 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 here 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 = 0;
>>   
>> -	if (sb->s_frozen == SB_UNFROZEN) {
>> +	mutex_lock(&sb->s_freeze_mutex);
>> +	if (!sb->s_freeze_count) {
>>   		error = -EINVAL;
>> -		goto out;
>> +		goto out_unlock;
>>   	}
>> +	sb->s_freeze_count = 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 locked 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