From: Hao Ge <hao.ge@linux.dev>
To: Abhishek Bapat <abhishekbapat@google.com>,
Suren Baghdasaryan <surenb@google.com>,
Andrew Morton <akpm@linux-foundation.org>,
Kent Overstreet <kent.overstreet@linux.dev>
Cc: Shuah Khan <skhan@linuxfoundation.org>,
Jonathan Corbet <corbet@lwn.net>,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, Sourav Panda <souravpanda@google.com>
Subject: Re: [PATCH 3/6] alloc_tag: add size-based filtering to ioctl
Date: Thu, 14 May 2026 14:53:56 +0800 [thread overview]
Message-ID: <b681fad1-1a27-47fa-b58a-1f639eb3f3d9@linux.dev> (raw)
In-Reply-To: <06b4fc2457fb4b75eb1ef18320a8722ddb5a850f.1777936301.git.abhishekbapat@google.com>
Hi Abhishek
On 2026/5/5 07:36, Abhishek Bapat 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@google.com>
> ---
> include/uapi/linux/alloc_tag.h | 8 +++++++-
> lib/alloc_tag.c | 15 +++++++++++++++
> 2 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/alloc_tag.h b/include/uapi/linux/alloc_tag.h
> index 0cc9db5298c6..229068efd24c 100644
> --- a/include/uapi/linux/alloc_tag.h
> +++ b/include/uapi/linux/alloc_tag.h
> @@ -20,6 +20,8 @@ struct allocinfo_tag {
> char function[ALLOCINFO_STR_SIZE];
> char filename[ALLOCINFO_STR_SIZE];
> __u64 lineno;
> + __u64 min_size;
> + __u64 max_size;
> };
allocinfo_tag is used both as a tag identifier in the output data
(allocinfo_tag_data.tag) and as filter criteria
(allocinfo_filter.fields). min_size and max_size are filter
parameters, not tag identity. Also, allocinfo_to_params() does not
fill these fields, so userspace gets zeros in the output, which is
a bit confusing. Might be cleaner to separate filter parameters
from tag identity.
> struct allocinfo_counter {
> @@ -39,13 +41,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)
> diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c
> index 7ff936e15e97..98a27c302928 100644
> --- a/lib/alloc_tag.c
> +++ b/lib/alloc_tag.c
> @@ -195,6 +195,9 @@ static int allocinfo_ioctl_get_content_id(struct seq_file *m, void __user *arg)
>
> static bool matches_filter(struct codetag *ct, struct allocinfo_filter *filter)
> {
> + struct alloc_tag *tag;
> + struct alloc_tag_counters counters;
> +
> if (!ct || !filter || !filter->mask)
> return true;
>
> @@ -214,6 +217,18 @@ 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) ||
> + (filter->mask & ALLOCINFO_FILTER_MASK_MAX_SIZE)) {
> + tag = ct_to_alloc_tag(ct);
> + counters = alloc_tag_read(tag);
alloc_tag_read() is called twice for matching tags
When size filtering is enabled, matches_filter() calls alloc_tag_read()
to check the size, and then allocinfo_to_params() calls it again to
fill the output data:
matches_filter():
counters = alloc_tag_read(tag); // 1st read
if (counters.bytes < min_size)
return false;
allocinfo_to_params():
counter = alloc_tag_read(tag); // 2nd read (same tag)
data->counter.bytes = counter.bytes;
For matching tags, the same per-CPU counter aggregation is done twice.
On large machines this is not trivial. Would it make sense to cache
the counters from matches_filter() and reuse them in allocinfo_to_params()?
> + if ((filter->mask & ALLOCINFO_FILTER_MASK_MIN_SIZE) &&
> + counters.bytes < filter->fields.min_size)
> + return false;
> + if ((filter->mask & ALLOCINFO_FILTER_MASK_MAX_SIZE) &&
> + counters.bytes > filter->fields.max_size)
> + return false;
> + }
> +
No validation for min_size > max_size.
If both MIN_SIZE and MAX_SIZE are set but min_size > max_size,
no records will match and the user gets no indication of the
invalid input. This could be checked alongside the existing
mask validation in allocinfo_ioctl_get_at():
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.fields.min_size > params.filter.fields.max_size)
+ return -EINVAL;
Thanks
Best Regards
Hao
> return true;
> }
>
next prev parent reply other threads:[~2026-05-14 6:54 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-04 23:36 [PATCH 0/6] alloc_tag: introduce IOCTL-based filtering for MAP Abhishek Bapat
2026-05-04 23:36 ` [PATCH 1/6] alloc_tag: add ioctl to /proc/allocinfo Abhishek Bapat
2026-05-14 4:37 ` Hao Ge
2026-05-04 23:36 ` [PATCH 2/6] alloc_tag: add ioctl filters " Abhishek Bapat
2026-05-14 6:15 ` Hao Ge
2026-05-04 23:36 ` [PATCH 3/6] alloc_tag: add size-based filtering to ioctl Abhishek Bapat
2026-05-14 6:53 ` Hao Ge [this message]
2026-05-04 23:36 ` [PATCH 4/6] alloc_tag: add accuracy based " Abhishek Bapat
2026-05-04 23:36 ` [PATCH 5/6] kselftest: alloc_tag: add kselftest for ioctl interface Abhishek Bapat
2026-05-04 23:36 ` [PATCH 6/6] kselftest: alloc_tag: extend the allocinfo ioctl kselftest Abhishek Bapat
2026-05-06 8:44 ` [PATCH 0/6] alloc_tag: introduce IOCTL-based filtering for MAP Hao Ge
2026-05-12 19:58 ` Suren Baghdasaryan
2026-05-14 2:05 ` Hao Ge
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b681fad1-1a27-47fa-b58a-1f639eb3f3d9@linux.dev \
--to=hao.ge@linux.dev \
--cc=abhishekbapat@google.com \
--cc=akpm@linux-foundation.org \
--cc=corbet@lwn.net \
--cc=kent.overstreet@linux.dev \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=skhan@linuxfoundation.org \
--cc=souravpanda@google.com \
--cc=surenb@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox