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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.