From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752758AbbBSCI1 (ORCPT ); Wed, 18 Feb 2015 21:08:27 -0500 Received: from aserp1040.oracle.com ([141.146.126.69]:28603 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751942AbbBSCI0 (ORCPT ); Wed, 18 Feb 2015 21:08:26 -0500 Message-ID: <54E54579.1060007@oracle.com> Date: Thu, 19 Feb 2015 10:07:53 +0800 From: Bob Liu User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130308 Thunderbird/17.0.4 MIME-Version: 1.0 To: Christoph Hellwig 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, avanzini.arianna@gmail.com Subject: Re: [PATCH 03/10] xen/blkfront: reorg info->io_lock after using blk-mq API References: <1423988345-4005-1-git-send-email-bob.liu@oracle.com> <1423988345-4005-4-git-send-email-bob.liu@oracle.com> <20150218170552.GC17813@infradead.org> In-Reply-To: <20150218170552.GC17813@infradead.org> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-Source-IP: aserv0021.oracle.com [141.146.126.233] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 02/19/2015 01:05 AM, Christoph Hellwig wrote: > 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; > } > Thank you! Will be updated. > Also this really should be merged into the first patch. > I thought it would be easier for people to review by split into three patches. Anyway, I can merge them into the first one. -- Regards, -Bob