Re: kern/58531: NetBSD 10.0 deadlock in nd_timer

[gnats-admin%netbsd.org@localhost]

The following reply was made to PR kern/58531; it has been noted by GNATS.

From: Rin Okuyama <rokuyama.rk%gmail.com@localhost>
To: matthew green <mrg%eterna23.net@localhost>, gnats-bugs%netbsd.org@localhost
Cc: kern-bug-people%netbsd.org@localhost, gnats-admin%netbsd.org@localhost,
 netbsd-bugs%netbsd.org@localhost, manu%netbsd.org@localhost, ozaki-r%NetBSD.org@localhost
Subject: Re: kern/58531: NetBSD 10.0 deadlock in nd_timer
Date: Tue, 13 Aug 2024 11:54:14 +0900

 On 2024/08/13 10:10, matthew green wrote:
 >>   We have a deadlock here:
 >>   
 >>   wm_handle_queue/wm_send_common_locked/bpf_deliver/selnotify/knote/filter_event waits for kernel_lock with txq->txq_lock held
 >>   
 >>   softint_dispatch/callout_softclock/nd_timer/arp_llinfo_output/arprequest/ether_output/if_transmit/wm_start waits for txq->txq_lock  with kernel_lock held
 > 
 > usually we'd want kernel lock taken before other locks so in
 > thie case, the first one would already have kernel lock and
 > would just take a ref on the existing lock
 
 I just discussed this with ozaki-r@. He pointed out that
 wm(4) is now marked IFEF_MPSAFE regardless of NET_MPSAFE:
 
 https://github.com/NetBSD/src/commit/2f5368b82e369741e8d99b3fd6cda9a14e76e550
 
 As a result, if(4) routines do not take KERNEL_LOCK for wm(4).
 
 These days, many drivers assert IFEF_MPSAFE unconditionally.
 We must revisit all of them, if it matters...
 
 > for the first case, see if you can ensure that kernel_lock is
 > held before taking txq_lock, perhaps only for the case that
 > calls the knote.
 > 
 > apparently we need to de-kernel-lock-ify select backends?
 
 Yeah, at least FILTEROP_MPSAFE should be turned on for bpf(4);
 it has a fine-grained lock, and this flag bit seems to be
 left untouched just accidentally.
 
 manu@, can you please test whether the attached patch below
 fixes your problem or not?
 
 Thanks,
 rin
 ----
 diff --git a/sys/net/bpf.c b/sys/net/bpf.c
 index f2532265e34..4ae5e7653f0 100644
 --- a/sys/net/bpf.c
 +++ b/sys/net/bpf.c
 @@ -1611,7 +1611,7 @@ filt_bpfread(struct knote *kn, long hint)
   }
 
   static const struct filterops bpfread_filtops = {
 -	.f_flags = FILTEROP_ISFD,
 +	.f_flags = FILTEROP_ISFD | FILTEROP_MPSAFE,
   	.f_attach = NULL,
   	.f_detach = filt_bpfrdetach,
   	.f_event = filt_bpfread,
 ----