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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
