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

[Hao Ge <hao.ge@linux.dev>]

Hi Suren


On 2026/6/9 04:55, Suren Baghdasaryan wrote:
> On Mon, Jun 8, 2026 at 1:25 AM Hao Ge <hao.ge@linux.dev> 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@google.com>
>>>> ---
>>>>    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);