| Message ID | 20210829221415.647744-1-sergio@lony.xyz (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/pcifront: Removed unnecessary __ref annotation | expand |
On 30.08.21 00:14, Sergio Miguéns Iglesias wrote: > An unnecessary "__ref" annotation was removed from the > "drivers/pci/xen_pcifront.c" file. The function where the annotation > was used was "pcifront_backend_changed()", which does not call any > functions annotated as "__*init" nor "__*exit". This makes "__ref" > unnecessary since this annotation is used to make the compiler ignore > section miss-matches when they are not happening here in the first > place. > > In addition to the aforementioned change, some code style issues were > fixed in the same file. > > Signed-off-by: Sergio Miguéns Iglesias <sergio@lony.xyz> > --- > drivers/pci/xen-pcifront.c | 24 ++++++++++++++---------- > 1 file changed, 14 insertions(+), 10 deletions(-) > > diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c > index b7a8f3a1921f..f06661704f3a 100644 > --- a/drivers/pci/xen-pcifront.c > +++ b/drivers/pci/xen-pcifront.c > @@ -115,7 +115,7 @@ static int do_pci_op(struct pcifront_device *pdev, struct xen_pci_op *op) > struct xen_pci_op *active_op = &pdev->sh_info->op; > unsigned long irq_flags; > evtchn_port_t port = pdev->evtchn; > - unsigned irq = pdev->irq; > + unsigned int irq = pdev->irq; > s64 ns, ns_timeout; > > spin_lock_irqsave(&pdev->sh_info_lock, irq_flags); > @@ -152,11 +152,10 @@ static int do_pci_op(struct pcifront_device *pdev, struct xen_pci_op *op) > } > } > > - /* > - * We might lose backend service request since we > - * reuse same evtchn with pci_conf backend response. So re-schedule > - * aer pcifront service. > - */ > + /* We might lose backend service request since we This is no net or drivers/net file, so please keep the initial "/*" line and fixup the other multi-line comments accordingly. > + * reuse same evtchn with pci_conf backend response. So re-schedule > + * aer pcifront service. > + */ > if (test_bit(_XEN_PCIB_active, > (unsigned long *)&pdev->sh_info->flags)) { > dev_err(&pdev->xdev->dev, > @@ -493,7 +492,8 @@ static int pcifront_scan_root(struct pcifront_device *pdev, > list_add(&bus_entry->list, &pdev->root_buses); > > /* pci_scan_root_bus skips devices which do not have a > - * devfn==0. The pcifront_scan_bus enumerates all devfn. */ > + * devfn==0. The pcifront_scan_bus enumerates all devfn. > + */ > err = pcifront_scan_bus(pdev, domain, bus, b); > > /* Claim resources before going "live" with our devices */ > @@ -651,8 +651,9 @@ static void pcifront_do_aer(struct work_struct *data) > pci_channel_state_t state = > (pci_channel_state_t)pdev->sh_info->aer_op.err; > > - /*If a pci_conf op is in progress, > - we have to wait until it is done before service aer op*/ > + /* If a pci_conf op is in progress, we have to wait until it is done > + * before service aer op > + */ > dev_dbg(&pdev->xdev->dev, > "pcifront service aer bus %x devfn %x\n", > pdev->sh_info->aer_op.bus, pdev->sh_info->aer_op.devfn); > @@ -676,6 +677,7 @@ static void pcifront_do_aer(struct work_struct *data) > static irqreturn_t pcifront_handler_aer(int irq, void *dev) > { > struct pcifront_device *pdev = dev; > + > schedule_pcifront_aer_op(pdev); > return IRQ_HANDLED; > } > @@ -1027,6 +1029,7 @@ static int pcifront_detach_devices(struct pcifront_device *pdev) > /* Find devices being detached and remove them. */ > for (i = 0; i < num_devs; i++) { > int l, state; > + > l = snprintf(str, sizeof(str), "state-%d", i); > if (unlikely(l >= (sizeof(str) - 1))) { > err = -ENOMEM; > @@ -1078,7 +1081,7 @@ static int pcifront_detach_devices(struct pcifront_device *pdev) > return err; > } > > -static void __ref pcifront_backend_changed(struct xenbus_device *xdev, > +static void pcifront_backend_changed(struct xenbus_device *xdev, > enum xenbus_state be_state) > { > struct pcifront_device *pdev = dev_get_drvdata(&xdev->dev); > @@ -1137,6 +1140,7 @@ static int pcifront_xenbus_probe(struct xenbus_device *xdev, > static int pcifront_xenbus_remove(struct xenbus_device *xdev) > { > struct pcifront_device *pdev = dev_get_drvdata(&xdev->dev); > + > if (pdev) > free_pdev(pdev); > > Juergen
Thanks a lot for your reply! I am sending a v2 patch to fix all the issues. Sergio M. Iglesias. On 21/08/30 06:55, Juergen Gross wrote: > On 30.08.21 00:14, Sergio Miguéns Iglesias wrote: > > An unnecessary "__ref" annotation was removed from the > > "drivers/pci/xen_pcifront.c" file. The function where the annotation > > was used was "pcifront_backend_changed()", which does not call any > > functions annotated as "__*init" nor "__*exit". This makes "__ref" > > unnecessary since this annotation is used to make the compiler ignore > > section miss-matches when they are not happening here in the first > > place. > > > > In addition to the aforementioned change, some code style issues were > > fixed in the same file. > > > > Signed-off-by: Sergio Miguéns Iglesias <sergio@lony.xyz> > > --- > > drivers/pci/xen-pcifront.c | 24 ++++++++++++++---------- > > 1 file changed, 14 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c > > index b7a8f3a1921f..f06661704f3a 100644 > > --- a/drivers/pci/xen-pcifront.c > > +++ b/drivers/pci/xen-pcifront.c > > @@ -115,7 +115,7 @@ static int do_pci_op(struct pcifront_device *pdev, struct xen_pci_op *op) > > struct xen_pci_op *active_op = &pdev->sh_info->op; > > unsigned long irq_flags; > > evtchn_port_t port = pdev->evtchn; > > - unsigned irq = pdev->irq; > > + unsigned int irq = pdev->irq; > > s64 ns, ns_timeout; > > spin_lock_irqsave(&pdev->sh_info_lock, irq_flags); > > @@ -152,11 +152,10 @@ static int do_pci_op(struct pcifront_device *pdev, struct xen_pci_op *op) > > } > > } > > - /* > > - * We might lose backend service request since we > > - * reuse same evtchn with pci_conf backend response. So re-schedule > > - * aer pcifront service. > > - */ > > + /* We might lose backend service request since we > > This is no net or drivers/net file, so please keep the initial "/*" > line and fixup the other multi-line comments accordingly. > > > + * reuse same evtchn with pci_conf backend response. So re-schedule > > + * aer pcifront service. > > + */ > > if (test_bit(_XEN_PCIB_active, > > (unsigned long *)&pdev->sh_info->flags)) { > > dev_err(&pdev->xdev->dev, > > @@ -493,7 +492,8 @@ static int pcifront_scan_root(struct pcifront_device *pdev, > > list_add(&bus_entry->list, &pdev->root_buses); > > /* pci_scan_root_bus skips devices which do not have a > > - * devfn==0. The pcifront_scan_bus enumerates all devfn. */ > > + * devfn==0. The pcifront_scan_bus enumerates all devfn. > > + */ > > err = pcifront_scan_bus(pdev, domain, bus, b); > > /* Claim resources before going "live" with our devices */ > > @@ -651,8 +651,9 @@ static void pcifront_do_aer(struct work_struct *data) > > pci_channel_state_t state = > > (pci_channel_state_t)pdev->sh_info->aer_op.err; > > - /*If a pci_conf op is in progress, > > - we have to wait until it is done before service aer op*/ > > + /* If a pci_conf op is in progress, we have to wait until it is done > > + * before service aer op > > + */ > > dev_dbg(&pdev->xdev->dev, > > "pcifront service aer bus %x devfn %x\n", > > pdev->sh_info->aer_op.bus, pdev->sh_info->aer_op.devfn); > > @@ -676,6 +677,7 @@ static void pcifront_do_aer(struct work_struct *data) > > static irqreturn_t pcifront_handler_aer(int irq, void *dev) > > { > > struct pcifront_device *pdev = dev; > > + > > schedule_pcifront_aer_op(pdev); > > return IRQ_HANDLED; > > } > > @@ -1027,6 +1029,7 @@ static int pcifront_detach_devices(struct pcifront_device *pdev) > > /* Find devices being detached and remove them. */ > > for (i = 0; i < num_devs; i++) { > > int l, state; > > + > > l = snprintf(str, sizeof(str), "state-%d", i); > > if (unlikely(l >= (sizeof(str) - 1))) { > > err = -ENOMEM; > > @@ -1078,7 +1081,7 @@ static int pcifront_detach_devices(struct pcifront_device *pdev) > > return err; > > } > > -static void __ref pcifront_backend_changed(struct xenbus_device *xdev, > > +static void pcifront_backend_changed(struct xenbus_device *xdev, > > enum xenbus_state be_state) > > { > > struct pcifront_device *pdev = dev_get_drvdata(&xdev->dev); > > @@ -1137,6 +1140,7 @@ static int pcifront_xenbus_probe(struct xenbus_device *xdev, > > static int pcifront_xenbus_remove(struct xenbus_device *xdev) > > { > > struct pcifront_device *pdev = dev_get_drvdata(&xdev->dev); > > + > > if (pdev) > > free_pdev(pdev); > > > > Juergen
diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c index b7a8f3a1921f..f06661704f3a 100644 --- a/drivers/pci/xen-pcifront.c +++ b/drivers/pci/xen-pcifront.c @@ -115,7 +115,7 @@ static int do_pci_op(struct pcifront_device *pdev, struct xen_pci_op *op) struct xen_pci_op *active_op = &pdev->sh_info->op; unsigned long irq_flags; evtchn_port_t port = pdev->evtchn; - unsigned irq = pdev->irq; + unsigned int irq = pdev->irq; s64 ns, ns_timeout; spin_lock_irqsave(&pdev->sh_info_lock, irq_flags); @@ -152,11 +152,10 @@ static int do_pci_op(struct pcifront_device *pdev, struct xen_pci_op *op) } } - /* - * We might lose backend service request since we - * reuse same evtchn with pci_conf backend response. So re-schedule - * aer pcifront service. - */ + /* We might lose backend service request since we + * reuse same evtchn with pci_conf backend response. So re-schedule + * aer pcifront service. + */ if (test_bit(_XEN_PCIB_active, (unsigned long *)&pdev->sh_info->flags)) { dev_err(&pdev->xdev->dev, @@ -493,7 +492,8 @@ static int pcifront_scan_root(struct pcifront_device *pdev, list_add(&bus_entry->list, &pdev->root_buses); /* pci_scan_root_bus skips devices which do not have a - * devfn==0. The pcifront_scan_bus enumerates all devfn. */ + * devfn==0. The pcifront_scan_bus enumerates all devfn. + */ err = pcifront_scan_bus(pdev, domain, bus, b); /* Claim resources before going "live" with our devices */ @@ -651,8 +651,9 @@ static void pcifront_do_aer(struct work_struct *data) pci_channel_state_t state = (pci_channel_state_t)pdev->sh_info->aer_op.err; - /*If a pci_conf op is in progress, - we have to wait until it is done before service aer op*/ + /* If a pci_conf op is in progress, we have to wait until it is done + * before service aer op + */ dev_dbg(&pdev->xdev->dev, "pcifront service aer bus %x devfn %x\n", pdev->sh_info->aer_op.bus, pdev->sh_info->aer_op.devfn); @@ -676,6 +677,7 @@ static void pcifront_do_aer(struct work_struct *data) static irqreturn_t pcifront_handler_aer(int irq, void *dev) { struct pcifront_device *pdev = dev; + schedule_pcifront_aer_op(pdev); return IRQ_HANDLED; } @@ -1027,6 +1029,7 @@ static int pcifront_detach_devices(struct pcifront_device *pdev) /* Find devices being detached and remove them. */ for (i = 0; i < num_devs; i++) { int l, state; + l = snprintf(str, sizeof(str), "state-%d", i); if (unlikely(l >= (sizeof(str) - 1))) { err = -ENOMEM; @@ -1078,7 +1081,7 @@ static int pcifront_detach_devices(struct pcifront_device *pdev) return err; } -static void __ref pcifront_backend_changed(struct xenbus_device *xdev, +static void pcifront_backend_changed(struct xenbus_device *xdev, enum xenbus_state be_state) { struct pcifront_device *pdev = dev_get_drvdata(&xdev->dev); @@ -1137,6 +1140,7 @@ static int pcifront_xenbus_probe(struct xenbus_device *xdev, static int pcifront_xenbus_remove(struct xenbus_device *xdev) { struct pcifront_device *pdev = dev_get_drvdata(&xdev->dev); + if (pdev) free_pdev(pdev);
An unnecessary "__ref" annotation was removed from the "drivers/pci/xen_pcifront.c" file. The function where the annotation was used was "pcifront_backend_changed()", which does not call any functions annotated as "__*init" nor "__*exit". This makes "__ref" unnecessary since this annotation is used to make the compiler ignore section miss-matches when they are not happening here in the first place. In addition to the aforementioned change, some code style issues were fixed in the same file. Signed-off-by: Sergio Miguéns Iglesias <sergio@lony.xyz> --- drivers/pci/xen-pcifront.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-)