Fri, Sep 02, 2011 at 08:46:01PM CEST, bhutchings@xxxxxxxxxxxxxx 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. > >-- >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. > -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
