Re: [bug] as_merged_requests(): possible recursive locking detected

[Nikanth Karthikesan]

On Thu, 2008-01-31 at 23:14 +0100, Ingo Molnar wrote:
> 
> Jens,
> 
> AS still has some locking issues - see the lockdep warning below that 
> the x86 test-rig just triggered. Config attached. Never saw this one 
> before. Can send more info if needed.
> 

The io_contexts are swapped. And while swapping, the locks were also
getting swapped, which will change the order of locking after that. This
may be the cause of these warning. I am not sure whether not swapping
the locks is the right way to fix this. Using a field of spinlock_t
itself to order locking might be better, instead of the address of the
container.

Now while adding a new member to io_context, one should not forget to
add it here. Also copying whole io_context and then restoring the locks
might have a window where this warning could be triggered.

Thanks
Nikanth Karthikesan

Do not swap locks while swapping io_contexts

Signed-off-by: Nikanth Karthikesan <knikanth@xxxxxxx>

---
diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index 6d16755..b9c6e39 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -179,9 +179,32 @@ EXPORT_SYMBOL(copy_io_context);
 void swap_io_context(struct io_context **ioc1, struct io_context **ioc2)
 {
 	struct io_context *temp;
+
+	/*
+	 * Do not swap the locks to preserve locking order
+	 */
+
 	temp = *ioc1;
-	*ioc1 = *ioc2;
-	*ioc2 = temp;
+
+	(*ioc1)->refcount = (*ioc2)->refcount;
+	(*ioc1)->nr_tasks = (*ioc2)->nr_tasks;
+	(*ioc1)->ioprio = (*ioc2)->ioprio;
+	(*ioc1)->ioprio_changed = (*ioc2)->ioprio_changed;
+	(*ioc1)->last_waited = (*ioc2)->last_waited;
+	(*ioc1)->nr_batch_requests = (*ioc2)->nr_batch_requests;
+	(*ioc1)->aic = (*ioc2)->aic;
+	(*ioc1)->radix_root = (*ioc2)->radix_root;
+	(*ioc1)->ioc_data = (*ioc2)->ioc_data;
+
+	(*ioc2)->refcount = (temp)->refcount;
+	(*ioc2)->nr_tasks = (temp)->nr_tasks;
+	(*ioc2)->ioprio = (temp)->ioprio;
+	(*ioc2)->ioprio_changed = (temp)->ioprio_changed;
+	(*ioc2)->last_waited = (temp)->last_waited;
+	(*ioc2)->nr_batch_requests = (temp)->nr_batch_requests;
+	(*ioc2)->aic = (temp)->aic;
+	(*ioc2)->radix_root = (temp)->radix_root;
+	(*ioc2)->ioc_data = (temp)->ioc_data;
 }
 EXPORT_SYMBOL(swap_io_context);
 


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