b43: Remove unnecessary synchronize_irq() before free_irq()

PATCH Superseded Lars-Peter Clausen 4 participants 6 messages Feb 8, 2016 – Feb 25, 2016

Subsystems (2)

Realtek wireless driver (rtlwifi family) B43 wireless driver

lore

Share & tools

lore

Lars-Peter Clausen

author

2016-02-08 20:41 UTC #0 lore raw

PATCH 1/2 b43: Remove unnecessary synchronize_irq() before free_irq()

Calling synchronize_irq() right before free_irq() is quite useless. On one hand the IRQ can easily fire again before free_irq() is entered, on the other hand free_irq() itself calls synchronize_irq() internally (in a race condition free way), before any state associated with the IRQ is freed.

Patch was generated using the following semantic patch:
// <smpl>
@@
expression irq;
@@
-synchronize_irq(irq);
free_irq(irq, ...);
// </smpl>

Signed-off-by Lars-Peter Clausen <[email protected]>

---
 drivers/net/wireless/broadcom/b43/main.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

drivers/net/wireless/broadcom/b43/main.c +2 -4

index c279211..72380af 100644
@@ -4375,12 +4375,10 @@ redo:
 	/* Synchronize and free the interrupt handlers. Unlock to avoid deadlocks. */
 	orig_dev = dev;
 	mutex_unlock(&wl->mutex);
-	if (b43_bus_host_is_sdio(dev->dev)) {
+	if (b43_bus_host_is_sdio(dev->dev))
 		b43_sdio_free_irq(dev);
-	} else {
-		synchronize_irq(dev->dev->irq);
+	else
 		free_irq(dev->dev->irq, dev);
-	}
 	mutex_lock(&wl->mutex);
 	dev = wl->current_dev;
 	if (!dev)

Signature · 1 line

2.1.4

Lars-Peter Clausen

author

2016-02-08 20:41 UTC #1 lore raw

PATCH 2/2 rtlwifi: Remove unnecessary synchronize_irq() before free_irq()

Patch was generated using the following semantic patch:
// <smpl>
@@
expression irq;
@@
-synchronize_irq(irq);
free_irq(irq, ...);
// </smpl>

Signed-off-by Lars-Peter Clausen <[email protected]>

---
 drivers/net/wireless/realtek/rtlwifi/pci.c | 1 -
 1 file changed, 1 deletion(-)

drivers/net/wireless/realtek/rtlwifi/pci.c +0 -1

index 7f471bf..140d254 100644
@@ -2392,7 +2392,6 @@ void rtl_pci_disconnect(struct pci_dev *pdev)
 	rtlpriv->cfg->ops->deinit_sw_vars(hw);
 
 	if (rtlpci->irq_alloc) {
-		synchronize_irq(rtlpci->pdev->irq);
 		free_irq(rtlpci->pdev->irq, hw);
 		rtlpci->irq_alloc = 0;
 	}

Signature · 1 line

2.1.4

Larry Finger

2016-02-08 21:44 UTC #2 lore raw

PATCH 2/2 rtlwifi: Remove unnecessary synchronize_irq() before free_irq()

Quoted 25 lines · Lars-Peter Clausen

Calling synchronize_irq() right before free_irq() is quite useless. On one hand the IRQ can easily fire again before free_irq() is entered, on the other hand free_irq() itself calls synchronize_irq() internally (in a race condition free way), before any state associated with the IRQ is freed.
Patch was generated using the following semantic patch:
// <smpl>
@@
expression irq;
@@
-synchronize_irq(irq);
free_irq(irq, ...);
// </smpl>
Signed-off-by Lars-Peter Clausen <[email protected]>
---
  drivers/net/wireless/realtek/rtlwifi/pci.c | 1 -
  1 file changed, 1 deletion(-)
drivers/net/wireless/realtek/rtlwifi/pci.c +0 -0
index 7f471bf..140d254 100644

drivers/net/wireless/realtek/rtlwifi/pci.c void rtl_pci_disconnect(struct pci_dev *pdev) · line 2397

Quoted 7 lines

drivers/net/wireless/realtek/rtlwifi/pci.c +0 -1

@@ -2392,7 +2392,6 @@ void rtl_pci_disconnect(struct pci_dev *pdev)
  	rtlpriv->cfg->ops->deinit_sw_vars(hw);
 
  	if (rtlpci->irq_alloc) {
-		synchronize_irq(rtlpci->pdev->irq);
  		free_irq(rtlpci->pdev->irq, hw);
  		rtlpci->irq_alloc = 0;
  	}

Acked-by Larry Finger <[email protected]>

Makes sense to me.

Thanks,

Larry

Michael Büsch

2016-02-08 22:22 UTC #3 lore raw

PATCH 1/2 b43: Remove unnecessary synchronize_irq() before free_irq()

In reply to Lars-Peter Clausen

Quoted 2 lines · On Mon, 8 Feb 2016 21:41:12 +0100 Lars-Peter Clausen <[email protected]> wrote:

Calling synchronize_irq() right before free_irq() is quite useless. On one hand the IRQ can easily fire again before free_irq() is entered,

Well, that depends on whether the interrupt is shared and whether we disabled the interrupt mask inside of the device (which we did).

Quoted 3 lines

on the
other hand free_irq() itself calls synchronize_irq() internally (in a race condition free way), before any state associated with the IRQ is freed.

Ok, fair enough.

Signature · 8 lines

Michael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/b43-dev/attachments/20160208/9a3d5f46/attachment.sig>

Kalle Valo

2016-02-25 10:00 UTC #4 lore raw

PATCH 1/2 b43: Remove unnecessary synchronize_irq() before free_irq()

In reply to Lars-Peter Clausen

Quoted 15 lines

Calling synchronize_irq() right before free_irq() is quite useless. On one hand the IRQ can easily fire again before free_irq() is entered, on the other hand free_irq() itself calls synchronize_irq() internally (in a race condition free way), before any state associated with the IRQ is freed.
Patch was generated using the following semantic patch:
// <smpl>
@@
expression irq;
@@
-synchronize_irq(irq);
free_irq(irq, ...);
// </smpl>

Quoted 1 line

Signed-off-by Lars-Peter Clausen <[email protected]>

Thanks, 2 patches applied to wireless-drivers-next.git:

10fbc7cf031d b43: Remove unnecessary synchronize_irq() before free_irq() 400b43c58269 rtlwifi: Remove unnecessary synchronize_irq() before free_irq()

Kalle Valo

2016-02-25 10:00 UTC #5 lore raw

In reply to Lars-Peter Clausen

Quoted 15 lines

Calling synchronize_irq() right before free_irq() is quite useless. On one hand the IRQ can easily fire again before free_irq() is entered, on the other hand free_irq() itself calls synchronize_irq() internally (in a race condition free way), before any state associated with the IRQ is freed.
Patch was generated using the following semantic patch:
// <smpl>
@@
expression irq;
@@
-synchronize_irq(irq);
free_irq(irq, ...);
// </smpl>

Quoted 1 line

Signed-off-by Lars-Peter Clausen <[email protected]>

Thanks, 2 patches applied to wireless-drivers-next.git:

10fbc7cf031d b43: Remove unnecessary synchronize_irq() before free_irq() 400b43c58269 rtlwifi: Remove unnecessary synchronize_irq() before free_irq()

Kalle Valo