Re: [PATCH 4/9] fsfreeze: emergency thaw will deadlock on s_umount

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

On 2012/09/25 18:24, Jan Kara wrote:
> On Fri 14-09-12 15:48:56, Fernando Luis Vázquez 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.
>>
>> Use the unlocked version of thaw_super() to do the thawing and replace
>> iterate_supers() with __iterate_supers() so that the unfreeze operation can
>> be performed with s_umount held as the locking rules for fsfreeze indicate.
>>
>> As a bonus, by using thaw_super(), which does not nest, instead of thaw_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() changes
> because there are filesystems without block devices...).

Yes, we may end up with an inconsistent state when
bdev->bd_fsfreeze_count == 1 entering thaw_bdev
(other cases will work as expected) because we have
this code:

         error = thaw_super(sb);
         if (error) {
                 bdev->bd_fsfreeze_count++;
                 mutex_unlock(&bdev->bd_fsfreeze_mutex);
                 return error;
         }

After the emergency unfreeze thaw_super() will return
-EINVAL and bdev->bd_fsfreeze_count will be restored
to one which can confuse the caller.

I think that the best solution to this problem is
considering the thaw successful when thaw_super()
returns -EINVAL, because that means the filesystem
is already unfrozen. I prefer this approach to
thawing bdevs too during emergency thaw,
because dm or external users  may call thaw_bdev
after that, which would fail returning -EINVAL.

If you agree with this I will implement it.

Thanks,
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