On Tue 29-11-16 11:43:50, Tejun Heo wrote: > Hello, Jan. > > On Tue, Nov 29, 2016 at 10:30:35AM +0100, Jan Kara wrote: > > > It's kinda weird that sync() is ends up accessing bdev's without any > > > synchronization. Can't we just make iterate_bdevs() grab bd_mutex and > > > verify bd_disk isn't NULL before calling into the callback? > > > > This reminded me I've already seen something like this and indeed I've > > already had a very similar discussion in March - > > https://patchwork.kernel.org/patch/8556941/ > > lol > > > Holding bd_mutex in iterate_devs() works but still nothing protects from > > flusher thread just walking across the block device inode and trying to > > write it which would result in the very same oops. > > Ah, right. We aren't implementing either sever or refcnted draining > semantics properly. I wonder whether we'd be able to retire the inode > synchronously during blkdev_put. Yeah, I've realized flusher thread is mostly taken care of by the fact that __blkdev_put() does bdev_write_inode() which waits for I_SYNC to get cleared and then the inode is clean so writeback code mostly ignores it. It is fragile but likely it works. So for now I've decided to just push the patch mentioned above to get at least obvious breakage fixed as playing with bdev lifetime rules definitely won't be a stable material anyway. I was also thinking about completely tearing down bdev inode in __blkdev_put() and it could be doable although hunting all inodes referencing bdev inode through i_bdev will be tricky. Probably we'll need i_devices things for block devices back... Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html