[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4684E4ED.90604@garzik.org>
Date: Fri, 29 Jun 2007 06:54:37 -0400
From: Jeff Garzik <jeff@...zik.org>
To: David Miller <davem@...emloft.net>
CC: peter.p.waskiewicz.jr@...el.com, netdev@...r.kernel.org,
auke-jan.h.kok@...el.com, hadi@...erus.ca, kaber@...sh.net
Subject: Re: [PATCH 2/3] NET: [CORE] Stack changes to add multiqueue hardware
support API
David Miller wrote:
> From: PJ Waskiewicz <peter.p.waskiewicz.jr@...el.com>
> Date: Thu, 28 Jun 2007 09:21:13 -0700
>
>> -struct net_device *alloc_netdev(int sizeof_priv, const char *name,
>> - void (*setup)(struct net_device *))
>> +struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
>> + void (*setup)(struct net_device *), int queue_count)
>> {
>> void *p;
>> struct net_device *dev;
>> @@ -3557,7 +3564,9 @@ struct net_device *alloc_netdev(int sizeof_priv, const char *name,
>> BUG_ON(strlen(name) >= sizeof(dev->name));
>>
>> /* ensure 32-byte alignment of both the device and private area */
>> - alloc_size = (sizeof(*dev) + NETDEV_ALIGN_CONST) & ~NETDEV_ALIGN_CONST;
>> + alloc_size = (sizeof(*dev) + NETDEV_ALIGN_CONST +
>> + (sizeof(struct net_device_subqueue) * queue_count)) &
>> + ~NETDEV_ALIGN_CONST;
>> alloc_size += sizeof_priv + NETDEV_ALIGN_CONST;
>>
>> p = kzalloc(alloc_size, GFP_KERNEL);
>> @@ -3573,12 +3582,14 @@ struct net_device *alloc_netdev(int sizeof_priv, const char *name,
>> if (sizeof_priv)
>> dev->priv = netdev_priv(dev);
>>
>> + dev->egress_subqueue_count = queue_count;
>> +
>> dev->get_stats = internal_stats;
>> setup(dev);
>> strcpy(dev->name, name);
>> return dev;
>> }
>
> This isn't going to work.
>
> The pointer returned from netdev_priv() doesn't take into account the
> variable sized queues at the end of struct netdev, so we can stomp
> over the queues with the private area.
>
> This probably works by luck because of NETDEV_ALIGN.
>
> The simplest fix is to just make netdev_priv() use dev->priv,
> except when it's being initialized during allocation, and
> that's what I'm going to do when I apply your patch.
Ugh. That will reverse the gains we had with the current setup, won't it?
Also, what happens when we want to add ingress_queue[0] ?
Jeff
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists