From patchwork Wed Oct 6 21:44:40 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Michael S. Tsirkin" X-Patchwork-Id: 66984 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [199.232.76.165]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id EE9BCB6F0D for ; Thu, 7 Oct 2010 09:13:56 +1100 (EST) Received: from localhost ([127.0.0.1]:59652 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1P3cEf-0000Xs-5l for incoming@patchwork.ozlabs.org; Wed, 06 Oct 2010 18:13:17 -0400 Received: from [140.186.70.92] (port=47834 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1P3c5K-0005nL-4M for qemu-devel@nongnu.org; Wed, 06 Oct 2010 18:03:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1P3bss-0002V6-AS for qemu-devel@nongnu.org; Wed, 06 Oct 2010 17:50:47 -0400 Received: from mx1.redhat.com ([209.132.183.28]:32609) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1P3bss-0002V0-2k for qemu-devel@nongnu.org; Wed, 06 Oct 2010 17:50:46 -0400 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o96Loj7p020150 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 6 Oct 2010 17:50:45 -0400 Received: from redhat.com (vpn-8-214.rdu.redhat.com [10.11.8.214]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with SMTP id o96LogLN013925; Wed, 6 Oct 2010 17:50:42 -0400 Date: Wed, 6 Oct 2010 23:44:40 +0200 From: "Michael S. Tsirkin" To: Alex Williamson Message-ID: <20101006214440.GA17149@redhat.com> References: <20101006145650.GA10968@redhat.com> <1286383724.3020.8.camel@x201> <20101006170222.GB13486@redhat.com> <1286385864.3020.26.camel@x201> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1286385864.3020.26.camel@x201> User-Agent: Mutt/1.5.21 (2010-09-15) X-Scanned-By: MIMEDefang 2.67 on 10.5.11.11 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. Cc: kvm@vger.kernel.org, Juan Quintela , mtosatti@redhat.com, qemu-devel@nongnu.org, avi@redhat.com, Amit Shah Subject: [Qemu-devel] Re: [PATCHv2] qemu-kvm/vhost: fix up irqfd support X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On Wed, Oct 06, 2010 at 11:24:24AM -0600, Alex Williamson wrote: > You could always keep the functions as separate wrapper callers of the > common function so you only need to keep true = unset, false = set > straight in one place. Thanks, Just to show why it does not work, I did exactly this: as you see the code is shorter but the true/false magic gets spread: it was in 2 places, (set/unset) now it is in 4 places and it is within the loop, in code that is more complex. So I think I'll stick to the original version and we can patch it up later if there's a will. diff --git a/hw/msix.c b/hw/msix.c index 3d4dd61..4b705a0 100644 --- a/hw/msix.c +++ b/hw/msix.c @@ -583,40 +583,15 @@ void msix_unuse_all_vectors(PCIDevice *dev) msix_free_irq_entries(dev); } -static int msix_set_mask_notifier_for_vector(PCIDevice *dev, unsigned vector) +/* Invoke the notifier if vector entry is used and unmasked. */ +static int msix_notify_if_unmasked(PCIDevice *dev, unsigned vector, bool masked) { int r = 0; - if (vector >= dev->msix_entries_nr || !dev->msix_entry_used[vector]) + if (!dev->msix_entry_used[vector] || msix_is_masked(dev, vector)) { return 0; - - assert(dev->msix_mask_notifier); - - /* Unmask the new notifier unless vector is masked. */ - if (!msix_is_masked(dev, vector)) { - r = dev->msix_mask_notifier(dev, vector, false); - if (r < 0) { - return r; - } } - return r; -} - -static int msix_unset_mask_notifier_for_vector(PCIDevice *dev, unsigned vector) -{ - int r = 0; - if (vector >= dev->msix_entries_nr || !dev->msix_entry_used[vector]) - return 0; - assert(dev->msix_mask_notifier); - - /* Mask the old notifier unless it is already masked. */ - if (!msix_is_masked(dev, vector)) { - r = dev->msix_mask_notifier(dev, vector, true); - if (r < 0) { - return r; - } - } - return r; + return dev->msix_mask_notifier(dev, vector, masked); } int msix_set_mask_notifier(PCIDevice *dev, msix_mask_notifier_func f) @@ -625,7 +600,7 @@ int msix_set_mask_notifier(PCIDevice *dev, msix_mask_notifier_func f) assert(!dev->msix_mask_notifier); dev->msix_mask_notifier = f; for (n = 0; n < dev->msix_entries_nr; ++n) { - r = msix_set_mask_notifier_for_vector(dev, n); + r = msix_notify_if_unmasked(dev, n, false); if (r < 0) { goto undo; } @@ -634,7 +609,7 @@ int msix_set_mask_notifier(PCIDevice *dev, msix_mask_notifier_func f) undo: while (--n >= 0) { - msix_unset_mask_notifier_for_vector(dev, n); + msix_notify_if_unmasked(dev, n, true); } dev->msix_mask_notifier = NULL; return r; @@ -645,7 +620,7 @@ int msix_unset_mask_notifier(PCIDevice *dev) int r, n; assert(dev->msix_mask_notifier); for (n = 0; n < dev->msix_entries_nr; ++n) { - r = msix_unset_mask_notifier_for_vector(dev, n); + r = msix_notify_if_unmasked(dev, n, true); if (r < 0) { goto undo; } @@ -655,7 +630,7 @@ int msix_unset_mask_notifier(PCIDevice *dev) undo: while (--n >= 0) { - msix_set_mask_notifier_for_vector(dev, n); + msix_notify_if_unmasked(dev, n, false); } return r; }