Re: [PATCH RESEND v4 6/8] vfs: Add get_vfsmount_sb() function to get vfsmount from a given sb.

[Qu Wenruo <quwenruo@cn.fujitsu.com>]

-------- Original Message --------
Subject: Re: [PATCH RESEND v4 6/8] vfs: Add get_vfsmount_sb() function 
to get vfsmount from a given sb.
From: Miao Xie <miaoxie@huawei.com>
To: Al Viro <viro@ZenIV.linux.org.uk>, Qu Wenruo <quwenruo@cn.fujitsu.com>
Date: 2015年01月30日 12:14
> On Fri, 30 Jan 2015 02:14:45 +0000, Al Viro wrote:
>> On Fri, Jan 30, 2015 at 09:44:03AM +0800, Qu Wenruo wrote:
>>
>>> This shouldn't happen. If someone is ro, the whole fs should be ro, right?
>> Wrong.  Individual vfsmounts over an r/w superblock might very well be r/o.
>> As for that trylock...  What for?  It invites transient failures for no
>> good reason.  Removal of sysfs entry will block while write(2) to that sucker
>> is in progress, so btrfs shutdown will block at that point in ctree_close().
>> It won't go away under you.
> could you explain the race condition? I think the deadlock won't happen, during
> the btrfs shutdown, we hold s_umount, the write operation will fail to lock it,
> and quit quickly, and then umount will continue.
>
> I think sb_want_write() is similar to trylock(s_umount), the difference is that
> sb_want_write() is more complex.
How?
sb_want_write() should be much like mnt_want_write(), except no need to 
increase vfsmout ref count things
and no need to check per mount ro/rw things.

Thanks,
Qu
>
>> Now, you might want to move those sysfs entry removals to the very beginning
>> of btrfs_kill_super(), but that's a different story - you need only to make
>> sure that they are removed not later than the destruction of the data
>> structures they need (IOW, the current location might very well be OK - I
>> hadn't checked the details).
> Yes, we need move those sysfs entry removals, but needn't move to the very
> beginning of btrfs_kill_super(), just at the beginning of close_ctree();
>
> The current location is not right, it will introduce the use-after-free
> problem. because we remove the sysfs entry after we release
> transaction_kthread, use-after-free problem might happen in this case
> 	Task1				Task2
> 	change Label by sysfs
> 					close_ctree
> 					  kthread_stop(transaction_kthread);
> 	  change label
> 	  wake_up(transaction_kthread)
>
>
> Thanks
> Miao
>
>> As for "it won't go r/o under us" - sb_want_write() will do that just fine.
>> .
>>

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