Message-ID: <CAMB2axN+7O=vBDpUfkCmZmSzcPrwrBRkvwx7PMBYvr01GBEk_g@mail.gmail.com>
Date: Tue, 29 Jul 2025 12:55:39 -0700
From: Amery Hung <ameryhung@...il.com>
To: Emil Tsalapatis <linux-lists@...alapatis.com>
Cc: bpf@...r.kernel.org, netdev@...r.kernel.org, alexei.starovoitov@...il.com, 
	andrii@...nel.org, daniel@...earbox.net, tj@...nel.org, memxor@...il.com, 
	martin.lau@...nel.org, kernel-team@...a.com
Subject: Re: [PATCH bpf-next v6 1/3] selftests/bpf: Introduce task local data

On Mon, Jul 28, 2025 at 10:45 AM Emil Tsalapatis
<linux-lists@...alapatis.com> wrote:
>
> On Thu, Jul 17, 2025 at 12:49 PM Amery Hung <ameryhung@...il.com> wrote:
> >
> > Task local data defines an abstract storage type for storing task-
> > specific data (TLD). This patch provides user space and bpf
> > implementation as header-only libraries for accessing task local data.
> >
> > Task local data is a bpf task local storage map with two UPTRs:
> > 1) u_tld_metadata, shared by all tasks of the same process, consists of
> > the total count of TLDs and an array of metadata of TLDs. A metadata of
> > a TLD comprises the size and the name. The name is used to identify a
> > specific TLD in bpf 2) u_tld_data points to a task-specific memory region
> > for storing TLDs.
> >
> > Below are the core task local data API:
> >
> >                      User space                           BPF
> > Define TLD    TLD_DEFINE_KEY(), tld_create_key()           -
> > Get data           tld_get_data()                    tld_get_data()
> >
> > A TLD is first defined by the user space with TLD_DEFINE_KEY() or
> > tld_create_key(). TLD_DEFINE_KEY() defines a TLD statically and allocates
> > just enough memory during initialization. tld_create_key() allows
> > creating TLDs on the fly, but has a fix memory budget, TLD_DYN_DATA_SIZE.
> > Internally, they all go through the metadata array to check if the TLD can
> > be added. The total TLD size needs to fit into a page (limited by UPTR),
> > and no two TLDs can have the same name. It also calculates the offset, the
> > next available space in u_tld_data, by summing sizes of TLDs. If the TLD
> > can be added, it increases the count using cmpxchg as there may be other
> > concurrent tld_create_key(). After a successful cmpxchg, the last
> > metadata slot now belongs to the calling thread and will be updated.
> > tld_create_key() returns the offset encapsulated as a opaque object key
> > to prevent user misuse.
> >
> > Then, user space can pass the key to tld_get_data() to get a pointer
> > to the TLD. The pointer will remain valid for the lifetime of the
> > thread.
> >
> > BPF programs can also locate the TLD by tld_get_data(), but with both
> > name and key. The first time tld_get_data() is called, the name will
> > be used to lookup the metadata. Then, the key will be saved to a
> > task_local_data map, tld_keys_map. Subsequent call to tld_get_data()
> > will use the key to quickly locate the data.
> >
> > User space task local data library uses a light way approach to ensure
> > thread safety (i.e., atomic operation + compiler and memory barriers).
> > While a metadata is being updated, other threads may also try to read it.
> > To prevent them from seeing incomplete data, metadata::size is used to
> > signal the completion of the update, where 0 means the update is still
> > ongoing. Threads will wait until seeing a non-zero size to read a
> > metadata.
> >
> > Signed-off-by: Amery Hung <ameryhung@...il.com>
>
> Reviewed-by: Emil Tsalapatis <emil@...alapatis.com>
>
> > ---
> >  .../bpf/prog_tests/task_local_data.h          | 388 ++++++++++++++++++
> >  .../selftests/bpf/progs/task_local_data.bpf.h | 227 ++++++++++
> >  2 files changed, 615 insertions(+)
> >  create mode 100644 tools/testing/selftests/bpf/prog_tests/task_local_data.h
> >  create mode 100644 tools/testing/selftests/bpf/progs/task_local_data.bpf.h
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/task_local_data.h b/tools/testing/selftests/bpf/prog_tests/task_local_data.h
> > new file mode 100644
> > index 000000000000..73ee122daf81
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/prog_tests/task_local_data.h
> > @@ -0,0 +1,388 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef __TASK_LOCAL_DATA_H
> > +#define __TASK_LOCAL_DATA_H
> > +
> > +#include <errno.h>
> > +#include <fcntl.h>
> > +#include <sched.h>
> > +#include <stdatomic.h>
> > +#include <stddef.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <unistd.h>
> > +#include <sys/syscall.h>
> > +#include <sys/types.h>
> > +
> > +#ifdef TLD_FREE_DATA_ON_THREAD_EXIT
> > +#include <pthread.h>
> > +#endif
> > +
> > +#include <bpf/bpf.h>
> > +
> > +/*
> > + * OPTIONS
> > + *
> > + *   Define the option before including the header
> > + *
> > + *   TLD_FREE_DATA_ON_THREAD_EXIT - Frees memory on thread exit automatically
> > + *
> > + *   Thread-specific memory for storing TLD is allocated lazily on the first call to
> > + *   tld_get_data(). The thread that calls it must also calls tld_free() on thread exit
>
> Typo: must also calls -> must also call
>
> > + *   to prevent memory leak. Pthread will be included if the option is defined. A pthread
> > + *   key will be registered with a destructor that calls tld_free().
> > + *
> > + *
> > + *   TLD_DYN_DATA_SIZE - The maximum size of memory allocated for TLDs created dynamically
> > + *   (default: 64 bytes)
> > + *
> > + *   A TLD can be defined statically using TLD_DEFINE_KEY() or created on the fly using
> > + *   tld_create_key(). As the total size of TLDs created with tld_create_key() cannot be
> > + *   possibly known statically, a memory area of size TLD_DYN_DATA_SIZE will be allocated
> > + *   for these TLDs. This additional memory is allocated for every thread that calls
> > + *   tld_get_data() even if no tld_create_key are actually called, so be mindful of
> > + *   potential memory wastage. Use TLD_DEFINE_KEY() whenever possible as just enough memory
> > + *   will be allocated for TLDs created with it.
> > + *
> > + *
> > + *   TLD_NAME_LEN - The maximum length of the name of a TLD (default: 62)
> > + *
> > + *   Setting TLD_NAME_LEN will affect the maximum number of TLDs a process can store,
> > + *   TLD_MAX_DATA_CNT.
> > + *
> > + *
> > + *   TLD_DATA_USE_ALIGNED_ALLOC - Always use aligned_alloc() instead of malloc()
> > + *
> > + *   When allocating the memory for storing TLDs, we need to make sure there is a memory
> > + *   region of the X bytes within a page. This is due to the limit posed by UPTR: memory
> > + *   pinned to the kernel cannot exceed a page nor can it cross the page boundary. The
> > + *   library normally calls malloc(2*X) given X bytes of total TLDs, and only uses
> > + *   aligned_alloc(PAGE_SIZE, X) when X >= PAGE_SIZE / 2. This is to reduce memory wastage
> > + *   as not all memory allocator can use the exact amount of memory requested to fulfill
> > + *   aligned_alloc(). For example, some may round the size up to the alignment. Enable the
> > + *   option to always use aligned_alloc() if the implementation has low memory overhead.
> > + */
> > +
> > +#define TLD_PIDFD_THREAD O_EXCL
> > +
>
> Is this #define necessary? The alias is used only once in the code and
> passed directly into syscall().
> Is it supposed to be called by the user in any way?
>

Nope. I will change to use O_EXCL directly.

> > +#define TLD_PAGE_SIZE getpagesize()
> > +#define TLD_PAGE_MASK (~(TLD_PAGE_SIZE - 1))
> > +
> > +#define TLD_ROUND_MASK(x, y) ((__typeof__(x))((y) - 1))
> > +#define TLD_ROUND_UP(x, y) ((((x) - 1) | TLD_ROUND_MASK(x, y)) + 1)
> > +
> > +#define TLD_READ_ONCE(x) (*(volatile typeof(x) *)&(x))
> > +
> > +#ifndef TLD_DYN_DATA_SIZE
> > +#define TLD_DYN_DATA_SIZE 64
> > +#endif
> > +
> > +#define TLD_MAX_DATA_CNT (TLD_PAGE_SIZE / sizeof(struct tld_metadata) - 1)
> > +
> > +#ifndef TLD_NAME_LEN
> > +#define TLD_NAME_LEN 62
> > +#endif
> > +
> > +#ifdef __cplusplus
> > +extern "C" {
> > +#endif
> > +
> > +typedef struct {
> > +       __s16 off;
> > +} tld_key_t;
> > +
> > +struct tld_metadata {
> > +       char name[TLD_NAME_LEN];
> > +       _Atomic __u16 size;
> > +};
> > +
> > +struct u_tld_metadata {
> > +       _Atomic __u8 cnt;
> > +       __u16 size;
> > +       struct tld_metadata metadata[];
> > +};
> > +
> > +struct u_tld_data {
> > +       __u64 start; /* offset of u_tld_data->data in a page */
> > +       char data[];
> > +};
> > +
> > +struct tld_map_value {
> > +       void *data;
> > +       struct u_tld_metadata *metadata;
> > +};
> > +
> > +struct u_tld_metadata * _Atomic tld_metadata_p __attribute__((weak));
> > +__thread struct u_tld_data *tld_data_p __attribute__((weak));
> > +__thread void *tld_data_alloc_p __attribute__((weak));
> > +
> > +#ifdef TLD_FREE_DATA_ON_THREAD_EXIT
> > +pthread_key_t tld_pthread_key __attribute__((weak));
> > +
> > +static void tld_free(void);
> > +
> > +static void __tld_thread_exit_handler(void *unused)
> > +{
> > +       tld_free();
> > +}
> > +#endif
> > +
> > +static int __tld_init_metadata(void)
>
> Nit: rename it to init_metadata_p? It's about the per-process metadata, not the
> per-key metadata.

I will try to make the distinction of the metadata page and the per
TLD metadata clear in the naming.

>
> > +{
> > +       struct u_tld_metadata *meta, *uninit = NULL;
> > +       int err = 0;
> > +
> > +       meta = (struct u_tld_metadata *)aligned_alloc(TLD_PAGE_SIZE, TLD_PAGE_SIZE);
> > +       if (!meta) {
> > +               err = -ENOMEM;
> > +               goto out;
> > +       }
> > +
> > +       memset(meta, 0, TLD_PAGE_SIZE);
> > +       meta->size = TLD_DYN_DATA_SIZE;
> > +
> > +       if (!atomic_compare_exchange_strong(&tld_metadata_p, &uninit, meta)) {
> > +               free(meta);
> > +               goto out;
> > +       }
> > +
> > +#ifdef TLD_FREE_DATA_ON_THREAD_EXIT
> > +       pthread_key_create(&tld_pthread_key, __tld_thread_exit_handler);
> > +#endif
> > +out:
> > +       return err;
> > +}
> > +
> > +static int __tld_init_data(int map_fd)
> > +{
> > +       bool use_aligned_alloc = false;
> > +       struct tld_map_value map_val;
> > +       struct u_tld_data *data;
> > +       int err, tid_fd = -1;
> > +       void *d = NULL;
> > +
> > +       tid_fd = syscall(SYS_pidfd_open, gettid(), TLD_PIDFD_THREAD);
> > +       if (tid_fd < 0) {
> > +               err = -errno;
> > +               goto out;
> > +       }
> > +
> > +#ifdef TLD_DATA_USE_ALIGNED_ALLOC
> > +       use_aligned_alloc = true;
> > +#endif
> > +
> > +       /*
> > +        * tld_metadata_p->size = TLD_DYN_DATA_SIZE +
> > +        *          total size of TLDs defined via TLD_DEFINE_KEY()
> > +        */
> > +       if (use_aligned_alloc || tld_metadata_p->size >= TLD_PAGE_SIZE / 2)
> > +               d = aligned_alloc(TLD_PAGE_SIZE, tld_metadata_p->size);
>
> Comment: The manpage for aligned_alloc() says tld_metadata_p->size
> should be a multiple
> of the alignment, but at least glibc doesn't actually enforce that so
> this call should work fine.

Some additional context for the TLD_DATA_USE_ALIGNED_ALLOC option

Recent glibc aligned_alloc() implementation is able to fulfill
aligned_alloc(align, sz) with sz + a few bytes of overhead. While for
jemalloc, it will use align byte of memory in this case. Hence, the
option is here to allow users to reduce their memory usage if the
memory allocator allows.

>
> > +       else
> > +               d = malloc(tld_metadata_p->size * 2);
> > +       if (!d) {
> > +               err = -ENOMEM;
> > +               goto out;
> > +       }
> > +
> > +       /*
> > +        * Always pass a page-aligned address to UPTR since the size of tld_map_value::data
> > +        * is a page in BTF. If d spans across two pages, use the page that contains large
> > +        * enough memory.
> > +        */
> > +       if (TLD_PAGE_SIZE - (~TLD_PAGE_MASK & (intptr_t)d) >= tld_metadata_p->size) {
> > +               map_val.data = (void *)(TLD_PAGE_MASK & (intptr_t)d);
> > +               data = d;
> > +               data->start = (~TLD_PAGE_MASK & (intptr_t)d) + offsetof(struct u_tld_data, data);
> > +       } else {
> > +               map_val.data = (void *)(TLD_ROUND_UP((intptr_t)d, TLD_PAGE_SIZE));
> > +               data = (void *)(TLD_ROUND_UP((intptr_t)d, TLD_PAGE_SIZE));
> > +               data->start = offsetof(struct u_tld_data, data);
> > +       }
> > +       map_val.metadata = TLD_READ_ONCE(tld_metadata_p);
> > +
> > +       err = bpf_map_update_elem(map_fd, &tid_fd, &map_val, 0);
> > +       if (err) {
> > +               free(d);
> > +               goto out;
> > +       }
> > +
> > +       tld_data_p = (struct u_tld_data *)data;
> > +       tld_data_alloc_p = d;
> > +#ifdef TLD_FREE_DATA_ON_THREAD_EXIT
> > +       pthread_setspecific(tld_pthread_key, (void *)1);
> > +#endif
> > +out:
> > +       if (tid_fd >= 0)
> > +               close(tid_fd);
> > +       return err;
> > +}
> > +
> > +static tld_key_t __tld_create_key(const char *name, size_t size, bool dyn_data)
> > +{
> > +       int err, i, sz, off = 0;
> > +       __u8 cnt;
> > +
> > +       if (!TLD_READ_ONCE(tld_metadata_p)) {
> > +               err = __tld_init_metadata();
> > +               if (err)
> > +                       return (tld_key_t){err};
> > +       }
> > +
> > +       for (i = 0; i < TLD_MAX_DATA_CNT; i++) {
> > +retry:
> > +               cnt = atomic_load(&tld_metadata_p->cnt);
> > +               if (i < cnt) {
> > +                       /* A metadata is not ready until size is updated with a non-zero value */
> > +                       while (!(sz = atomic_load(&tld_metadata_p->metadata[i].size)))
> > +                               sched_yield();
> > +
> > +                       if (!strncmp(tld_metadata_p->metadata[i].name, name, TLD_NAME_LEN))
> > +                               return (tld_key_t){-EEXIST};
> > +
> > +                       off += TLD_ROUND_UP(sz, 8);
> > +                       continue;
> > +               }
> > +
> > +               /*
> > +                * TLD_DEFINE_KEY() is given memory upto a page while at most
>
> Typo: upto -> up to
>

Thanks for catching all the typos!

>
> > +                * TLD_DYN_DATA_SIZE is allocated for tld_create_key()
> > +                */
> > +               if (dyn_data) {
> > +                       if (off + TLD_ROUND_UP(size, 8) > tld_metadata_p->size)
> > +                               return (tld_key_t){-E2BIG};
> > +               } else {
> > +                       if (off + TLD_ROUND_UP(size, 8) > TLD_PAGE_SIZE - sizeof(struct u_tld_data))
> > +                               return (tld_key_t){-E2BIG};
> > +                       tld_metadata_p->size += TLD_ROUND_UP(size, 8);
>
> Discussion: Would there be advantages to adjusting tld_metadata_p with
> dynamic keys defined before
> __tld_init_data is called? That way we wouldn't have to worry about
> rightsizing TLD_DYN_DATA_SIZE,
> and it may be a reasonable expectation that all dynamic keys are
> defined before the program starts using
> the TLDs. Imo it's not necessary now because we don't know how this
> API will end up being used, but
> just wondering if it is a possibility.

TLD_DEFINE_KEY() is introduced in this iteration to address the memory
wastage and rightsizing issue of tld_create_key().

I also think that keys should be known before the program starts in
most cases, and TLD_DEFINE_KEY() should be used whenever possible.
But, I prefer to keep the dynamic API here just in case it is useful.
Plus the code is mostly shared.

>
> > +               }
> > +
> > +               /*
> > +                * Only one tld_create_key() can increase the current cnt by one and
> > +                * takes the latest available slot. Other threads will check again if a new
> > +                * TLD can still be added, and then compete for the new slot after the
> > +                * succeeding thread update the size.
> > +                */
> > +               if (!atomic_compare_exchange_strong(&tld_metadata_p->cnt, &cnt, cnt + 1))
> > +                       goto retry;
> > +
> > +               strncpy(tld_metadata_p->metadata[i].name, name, TLD_NAME_LEN);
> > +               atomic_store(&tld_metadata_p->metadata[i].size, size);
> > +               return (tld_key_t){(__s16)off};
> > +       }
> > +
> > +       return (tld_key_t){-ENOSPC};
> > +}
> > +
> > +/**
> > + * TLD_DEFINE_KEY() - Define a TLD and a global variable key associated with the TLD.
> > + *
> > + * @name: The name of the TLD
> > + * @size: The size of the TLD
> > + * @key: The variable name of the key. Cannot exceed TLD_NAME_LEN
> > + *
> > + * The macro can only be used in file scope.
> > + *
> > + * A global variable key of opaque type, tld_key_t, will be declared and initialized before
> > + * main() starts. Use tld_key_is_err() or tld_key_err_or_zero() later to check if the key
> > + * creation succeeded. Pass the key to tld_get_data() to get a pointer to the TLD.
> > + * bpf programs can also fetch the same key by name.
> > + *
> > + * The total size of TLDs created using TLD_DEFINE_KEY() cannot exceed a page. Just
> > + * enough memory will be allocated for each thread on the first call to tld_get_data().
> > + */
> > +#define TLD_DEFINE_KEY(key, name, size)                        \
> > +tld_key_t key;                                         \
> > +                                                       \
> > +__attribute__((constructor))                           \
> > +void __tld_define_key_##key(void)                      \
> > +{                                                      \
> > +       key = __tld_create_key(name, size, false);      \
> > +}
> > +
> > +/**
> > + * tld_create_key() - Create a TLD and return a key associated with the TLD.
> > + *
> > + * @name: The name the TLD
> > + * @size: The size of the TLD
> > + *
> > + * Return an opaque object key. Use tld_key_is_err() or tld_key_err_or_zero() to check
> > + * if the key creation succeeded. Pass the key to tld_get_data() to get a pointer to
> > + * locate the TLD. bpf programs can also fetch the same key by name.
> > + *
> > + * Use tld_create_key() only when a TLD needs to be created dynamically (e.g., @name is
> > + * not known statically or a TLD needs to be created conditionally)
> > + *
> > + * An additional TLD_DYN_DATA_SIZE bytes are allocated per-thread to accommodate TLDs
> > + * created dynamically with tld_create_key(). Since only a user page is pinned to the
> > + * kernel, when TLDs created with TLD_DEFINE_KEY() uses more than TLD_PAGE_SIZE -
> > + * TLD_DYN_DATA_SIZE, the buffer size will be limited to the rest of the page.
> > + */
> > +__attribute__((unused))
> > +static tld_key_t tld_create_key(const char *name, size_t size)
> > +{
> > +       return __tld_create_key(name, size, true);
> > +}

[...]

Powered by blists - more mailing lists