All of lore.kernel.org
 help / color / mirror / Atom feed
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;
>   }
>   


  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.