Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752259AbbKPXNs (ORCPT ); Mon, 16 Nov 2015 18:13:48 -0500 Received: from aserp1040.oracle.com ([141.146.126.69]:31654 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751474AbbKPXNq (ORCPT ); Mon, 16 Nov 2015 18:13:46 -0500 Message-ID: <564A631E.7040905@oracle.com> Date: Tue, 17 Nov 2015 07:13:34 +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: xen-devel@lists.xen.org, linux-kernel@vger.kernel.org, roger.pau@citrix.com, felipe.franciosi@citrix.com, axboe@fb.com, avanzini.arianna@gmail.com, rafal.mielniczuk@citrix.com, jonathan.davies@citrix.com, david.vrabel@citrix.com Subject: Re: [PATCH v5 05/10] xen/blkfront: negotiate number of queues/rings to be used with backend References: <1447470739-18136-1-git-send-email-bob.liu@oracle.com> <1447470739-18136-6-git-send-email-bob.liu@oracle.com> <20151116212702.GA12823@char.us.oracle.com> In-Reply-To: <20151116212702.GA12823@char.us.oracle.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit 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: 2404 Lines: 73 On 11/17/2015 05:27 AM, Konrad Rzeszutek Wilk wrote: >> /* Common code used when first setting up, and when resuming. */ >> static int talk_to_blkback(struct xenbus_device *dev, >> @@ -1527,10 +1582,9 @@ static int talk_to_blkback(struct xenbus_device *dev, >> { >> const char *message = NULL; >> struct xenbus_transaction xbt; >> - int err, i; >> - unsigned int max_page_order = 0; >> + int err; >> + unsigned int i, max_page_order = 0; >> unsigned int ring_page_order = 0; >> - struct blkfront_ring_info *rinfo; > > Why? You end up doing the 'struct blkfront_ring_info' decleration > in two of the loops below? Oh, that's because Roger mentioned we would be tempted to declare rinfo only inside the for loop, to limit the scope. >> >> err = xenbus_scanf(XBT_NIL, info->xbdev->otherend, >> "max-ring-page-order", "%u", &max_page_order); >> @@ -1542,7 +1596,8 @@ static int talk_to_blkback(struct xenbus_device *dev, >> } >> >> for (i = 0; i < info->nr_rings; i++) { >> - rinfo = &info->rinfo[i]; >> + struct blkfront_ring_info *rinfo = &info->rinfo[i]; >> + > > Here.. > >> @@ -1617,7 +1677,7 @@ again: >> >> for (i = 0; i < info->nr_rings; i++) { >> int j; >> - rinfo = &info->rinfo[i]; >> + struct blkfront_ring_info *rinfo = &info->rinfo[i]; > > And here? > > It is not a big deal but I am curious of why add this change? > >> @@ -1717,7 +1789,6 @@ static int blkfront_probe(struct xenbus_device *dev, >> >> mutex_init(&info->mutex); >> spin_lock_init(&info->dev_lock); >> - info->xbdev = dev; > > That looks like a spurious change? Ah, I see that we do the same exact > operation earlier in the blkfront_probe. > The place of this line was changed because: 1738 info->xbdev = dev; 1739 /* Check if backend supports multiple queues. */ 1740 err = xenbus_scanf(XBT_NIL, info->xbdev->otherend, ^^^^ We need xbdev to be set in advance. 1741 "multi-queue-max-queues", "%u", &backend_max_queues); 1742 if (err < 0) 1743 backend_max_queues = 1; -- 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/