[PATCH] xen/pcifront: Removed unnecessary __ref annotation

[PATCH] xen/pcifront: Removed unnecessary __ref annotation

[Sergio Miguéns Iglesias]

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
+	 * 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

Re: [PATCH] xen/pcifront: Removed unnecessary __ref annotation

[Juergen Gross]

[-- Attachment #1.1.1: Type: text/plain, Size: 4322 bytes --]

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

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3135 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH] xen/pcifront: Removed unnecessary __ref annotation

[Sergio Miguéns Iglesias]

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