MIME-Version: 1.0
References: <20230312190600.324573-1-joe@isovalent.com> <ZBB8vZ8EJRv2d7mD@mail.gmail.com>
In-Reply-To: <ZBB8vZ8EJRv2d7mD@mail.gmail.com>
From:   Joe Stringer <joe@isovalent.com>
Date:   Wed, 15 Mar 2023 18:22:18 -0700
Message-ID: <CADa=RywQPwBib1MKs3+TFK4K6yh8sd2UkERkU5bzHZ9VS77hyw@mail.gmail.com>
Subject: Re: [PATCH bpf-next v3] docs/bpf: Add LRU internals description and graph
To:     Maxim Mikityanskiy <maxtram95@gmail.com>
Cc:     bpf@vger.kernel.org, linux-doc@vger.kernel.org,
        linux-kernel@vger.kernel.org, ast@kernel.org, corbet@lwn.net,
        martin.lau@linux.dev, bagasdotme@gmail.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Tue, Mar 14, 2023 at 6:55=E2=80=AFAM Maxim Mikityanskiy <maxtram95@gmail=
.com> wrote:
>
> On Sun, Mar 12, 2023 at 12:05:59PM -0700, Joe Stringer wrote:

<snip>

> I believe there are some inaccuracies, though. As far as I see it,
> local_freelist_check corresponds to __local_list_pop_free in the common
> LRU case, specifically, to checking its return value; use_local_node
> corresponds to returning that value; and common_lru_check corresponds
> to bpf_lru_pop_free (for both common and percpu LRU, that's where the
> distinction is made).

Ah yes, thanks for the pointers, will fix up. I started with reviewing
the shared case since I was primarily interested in the behaviour
there, then I added the other cases later. Adding the function names
was one of the later ideas but it's difficult to get accurate.

> > +  local_freelist_check [shape=3Ddiamond,fillcolor=3D1,
> > +    label=3D"Local freelist\nnode available?"];
> > +  // The following corresponds to __local_list_pop_free() for common L=
RU case.
> > +  use_local_node [shape=3Drectangle,
> > +    label=3D"Use node owned\nby this CPU"]
> > +
> > +  common_lru_check [shape=3Ddiamond,
> > +    label=3D"Map created with\ncommon LRU?\n(!BPF_NO_COMMON_LRU)"];
>
> Nit: the exact flag name is BPF_F_NO_COMMON_LRU.

Will fix.

> Thanks again for this patch, this piece of documentation really helped
> me understand internals of the LRU hashmap.

Glad to hear, thanks for the feedback on the patch!