Re: [PATCH v3 4/6] alloc_tag: add accuracy based filtering to ioctl

[Hao Ge]

Hi Suren


On 2026/6/9 04:55, Suren Baghdasaryan wrote:
> On Mon, Jun 8, 2026 at 1:25 AM Hao Ge <hao.ge@xxxxxxxxx> wrote:
> > On 2026/6/8 14:22, Hao Ge wrote:
> > > Hi Abhishek
> > >
> > >
> > > On 2026/6/6 07:36, Abhishek Bapat wrote:
> > > > Extend the allocinfo filtering mechanism to allow users to filter tags
> > > > based on their accuracy.
> > > >
> > > > Signed-off-by: Abhishek Bapat <abhishekbapat@xxxxxxxxxx>
> > > > ---
> > > >    include/uapi/linux/alloc_tag.h | 3 +++
> > > >    lib/alloc_tag.c                | 8 ++++++++
> > > >    2 files changed, 11 insertions(+)
> > > >
> > > > diff --git a/include/uapi/linux/alloc_tag.h
> > > > b/include/uapi/linux/alloc_tag.h
> > > > index 0e648192df4d..42445bdb11c5 100644
> > > > --- a/include/uapi/linux/alloc_tag.h
> > > > +++ b/include/uapi/linux/alloc_tag.h
> > > > @@ -20,6 +20,7 @@ struct allocinfo_tag {
> > > >        char function[ALLOCINFO_STR_SIZE];
> > > >        char filename[ALLOCINFO_STR_SIZE];
> > > >        __u64 lineno;
> > > > +    __u64 inaccurate;
> > >
> > > I was wondering if it would make sense to define inaccurate as a flags
> > > field
> > >
> > > (e.g. __u64 flags with ALLOCINFO_TAG_F_INACCURATE (1 <<0)),
> > >
> > > so that only bit 0 is used today and the upper bits are reserved for
> > > future use,
> > >
> > > aligning with current kernel codebase.
> > >
> > > This design also allows for better extensibility if we need to
> > >
> > > add new flags for any reason in the future.
> > >
> > > We also need to add flag validity checks if we go this route.
> > And I've reviewed the issue reported by Sashiko, and I think it's valid.
> >
> > When we expand the allocinfo_tag_data structure
> >
> > struct allocinfo_tag_data{
> >
> >       char modname[64];
> >
> >       char function[64];
> >
> >       char filename[64];
> >
> >       __u64 lineno;
> >
> >       __u64 inaccurate;
> >
> >       __u64 bytes;
> >
> >       __u64 calls;
> >
> >       __u8 accurate;
> >     /* padding */
> >
> > }
> >
> > I think user space may see two fields related to inaccuracy.
> Yes but one field (inside allocinfo_tag) is the input parameter which
> user provides to specify the filtering criteria and the other is the
> returned tag information. It's similar to any other tag attribute
> which you can be included in the filters.
>
> > How do you like these modifications?
> >
> >
> > diff --git a/include/uapi/linux/alloc_tag.h b/include/uapi/linux/alloc_tag.h
> > --- a/include/uapi/linux/alloc_tag.h
> > +++ b/include/uapi/linux/alloc_tag.h
> > @@ -20,7 +20,6 @@ struct allocinfo_tag {
> >        char function[ALLOCINFO_STR_SIZE];
> >        char filename[ALLOCINFO_STR_SIZE];
> >        __u64 lineno;
> > -    __u64 inaccurate;
> >    };
> >
> >    /* The alignment ensures 32-bit compatible interfaces are not broken */
> > @@ -40,7 +39,7 @@ enum {
> >        ALLOCINFO_FILTER_FUNCTION,
> >        ALLOCINFO_FILTER_FILENAME,
> >        ALLOCINFO_FILTER_LINENO,
> > -    ALLOCINFO_FILTER_INACCURATE,
> > +    ALLOCINFO_FILTER_FLAGS,
> >        ALLOCINFO_FILTER_MIN_SIZE,
> >        ALLOCINFO_FILTER_MAX_SIZE,
> >        __ALLOCINFO_FILTER_LAST = ALLOCINFO_FILTER_MAX_SIZE
> > @@ -50,16 +49,20 @@ enum {
> >    #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_INACCURATE    (1 <<
> > ALLOCINFO_FILTER_INACCURATE)
> > +#define ALLOCINFO_FILTER_MASK_FLAGS        (1 << ALLOCINFO_FILTER_FLAGS)
> >    #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)
> >
> > +#define ALLOCINFO_FILTER_F_INACCURATE    (1ULL << 0)
> > +#define ALLOCINFO_FILTER_FLAGS_ALL ALLOCINFO_FILTER_F_INACCURATE
> > +
> >    struct allocinfo_filter {
> >        __u64 mask; /* bitmask of the filter fields used */
> >        struct allocinfo_tag fields;
> > +    __u64 flags; /* bitmask of ALLOCINFO_FILTER_F_* */
> >        __u64 min_size;
> >        __u64 max_size;
> >    };
> > diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c
> > --- a/lib/alloc_tag.c
> > +++ b/lib/alloc_tag.c
> > @@ -249,8 +249,6 @@ static bool matches_filter(struct codetag *ct,
> > struct allocinfo_filter *filter,
> >                   struct alloc_tag_counters *counters,
> >                   bool *fetched_counters)
> >    {
> > -    bool inaccurate;
> > -
> >        if (!filter || !filter->mask)
> >            return true;
> >
> > @@ -277,10 +275,11 @@ static bool matches_filter(struct codetag *ct,
> > struct allocinfo_filter *filter,
> >            ct->lineno != filter->fields.lineno)
> >            return false;
> >
> > -    if (filter->mask & ALLOCINFO_FILTER_MASK_INACCURATE) {
> > -        inaccurate = !!(ct->flags & CODETAG_FLAG_INACCURATE);
> > -        if (inaccurate != !!(filter->fields.inaccurate))
> > -            return false;
> > +    if (filter->mask & ALLOCINFO_FILTER_MASK_FLAGS) {
> > +        if (filter->flags & ALLOCINFO_FILTER_F_INACCURATE) {
> > +            if (!(ct->flags & CODETAG_FLAG_INACCURATE))
> How would you filter records which have only accurate data?


Sorry, I overlooked this case.

Since allocinfo_tag_data exposes both inaccurate (from allocinfo_tag) and

accurate (from allocinfo_counter), userspace developers might mistakenly 
read

inaccurate instead of accurate when checking accuracy.

How about we add a comment to clarify?

struct allocinfo_tag {

    /* ... */

    __u64 lineno;

    /* filter criteria only; see allocinfo_counter.accurate for actual 
accuracy */

    __u64 inaccurate;

};


LGTM for the rest.


Thanks

Best Regards

Hao

> Overall I would prefer ALLOCINFO_FILTER_MASK_INACCURATE rather than
> ALLOCINFO_FILTER_MASK_FLAGS. The fact that this attribute is a
> single-bit flag is a technical detail. It's still a tag attribuite
> like file and module names and IMO deserves its own filter.
>
>
>
> > +                return false;
> > +        }
> >        }
> >
> >        if (filter->mask & (ALLOCINFO_FILTER_MASK_MIN_SIZE |
> > ALLOCINFO_FILTER_MASK_MAX_SIZE)) {
> > @@ -318,6 +317,10 @@ 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_FLAGS) &&
> > +        (params.filter.flags & ~ALLOCINFO_FILTER_FLAGS_ALL))
> > +        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)
> >
> >
> > Thanks
> >
> > Best Regards
> >
> > Hao
> >
> >
> > > Thanks
> > >
> > > Best Regards
> > >
> > > Hao
> > >
> > >
> > > >    };
> > > >      /* The alignment ensures 32-bit compatible interfaces are not
> > > > broken */
> > > > @@ -39,6 +40,7 @@ enum {
> > > >        ALLOCINFO_FILTER_FUNCTION,
> > > >        ALLOCINFO_FILTER_FILENAME,
> > > >        ALLOCINFO_FILTER_LINENO,
> > > > +    ALLOCINFO_FILTER_INACCURATE,
> > > >        ALLOCINFO_FILTER_MIN_SIZE,
> > > >        ALLOCINFO_FILTER_MAX_SIZE,
> > > >        __ALLOCINFO_FILTER_LAST = ALLOCINFO_FILTER_MAX_SIZE
> > > > @@ -48,6 +50,7 @@ enum {
> > > >    #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_INACCURATE    (1 <<
> > > > ALLOCINFO_FILTER_INACCURATE)
> > > >    #define ALLOCINFO_FILTER_MASK_MIN_SIZE        (1 <<
> > > > ALLOCINFO_FILTER_MIN_SIZE)
> > > >    #define ALLOCINFO_FILTER_MASK_MAX_SIZE        (1 <<
> > > > ALLOCINFO_FILTER_MAX_SIZE)
> > > >    diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c
> > > > index ddc6946f56ab..cbcd12c4ef9c 100644
> > > > --- a/lib/alloc_tag.c
> > > > +++ b/lib/alloc_tag.c
> > > > @@ -249,6 +249,8 @@ static bool matches_filter(struct codetag *ct,
> > > > struct allocinfo_filter *filter,
> > > >                   struct alloc_tag_counters *counters,
> > > >                   bool *fetched_counters)
> > > >    {
> > > > +    bool inaccurate;
> > > > +
> > > >        if (!filter || !filter->mask)
> > > >            return true;
> > > >    @@ -275,6 +277,12 @@ static bool matches_filter(struct codetag *ct,
> > > > struct allocinfo_filter *filter,
> > > >            ct->lineno != filter->fields.lineno)
> > > >            return false;
> > > >    +    if (filter->mask & ALLOCINFO_FILTER_MASK_INACCURATE) {
> > > > +        inaccurate = !!(ct->flags & CODETAG_FLAG_INACCURATE);
> > > > +        if (inaccurate != !!(filter->fields.inaccurate))
> > > > +            return false;
> > > > +    }
> > > > +
> > > >        if (filter->mask & (ALLOCINFO_FILTER_MASK_MIN_SIZE |
> > > > ALLOCINFO_FILTER_MASK_MAX_SIZE)) {
> > > >            if (!*fetched_counters) {
> > > >                *counters = allocinfo_prefetch_counters(ct);