Re: [PATCH v5 3/6] alloc_tag: add size-based filtering to ioctl
From: Suren Baghdasaryan
Date: Wed Jun 17 2026 - 18:35:05 EST
On Wed, Jun 17, 2026 at 1:55 PM Abhishek Bapat <abhishekbapat@xxxxxxxxxx> wrote:
>
> On Wed, Jun 17, 2026 at 9:29 AM Suren Baghdasaryan <surenb@xxxxxxxxxx> wrote:
> >
> > On Mon, Jun 15, 2026 at 4:04 PM Abhishek Bapat <abhishekbapat@xxxxxxxxxx> wrote:
> > >
> > > Extend the allocinfo filtering mechanism to allow users to filter tags
> > > based on the total number of bytes allocated [min_size, max_size]. The
> > > size range is inclusive.
> > >
> > > Filtering by size involves retrieving allocinfo per-CPU counters, which
> > > is an expensive operation. Hence, the performance of size-based
> > > filtering will be worse than other filters.
> > >
> > > Signed-off-by: Abhishek Bapat <abhishekbapat@xxxxxxxxxx>
> > > Acked-by: Hao Ge <hao.ge@xxxxxxxxx>
> > > ---
> > > include/uapi/linux/alloc_tag.h | 8 ++++-
> > > lib/alloc_tag.c | 63 ++++++++++++++++++++++++++++------
> > > 2 files changed, 59 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/include/uapi/linux/alloc_tag.h b/include/uapi/linux/alloc_tag.h
> > > index 3b11877955b9..7f5acbb44c14 100644
> > > --- a/include/uapi/linux/alloc_tag.h
> > > +++ b/include/uapi/linux/alloc_tag.h
> > > @@ -45,13 +45,17 @@ enum {
> > > ALLOCINFO_FILTER_FUNCTION,
> > > ALLOCINFO_FILTER_FILENAME,
> > > ALLOCINFO_FILTER_LINENO,
> > > - __ALLOCINFO_FILTER_LAST = ALLOCINFO_FILTER_LINENO
> > > + ALLOCINFO_FILTER_MIN_SIZE,
> > > + ALLOCINFO_FILTER_MAX_SIZE,
> > > + __ALLOCINFO_FILTER_LAST = ALLOCINFO_FILTER_MAX_SIZE
> > > };
> > >
> > > #define ALLOCINFO_FILTER_MASK_MODNAME (1 << ALLOCINFO_FILTER_MODNAME)
> > > #define ALLOCINFO_FILTER_MASK_FUNCTION (1 << ALLOCINFO_FILTER_FUNCTION)
> > > #define ALLOCINFO_FILTER_MASK_FILENAME (1 << ALLOCINFO_FILTER_FILENAME)
> > > #define ALLOCINFO_FILTER_MASK_LINENO (1 << ALLOCINFO_FILTER_LINENO)
> > > +#define ALLOCINFO_FILTER_MASK_MIN_SIZE (1 << ALLOCINFO_FILTER_MIN_SIZE)
> > > +#define ALLOCINFO_FILTER_MASK_MAX_SIZE (1 << ALLOCINFO_FILTER_MAX_SIZE)
> > >
> > > #define ALLOCINFO_FILTER_MASKS \
> > > ((1 << (__ALLOCINFO_FILTER_LAST + 1)) - 1)
> > > @@ -59,6 +63,8 @@ enum {
> > > struct allocinfo_filter {
> > > __u64 mask; /* bitmask of the filter fields used */
> > > struct allocinfo_tag fields;
> > > + __u64 min_size;
> > > + __u64 max_size;
> > > };
> > >
> > > struct allocinfo_get_at {
> > > diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c
> > > index 5feb61d9fb92..b3d21834b61e 100644
> > > --- a/lib/alloc_tag.c
> > > +++ b/lib/alloc_tag.c
> > > @@ -195,15 +195,26 @@ static int allocinfo_cmp_str(const char *str, const char *template)
> > > return strncmp(allocinfo_str(str), template, ALLOCINFO_STR_SIZE);
> > > }
> > >
> > > +/* Fetch the per-CPU counters */
> > > +static inline struct alloc_tag_counters allocinfo_prefetch_counters(struct codetag *ct)
> > > +{
> > > + return alloc_tag_read(ct_to_alloc_tag(ct));
> > > +}
> > > +
> > > /*
> > > * Populates the UAPI allocinfo_tag_data structure with active runtime
> > > * profiling counters extracted from the given kernel codetag.
> > > */
> > > static void allocinfo_to_params(struct codetag *ct,
> > > - struct allocinfo_tag_data *data)
> > > + struct allocinfo_tag_data *data,
> > > + struct alloc_tag_counters *counters)
> > > {
> > > - struct alloc_tag *tag = ct_to_alloc_tag(ct);
> > > - struct alloc_tag_counters counter = alloc_tag_read(tag);
> > > + struct alloc_tag_counters local_counters;
> > > +
> > > + if (!counters) {
> > > + local_counters = allocinfo_prefetch_counters(ct);
> > > + counters = &local_counters;
> > > + }
> > >
> > > if (ct->modname)
> > > allocinfo_copy_str(data->tag.modname, ct->modname);
> > > @@ -212,9 +223,9 @@ static void allocinfo_to_params(struct codetag *ct,
> > > allocinfo_copy_str(data->tag.function, ct->function);
> > > allocinfo_copy_str(data->tag.filename, ct->filename);
> > > data->tag.lineno = ct->lineno;
> > > - data->counter.bytes = counter.bytes;
> > > - data->counter.calls = counter.calls;
> > > - data->counter.accurate = !alloc_tag_is_inaccurate(tag);
> > > + data->counter.bytes = counters->bytes;
> > > + data->counter.calls = counters->calls;
> > > + data->counter.accurate = !alloc_tag_is_inaccurate(ct_to_alloc_tag(ct));
> > > }
> > >
> > > /*
> > > @@ -238,7 +249,9 @@ static int allocinfo_ioctl_get_content_id(struct seq_file *m, void __user *arg)
> > > * Verifies whether a given codetag satisfies the active filtering criteria by
> > > * matching its characteristics against the specified filter.
> > > */
> > > -static bool matches_filter(struct codetag *ct, struct allocinfo_filter *filter)
> > > +static bool matches_filter(struct codetag *ct, struct allocinfo_filter *filter,
> > > + struct alloc_tag_counters *counters,
> > > + bool *fetched_counters)
> > > {
> > > if (!filter || !filter->mask)
> > > return true;
> > > @@ -265,6 +278,19 @@ static bool matches_filter(struct codetag *ct, struct allocinfo_filter *filter)
> > > ct->lineno != filter->fields.lineno)
> > > return false;
> > >
> > > + if (filter->mask & (ALLOCINFO_FILTER_MASK_MIN_SIZE | ALLOCINFO_FILTER_MASK_MAX_SIZE)) {
> > > + if (!*fetched_counters) {
> > > + *counters = allocinfo_prefetch_counters(ct);
> > > + *fetched_counters = true;
> > > + }
> > > + if ((filter->mask & ALLOCINFO_FILTER_MASK_MIN_SIZE) &&
> > > + counters->bytes < filter->min_size)
> > > + return false;
> > > + if ((filter->mask & ALLOCINFO_FILTER_MASK_MAX_SIZE) &&
> > > + counters->bytes > filter->max_size)
> > > + return false;
> > > + }
> > > +
> > > return true;
> > > }
> > >
> > > @@ -278,6 +304,8 @@ static int allocinfo_ioctl_get_at(struct seq_file *m, void __user *arg)
> > > struct codetag *ct;
> > > struct allocinfo_get_at params = {0};
> > > __u64 skip_count;
> > > + struct alloc_tag_counters counters;
> > > + bool fetched_counters;
> > >
> > > if (copy_from_user(¶ms, arg, sizeof(params)))
> > > return -EFAULT;
> > > @@ -285,6 +313,11 @@ static int allocinfo_ioctl_get_at(struct seq_file *m, void __user *arg)
> > > if (params.filter.mask & ~ALLOCINFO_FILTER_MASKS)
> > > return -EINVAL;
> > >
> > > + if ((params.filter.mask & ALLOCINFO_FILTER_MASK_MIN_SIZE) &&
> > > + (params.filter.mask & ALLOCINFO_FILTER_MASK_MAX_SIZE) &&
> > > + params.filter.min_size > params.filter.max_size)
> > > + return -EINVAL;
> > > +
> > > priv = m->private;
> > >
> > > mutex_lock(&priv->ioctl_lock);
> > > @@ -308,7 +341,8 @@ static int allocinfo_ioctl_get_at(struct seq_file *m, void __user *arg)
> > > ct = codetag_next_ct(&priv->ioctl_iter);
> > >
> > > while (ct) {
> > > - if (matches_filter(ct, &priv->filter)) {
> > > + fetched_counters = false;
> > > + if (matches_filter(ct, &priv->filter, &counters, &fetched_counters)) {
> >
> > Do we really need this "fetched_counters" parameter? Here are the
> > possible cases:
> > 1. If the filter does not include ALLOCINFO_FILTER_MASK_MIN_SIZE |
> > ALLOCINFO_FILTER_MASK_MAX_SIZE then counters would not be fetched.
> > 2. If the filter includes ALLOCINFO_FILTER_MASK_MIN_SIZE |
> > ALLOCINFO_FILTER_MASK_MAX_SIZE and
> > 2.1. matches_filter() returns true then we know counters were fetched
> > because they had to be validated.
> > 2.2. matches_filter() returns false then we don't care if the counters
> > were fetched. We do not report that tag anyway.
> >
> > So, instead of passing fetched_counters to matches_filter() we could do this:
> >
> > bool filter_by_size = (params.filter.mask &
> > (ALLOCINFO_FILTER_MASK_MIN_SIZE | ALLOCINFO_FILTER_MASK_MAX_SIZE)) !=
> > 0;
> > while (ct) {
> > if (matches_filter(ct, &priv->filter, &counters)) {
> > ...
> > }
> > if (ct) {
> > allocinfo_to_params(ct, ¶ms.data, filter_by_size ?
> > &counters : NULL);
> > ...
> > }
> >
> > Wouldn't that work?
> >
>
> While we can deduce whether counters were fetched outside the
> matches_filter function, I think the current implementation is more
> intuitive from a readability perspective. I believe it should be kept
> as is for that reason. If we extract the logic, we'll first have to
> replicate the boolean logic at two places. Second, we'd need to add a
> comment explaining the boolean calculation, and the reader might have
> a higher cognitive load trying to determine which function populates
> the counters. The current implementation makes it easy for the reader
> to deduce the original intention. Let me know what you think.
Ok, I guess you have a point.
I was also thinking why we are passing NULL to allocinfo_to_params()
to fetch the counters into a local variable? Why can't we simply call
allocinfo_prefetch_counters() before calling allocinfo_to_params()
when fetched_counters==false? Basically:
if (!fetched_counters)
counters = allocinfo_prefetch_counters(ct);
allocinfo_to_params(ct, ¶ms.data, &counters);
This would simplify allocinfo_to_params() because counter will never
be NULL and it would not need local counters.
>
> > > if (skip_count == 0)
> > > break;
> > > skip_count--;
> > > @@ -317,7 +351,7 @@ static int allocinfo_ioctl_get_at(struct seq_file *m, void __user *arg)
> > > }
> > >
> > > if (ct) {
> > > - allocinfo_to_params(ct, ¶ms.data);
> > > + allocinfo_to_params(ct, ¶ms.data, fetched_counters ? &counters : NULL);
> > > priv->positioned = true;
> > > }
> > >
> > > @@ -343,6 +377,8 @@ static int allocinfo_ioctl_get_next(struct seq_file *m, void __user *arg)
> > > struct codetag *ct;
> > > struct allocinfo_tag_data params;
> > > int ret = 0;
> > > + struct alloc_tag_counters counters;
> > > + bool fetched_counters;
> > >
> > > memset(¶ms, 0, sizeof(params));
> > > priv = m->private;
> > > @@ -356,10 +392,15 @@ static int allocinfo_ioctl_get_next(struct seq_file *m, void __user *arg)
> > > }
> > >
> > > ct = codetag_next_ct(&priv->ioctl_iter);
> > > - while (ct && !matches_filter(ct, &priv->filter))
> > > + while (ct) {
> > > + fetched_counters = false;
> > > + if (matches_filter(ct, &priv->filter, &counters, &fetched_counters))
> > > + break;
> > > ct = codetag_next_ct(&priv->ioctl_iter);
> > > + }
> > > +
> > > if (ct)
> > > - allocinfo_to_params(ct, ¶ms);
> > > + allocinfo_to_params(ct, ¶ms, fetched_counters ? &counters : NULL);
> > >
> > > if (!ct) {
> > > priv->positioned = false;
> > > --
> > > 2.54.0.1136.gdb2ca164c4-goog
> > >