Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752711AbbGKLR4 (ORCPT ); Sat, 11 Jul 2015 07:17:56 -0400 Received: from userp1040.oracle.com ([156.151.31.81]:26085 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751937AbbGKLRz convert rfc822-to-8bit (ORCPT ); Sat, 11 Jul 2015 07:17:55 -0400 User-Agent: K-9 Mail for Android In-Reply-To: <55A0D162.1020002@oracle.com> References: <1436176608-18237-1-git-send-email-bob.liu@oracle.com> <20150710195714.GB31146@l.oracle.com> <55A0D162.1020002@oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Subject: Re: [RESEND PATCH] xen/blkfront: convert to blk-mq APIs From: Konrad Rzeszutek Wilk Date: Sat, 11 Jul 2015 07:17:18 -0400 To: Bob Liu 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 Message-ID: X-Source-IP: aserv0022.oracle.com [141.146.126.234] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2347 Lines: 83 On July 11, 2015 4:18:42 AM EDT, Bob Liu wrote: > >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? The 'locked' parameter can be used to tell the function to not take the lock. But it would drop the lock in both cases. > >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 >> -- 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/