netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: David Miller <davem@davemloft.net>
Cc: shemminger@osdl.org, netdev@vger.kernel.org,
	support@pathscale.com, samuel@sortiz.org, kkeil@suse.de,
	kai.germaschewski@gmx.de, chas@cmf.nrl.navy.mil,
	paulus@samba.org
Subject: Re: [PATCH 2/2] correct dev_alloc_skb kerneldoc
Date: Thu, 13 Jul 2006 22:36:16 +0200	[thread overview]
Message-ID: <20060713203615.GA26954@lst.de> (raw)
In-Reply-To: <20060708.133738.123994596.davem@davemloft.net>

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.

  reply	other threads:[~2006-07-13 20:37 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-07-07  9:09 [PATCH 2/2] correct dev_alloc_skb kerneldoc Christoph Hellwig
2006-07-07 22:52 ` Stephen Hemminger
2006-07-07 23:55   ` David Miller
2006-07-08  3:11     ` Herbert Xu
2006-07-08 10:48     ` Christoph Hellwig
2006-07-08 20:37       ` David Miller
2006-07-13 20:36         ` Christoph Hellwig [this message]
2006-07-13 20:47           ` Randy.Dunlap
2006-07-13 20:47           ` David Miller
2006-07-14 15:24             ` chas williams - CONTRACTOR
2006-07-13 21:24           ` Stephen Hemminger
2006-07-13 21:29             ` Christoph Hellwig
2006-07-13 22:01               ` David Miller
2006-07-14  7:27           ` Samuel Ortiz
2006-07-14  6:49             ` Christoph Hellwig
2006-07-24 22:31 ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20060713203615.GA26954@lst.de \
    --to=hch@lst.de \
    --cc=chas@cmf.nrl.navy.mil \
    --cc=davem@davemloft.net \
    --cc=kai.germaschewski@gmx.de \
    --cc=kkeil@suse.de \
    --cc=netdev@vger.kernel.org \
    --cc=paulus@samba.org \
    --cc=samuel@sortiz.org \
    --cc=shemminger@osdl.org \
    --cc=support@pathscale.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).