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 <[email protected]>
---
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
+ * 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);
--
2.33.0
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 <[email protected]>
> ---
> 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 <[email protected]>
> > ---
> > 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