Messages in this thread Patch in this message | | | Date | Mon, 3 Jun 2002 10:39:37 +0200 | From | Jens Axboe <> | Subject | Re: [patch 1/16] unplugging fix |
| |
On Sun, Jun 02 2002, Jens Axboe wrote: > On Sun, Jun 02 2002, Andrew Morton wrote: > > Andrew Morton wrote: > > > > > > There's a plugging bug in 2.5.19. Once you start pushing several disks > > > hard, the new unplug code gets confused and queues are left in plugged > > > state, but not on the plug list. They never get unplugged and the > > > machine dies mysteriously. > > > > > > > This patch didn't fix it. It made it tons better, but I again have a > > wedged box. Same symptoms - against IDE this time. > > > > blk_plug_list is empty. queue_flags=0x03. Interestingly, > > q->plug_list is non-empty, non-zero, both list members pointing at > > a list which isn't either itself or blk_plug_list. > > > > I note that the code isn't taking queues off the plug list when the queue > > is destroyed. Guess that doesn't matter - we never destroy a plugged > > queue... > > > > This one is killing me. > > I've got a good handle on how to clean the whole plugging thing up, I > suspect it will make this case easier to fix. I'll be back with that > tomorrow, still got guests...
Does this work? I can't poke holes in it, but then again...
--- /opt/kernel/linux-2.5.20/drivers/block/ll_rw_blk.c 2002-06-03 10:35:35.000000000 +0200 +++ linux/drivers/block/ll_rw_blk.c 2002-06-03 10:37:26.000000000 +0200 @@ -821,7 +821,7 @@ /* * not plugged */ - if (!__test_and_clear_bit(QUEUE_FLAG_PLUGGED, &q->queue_flags)) + if (!test_and_clear_bit(QUEUE_FLAG_PLUGGED, &q->queue_flags)) return; if (test_bit(QUEUE_FLAG_STOPPED, &q->queue_flags)) @@ -893,6 +893,17 @@ **/ void blk_stop_queue(request_queue_t *q) { + /* + * remove from the plugged list, queue must not be called. + */ + if (test_and_clear_bit(QUEUE_FLAG_PLUGGED, &q->queue_flags)) { + unsigned long flags; + + spin_lock_irqsave(&blk_plug_lock, flags); + list_del(&q->plug_list); + spin_unlock_irqrestore(&blk_plug_lock, flags); + } + set_bit(QUEUE_FLAG_STOPPED, &q->queue_flags); } @@ -904,45 +915,36 @@ * are currently stopped are ignored. This is equivalent to the older * tq_disk task queue run. **/ +#define blk_plug_entry(entry) list_entry((entry), request_queue_t, plug_list) void blk_run_queues(void) { - struct list_head *n, *tmp, local_plug_list; - unsigned long flags; + struct list_head local_plug_list; INIT_LIST_HEAD(&local_plug_list); + spin_lock_irq(&blk_plug_lock); + /* * this will happen fairly often */ - spin_lock_irqsave(&blk_plug_lock, flags); if (list_empty(&blk_plug_list)) { - spin_unlock_irqrestore(&blk_plug_lock, flags); + spin_unlock_irq(&blk_plug_lock); return; } list_splice(&blk_plug_list, &local_plug_list); INIT_LIST_HEAD(&blk_plug_list); - spin_unlock_irqrestore(&blk_plug_lock, flags); + spin_unlock_irq(&blk_plug_lock); + + while (!list_empty(&local_plug_list)) { + request_queue_t *q = blk_plug_entry(local_plug_list.next); - /* - * local_plug_list is now a private copy we can traverse lockless - */ - list_for_each_safe(n, tmp, &local_plug_list) { - request_queue_t *q = list_entry(n, request_queue_t, plug_list); - - if (!test_bit(QUEUE_FLAG_STOPPED, &q->queue_flags)) { - list_del(&q->plug_list); - generic_unplug_device(q); - } - } + BUG_ON(test_bit(QUEUE_FLAG_STOPPED, &q->queue_flags)); - /* - * add any remaining queue back to plug list - */ - if (!list_empty(&local_plug_list)) { - spin_lock_irqsave(&blk_plug_lock, flags); - list_splice(&local_plug_list, &blk_plug_list); - spin_unlock_irqrestore(&blk_plug_lock, flags); + spin_lock_irq(q->queue_lock); + list_del(&q->plug_list); + __generic_unplug_device(q); + spin_unlock_irq(q->queue_lock); } } -- Jens Axboe
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |