Re: [Xen-devel] [PATCH v3 2/3] xen/pciback: avoid multiple entries in slot list
On 23/09/16 12:15, David Vrabel wrote: > On 22/09/16 22:02, Boris Ostrovsky wrote: >> 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) > > I think this should allocate the new pcistub_device_id if needed. You > can pass in GFP flags if needed. > > Then it can return the newly allocated one, or the existing one. > > static struct pcistub_device_id *pcistub_device_id_add_list( > int domain, int bus, unsigned int devfn) Patch 3 will be very nasty then: in case of an allocation failure all of the actions done in pcistub_seize() will have to be undone again. I'd really like to avoid that. Juergen
Re: [Xen-devel] [PATCH v3 2/3] xen/pciback: avoid multiple entries in slot list
On 23/09/16 12:15, David Vrabel wrote: > On 22/09/16 22:02, Boris Ostrovsky wrote: >> 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) > > I think this should allocate the new pcistub_device_id if needed. You > can pass in GFP flags if needed. > > Then it can return the newly allocated one, or the existing one. > > static struct pcistub_device_id *pcistub_device_id_add_list( > int domain, int bus, unsigned int devfn) Patch 3 will be very nasty then: in case of an allocation failure all of the actions done in pcistub_seize() will have to be undone again. I'd really like to avoid that. Juergen
Re: [Xen-devel] [PATCH v3 2/3] xen/pciback: avoid multiple entries in slot list
On 22/09/16 22:02, Boris Ostrovsky wrote: > 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) I think this should allocate the new pcistub_device_id if needed. You can pass in GFP flags if needed. Then it can return the newly allocated one, or the existing one. static struct pcistub_device_id *pcistub_device_id_add_list( int domain, int bus, unsigned int devfn) David
Re: [Xen-devel] [PATCH v3 2/3] xen/pciback: avoid multiple entries in slot list
On 22/09/16 22:02, Boris Ostrovsky wrote: > 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) I think this should allocate the new pcistub_device_id if needed. You can pass in GFP flags if needed. Then it can return the newly allocated one, or the existing one. static struct pcistub_device_id *pcistub_device_id_add_list( int domain, int bus, unsigned int devfn) David