From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1034657AbcIVVBC (ORCPT ); Thu, 22 Sep 2016 17:01:02 -0400 Received: from aserp1040.oracle.com ([141.146.126.69]:40810 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1034494AbcIVVBB (ORCPT ); Thu, 22 Sep 2016 17:01:01 -0400 Subject: Re: [PATCH v3 2/3] xen/pciback: avoid multiple entries in slot list To: Juergen Gross , linux-kernel@vger.kernel.org, xen-devel@lists.xenproject.org References: <1474533941-6976-1-git-send-email-jgross@suse.com> <1474533941-6976-3-git-send-email-jgross@suse.com> Cc: david.vrabel@citrix.com From: Boris Ostrovsky Message-ID: Date: Thu, 22 Sep 2016 17:02:40 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: <1474533941-6976-3-git-send-email-jgross@suse.com> Content-Type: text/plain; charset=windows-1252 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 On 09/22/2016 04:45 AM, Juergen Gross wrote: > The Xen pciback driver has a list of all pci devices it is ready to > seize. There is no check whether a to be added entry already exists. > While this might be no problem in the common case it might confuse > those which consume the list via sysfs. > > Modify the handling of this list by not adding an entry which already > exists. As this will be needed later split out the list handling into > a separate function. > > Signed-off-by: Juergen Gross > --- > drivers/xen/xen-pciback/pci_stub.c | 39 ++++++++++++++++++++++++++++++-------- > 1 file changed, 31 insertions(+), 8 deletions(-) > > diff --git a/drivers/xen/xen-pciback/pci_stub.c b/drivers/xen/xen-pciback/pci_stub.c > index 79a9e4d..0179333 100644 > --- a/drivers/xen/xen-pciback/pci_stub.c > +++ b/drivers/xen/xen-pciback/pci_stub.c > @@ -478,6 +478,36 @@ static int __init pcistub_init_devices_late(void) > return 0; > } > > +static void pcistub_device_id_add_list(struct pcistub_device_id *new, > + int domain, int bus, unsigned int devfn) > +{ > + struct pcistub_device_id *pci_dev_id; > + unsigned long flags; > + int found = 0; > + > + spin_lock_irqsave(&device_ids_lock, flags); > + > + list_for_each_entry(pci_dev_id, &pcistub_device_ids, slot_list) { > + if (pci_dev_id->domain == domain && pci_dev_id->bus == bus && > + pci_dev_id->devfn == devfn) { > + found = 1; > + break; > + } > + } > + > + if (!found) { > + new->domain = domain; > + new->bus = bus; > + new->devfn = devfn; > + list_add_tail(&new->slot_list, &pcistub_device_ids); > + } > + > + spin_unlock_irqrestore(&device_ids_lock, flags); > + > + if (found) > + kfree(new); I'd rather free 'new' in the caller (who allocated it) and return something like -EEXIST if device is already on the list. -boris > +} > + > static int pcistub_seize(struct pci_dev *dev) > { > struct pcistub_device *psdev; > @@ -1012,7 +1042,6 @@ static inline int str_to_quirk(const char *buf, int *domain, int *bus, int > static int pcistub_device_id_add(int domain, int bus, int slot, int func) > { > struct pcistub_device_id *pci_dev_id; > - unsigned long flags; > int rc = 0, devfn = PCI_DEVFN(slot, func); > > if (slot < 0) { > @@ -1042,16 +1071,10 @@ static int pcistub_device_id_add(int domain, int bus, int slot, int func) > if (!pci_dev_id) > return -ENOMEM; > > - pci_dev_id->domain = domain; > - pci_dev_id->bus = bus; > - pci_dev_id->devfn = devfn; > - > pr_debug("wants to seize %04x:%02x:%02x.%d\n", > domain, bus, slot, func); > > - spin_lock_irqsave(&device_ids_lock, flags); > - list_add_tail(&pci_dev_id->slot_list, &pcistub_device_ids); > - spin_unlock_irqrestore(&device_ids_lock, flags); > + pcistub_device_id_add_list(pci_dev_id, domain, bus, devfn); > > return 0; > }