Hello,
On Tue, May 22, 2012 at 03:30:37PM +0800, Asias He wrote:
> On 05/21/2012 11:42 PM, Tejun Heo wrote:
> 1) if the queue is stopped, q->request_fn() will never call called.
> we will be stuck in the loop forever. This can happen if the remove
> method is called after the q->request_fn() calls blk_stop_queue() to
> stop the queue when the device is full, and before the device
> interrupt handler to start the queue. This can be fixed by calling
> blk_start_queue() before __blk_run_queue(q).
>
> blk_drain_queue() {
> while(true) {
> ...
> if (!list_empty(&q->queue_head))
> __blk_run_queue(q);
> ...
> }
> }
Wouldn't that be properly fixed by making queue cleanup override
stopped state?
> 2) Since the device is gonna be removed, is it safe to rely on the
> device to finish the request before the DEAD marking? E.g, In
> vritio-blk, We reset the device and thus disable the interrupt
> before we call blk_cleanup_queue(). I also suspect that the real
> hardware can finish the pending requests when being hot-unplugged.
Yes, it should be safe (otherwise it's a driver bug). Device driver
already knows the state of the device it is driving. If the device
can't service requests for whatever reason, the device driver should
abort any in-flight and future requests. That's how other block
drivers behave and I don't see why virtio should be any different.
Also, blk_drain_queue() is used for other purposes too - elevator
switch and blkcg policy changes. You definitely don't want to be
aborting requests across those events.
So, NACK.
Thanks.
--
tejun
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
