[6/7] dm core: disable interrupt when taking map_lock

Commit Message

Kiyoshi Ueda April 24, 2009, 8:15 a.m. UTC

This patch disables interrupt when taking map_lock to prevent
needless lockdep warnings in request-based dm.

request-based dm takes map_lock after taking queue_lock with
disabling interrupt:
  spin_lock_irqsave(queue_lock)
  q->request_fn() == dm_request_fn()
    => dm_get_table()
         => read_lock(map_lock)
while queue_lock could be taken in interrupt context.

So lockdep warns that a deadlock can happen:
  write_lock(map_lock)
  <interrupt>
  spin_lock_irqsave(queue_lock)
  q->request_fn() == dm_request_fn()
    => dm_get_table()
         => read_lock(map_lock)

Currently there is no such code path in request-based dm where
q->request_fn() is called from interrupt context, so no such deadlock
happens.
But such warning messages confuse users, so prevent them by disabling
interrupt when taking map_lock.


Signed-off-by: Kiyoshi Ueda <[e-mail]>
Signed-off-by: Jun'ichi Nomura <[e-mail]>
Cc: Christof Schmitt <[e-mail]>
Cc: Alasdair G Kergon <[e-mail]>
---
 drivers/md/dm.c |   15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)


--
dm-devel mailing list
[e-mail]
https://www.redhat.com/mailman/listinfo/dm-devel

Comments

Hannes Reinecke April 24, 2009, 9 a.m. UTC | #1

Kiyoshi Ueda wrote:
> This patch disables interrupt when taking map_lock to prevent
> needless lockdep warnings in request-based dm.
> 
> request-based dm takes map_lock after taking queue_lock with
> disabling interrupt:
>   spin_lock_irqsave(queue_lock)
>   q->request_fn() == dm_request_fn()
>     => dm_get_table()
>          => read_lock(map_lock)
> while queue_lock could be taken in interrupt context.
> 
> So lockdep warns that a deadlock can happen:
>   write_lock(map_lock)
>   <interrupt>
>   spin_lock_irqsave(queue_lock)
>   q->request_fn() == dm_request_fn()
>     => dm_get_table()
>          => read_lock(map_lock)
> 
> Currently there is no such code path in request-based dm where
> q->request_fn() is called from interrupt context, so no such deadlock
> happens.
> But such warning messages confuse users, so prevent them by disabling
> interrupt when taking map_lock.
> 
> 
> Signed-off-by: Kiyoshi Ueda <[e-mail]>
> Signed-off-by: Jun'ichi Nomura <[e-mail]>
Acked-by: Hannes Reinecke <[e-mail]>

Cheers,

Hannes

Patch

Index: 2.6.30-rc3/drivers/md/dm.c
===================================================================
--- 2.6.30-rc3.orig/drivers/md/dm.c
+++ 2.6.30-rc3/drivers/md/dm.c
@@ -510,12 +510,13 @@  static void queue_io(struct mapped_devic
 struct dm_table *dm_get_table(struct mapped_device *md)
 {
 	struct dm_table *t;
+	unsigned long flags;
 
-	read_lock(&md->map_lock);
+	read_lock_irqsave(&md->map_lock, flags);
 	t = md->map;
 	if (t)
 		dm_table_get(t);
-	read_unlock(&md->map_lock);
+	read_unlock_irqrestore(&md->map_lock, flags);
 
 	return t;
 }
@@ -1940,6 +1941,7 @@  static int __bind(struct mapped_device *
 {
 	struct request_queue *q = md->queue;
 	sector_t size;
+	unsigned long flags;
 
 	size = dm_table_get_size(t);
 
@@ -1971,10 +1973,10 @@  static int __bind(struct mapped_device *
 
 	__bind_mempools(md, t);
 
-	write_lock(&md->map_lock);
+	write_lock_irqsave(&md->map_lock, flags);
 	md->map = t;
 	dm_table_set_restrictions(t, q);
-	write_unlock(&md->map_lock);
+	write_unlock_irqrestore(&md->map_lock, flags);
 
 	return 0;
 }
@@ -1982,14 +1984,15 @@  static int __bind(struct mapped_device *
 static void __unbind(struct mapped_device *md)
 {
 	struct dm_table *map = md->map;
+	unsigned long flags;
 
 	if (!map)
 		return;
 
 	dm_table_event_callback(map, NULL, NULL);
-	write_lock(&md->map_lock);
+	write_lock_irqsave(&md->map_lock, flags);
 	md->map = NULL;
-	write_unlock(&md->map_lock);
+	write_unlock_irqrestore(&md->map_lock, flags);
 	dm_table_destroy(map);
 }