Thread (30 messages) 30 messages, 4 authors, 2020-09-11

Re: [PATCH v2 01/20] ethernet: alteon: convert tasklets to use new tasklet_setup() API

From: David Miller <davem@davemloft.net>
Date: 2020-09-09 21:33:30

From: Allen <redacted>
Date: Thu, 10 Sep 2020 00:06:47 +0530
quoted
quoted
@@ -1562,10 +1562,11 @@ static void ace_watchdog(struct net_device *data, unsigned int txqueue)
 }


-static void ace_tasklet(unsigned long arg)
+static void ace_tasklet(struct tasklet_struct *t)
 {
-     struct net_device *dev = (struct net_device *) arg;
-     struct ace_private *ap = netdev_priv(dev);
+     struct ace_private *ap = from_tasklet(ap, t, ace_tasklet);
+     struct net_device *dev = (struct net_device *)((char *)ap -
+                             ALIGN(sizeof(struct net_device), NETDEV_ALIGN));
      int cur_size;
I don't see this is as an improvement.  The 'dev' assignment looks so
incredibly fragile and exposes so many internal details about netdev
object allocation, alignment, and layout.

Who is going to find and fix this if someone changes how netdev object
allocation works?
Thanks for pointing it out. I'll see if I can fix it to keep it simple.
Just add a backpointer to the netdev from the netdev_priv() if you
absolutely have too.
quoted
I don't want to apply this, it sets a very bad precedent.  The existing
code is so much cleaner and easier to understand and audit.
Will you pick the rest of the patches or would they have to wait till
this one is
fixed.
I never pick up a partial series, ever.  So yes you will have to fix these
two patches up and resubmit the entire thing.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help