On Sun, Aug 21, 2022 at 07:19:23PM +0200, Alexander Bluhm wrote:
> Hi,
>
> When testing parallel UDP input, I see contention on the netlock
> from the protocol timer. There is a check to avoid loop traversal
> if igmp and mld6 are not active. Move the shortcut to avoid grabbing
> netlock. This should be save as variables igmp_timers_are_running
> and mld_timers_are_running are read atomically. In case we miss
> an fast timer due to MP races, we just run it next time.
>
> ok?
>
Could we have a commentary like this before unlocked check? The rest of
diff looks ok by me.
> This should be save as variables igmp_timers_are_running
> and mld_timers_are_running are read atomically. In case we miss
> an fast timer due to MP races, we just run it next time.
> bluhm
>
> Index: netinet/igmp.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/igmp.c,v
> retrieving revision 1.78
> diff -u -p -r1.78 igmp.c
> --- netinet/igmp.c 28 Mar 2022 16:31:26 -0000 1.78
> +++ netinet/igmp.c 21 Aug 2022 17:01:44 -0000
> @@ -526,23 +526,21 @@ igmp_fasttimo(void)
> {
> struct ifnet *ifp;
>
> - NET_LOCK();
> -
> /*
> * Quick check to see if any work needs to be done, in order
> * to minimize the overhead of fasttimo processing.
> */
> if (!igmp_timers_are_running)
> - goto out;
> + return;
> +
> + NET_LOCK();
>
> igmp_timers_are_running = 0;
> TAILQ_FOREACH(ifp, &ifnet, if_list)
> igmp_checktimer(ifp);
>
> -out:
> NET_UNLOCK();
> }
> -
>
> void
> igmp_checktimer(struct ifnet *ifp)
> Index: netinet6/mld6.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/mld6.c,v
> retrieving revision 1.56
> diff -u -p -r1.56 mld6.c
> --- netinet6/mld6.c 30 Nov 2018 09:28:34 -0000 1.56
> +++ netinet6/mld6.c 21 Aug 2022 17:01:26 -0000
> @@ -327,20 +327,19 @@ mld6_fasttimeo(void)
> {
> struct ifnet *ifp;
>
> - NET_LOCK();
> -
> /*
> * Quick check to see if any work needs to be done, in order
> * to minimize the overhead of fasttimo processing.
> */
> if (!mld_timers_are_running)
> - goto out;
> + return;
> +
> + NET_LOCK();
>
> mld_timers_are_running = 0;
> TAILQ_FOREACH(ifp, &ifnet, if_list)
> mld6_checktimer(ifp);
>
> -out:
> NET_UNLOCK();
> }
>
>
