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: Allen <hidden>
Date: 2020-09-11 10:01:08

quoted
quoted
quoted

-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.
How does this look?
diff --git a/drivers/net/ethernet/alteon/acenic.c
b/drivers/net/ethernet/alteon/acenic.c
index 8470c836fa18..1a7e4df9b3e9 100644
--- a/drivers/net/ethernet/alteon/acenic.c
+++ b/drivers/net/ethernet/alteon/acenic.c
@@ -465,6 +465,7 @@ static int acenic_probe_one(struct pci_dev *pdev,
        SET_NETDEV_DEV(dev, &pdev->dev);

        ap = netdev_priv(dev);
+       ap->ndev = dev;
        ap->pdev = pdev;
        ap->name = pci_name(pdev);
@@ -1562,10 +1563,10 @@ 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 = ap->ndev;
        int cur_size;

        cur_size = atomic_read(&ap->cur_rx_bufs);
@@ -2269,7 +2270,7 @@ static int ace_open(struct net_device *dev)
        /*
         * Setup the bottom half rx ring refill handler
         */
-       tasklet_init(&ap->ace_tasklet, ace_tasklet, (unsigned long)dev);
+       tasklet_setup(&ap->ace_tasklet, ace_tasklet);
        return 0;
 }
diff --git a/drivers/net/ethernet/alteon/acenic.h
b/drivers/net/ethernet/alteon/acenic.h
index c670067b1541..265fa601a258 100644
--- a/drivers/net/ethernet/alteon/acenic.h
+++ b/drivers/net/ethernet/alteon/acenic.h
@@ -633,6 +633,7 @@ struct ace_skb
  */
 struct ace_private
 {
+       struct net_device       *ndev;          /* backpointer */
        struct ace_info         *info;
        struct ace_regs __iomem *regs;          /* register base */
        struct ace_skb          *skb;
@@ -776,7 +777,7 @@ static int ace_open(struct net_device *dev);
 static netdev_tx_t ace_start_xmit(struct sk_buff *skb,
                                  struct net_device *dev);
 static int ace_close(struct net_device *dev);
-static void ace_tasklet(unsigned long dev);
+static void ace_tasklet(struct tasklet_struct *t);
 static void ace_dump_trace(struct ace_private *ap);
 static void ace_set_multicast_list(struct net_device *dev);
 static int ace_change_mtu(struct net_device *dev, int new_mtu);
Let me know what you think.

Thanks.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help