[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v5 03/14] vpci: move lock outside of struct vpci
Hello, Roger, Jan! On 12.01.22 16:42, Jan Beulich wrote: > On 11.01.2022 16:17, Roger Pau Monné wrote: >> On Thu, Nov 25, 2021 at 01:02:40PM +0200, Oleksandr Andrushchenko wrote: >>> diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c >>> index 657697fe3406..ceaac4516ff8 100644 >>> --- a/xen/drivers/vpci/vpci.c >>> +++ b/xen/drivers/vpci/vpci.c >>> @@ -35,12 +35,10 @@ extern vpci_register_init_t *const __start_vpci_array[]; >>> extern vpci_register_init_t *const __end_vpci_array[]; >>> #define NUM_VPCI_INIT (__end_vpci_array - __start_vpci_array) >>> >>> -void vpci_remove_device(struct pci_dev *pdev) >>> +static void vpci_remove_device_handlers_locked(struct pci_dev *pdev) >>> { >>> - if ( !has_vpci(pdev->domain) ) >>> - return; >>> + ASSERT(spin_is_locked(&pdev->vpci_lock)); >>> >>> - spin_lock(&pdev->vpci->lock); >>> while ( !list_empty(&pdev->vpci->handlers) ) >>> { >>> struct vpci_register *r = list_first_entry(&pdev->vpci->handlers, >>> @@ -50,15 +48,33 @@ void vpci_remove_device(struct pci_dev *pdev) >>> list_del(&r->node); >>> xfree(r); >>> } >>> - spin_unlock(&pdev->vpci->lock); >>> +} >>> + >>> +void vpci_remove_device_locked(struct pci_dev *pdev) >> I think this could be static instead, as it's only used by >> vpci_remove_device and vpci_add_handlers which are local to the >> file. This is going to be used outside later on while processing pending mappings, so I think it is not worth it defining it static here and then removing the static key word later on: please see [PATCH v5 04/14] vpci: cancel pending map/unmap on vpci removal [1] > Does the splitting out of vpci_remove_device_handlers_locked() belong in > this patch in the first place? There's no second caller being added, so > this looks to be an orthogonal adjustment. I think of it as a preparation for the upcoming code: although the reason for the change might not be immediately seen in this patch it is still in line with what happens next. So, I would prefer to keep the change as is: anyways the whole series should probably be committed as a single piece of work, so it won't look inconsistent then Thank you, Oleksandr > > Jan > [1] https://patchwork.kernel.org/project/xen-devel/patch/20211125110251.2877218-5-andr2000@xxxxxxxxx/
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |