On Tue, Sep 19, 2017 at 1:38 PM, Kengo NAKAHARA <k-nakah...@iij.ad.jp> wrote: > Hi, > > To fix PR kern/52515(*), in particular psref(9), I implement PERCPU_LIST which > is dedicated for percpu_alloc'ed percpu struct. Here is the patch which > consists > of PERCULI_LIST implementation and applying to subr_psref.c. > http://www.NetBSD.org/~knakahara/percpu-list/percpu-list.patch > > (*) http://gnats.netbsd.org/cgi-bin/query-pr-single.pl?number=52515 > > The details are following. > > As written in PR/52515, the root of the problem is back-pointer to > percpu_alloc'ed struct. At first, I tried generic solution to let > percpu_alloc'ed struct use a pointer instead of itself. Here is a part of the > PoC patch. > ========== bad patch ========== > @@ -103,9 +103,18 @@ struct psref_class { > * Not exposed by the API. > */ > struct psref_cpu { > - struct psref_head pcpu_head; > + struct psref_head *pcpu_head; > }; > > +static void > +psref_class_pcpu_init_p(void *p, void *cookie, struct cpu_info *ci) > +{ > + struct psref_cpu *pcpu = p; > + > + pcpu->pcpu_head = kmem_alloc(sizeof(*(pcpu->pcpu_head)), KM_SLEEP); > + LIST_INIT(pcpu->pcpu_head); > +} > + > /* > * psref_class_create(name, ipl) > * > @@ -121,6 +130,7 @@ psref_class_create(const char *name, int ipl) > > class = kmem_alloc(sizeof(*class), KM_SLEEP); > class->prc_percpu = percpu_alloc(sizeof(struct psref_cpu)); > + percpu_foreach(class->prc_percpu, &psref_class_pcpu_init_p, NULL); > mutex_init(&class->prc_lock, MUTEX_DEFAULT, ipl); > cv_init(&class->prc_cv, name); > class->prc_iplcookie = makeiplcookie(ipl); > ========== bad patch ========== > > Howerver, it causes initialization dependency problems to apply this > modification to psref(9). That result from that percpu_foreach() must be > called > after ncpu is fixed, that is, > (A) psref_class_create() for "ifnet_psref_class" must be called after ncpu > is fixed > - as psref_class_create() use percpu_foreach() in this > modification > (B) "ifnet_psref_class" must be initialized before any (real and pseudo) > network interfaces are attached > > To satisfy both (A) and (B), it is required the MI hook called immediately > after ncpu is fixed, yes, that hook is not implemented yet. Furthermore, > the modification is too large even if that hook is already implemented. > So, I gave up to use this generic solution. > > After that, I decide to use list specific solution to use PERCPU_LIST > implemented for percpu_alloc'ed struct. The key design is an omission of the > back-pointer to list head in the first list element, that is, when it wants > to access list head, it always call percpu_getref() and then use the returned > pointer. > # There are some PR kern/52515 problems which cannot be fix by percpu list, > # such as ipforward_rt_percpu. They can be fixed by using a pointer instead > # of itself because they don't have complicated initialization dependency.
For rtcaches, I recommend that we stop manipulating rtcaches by global lists (dom->dom_rtcache) for cache invalidation and instead use a global generation counter for cache invalidation (inspired by IPsec SP cache). By doing so we can remove LIST_ENTRY from struct route and avoid the issue. This is a patch: http://www.netbsd.org/~ozaki-r/rtcache-gen.diff Thanks, ozaki-r