-------- 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 <redacted>
To: Al Viro <viro@ZenIV.linux.org.uk>, Qu Wenruo <redacted>
Date: 2015年01月30日 12:14

On Fri, 30 Jan 2015 02:14:45 +0000, Al Viro wrote:

quoted

On Fri, Jan 30, 2015 at 09:44:03AM +0800, Qu Wenruo wrote:

quoted

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

quoted

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

quoted

As for "it won't go r/o under us" - sb_want_write() will do that just fine.
.