[Top] [All Lists]

Re: [patch net-next-2.6 v2] net: consolidate and fix ethtool_ops->get_se

To: Ben Hutchings <>
Subject: Re: [patch net-next-2.6 v2] net: consolidate and fix ethtool_ops->get_settings calling
From: Jiri Pirko <>
Date: Fri, 2 Sep 2011 22:42:41 +0200
In-reply-to: <1314989161.3419.5.camel@bwh-desktop>
References: <> <20110902122630.GC1991@minipsycho> <1314989161.3419.5.camel@bwh-desktop>
User-agent: Mutt/1.5.21 (2010-09-15)
Fri, Sep 02, 2011 at 08:46:01PM CEST, wrote:
>On Fri, 2011-09-02 at 14:26 +0200, Jiri Pirko wrote:
>> This patch does several things:
>> - introduces __ethtool_get_settings which is called from ethtool code and
>>   from dev_ethtool_get_settings() as well.
>> - dev_ethtool_get_settings() becomes rtnl wrapper for
>>   __ethtool_get_settings()
>I don't like this locking change.  Most other dev_*() functions require
>the caller to hold RTNL, and it will break any OOT module calling
>dev_ethtool_get_settings() without producing any warning at compile
>time.  Why not put an ASSERT_RTNL() in it instead?

Hmm. Okay, then I would remove dev_ethtool_get_settings() from
net/core/dev.c and only put __ethtool_get_settings() to
net/core/ethtool.c. Makes more sense to me to have it there...
ASSERT_RTNL woudl be good there as well.

>The rest of this looks fine.
>Ben Hutchings, Staff Engineer, Solarflare
>Not speaking for my employer; that's the marketing department's job.
>They asked us to note that Solarflare product names are trademarked.

<Prev in Thread] Current Thread [Next in Thread>