Hello, On Fri, Feb 19, 2016 at 09:58:11PM +0000, Al Viro wrote: > Um... What ordering requirements do you have? You obviously shouldn't > let it continue past the shutdown - as the matter of fact, you *can't* let > it continue past generic_shutdown_super(), since any inode references > held at evict_inodes() time will make it very unhappy. Attempts to do > any IO after that will make things a lot worse than unhappy - data structures > needed to do it might be gone (and if you hold a bit longer, filesystem > driver itself might very well be gone, along with the functions you were > going to call). It can be thought of as an extension of fs writeback operation and it'd be ideal if it can hold off sb shutdown as on-going writeback does through holding s_umount. Unfortunately, that doesn't seem possible because there's no way to transfer rwsem ownership. It doesn't generate any IO. The reason it's done asynchronously is because the operation requires an RCU grace period. After the grace period, it accesses only the generic inode and address_space and the only time it ends up accessing sb is through the iput call. Everything else AFAICS doesn't really care whether the underlying sb is shut down or not. > Grabbing ->s_active is a seriously bad idea for another reason - in > a situation when there's only one mount of given fs, plain umount() should > _not_ return 0 before fs shutdown is over. Sure, it is possible that there's > a binding somewhere, or that it's a lazy umount, etc., but those are "you've > asked for it" situations; having plain umount of e.g. ext3 on a USB stick > return success before it is safe to pull that stick out is a Bloody Bad Idea, > for obvious usability reasons. I see. > IOW, while fs shutdown may be async, making it *always* async would be a bad > bug. And bumping ->s_active does just that. > > I'd go for trylock inside that work + making generic_shutdown_super() > kill all such works. I assume that it *can* be abandoned in situation > when we know that sync_filesystem() is about to be called and that > said sync_filesystem() won't, in turn, schedule any such works, of course... I'll make generic_shutdown_super() to kill all such work items. I don't think the work item itself would need further locking tho. Can you please elaborate why you thought adding trylock to the work would be necessary? Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe cgroups" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
