On Thu, 13 Jul 2006 22:36:16 +0200
Christoph Hellwig <[EMAIL PROTECTED]> wrote:
> On Sat, Jul 08, 2006 at 01:37:38PM -0700, David Miller wrote:
> > > My plan is to give dev_alloc_skb a struct netdevice * argument and
> > > introduce a alloc_netdev_node so the driver can tell what node the
> > > device is on. This gives a significant speedup for cell. I already
> > > have this implemented in fact but only converted a handfull of drivers.
> > >
> > > Does this approach sound okay?
> >
> > I do not think it is feasible to add a netdevice argument to
> > such a widely used interface.
> >
> > I would instead recommend that you add a new interface, and
> > we convert drivers gradually.
>
> I gave converting the dev_alloc_skb prototype everywhere a spin and it
> doesn't look too bad. There's only a few places that don't have a
> netdevice at hand, and all these look bogus:
>
> - drivers/atm/{idt77252.c, nicstar.c}, drivers/usb/atm/usbatm.c:
> These use private skb queues and do odd things. I can't see
> any point for using dev_alloc_skb with it's additional headroom
> reservation here.
> - drivers/isdn/<everything> :
> These don't even have a netdevice! Also I can't see why they
> need the headroom.
> - drivers/net/{ppp_async.c,drivers/net/ppp_async.c}:
> They take data from the underlying tty device. Because of that
> there's not point for he additional headroom.
> - net/irda/*:
> They allocate the skb in protocol code for TX. Should probably
> do a normal alloc_skb like all the other protocol code.
> - drivers/infiniband/hw/ipath/ipath_driver.c:
> Uses skbbuffs in a driver that has absolutely nothing to do
> with networking. Duh..
>
> I've added the respective maintainers to the cc list here so they can
> comment on these usages.
>
> The patchkit for this is at http://verein.lst.de/~hch/patches.skb.tgz, it
> includes the first two cleanup patches I posted previously (Any plans
> to put them in?), a patch to move __dev_alloc_skb out of line because
> otherwise we'd need to include netdevice.h in skbuff.h which creates
> lots of problems (and moving it out of lines shaves 10kb off a
> allyesconfig), the dev_alloc_skb prototype changes and some experimental
> patches to make dev_alloc_skb nodeaware.
>
> > That netdevice argument is useful for other things. For example,
> > you can use it to apply a per-netdev random memory allocation
> > failure setting, to test the robustness of drivers. I came up
> > with that idea something like 7 years ago but never got around
> > to doing it :)
>
> Sounds cool. While doing the change I also noticed that assigning
> skb->dev can move in there aswell, but I haven't done that change yet.
What's the new name? For sanity, don't reuse the name dev_alloc_skb.
--
Stephen Hemminger <[EMAIL PROTECTED]>
"And in the Packet there writ down that doome"
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
