Re: [PATCH 2/2] blk: Fix lock inversion between ioc lock and bfqd lock

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri 21-05-21 14:54:09, Ming Lei wrote:
> On Thu, May 20, 2021 at 08:29:49PM -0700, Khazhy Kumykov wrote:
> > On Thu, May 20, 2021 at 5:57 PM Ming Lei <ming.lei@xxxxxxxxxx> wrote:
> > >
> > > On Fri, May 21, 2021 at 12:33:53AM +0200, Jan Kara wrote:
> > > > Lockdep complains about lock inversion between ioc->lock and bfqd->lock:
> > > >
> > > > bfqd -> ioc:
> > > >  put_io_context+0x33/0x90 -> ioc->lock grabbed
> > > >  blk_mq_free_request+0x51/0x140
> > > >  blk_put_request+0xe/0x10
> > > >  blk_attempt_req_merge+0x1d/0x30
> > > >  elv_attempt_insert_merge+0x56/0xa0
> > > >  blk_mq_sched_try_insert_merge+0x4b/0x60
> > > >  bfq_insert_requests+0x9e/0x18c0 -> bfqd->lock grabbed
> > >
> > > We could move blk_put_request() into scheduler code, then the lock
> > > inversion is avoided. So far only mq-deadline and bfq calls into
> > > blk_mq_sched_try_insert_merge(), and this change should be small.
> > 
> > We'd potentially be putting multiple requests if we keep the recursive merge.
> 
> Oh, we still can pass a list to hold all requests to be freed, then free
> them all outside in scheduler code.

If we cannot really get rid of the recursive merge (not yet convinced),
this is also an option I've considered. I was afraid what can we use in
struct request to attach request to a list but it seems .merged_requests
handlers remove the request from the queuelist already so we should be fine
using that.

								Honza
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR



[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux