From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752570AbbBRRF4 (ORCPT ); Wed, 18 Feb 2015 12:05:56 -0500 Received: from bombadil.infradead.org ([198.137.202.9]:55350 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751571AbbBRRFz (ORCPT ); Wed, 18 Feb 2015 12:05:55 -0500 Date: Wed, 18 Feb 2015 09:05:52 -0800 From: Christoph Hellwig To: Bob Liu Cc: xen-devel@lists.xen.org, david.vrabel@citrix.com, linux-kernel@vger.kernel.org, roger.pau@citrix.com, konrad.wilk@oracle.com, felipe.franciosi@citrix.com, axboe@fb.com, hch@infradead.org, avanzini.arianna@gmail.com Subject: Re: [PATCH 03/10] xen/blkfront: reorg info->io_lock after using blk-mq API Message-ID: <20150218170552.GC17813@infradead.org> References: <1423988345-4005-1-git-send-email-bob.liu@oracle.com> <1423988345-4005-4-git-send-email-bob.liu@oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1423988345-4005-4-git-send-email-bob.liu@oracle.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-SRS-Rewrite: SMTP reverse-path rewritten from by bombadil.infradead.org See http://www.infradead.org/rpr.html Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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.