Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753474AbbGKITa (ORCPT ); Sat, 11 Jul 2015 04:19:30 -0400 Received: from aserp1040.oracle.com ([141.146.126.69]:36575 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753331AbbGKIT3 (ORCPT ); Sat, 11 Jul 2015 04:19:29 -0400 Message-ID: <55A0D162.1020002@oracle.com> Date: Sat, 11 Jul 2015 16:18:42 +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: Konrad Rzeszutek Wilk CC: linux-kernel@vger.kernel.org, axboe@fb.com, hch@infradead.org, xen-devel@lists.xenproject.org, avanzini.arianna@gmail.com, david.vrabel@citrix.com, marcus.granado@citrix.com, roger.pau@citrix.com Subject: Re: [RESEND PATCH] xen/blkfront: convert to blk-mq APIs References: <1436176608-18237-1-git-send-email-bob.liu@oracle.com> <20150710195714.GB31146@l.oracle.com> In-Reply-To: <20150710195714.GB31146@l.oracle.com> 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 Content-Length: 2102 Lines: 71 On 07/11/2015 03:57 AM, Konrad Rzeszutek Wilk wrote: > On Mon, Jul 06, 2015 at 05:56:48PM +0800, Bob Liu wrote: >> From: Arianna Avanzini >> >> This patch converts xen-blkfront driver to use the block multiqueue APIs. >> Only one hardware queue is used now, so there is no performance change. >> >> The legacy non-mq code was deleted completely which is the same as other drivers >> like virtio, mtip, and nvme. >> >> Also dropped unnecessary holding of info->io_lock when calling into blk-mq APIs. > > Yeey! > > Two points: > > - The io_lock is now used to guard against concurrent access to the ring. > We should rename it to 'ring_lock'. > Sure. > - The kick_pending_request_queues should have an extra argument - 'bool locked'. > This is so that you don't drop and immediately grab the lock from the blkif_interrupt. > Then where to drop the lock? In kick_pending_request_queues(), the lock have to be dropped before calling blk_mq_start_stopped_hw_queues(). static void kick_pending_request_queues(struct blkfront_info *info) { + unsigned long flags; + + spin_lock_irqsave(&info->io_lock, flags); if (!RING_FULL(&info->ring)) { - /* Re-enable calldowns. */ - blk_start_queue(info->rq); - /* Kick things off immediately. */ - do_blkif_request(info->rq); + spin_unlock_irqrestore(&info->io_lock, flags); + blk_mq_start_stopped_hw_queues(info->rq, true); + return; } > See: > >> @@ -1243,9 +1243,8 @@ static irqreturn_t blkif_interrupt(int irq, void *dev_id) >> } else >> info->ring.sring->rsp_event = i + 1; >> >> - kick_pending_request_queues(info); >> - >> spin_unlock_irqrestore(&info->io_lock, flags); >> + kick_pending_request_queues(info); >> >> return IRQ_HANDLED; >> } > > Otherwise Reviewed-by: Konrad Rzeszutek Wilk > -- Regards, -Bob -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/