diff mbox

[V3] backport: handle change in netdevice destructor usage

Message ID 1498116664-18399-1-git-send-email-arend.vanspriel@broadcom.com (mailing list archive)
State Accepted
Headers show

Commit Message

Arend van Spriel June 22, 2017, 7:31 a.m. UTC 
This patch deals with changes made in struct net_device by commit
cf124db566e6 ("net: Fix inconsistent teardown and release of private
netdev state.").

Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
After a (somewhat) good night sleep here is V3.

Regards,
Arend

changes:
 V2:
  - changed netdev_set_priv_destructor() to macro.
  - revised semantic patch covering drivers only needing free_netdev().
  - used some tricks suggested by Julia in semantic patch.
  - added more version checks against 4.13.
 V3:
  - fixed netdev_set_priv_destructor() for >= 4.13.
---
 backport/backport-include/linux/netdevice.h | 10 ++++
 patches/0079-netdev-destructor.cocci        | 84 +++++++++++++++++++++++++++++
 2 files changed, 94 insertions(+)
 create mode 100644 patches/0079-netdev-destructor.cocci

Comments

Emmanuel Grumbach July 10, 2017, 9:43 a.m. UTC | #1 
On Thu, Jun 22, 2017 at 10:31 AM, Arend van Spriel
<arend.vanspriel@broadcom.com> wrote:
> This patch deals with changes made in struct net_device by commit
> cf124db566e6 ("net: Fix inconsistent teardown and release of private
> netdev state.").
>
> Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> ---
> After a (somewhat) good night sleep here is V3.
>

I can't claim I understand any of this, but I can say that this
davem's patch has been applied on 4.11.X and is included in 4.11.9.
--
To unsubscribe from this list: send the line "unsubscribe backports" in
Luca Coelho July 18, 2017, 9:36 a.m. UTC | #2 
Hi,

On Mon, 2017-07-10 at 12:43 +0300, Emmanuel Grumbach wrote:
> On Thu, Jun 22, 2017 at 10:31 AM, Arend van Spriel
> <arend.vanspriel@broadcom.com> wrote:
> > This patch deals with changes made in struct net_device by commit
> > cf124db566e6 ("net: Fix inconsistent teardown and release of private
> > netdev state.").
> > 
> > Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> > ---
> > After a (somewhat) good night sleep here is V3.
> > 
> 
> I can't claim I understand any of this, but I can say that this
> davem's patch has been applied on 4.11.X and is included in 4.11.9.

It seems that this made it into 4.12-rc6 as well, so stable aside we
need at least to change it to LINUX_VERSION_IS_LESS(4,12,0)...

I'll send a patch for that.

--
Cheers,
Luca.
--
To unsubscribe from this list: send the line "unsubscribe backports" in
Luca Coelho July 18, 2017, 11:38 a.m. UTC | #3 
On Tue, 2017-07-18 at 12:36 +0300, Luca Coelho wrote:
> Hi,
> 
> On Mon, 2017-07-10 at 12:43 +0300, Emmanuel Grumbach wrote:
> > On Thu, Jun 22, 2017 at 10:31 AM, Arend van Spriel
> > <arend.vanspriel@broadcom.com> wrote:
> > > This patch deals with changes made in struct net_device by commit
> > > cf124db566e6 ("net: Fix inconsistent teardown and release of private
> > > netdev state.").
> > > 
> > > Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> > > ---
> > > After a (somewhat) good night sleep here is V3.
> > > 
> > 
> > I can't claim I understand any of this, but I can say that this
> > davem's patch has been applied on 4.11.X and is included in 4.11.9.
> 
> It seems that this made it into 4.12-rc6 as well, so stable aside we
> need at least to change it to LINUX_VERSION_IS_LESS(4,12,0)...
> 
> I'll send a patch for that.

I'm also having problems with mac80211_hwsim.  The reason is that if we
have needs_free_netdev = true but don't define explicitly set the
destructor function, we hit this rule:

@r5@
struct net_device *NDEV;
identifier TRUE =~ "true";
@@

-NDEV->needs_free_netdev = TRUE;
+netdev_set_priv_destructor(NDEV, free_netdev);

But free_netdev() is not define here, so the rule that should add
__free_netdev() doesn't match and we end up with an undefined function
call:

In file included from ./include/net/dst.h:12:0,
                 from /home/luca/iwlwifi/stack-dev/drivers/net/wireless/mac80211_hwsim.c:22:
/home/luca/iwlwifi/stack-dev/drivers/net/wireless/mac80211_hwsim.c: In function ‘hwsim_mon_setup’:
/home/luca/iwlwifi/stack-dev/backport-include/linux/netdevice.h:325:23: error: ‘__free_netdev’ undeclared (first use in this function)
  (_dev)->destructor = __ ## _destructor
                       ^
/home/luca/iwlwifi/stack-dev/drivers/net/wireless/mac80211_hwsim.c:2977:2: note: in expansion of macro ‘netdev_set_priv_destructor’
  netdev_set_priv_destructor(dev, free_netdev);
  ^~~~~~~~~~~~~~~~~~~~~~~~~~
/home/luca/iwlwifi/stack-dev/backport-include/linux/netdevice.h:325:23: note: each undeclared identifier is reported only once for each function it appears in
  (_dev)->destructor = __ ## _destructor
                       ^
/home/luca/iwlwifi/stack-dev/drivers/net/wireless/mac80211_hwsim.c:2977:2: note: in expansion of macro ‘netdev_set_priv_destructor’
  netdev_set_priv_destructor(dev, free_netdev);
  ^~~~~~~~~~~~~~~~~~~~~~~~~~
scripts/Makefile.build:300: recipe for target '/home/luca/iwlwifi/stack-dev/drivers/net/wireless/mac80211_hwsim.o' failed


I guess one way to fix that would be to define another macro,
netdev_set_default_destructor or something...

I'll try to cook something up.

--
Cheers,
Luca.
--
To unsubscribe from this list: send the line "unsubscribe backports" in
diff mbox

Patch

diff --git a/backport/backport-include/linux/netdevice.h b/backport/backport-include/linux/netdevice.h
index 06230b5..98e781e 100644
--- a/backport/backport-include/linux/netdevice.h
+++ b/backport/backport-include/linux/netdevice.h
@@ -320,4 +320,14 @@  static inline void netif_trans_update(struct net_device *dev)
 }
 #endif
 
+#if LINUX_VERSION_IS_LESS(4,13,0)
+#define netdev_set_priv_destructor(_dev, _destructor) \
+	(_dev)->destructor = __ ## _destructor
+#else
+#define netdev_set_priv_destructor(_dev, _destructor) \
+	(_dev)->needs_free_netdev = true; \
+	if ((_destructor) != free_netdev) \
+		(_dev)->priv_destructor = (_destructor);
+#endif
+
 #endif /* __BACKPORT_NETDEVICE_H */
diff --git a/patches/0079-netdev-destructor.cocci b/patches/0079-netdev-destructor.cocci
new file mode 100644
index 0000000..fab8af1
--- /dev/null
+++ b/patches/0079-netdev-destructor.cocci
@@ -0,0 +1,84 @@ 
+@r1@
+struct net_device *NDEV;
+identifier D, C;
+identifier TRUE =~ "true";
+@@
+C(...)
+{
+	<...
+-	NDEV->needs_free_netdev = TRUE;
+-	NDEV->priv_destructor = D;
++	netdev_set_priv_destructor(NDEV, D);
+	...>
+}
+
+@r2 depends on r1@
+identifier r1.D, r1.C;
+fresh identifier E = "__" ## D;
+@@
+
++#if LINUX_VERSION_IS_LESS(4,13,0)
++static void E(struct net_device *ndev)
++{
++	D(ndev);
++	free_netdev(ndev);
++}
++#endif
++
+C(...)
+{
+	...
+}
+
+@r3 depends on r1@
+type T;
+identifier NDEV;
+identifier r1.D;
+T RET;
+@@
+
+RET = \(register_netdevice\|register_ndev\)(NDEV);
+if (<+... RET ...+>) {
+	<...
++#if LINUX_VERSION_IS_LESS(4,13,0)
++	D(NDEV);
++#endif
+	free_netdev(NDEV);
+	...>
+}
+
+@r4 depends on r1@
+identifier NDEV;
+identifier r1.D;
+type T;
+T RET;
+@@
+
+if (...)
+	RET = register_netdevice(NDEV);
+else
+	RET = register_netdev(NDEV);
+if (<+... RET ...+>) {
+	<...
++#if LINUX_VERSION_IS_LESS(4,13,0)
++	D(NDEV);
++#endif
+	free_netdev(NDEV);
+	...>
+}
+
+@r5@
+struct net_device *NDEV;
+identifier TRUE =~ "true";
+@@
+
+-NDEV->needs_free_netdev = TRUE;
++netdev_set_priv_destructor(NDEV, free_netdev);
+
+@r6@
+struct net_device *NDEV;
+identifier D;
+@@
+
+-NDEV->priv_destructor = D;
++netdev_set_priv_destructor(NDEV, D);