Hello, On Thu, Apr 12, 2018 at 04:03:52PM +0000, Bart Van Assche wrote: > On Thu, 2018-04-12 at 08:37 -0700, Tejun Heo wrote: > > On Thu, Apr 12, 2018 at 08:09:17AM -0600, Bart Van Assche wrote: > > > I have retested hotunplugging by rerunning the srp-test software. It > > > seems like you overlooked that this patch does not remove the > > > blkcg_exit_queue() call from blk_cleanup_queue()? If a device is > > > hotunplugged it is up to the block driver to call > > > blk_cleanup_queue(). And blk_cleanup_queue() will call > > > blkcg_exit_queue(). > > > > Hmm... what'd prevent blg_lookup_and_create() racing against that? > > Hello Tejun, > > Did you perhaps mean blkg_lookup_create()? That function has one caller, Ah yeah, sorry about the sloppiness. > namely blkcg_bio_issue_check(). The only caller of that function is > generic_make_request_checks(). A patch was posted on the linux-block mailing > list recently that surrounds that call with blk_queue_enter() / blk_queue_exit(). > I think that prevents that blkcg_exit_queue() is called concurrently with > blkg_lookup_create(). Yeah, that'd solve the problem for that particular path, but what that's doing is adding another layer of refcnting which is used to implement the revoke (or sever) semantics. This is a fragile approach tho because it isn't clear who's protecting what and there's nothing in blkg's usage which suggests it'd be protected that way and we're basically working around a different underlying problem (lock switching) by expanding the coverage of a different lock. I'd much prefer fixing the lock switching problem properly than working around that shortcoming this way. Thans. -- tejun
