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

[Suren Baghdasaryan]

On Mon, Jun 8, 2026 at 6:26 PM Hao Ge <hao.ge@xxxxxxxxx> wrote:
>
> 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;

I think we had comments showing which block of parameters are inputs
and which ones are outputs but I'm not opposed to an additional
reminder here.

>
> };
>
>
> 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);