Date: Sat, 20 Aug 2016 13:27:59 -0700 From: Adrian Chadd <adrian@freebsd.org> To: "Andrey V. Elsukov" <ae@freebsd.org> Cc: FreeBSD Net <freebsd-net@freebsd.org>, Robert Watson <rwatson@freebsd.org> Subject: Re: svn commit: r304313 - head/sys/net Message-ID: <CAJ-Vmonu3ZSKZtZZMa68FAS9nDdiriE3iv81cjoW%2Bk3mHKO2=g@mail.gmail.com> In-Reply-To: <201608172021.u7HKLXJ4001584@repo.freebsd.org> References: <201608172021.u7HKLXJ4001584@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 17 August 2016 at 13:21, Andrey V. Elsukov <ae@freebsd.org> wrote: > Author: ae > Date: Wed Aug 17 20:21:33 2016 > New Revision: 304313 > URL: https://svnweb.freebsd.org/changeset/base/304313 > > Log: > Teach netisr_get_cpuid() to limit a given value to supported by netisr. > Use netisr_get_cpuid() in netisr_select_cpuid() to limit cpuid value > returned by protocol to be sure that it is not greather than nws_count. > > PR: 211836 > Reviewed by: adrian > MFC after: 3 days > > Modified: > head/sys/net/if_epair.c > head/sys/net/netisr.c > > Modified: head/sys/net/if_epair.c > ============================================================================== > --- head/sys/net/if_epair.c Wed Aug 17 19:43:45 2016 (r304312) > +++ head/sys/net/if_epair.c Wed Aug 17 20:21:33 2016 (r304313) > @@ -807,9 +807,9 @@ epair_clone_create(struct if_clone *ifc, > * cache locality but we can at least allow parallelism. > */ > sca->cpuid = > - netisr_get_cpuid(sca->ifp->if_index % netisr_get_cpucount()); > + netisr_get_cpuid(sca->ifp->if_index); > scb->cpuid = > - netisr_get_cpuid(scb->ifp->if_index % netisr_get_cpucount()); > + netisr_get_cpuid(scb->ifp->if_index); > > /* Initialise pseudo media types. */ > ifmedia_init(&sca->media, 0, epair_media_change, epair_media_status); > > Modified: head/sys/net/netisr.c > ============================================================================== > --- head/sys/net/netisr.c Wed Aug 17 19:43:45 2016 (r304312) > +++ head/sys/net/netisr.c Wed Aug 17 20:21:33 2016 (r304313) > @@ -272,10 +272,7 @@ u_int > netisr_get_cpuid(u_int cpunumber) > { > > - KASSERT(cpunumber < nws_count, ("%s: %u > %u", __func__, cpunumber, > - nws_count)); > - > - return (nws_array[cpunumber]); > + return (nws_array[cpunumber % nws_count]); > } > > /* > @@ -810,10 +807,12 @@ netisr_select_cpuid(struct netisr_proto > * dispatch. In the queued case, fall back on the SOURCE > * policy. > */ > - if (*cpuidp != NETISR_CPUID_NONE) > + if (*cpuidp != NETISR_CPUID_NONE) { > + *cpuidp = netisr_get_cpuid(*cpuidp); > return (m); > + } > if (dispatch_policy == NETISR_DISPATCH_HYBRID) { > - *cpuidp = curcpu; > + *cpuidp = netisr_get_cpuid(curcpu); > return (m); > } > policy = NETISR_POLICY_SOURCE; > I wonder if the right-er thing to do here is to allow the cpuid to be whatever it needs to be, but limit the cpuid lookups when it resolves to a netisr array. that'd mean the hybrid model would still return the current CPU up to the max CPU id, but anything trying to queue into a netisr would not overflow the netisr queue count. Thoughts? -a
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ-Vmonu3ZSKZtZZMa68FAS9nDdiriE3iv81cjoW%2Bk3mHKO2=g>