Re: [RFC PATCH 1/5] block: Introduce q->abort_queue_fn()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux