LOCKDEP warning due to ext_devt_lock

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

LOCKDEP warning due to ext_devt_lock

NeilBrown

Hi Keith,
 I'm getting lockdep warning around use of ext_devt_lock.

[11428.266019] inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.

 As you say in
    2da78092dda1 ("block: Fix dev_t minor allocation lifetime")
ext_devt_lock can be taken from call_rcu's soft-irq.
I think that means it should always use the _irq verion of spinlock.

I've been using the following patch and the LOCKDEP message are gone.

If you agree, could you forward it or something similar upstream?
Thanks,
NeilBrown


Signed-off-by: NeilBrown <[hidden email]>
diff --git a/block/genhd.c b/block/genhd.c
index 666e11b83983..d1338934562e 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -422,9 +422,9 @@ int blk_alloc_devt(struct hd_struct *part, dev_t *devt)
  /* allocate ext devt */
  idr_preload(GFP_KERNEL);
 
- spin_lock(&ext_devt_lock);
+ spin_lock_irq(&ext_devt_lock);
  idx = idr_alloc(&ext_devt_idr, part, 0, NR_EXT_DEVT, GFP_NOWAIT);
- spin_unlock(&ext_devt_lock);
+ spin_unlock_irq(&ext_devt_lock);
 
  idr_preload_end();
  if (idx < 0)
@@ -449,9 +449,9 @@ void blk_free_devt(dev_t devt)
  return;
 
  if (MAJOR(devt) == BLOCK_EXT_MAJOR) {
- spin_lock(&ext_devt_lock);
+ spin_lock_irq(&ext_devt_lock);
  idr_remove(&ext_devt_idr, blk_mangle_minor(MINOR(devt)));
- spin_unlock(&ext_devt_lock);
+ spin_unlock_irq(&ext_devt_lock);
  }
 }
 
@@ -690,13 +690,13 @@ struct gendisk *get_gendisk(dev_t devt, int *partno)
  } else {
  struct hd_struct *part;
 
- spin_lock(&ext_devt_lock);
+ spin_lock_irq(&ext_devt_lock);
  part = idr_find(&ext_devt_idr, blk_mangle_minor(MINOR(devt)));
  if (part && get_disk(part_to_disk(part))) {
  *partno = part->partno;
  disk = part_to_disk(part);
  }
- spin_unlock(&ext_devt_lock);
+ spin_unlock_irq(&ext_devt_lock);
  }
 
  return disk;

attachment0 (828 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: LOCKDEP warning due to ext_devt_lock

Jens Axboe-3
On 05/27/2015 08:32 PM, NeilBrown wrote:

>
> Hi Keith,
>   I'm getting lockdep warning around use of ext_devt_lock.
>
> [11428.266019] inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
>
>   As you say in
>      2da78092dda1 ("block: Fix dev_t minor allocation lifetime")
> ext_devt_lock can be taken from call_rcu's soft-irq.
> I think that means it should always use the _irq verion of spinlock.

Wouldn't spin_lock_bh() and friends suffice then?

--
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|

Re: LOCKDEP warning due to ext_devt_lock

NeilBrown
On Wed, 27 May 2015 21:32:21 -0600 Jens Axboe <[hidden email]> wrote:

> On 05/27/2015 08:32 PM, NeilBrown wrote:
> >
> > Hi Keith,
> >   I'm getting lockdep warning around use of ext_devt_lock.
> >
> > [11428.266019] inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
> >
> >   As you say in
> >      2da78092dda1 ("block: Fix dev_t minor allocation lifetime")
> > ext_devt_lock can be taken from call_rcu's soft-irq.
> > I think that means it should always use the _irq verion of spinlock.
>
> Wouldn't spin_lock_bh() and friends suffice then?
>
Quite possibly.  I've never actually used those so I don't tend to think
about them.  But only reflection I think they would be exactly right.
I'll modify my patch for future testing.

BTW, did you get my:

   [PATCH v2] block: discard bdi_unregister() in favour of bdi_destroy()

??  I haven't seen it appear in your 'block' tree.

Thanks,
NeilBrown

attachment0 (828 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: LOCKDEP warning due to ext_devt_lock

Jens Axboe-3
On 05/27/2015 09:45 PM, NeilBrown wrote:

> On Wed, 27 May 2015 21:32:21 -0600 Jens Axboe <[hidden email]> wrote:
>
>> On 05/27/2015 08:32 PM, NeilBrown wrote:
>>>
>>> Hi Keith,
>>>    I'm getting lockdep warning around use of ext_devt_lock.
>>>
>>> [11428.266019] inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
>>>
>>>    As you say in
>>>       2da78092dda1 ("block: Fix dev_t minor allocation lifetime")
>>> ext_devt_lock can be taken from call_rcu's soft-irq.
>>> I think that means it should always use the _irq verion of spinlock.
>>
>> Wouldn't spin_lock_bh() and friends suffice then?
>>
>
> Quite possibly.  I've never actually used those so I don't tend to think
> about them.  But only reflection I think they would be exactly right.
> I'll modify my patch for future testing.
>
> BTW, did you get my:
>
>     [PATCH v2] block: discard bdi_unregister() in favour of bdi_destroy()
>
> ??  I haven't seen it appear in your 'block' tree.

I missed that, thanks for the reminder. I've queued it up for this series.

--
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/