Re: [PATCH 3/3] vfs: fix filesystem_sync vs write race on rw=>ro remount v2

[Dmitry Monakhov <dmonakhov@openvz.org>]

Jan Kara <jack@suse.cz> writes:

> On Tue 02-03-10 17:24:36, Dmitry Monakhov wrote:
>> Jan Kara <jack@suse.cz> writes:
>> 
>> > On Tue 02-03-10 17:04:26, Dmitry Monakhov wrote:
>> >> Jan Kara <jack@suse.cz> writes:
>> >> 
>> >> > On Sat 27-02-10 03:25:49, Dmitry Monakhov wrote:
>> >> >> +/**
>> >> >> + * Check whenever is it possible to remount given sb to readonly.
>> >> >> + * @sb : super block in question
>> >> >> + *
>> >> >> + * Caller is responsible to set ST_REMOUNT_RO state before the call.
>> >> >> + */
>> >> >> +int fs_may_remount_ro(struct super_block *sb)
>> >> >> +{
>> >> >> +	struct vfsmount *mnt;
>> >> >> +	int ret = 1;
>> >> >> +	spin_lock(&vfsmount_lock);
>> >> >> +	list_for_each_entry(mnt, &sb->s_vfsmount, mnt_sb_list) {
>> >> >> +		ret = !mnt_check_writers(mnt, 0);
>> >> >> +		if (ret)
>> >> >> +			break;
>> >> >> +	}
>> >> >> +	spin_unlock(&vfsmount_lock);
>> >> >> +	/*
>> >> >> +	 * If new writer appears after we have checked all vfsmounts.
>> >> >> +	 * Then ST_REMOUNT_RO bit will be cleared.
>> >> >> +	 */
>> >> >> +	if (!test_bit(ST_REMOUNT_RO, &sb->s_state))
>> >> >> +		ret = 0;
>> >> >> +	return ret;
>> >> >> +}
>> >> >   This misses the case when the superblock as unlinked-but-open files.
>> >> > In such case we have to fail remount RO as well. The original
>> >> > fs_may_remount_ro checks for that..
>> >> Since file is opened for write one of vfsmnt struct would have non
>> >> zero write count. So -EBUSY will be returned from fs_may_remount_ro()
>> >   But file can be open for reading only...
>> Ohh.. i see. what is the reason to fail RO remount due to unlinked
>> files? Is this because not all filesystem has orphan list?
>   Yes, this is the main reason.
>
>> Shame on such FS. Seems that i have to also grab write count
>> in unlink path if i_nlink becomes zero and drop on inode release.
>   Yes, I'd imagine some solution like this as well.
I've take a look at the code. It appear not that easy as it looks
from the first glance.
It is not obvious how to store mnt on which write count was
incremented during unlink. Also there is no dedicated mnt which
is assessable from SB. We have s_root but don't know in which mnt
this dentry belongs. So the only option is to introduce real
super_block counter and use it for unlinked inodes tracking.