Re: RFC: PERCPU_LIST to fix PR kern/52515

[Ryota Ozaki <ozaki-r%netbsd.org@localhost>]

On Tue, Sep 19, 2017 at 1:38 PM, Kengo NAKAHARA <k-nakahara%iij.ad.jp@localhost> 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