Messages in this thread | | | Date | Wed, 18 Feb 2015 09:05:52 -0800 | From | Christoph Hellwig <> | Subject | Re: [PATCH 03/10] xen/blkfront: reorg info->io_lock after using blk-mq API |
| |
On Sun, Feb 15, 2015 at 04:18:58PM +0800, Bob Liu wrote: > diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c > index 3589436..5a90a51 100644 > --- a/drivers/block/xen-blkfront.c > +++ b/drivers/block/xen-blkfront.c > @@ -614,25 +614,28 @@ static int blk_mq_queue_rq(struct blk_mq_hw_ctx *hctx, > blk_mq_start_request(qd->rq); > spin_lock_irq(&info->io_lock); > if (RING_FULL(&info->ring)) { > + spin_unlock_irq(&info->io_lock); > blk_mq_stop_hw_queue(hctx); > ret = BLK_MQ_RQ_QUEUE_BUSY; > goto out; > } > > if (blkif_request_flush_invalid(qd->rq, info)) { > + spin_unlock_irq(&info->io_lock); > ret = BLK_MQ_RQ_QUEUE_ERROR; > goto out; > } > > if (blkif_queue_request(qd->rq)) { > + spin_unlock_irq(&info->io_lock); > blk_mq_stop_hw_queue(hctx); > ret = BLK_MQ_RQ_QUEUE_BUSY; > goto out; > } > > flush_requests(info); > -out: > spin_unlock_irq(&info->io_lock); > +out: > return ret; > }
I'd rather write the function something like:
spin_lock_irq(&info->io_lock); if (RING_FULL(&info->ring)) goto out_busy; if (blkif_request_flush_invalid(qd->rq, info)) goto out_error; if (blkif_queue_request(qd->rq)) goto out_busy;
flush_requests(info); spin_unlock_irq(&info->io_lock); return BLK_MQ_RQ_QUEUE_OK; out_error: spin_unlock_irq(&info->io_lock); return BLK_MQ_RQ_QUEUE_ERROR; out_busy: spin_unlock_irq(&info->io_lock); blk_mq_stop_hw_queue(hctx); return BLK_MQ_RQ_QUEUE_BUSY; }
Also this really should be merged into the first patch.
| |