Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 6 Jun 2019 11:00:00 -0700
From:      John Baldwin <jhb@FreeBSD.org>
To:        Mark Johnston <markj@freebsd.org>
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r348745 - head/sys/net
Message-ID:  <f181a5e6-8507-b8ac-6a33-fba82e3bdbd5@FreeBSD.org>
In-Reply-To: <20190606174339.GA3015@raichu>
References:  <201906061622.x56GMTmT076525@repo.freebsd.org> <61b60bb0-da61-00cf-5fca-69ac306ce6d2@FreeBSD.org> <20190606174339.GA3015@raichu>

next in thread | previous in thread | raw e-mail | index | archive | help
On 6/6/19 10:43 AM, Mark Johnston wrote:
> On Thu, Jun 06, 2019 at 10:13:13AM -0700, John Baldwin wrote:
>> On 6/6/19 9:22 AM, Mark Johnston wrote:
>>> Author: markj
>>> Date: Thu Jun  6 16:22:29 2019
>>> New Revision: 348745
>>> URL: https://svnweb.freebsd.org/changeset/base/348745
>>>
>>> Log:
>>>   Conditionalize an in_epoch() call on INVARIANTS.
>>>   
>>>   Its result is only used to determine whether to perform further
>>>   INVARIANTS-only checks.  Remove a stale comment while here.
>>>   
>>>   Submitted by:	Sebastian Huber <sebastian.huber@embedded-brains.de>
>>>   MFC after:	1 week
>>>
>>> Modified:
>>>   head/sys/net/if_lagg.c
>>>
>>> Modified: head/sys/net/if_lagg.c
>>> ==============================================================================
>>> --- head/sys/net/if_lagg.c	Thu Jun  6 16:20:50 2019	(r348744)
>>> +++ head/sys/net/if_lagg.c	Thu Jun  6 16:22:29 2019	(r348745)
>>> @@ -1955,12 +1955,10 @@ lagg_link_active(struct lagg_softc *sc, struct lagg_po
>>>  	 * Search a port which reports an active link state.
>>>  	 */
>>>  
>>> -	/*
>>> -	 * This is called with either LAGG_RLOCK() held or
>>> -	 * LAGG_XLOCK(sc) held.
>>> -	 */
>>> +#ifdef INVARIANTS
>>>  	if (!in_epoch(net_epoch_preempt))
>>>  		LAGG_XLOCK_ASSERT(sc);
>>> +#endif
>>
>> FWIW, the comment wasn't stale but on purpose (I added it when I added the check).
>> The idea is to be the equivalent of
>>
>> assert(in_epoch(net_epoch_preempt) || lagg_xlocked(sc))
>>
>> However, I couldn't write it that way, so I use LAGG_XLOCK_ASSERT when !in_epoch
>> returns false.  
> 
> We could add
> 
> #ifdef INVARIANTS
> #define LAGG_ASSERT_LOCKED(_sc) (sx_xlocked(&(_sc)->sc_sx) || in_epoch(net_epoch_preempt))
> #else
> #define LAGG_ASSERT_LOCKED(_sc)
> #endif

Hmm, ok I guess.  I just like using sx_assert when possible as you get line numbers,
etc.  Maybe MPASS would work though since it does that.  You also don't need the
extra #ifdef in that case either:

#define LAGG_ASSERT_LOCKED(_sc) MPASS(sx_xlocked(&(_sc)->sc_sx) || in_epoch(net_epoch_preempt))

-- 
John Baldwin



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?f181a5e6-8507-b8ac-6a33-fba82e3bdbd5>