Re: [PATCH v5 3/6] alloc_tag: add size-based filtering to ioctl
From: Abhishek Bapat
Date: Wed Jun 17 2026 - 16:55:29 EST
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.
> > 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
> >