On Fri, Jan 30, 2015 at 12:14:24PM +0800, Miao Xie wrote: > 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. First of all, ->s_umount is not a mutex; it's rwsem. So you mean down_read_trylock(). As for the transient failures - grep for down_write on it... E.g. have somebody call mount() from the same device. We call sget(), which finds existing superblock and calls grab_super(). Sure, that ->s_umount will be released shortly, but in the meanwhile your trylock will fail... > I think sb_want_write() is similar to trylock(s_umount), the difference is that > sb_want_write() is more complex. > > > > > 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(); So move them... It's a matter of moving one function call around a bit. -- 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
