Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754125AbaA1Ai0 (ORCPT ); Mon, 27 Jan 2014 19:38:26 -0500 Received: from ozlabs.org ([203.10.76.45]:52622 "EHLO ozlabs.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751571AbaA1AiZ (ORCPT ); Mon, 27 Jan 2014 19:38:25 -0500 From: Rusty Russell To: Leandro Dorileo , linux-kernel@vger.kernel.org Cc: virtio-dev@lists.oasis-open.org, virtualization@lists.linux-foundation.org, mst@redhat.com, Leandro Dorileo Subject: Re: [PATCH] virtio_balloon: don't call virtio_has_feature() twice on init_vqs() In-Reply-To: <1390679972-1406-1-git-send-email-l@dorileo.org> References: <1390679972-1406-1-git-send-email-l@dorileo.org> User-Agent: Notmuch/0.15.2 (http://notmuchmail.org) Emacs/23.4.1 (x86_64-pc-linux-gnu) Date: Tue, 28 Jan 2014 10:48:31 +1030 Message-ID: <87ppndyn54.fsf@rustcorp.com.au> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Leandro Dorileo writes: > Cchange init_vqs() to avoid calling twice the virtio_has_feature() > - attempting to find out if VIRTIO_BALLOON_F_STATS_VQ feature was negotiated - > consequently we prevent unnecessarily running the drivers' feature_table more > than needed. > > Signed-off-by: Leandro Dorileo Hi Leandro, This seems like a premature optimization. The current code is fairly clear, and there's no performance issue with init_vqs. Am I missing something? Rusty. > --- > drivers/virtio/virtio_balloon.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c > index 34bdaba..41771c1 100644 > --- a/drivers/virtio/virtio_balloon.c > +++ b/drivers/virtio/virtio_balloon.c > @@ -320,19 +320,21 @@ static int init_vqs(struct virtio_balloon *vb) > vq_callback_t *callbacks[] = { balloon_ack, balloon_ack, stats_request }; > const char *names[] = { "inflate", "deflate", "stats" }; > int err, nvqs; > + bool stats; > > /* > * We expect two virtqueues: inflate and deflate, and > * optionally stat. > */ > - nvqs = virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ) ? 3 : 2; > + stats = virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ); > + nvqs = stats ? 3 : 2; > err = vb->vdev->config->find_vqs(vb->vdev, nvqs, vqs, callbacks, names); > if (err) > return err; > > vb->inflate_vq = vqs[0]; > vb->deflate_vq = vqs[1]; > - if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ)) { > + if (stats) { > struct scatterlist sg; > vb->stats_vq = vqs[2]; > > -- > 1.8.5.3 -- 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/