Transitioning if_addr_lock to an rwlock

[Arnaud Lacombe]

Hi,

On Thu, Dec 22, 2011 at 11:30 AM, John Baldwin <jhb at freebsd.org> wrote:
> Another bit of lock contention I ran into between a device driver doing slow
> MAC filter updates and the receive path is IF_ADDR_LOCK().  NIC device drivers
> typically hold this lock while iterating the list of multicast addresses to
> program their MAC filter.  OTOH, ip_input() uses this lock to check to see if
> an incoming packet is a broadcast packet or not.  So even with the pcbinfo
> contention from my previous patch addressed, I still ran into a problem with
> IF_ADDR_LOCK().  We already have some partial support for making this lock be
> an rwlock (the APIs that drivers now use implies an rlock), so I finished the
> transition and checked various non-driver users to see which ones could use a
> read lock (most uses can).  The current patch I have for this is on 8, but if
> folks think this is a good idea I can work on porting it to HEAD.  For HEAD
> the strategy I would use would be to split this up into 2 phases:
>
> 1) Add IF_ADDR locking macros to differentiate read locks vs write locks along
>   with appropriate assertion macros.  Update current users of the locking
>   macros to use either read or write locks explicitly.  To preserve KPI,
>   the existing LOCK/UNLOCK macros would map to write lock operations.  In
>   the initial patch, the locking would still be implemented via a mtx.
>
> 2) Replace the mutex with an rwlock and update the locking macros as
>   appropriate.
>
out of curiosity, what do you expect from the conversion ? performance
improvement ? latency improvement ?

Does this particular lock show up in any significant way in lock
profiling that make the change noticeable ?

Thanks,
 - Arnaud

> Phase 1 should definitely be MFC'able.  Phase 2 may or may not be.  Robert had
> the foresight to change drivers to use explicit function wrappers around
> IF_ADDR_LOCK, and sizeof(struct mtx) == sizeof(struct rwlock), so if we
> changed the lock type the KBI for existing device drivers would all be fine.
> Most of the remaining uses of the locking macros are in parts of the kernel
> that aren't loadable (such as inet and inet6).  We can look at the places that
> to do change and if any of them are in kld's then it would be up to re@ to
> decide if 2) was actually safe to merge.  However, even if Phase 2 cannot be
> MFC'd, having phase 1 makes it easier for downstream consumers to apply Phase
> 2 locally if they need it.
>
> You can find the patch for 8.x at
> http://www.freebsd.org/~jhb/patches/if_addr_rwlock.patch
>


> --
> John Baldwin
> _______________________________________________
> freebsd-net at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-net
> To unsubscribe, send any mail to "freebsd-net-unsubscribe at freebsd.org"