On Fri, 30 Jan 2015 04:37:14 +0000, Al Viro wrote: > 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 know it, so I suggested to return -EBUSY in the previous mail. I think it is acceptable method, mount/umount operations are not so many after all. Thanks Miao > >> 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
