On Fri, Feb 3, 2017 at 7:25 PM, Joel Cunningham <joel.cunning...@me.com> wrote: > From the documentation in dev.c: > /* > * The @dev_base_head list is protected by @dev_base_lock and the rtnl > * semaphore. > * > * Pure readers hold dev_base_lock for reading, or rcu_read_lock() > * > * Writers must hold the rtnl semaphore while they loop through the > * dev_base_head list, and hold dev_base_lock for writing when they do the > * actual updates. This allows pure readers to access the list even > * while a writer is preparing to update it. > * > * To put it another way, dev_base_lock is held for writing only to > * protect against pure readers; the rtnl semaphore provides the > * protection against other writers. > * > * See, for example usages, register_netdevice() and > * unregister_netdevice(), which must be called with the rtnl > * semaphore held. > */
This comment is pretty old, it was added prior to git history. > Is the correct usage is to hold both rtnl_lock() and dev_base_lock when > modifying a member of a struct net_device? The wording seems vague as to > which synchronization issue holding both covers. What does “do the actual > update” mean, updating the list or structure member? If the latter, then > maybe the concurrent dev_ioctl() case has never been safe > I think dev_base_lock is supposed to speed up the readers when we only read one or a few fields from netdevice, otherwise it would be pretty pointless since we already have the RTNL lock. Unfortunately, as you noticed, not all of these fields are protected by dev_base_lock, therefore the readers who only take this read lock is not enough to read an atomic result. RCU doesn't seem to be the solution here, since it still requires a whole copy of netdevice even we only update, for example MTU. This is very inconvenient. It is also kinda messy due to the mix of dev_base_lock, RCU, and RTNL. Sigh...
