On Thu, Apr 02, 2020 at 04:49:37PM +0200, Nicolai Stange wrote: > Bart Van Assche <bvanassche@xxxxxxx> writes: > > > On 2020-04-01 17:00, Luis Chamberlain wrote: > > The description of this patch mentions a single blk_release_queue() call > > that happened in the past from a context from which sleeping is not > > allowed and from which sleeping is allowed today. Have all other > > blk_release_queue() / blk_put_queue() calls been verified to see whether > > none of these happens from a context from which sleeping is not allowed? > > I've just done this today and found the following potentially > problematic call paths to blk_put_queue(). > > 1.) mem_cgroup_throttle_swaprate() takes a spinlock and > calls blkcg_schedule_throttle()->blk_put_queue(). > > Also note that AFAICS mem_cgroup_try_charge_delay() can be called > with GFP_ATOMIC. I have a solution to this which would avoid having to deal with the concern completely. I'll post in my follow up. > 2.) scsi_unblock_requests() gets called from a lot of drivers and > invoke blk_put_queue() through > scsi_unblock_requests() -> scsi_run_host_queues() -> > scsi_starved_list_run() -> blk_put_queue(). sd_probe() calls device_add_disk(), and the scsi lib also has its own refcounting for scsi, but unless you call sd_remove() you'll be protecting the underlying block disk and request_queue, as sd_remove() calls the del_gendisk() which would in call call blk_unregister_queue() which calls the last blk_put_queue(). If sd_remove() can be called from atomic context we can also fix this, and this should be evident how in my next follow up series of patches. Luis
