Re: panic: mutex ncnegl not owned at /usr/src/sys/kern/vfs_cache.c:743 in 12.0-CURRENT r308447

The machine managed to survive 21 hours of building FreeBSD 12 ports.
All of the panic happened while building FreeBSD 10 ports, which build
faster.  That may be enough to affect timing and affect the likelyhood
of a race condition triggering the panic.

On 13 Nov, Mateusz Guzik wrote:
> On Sat, Nov 12, 2016 at 05:11:57PM -0800, Don Lewis wrote:
>> This !neg_locked code in cache_negative_remove() looks suspicious to me:
>> 
>>         if (!neg_locked) {
>>                 if (ncp->nc_flag & NCF_HOTNEGATIVE) {
>>                         hot_locked = true;
>>                         mtx_lock(&ncneg_hot.nl_lock);
>>                         if (!(ncp->nc_flag & NCF_HOTNEGATIVE)) {
>>                                 list_locked = true;
>>                                 mtx_lock(&neglist->nl_lock);
>>                         }
>>                 } else {
>>                         list_locked = true;
>>                         mtx_lock(&neglist->nl_lock);
>>                 }
>> 
>> It looks like you handle NCF_HOTNEGATIVE going away while waiting for
>> the ncneg_hot.nl_lock, but don't handle the possible appearance of
>> NCF_HOTNEGATIVE while waiting for neglist->nl_lock.
>> 
> 
> Promotions from regular to hot are only possible on a hit, which is
> prevented by the caller holding both the vnode and bucket lock. The only
> way to see the entry at this stage is from the shrinker, which can
> demote it and then take all the locks to try to remove it. But it checks
> after locking if the node is still there.
> 
>> What protects nc_flag, the lock for the list that it resides on?
>> 
> 
> It is supposed to be the hot list lock and I think this uncovers a bug
> here.
> 
> Consider a NCF_HOTNEGATIVE entry which is being evicted. It sets the
> NCV_DVDROP flag without the lock held, but the entry is still not
> removed from negative lists. So in principle we can either lose the
> newly set flag or the information that hotnegative is unset.
> 
> That said, I think the fix would be to remove from negative entries
> prior to setting the NCV_DVDROP flag.
> 
> Normally the flag is protected by the hotlist lock.
> 
> Untested, but should do the trick:
> 
> --- vfs_cache.c.old	2016-11-13 09:37:50.096000000 +0100
> +++ vfs_cache.c.new	2016-11-13 09:39:45.004000000 +0100
> _at__at_ -868,6 +868,13 _at__at_
>  		    nc_get_name(ncp), ncp->nc_neghits);
>  	}
>  	LIST_REMOVE(ncp, nc_hash);
> +	if (!(ncp->nc_flag & NCF_NEGATIVE)) {
> +		TAILQ_REMOVE(&ncp->nc_vp->v_cache_dst, ncp, nc_dst);
> +		if (ncp == ncp->nc_vp->v_cache_dd)
> +			ncp->nc_vp->v_cache_dd = NULL;
> +	} else {
> +		cache_negative_remove(ncp, neg_locked);
> +	}
>  	if (ncp->nc_flag & NCF_ISDOTDOT) {
>  		if (ncp == ncp->nc_dvp->v_cache_dd)
>  			ncp->nc_dvp->v_cache_dd = NULL;
> _at__at_ -878,13 +885,6 _at__at_
>  			atomic_subtract_rel_long(&numcachehv, 1);
>  		}
>  	}
> -	if (!(ncp->nc_flag & NCF_NEGATIVE)) {
> -		TAILQ_REMOVE(&ncp->nc_vp->v_cache_dst, ncp, nc_dst);
> -		if (ncp == ncp->nc_vp->v_cache_dd)
> -			ncp->nc_vp->v_cache_dd = NULL;
> -	} else {
> -		cache_negative_remove(ncp, neg_locked);
> -	}
>  	atomic_subtract_rel_long(&numcache, 1);
>  }

I will give this a try.