netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff Garzik <jeff@garzik.org>
To: David Miller <davem@davemloft.net>
Cc: akpm@linux-foundation.org, Yinghai.Lu@Sun.COM,
	Andi Kleen <ak@suse.de>, Al Viro <viro@ftp.linux.org.uk>,
	netdev <netdev@vger.kernel.org>
Subject: Re: [patch 10/11] net: use numa_node in net_device->dev instead of parent
Date: Thu, 27 Sep 2007 01:50:31 -0400	[thread overview]
Message-ID: <46FB44A7.3020506@garzik.org> (raw)
In-Reply-To: <20070926.223831.44964319.davem@davemloft.net>


(warning, adjusted CC's and added netdev mailing list)


David Miller wrote:
> From: akpm@linux-foundation.org
> Date: Wed, 26 Sep 2007 18:14:42 -0700
> 
>> From: Yinghai Lu <Yinghai.Lu@Sun.COM>
>>
>> Signed-off-by: Yinghai Lu <yinghai.lu@sun.com>
>> Cc: Christoph Lameter <clameter@sgi.com>
>> Cc: Andy Whitcroft <apw@shadowen.org>
>> Cc: Jeff Garzik <jeff@garzik.org>
>> Cc: Andi Kleen <ak@suse.de>
>> Cc: "David S. Miller" <davem@davemloft.net>
>> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> 
> I'm not applying this, I'm still not convinced it is right.
> 
> The device of netdev->dev is a network subsystem pseudo device and
> it's not a real I/O device at all.  It's there for creating the
> class/net/name info under sysfs for the network device.
> 
> So pulling the NUMA node information out of there is completely

Agreed.


> illogical even if you do add some ugly hack to propagate the NUMA
> information from the I/O device parent into the device struct the
> netdev has embedded in it.

I don't think it's an ugly hack at all.  The following is the standard 
way to tell the net device your parent:

#define SET_NETDEV_DEV(net, pdev)       ((net)->dev.parent = (pdev))

so therefore reading the parent is the standard (only?) way to retrieve 
that same information.

Using IETF RFC language:  Every net driver SHOULD have an associated 
struct device via SET_NETDEV_DEV(), even if it's another pseudo-device 
in the case where the net_device is not associated with real hardware.


However, that said, the overall /system/ employed here is a hack, 
because SET_NETDEV_DEV() does not actually adjust any reference counts 
or anything, for the associated net_device or associated struct device.

Al Viro pointed out problems related to this, ISTR, when someone tried 
to convert net drivers over to using the new devres stuff.  That's why I 
haven't been merging the devres net driver conversions -- they 
exacerbate existing object lifetime-related problems.

	Jeff



           reply	other threads:[~2007-09-27  5:50 UTC|newest]

Thread overview: expand[flat|nested]  mbox.gz  Atom feed
 [parent not found: <20070926.223831.44964319.davem@davemloft.net>]

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=46FB44A7.3020506@garzik.org \
    --to=jeff@garzik.org \
    --cc=Yinghai.Lu@Sun.COM \
    --cc=ak@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=netdev@vger.kernel.org \
    --cc=viro@ftp.linux.org.uk \
    /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).