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

[Abhishek Bapat]

On Tue, Jun 9, 2026 at 7:41 AM Suren Baghdasaryan <surenb@xxxxxxxxxx> wrote:
>
> 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.
>
Ack, I'll include the recommended comment.
> >
> > };
> >
> >
> > 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);