Linux Trace Kernel
 help / color / mirror / Atom feed
From: Hao Ge <hao.ge@linux.dev>
To: Abhishek Bapat <abhishekbapat@google.com>
Cc: Suren Baghdasaryan <surenb@google.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	linux-trace-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/4] alloc_tag: Introduce IOCTLs to toggle allocation tracepoints
Date: Thu, 15 Oct 2026 23:54:29 +0800	[thread overview]
Message-ID: <7473d6e6-cbcd-40f5-9b48-e2cac9c926f1@linux.dev> (raw)
In-Reply-To: <CAL41Mv6Kp9qLtPum36WaMpOtrmTG3LZoEXyD=s78DH+9ruKO7g@mail.gmail.com>

Hi Abhishek


On 10/15/26 01:18, Abhishek Bapat wrote:
> On Tue, Oct 13, 2026 at 12:41 AM Hao Ge <hao.ge@linux.dev> wrote:
>> Hi Abhishek
>>
>>
>> On 2026/10/8 06:40, Abhishek Bapat wrote:
>>> Introduce a new IOCTL (`ALLOCINFO_IOC_TOGGLE_TRACE`) to selectively
>>> toggle tracing on allocation call sites. Userspace tools can use the
>>> existing filtering mechanism to specify the set of tags to toggle
>>> tracing for.
>>>
>>> To facilitate low overhead execution for non-targeted call sites, add a
>>> new `CODETAG_FLAG_TRACE_ON` flag to `struct codetag` to track per-site
>>> activation. Protect these conditional branch evaluations using the global
>>> `alloc_tag_trace_key` static branch and an inline static key check
>>> pattern in the allocator hooks (`alloc_tag_add`, `alloc_tag_sub`, etc).
>>> This ensures that the trace events are entirely skipped when no
>>> allocation call sites are being actively traced, leaving only a NOP on
>>> the allocation fast path.
>>>
>>> Signed-off-by: Abhishek Bapat <abhishekbapat@google.com>
>>> ---
>>>   Documentation/mm/allocation-profiling.rst |  20 ++++
>>>   include/linux/alloc_tag.h                 |  47 +++++---
>>>   include/linux/codetag.h                   |   5 +-
>>>   include/uapi/linux/alloc_tag.h            |  13 +++
>>>   mm/alloc_tag.c                            | 128 ++++++++++++++++++++--
>>>   5 files changed, 187 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/Documentation/mm/allocation-profiling.rst b/Documentation/mm/allocation-profiling.rst
>>> index b241450394df..63b668466342 100644
>>> --- a/Documentation/mm/allocation-profiling.rst
>>> +++ b/Documentation/mm/allocation-profiling.rst
>>> @@ -149,6 +149,26 @@ expose the full call stack and the lifetime of individual allocations:
>>>   - slab and percpu: the address of the object's `codetag_ref`
>>>   - page allocator: the `struct page` pointer of the head page
>>>
>>> +Events are only emitted for tags that have tracing turned on and while profiling
>>> +is enabled. When no tag is traced, the hooks are behind a static branch.
>>> +
>>> +Enabling Tracing
>>> +----------------
>>> +
>>> +Tracing is toggled on and off per tag with the `ALLOCINFO_IOC_TOGGLE_TRACE`
>>> +ioctl on `/proc/allocinfo`. It takes the same filter as used by
>>> +`ALLOCINFO_IOC_GET_AT`; an empty mask selects all tags. Size limits in the
>>> +filter are checked once, against the tag's size at the time of the call. Tags
>>> +already in the requested state are skipped and a -ENOENT is returned if no
>>> +tag matched.
>>> +
>>> +Tracing is turned off for a module's tags when the module is unloaded.
>>> +
>>> +Memory allocated before a tag was enabled has no alloc event but will still
>>> +produce a free event. Likewise, memory alive when tag is disabled will not
>>> +produce a free event. Tools should expect unmatched frees after enabling and
>>> +missing frees after disabling.
>>> +
>>>   Correlating the events
>>>   ----------------------
>>>
>>> diff --git a/include/linux/alloc_tag.h b/include/linux/alloc_tag.h
>>> index 49825177d8c8..94697b28053a 100644
>>> --- a/include/linux/alloc_tag.h
>>> +++ b/include/linux/alloc_tag.h
>>> @@ -136,9 +136,36 @@ static inline bool mem_alloc_profiling_enabled(void)
>>>                                   &mem_alloc_profiling_key);
>>>   }
>>>
>>> -static inline bool alloc_tag_trace_enabled(void)
>>> +static inline void alloc_tag_set_inaccurate(struct alloc_tag *tag)
>>>   {
>>> -     return static_branch_unlikely(&alloc_tag_trace_key);
>>> +     atomic_or(CODETAG_FLAG_INACCURATE, &tag->ct.flags);
>>> +}
>>> +
>>> +static inline bool alloc_tag_is_inaccurate(struct alloc_tag *tag)
>>> +{
>>> +     return !!(atomic_read(&tag->ct.flags) & CODETAG_FLAG_INACCURATE);
>>> +}
>>> +
>>> +static inline void alloc_tag_set_traced(struct alloc_tag *tag)
>>> +{
>>> +     atomic_or(CODETAG_FLAG_TRACE_ON, &tag->ct.flags);
>>> +}
>>> +
>>> +static inline void alloc_tag_clear_traced(struct alloc_tag *tag)
>>> +{
>>> +     atomic_andnot(CODETAG_FLAG_TRACE_ON, &tag->ct.flags);
>>> +}
>>> +
>>> +static inline bool alloc_tag_is_traced(const struct alloc_tag *tag)
>>> +{
>>> +     return !!(atomic_read(&tag->ct.flags) & CODETAG_FLAG_TRACE_ON);
>>> +}
>>> +
>>> +static inline bool alloc_tag_trace_enabled(const struct alloc_tag *tag)
>>> +{
>>> +     if (static_branch_unlikely(&alloc_tag_trace_key))
>>> +             return tag && alloc_tag_is_traced(tag);
>>> +     return false;
>>>   }
>>>
>>>   void alloc_tag_trace_mem_alloc(const void *ptr, struct alloc_tag *tag,
>>> @@ -151,7 +178,7 @@ void __alloc_tag_trace_hit(struct alloc_tag *tag);
>>>
>>>   static __always_inline void alloc_tag_trace_hit(struct alloc_tag *tag)
>>>   {
>>> -     if (alloc_tag_trace_enabled())
>>> +     if (alloc_tag_trace_enabled(tag))
>>>                __alloc_tag_trace_hit(tag);
>>>   }
>>>
>>> @@ -225,7 +252,7 @@ static inline void alloc_tag_add(union codetag_ref *ref, struct alloc_tag *tag,
>>>        if (likely(alloc_tag_ref_set(ref, tag))) {
>>>                this_cpu_add(tag->counters->bytes, bytes);
>>>
>>> -             if (alloc_tag_trace_enabled())
>>> +             if (alloc_tag_trace_enabled(tag))
>>>                        /* Trace successful allocs with their unique ptr */
>>>                        alloc_tag_trace_mem_alloc(ptr, tag, bytes);
>>>        }
>>> @@ -249,23 +276,13 @@ static inline void alloc_tag_sub(union codetag_ref *ref, size_t bytes, const voi
>>>        this_cpu_sub(tag->counters->bytes, bytes);
>>>        this_cpu_dec(tag->counters->calls);
>>>
>>> -     if (alloc_tag_trace_enabled())
>>> +     if (alloc_tag_trace_enabled(tag))
>>>                /* Trace frees with their unique ptr */
>>>                alloc_tag_trace_mem_free(ptr, tag, bytes);
>>>
>>>        ref->ct = NULL;
>>>   }
>>>
>>> -static inline void alloc_tag_set_inaccurate(struct alloc_tag *tag)
>>> -{
>>> -     tag->ct.flags |= CODETAG_FLAG_INACCURATE;
>>> -}
>>> -
>>> -static inline bool alloc_tag_is_inaccurate(struct alloc_tag *tag)
>>> -{
>>> -     return !!(tag->ct.flags & CODETAG_FLAG_INACCURATE);
>>> -}
>>> -
>>>   #define alloc_tag_record(p)  ((p) = current->alloc_tag)
>>>
>>>   #else /* CONFIG_MEM_ALLOC_PROFILING */
>>> diff --git a/include/linux/codetag.h b/include/linux/codetag.h
>>> index a25a085c2df1..f728295d50c0 100644
>>> --- a/include/linux/codetag.h
>>> +++ b/include/linux/codetag.h
>>> @@ -18,6 +18,7 @@ struct module;
>>>
>>>   /* codetag flags */
>>>   #define CODETAG_FLAG_INACCURATE      (1 << 0)
>>> +#define CODETAG_FLAG_TRACE_ON        (1 << 1)
>>>
>>>   /*
>>>    * An instance of this structure is created in a special ELF section at every
>>> @@ -25,7 +26,7 @@ struct module;
>>>    * an array of these.
>>>    */
>>>   struct codetag {
>>> -     unsigned int flags;
>>> +     atomic_t flags;
>>>        unsigned int lineno;
>>>        const char *modname;
>>>        const char *function;
>>> @@ -71,7 +72,7 @@ struct codetag_iterator {
>>>        .function       = __func__,                     \
>>>        .filename       = __FILE__,                     \
>>>        .lineno         = __LINE__,                     \
>>> -     .flags          = 0,                            \
>>> +     .flags          = ATOMIC_INIT(0),               \
>>>   }
>>>
>>>   void codetag_lock_module_list(struct codetag_type *cttype);
>>> diff --git a/include/uapi/linux/alloc_tag.h b/include/uapi/linux/alloc_tag.h
>>> index 7d4618bea043..dafcba2f7e2c 100644
>>> --- a/include/uapi/linux/alloc_tag.h
>>> +++ b/include/uapi/linux/alloc_tag.h
>>> @@ -85,9 +85,20 @@ struct allocinfo_get_at {
>>>        struct allocinfo_tag_data data;
>>>   };
>>>
>>> +struct allocinfo_toggle_traces {
>>> +     /* inputs */
>>> +     /*
>>> +      * Tags matching the filter are toggled; an empty mask selects all tags.
>>> +      * Size criteria are checked once, against the tag's size at call time.
>>> +      */
>>> +     struct allocinfo_filter filter;
>>> +     __u64 enable;
>>> +};
>>> +
>>>   #define _ALLOCINFO_IOC_CONTENT_ID    0
>>>   #define _ALLOCINFO_IOC_GET_AT                1
>>>   #define _ALLOCINFO_IOC_GET_NEXT              2
>>> +#define _ALLOCINFO_IOC_TOGGLE_TRACE  3
>>>
>>>   #define ALLOCINFO_IOC_BASE           0xA6
>>>   #define ALLOCINFO_IOC_CONTENT_ID     _IOR(ALLOCINFO_IOC_BASE, _ALLOCINFO_IOC_CONTENT_ID,     \
>>> @@ -96,5 +107,7 @@ struct allocinfo_get_at {
>>>                                              struct allocinfo_get_at)
>>>   #define ALLOCINFO_IOC_GET_NEXT               _IOR(ALLOCINFO_IOC_BASE, _ALLOCINFO_IOC_GET_NEXT,       \
>>>                                             struct allocinfo_tag_data)
>>> +#define ALLOCINFO_IOC_TOGGLE_TRACE   _IOW(ALLOCINFO_IOC_BASE, _ALLOCINFO_IOC_TOGGLE_TRACE,   \
>>> +                                         struct allocinfo_toggle_traces)
>>>
>>>   #endif /* _UAPI_ALLOC_TAG_H */
>>> diff --git a/mm/alloc_tag.c b/mm/alloc_tag.c
>>> index ca2412a67312..262f00f2bba9 100644
>>> --- a/mm/alloc_tag.c
>>> +++ b/mm/alloc_tag.c
>>> @@ -60,6 +60,15 @@ DEFINE_STATIC_KEY_FALSE(mem_profiling_compressed);
>>>   DEFINE_STATIC_KEY_FALSE(alloc_tag_trace_key);
>>>   EXPORT_SYMBOL(alloc_tag_trace_key);
>>>
>>> +static atomic_t alloc_tag_trace_cnt = ATOMIC_INIT(0);
>>> +
>>> +/*
>>> + * As `codetag_lock_module_list` is a read lock, we need an additional mutex
>>> + * to protect against the race conditions involved in the alloc tag trace
>>> + * toggle path.
>>> + */
>>> +static DEFINE_MUTEX(alloc_tag_trace_mutex);
>>> +
>>>   struct alloc_tag_kernel_section kernel_tags = { NULL, 0 };
>>>   unsigned long alloc_tag_ref_mask;
>>>   int alloc_tag_ref_offs;
>>> @@ -296,7 +305,7 @@ static bool matches_filter(struct codetag *ct, struct allocinfo_filter *filter,
>>>                return false;
>>>
>>>        if (filter->mask & ALLOCINFO_FILTER_MASK_INACCURATE) {
>>> -             inaccurate = !!(ct->flags & CODETAG_FLAG_INACCURATE);
>>> +             inaccurate = alloc_tag_is_inaccurate(ct_to_alloc_tag(ct));
>>>                if (inaccurate != !!(filter->inaccurate))
>>>                        return false;
>>>        }
>>> @@ -317,6 +326,23 @@ static bool matches_filter(struct codetag *ct, struct allocinfo_filter *filter,
>>>        return true;
>>>   }
>>>
>>> +/*
>>> + * Checks that a user supplied filter only uses known fields and that its size
>>> + * range, if any, is not inverted.
>>> + */
>>> +static bool allocinfo_filter_valid(const struct allocinfo_filter *filter)
>>> +{
>>> +     if (filter->mask & ~ALLOCINFO_FILTER_MASKS)
>>> +             return false;
>>> +
>>> +     if ((filter->mask & ALLOCINFO_FILTER_MASK_MIN_SIZE) &&
>>> +         (filter->mask & ALLOCINFO_FILTER_MASK_MAX_SIZE) &&
>>> +         filter->min_size > filter->max_size)
>>> +             return false;
>>> +
>>> +     return true;
>>> +}
>>> +
>>>   /*
>>>    * Seeks the ioctl iterator to the specified 0-indexed tag position, reads its
>>>    * profiling data and returns it to userspace.
>>> @@ -333,12 +359,7 @@ static int allocinfo_ioctl_get_at(struct seq_file *m, void __user *arg)
>>>        if (copy_from_user(&params, arg, sizeof(params)))
>>>                return -EFAULT;
>>>
>>> -     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.min_size > params.filter.max_size)
>>> +     if (!allocinfo_filter_valid(&params.filter))
>>>                return -EINVAL;
>>>
>>>        priv = m->private;
>>> @@ -443,6 +464,75 @@ static int allocinfo_ioctl_get_next(struct seq_file *m, void __user *arg)
>>>        return ret;
>>>   }
>>>
>>> +static void alloc_tag_trace_toggle(struct alloc_tag *tag, bool enable)
>>> +{
>>> +     if (enable) {
>>> +             if (alloc_tag_is_traced(tag))
>>> +                     return;
>>> +
>>> +             alloc_tag_set_traced(tag);
>>> +             if (atomic_fetch_inc(&alloc_tag_trace_cnt) == 0)
>>> +                     static_branch_enable(&alloc_tag_trace_key);
>>> +     } else {
>>> +             if (!alloc_tag_is_traced(tag))
>>> +                     return;
>>> +
>>> +             alloc_tag_clear_traced(tag);
>>> +             if (atomic_dec_and_test(&alloc_tag_trace_cnt))
>>> +                     static_branch_disable(&alloc_tag_trace_key);
>>> +     }
>>> +}
>>> +
>>> +/*
>>> + * Toggles tracing on every allocation tag that matches the user supplied
>>> + * filter. An empty filter mask selects all tags, same as for
>>> + * ALLOCINFO_IOC_GET_AT. Tags already in the requested state are not an error.
>>> + */
>>> +static int allocinfo_ioctl_toggle_trace(struct seq_file *m, void __user *arg)
>>> +{
>>> +     struct allocinfo_toggle_traces params;
>>> +     struct codetag_iterator iter;
>>> +     struct codetag *ct;
>>> +     struct alloc_tag_counters counters;
>>> +     bool fetched_counters;
>>> +     int matches = 0, ret;
>>> +
>>> +     if (!capable(CAP_SYS_ADMIN))
>>> +             return -EPERM;
>>> +
>>> +     if (copy_from_user(&params, arg, sizeof(params)))
>>> +             return -EFAULT;
>>> +
>>> +     if (!allocinfo_filter_valid(&params.filter))
>>> +             return -EINVAL;
>>> +
>>> +     codetag_lock_module_list(alloc_tag_cttype);
>>> +
>>> +     iter = codetag_get_ct_iter(alloc_tag_cttype);
>>> +
>>> +     /* Toggle tracing on all codetags that match */
>>> +     while ((ct = codetag_next_ct(&iter))) {
>>> +             fetched_counters = false;
>>> +             if (matches_filter(ct, &params.filter, &counters, &fetched_counters)) {
>>> +                     matches++;
>>> +
>>> +                     mutex_lock(&alloc_tag_trace_mutex);
>> Could we grab and drop alloc_tag_trace_mutex outside the loop?
> Well, that is possible. But I did it this way to minimize the critical section.
>
> Since this mutex is specific to the trace ioctl, I'm comfortable doing
> what you recommended too. It would just mean that one thread would be
> blocked until another thread fully finishes its loop, which is not too
> bad. I'll let Suren comment on this as well.


Hmm, sorry, I misread this part of the code. (sorry for this) . Putting 
|mutex_lock| outside the loop is not a good idea.

It greatly increases the critical section.

My original concern was that we need to mutex_lock and mutex_unlock 
every time we get a match.

As the function comment states, An empty filter mask selects all tags, 
that means we need a

mutex_lock / mutex_unlock for every alloc_tag.

Let's see what Suren thinks.


Thanks

Best Regards

Hao

>>
>>> +                     alloc_tag_trace_toggle(ct_to_alloc_tag(ct), !!params.enable);
>>> +                     mutex_unlock(&alloc_tag_trace_mutex);
>>> +             }
>>> +     }
>>> +
>>> +     if (matches == 0)
>>> +             /* Nothing matched the filter */
>>> +             ret = -ENOENT;
>>> +     else
>>> +             ret = 0;
>>> +
>>> +     codetag_unlock_module_list(alloc_tag_cttype);
>>> +
>>> +     return ret;
>>> +}
>>> +
>>>   /*
>>>    * Entry point ioctl function for /proc/allocinfo routing requests to fetch the
>>>    * layout content ID, seek to a specific tag, or read sequential tags.
>>> @@ -463,6 +553,9 @@ static long allocinfo_ioctl(struct file *file, unsigned int cmd,
>>>        case ALLOCINFO_IOC_GET_NEXT:
>>>                ret = allocinfo_ioctl_get_next(file->private_data, arg);
>>>                break;
>>> +     case ALLOCINFO_IOC_TOGGLE_TRACE:
>>> +             ret = allocinfo_ioctl_toggle_trace(file->private_data, arg);
>>> +             break;
>>>        default:
>>>                ret = -ENOIOCTLCMD;
>>>                break;
>>> @@ -492,8 +585,6 @@ static const struct proc_ops allocinfo_proc_ops = {
>>>
>>>   void __alloc_tag_trace_hit(struct alloc_tag *tag)
>>>   {
>>> -     if (unlikely(!tag))
>>> -             return;
>>>        trace_alloc_tag_hit(tag);
>>>   }
>>>   EXPORT_SYMBOL(__alloc_tag_trace_hit);
>>> @@ -1031,6 +1122,24 @@ static int load_module(struct module *mod, struct codetag *start, struct codetag
>>>        return 0;
>>>   }
>>>
>>> +static void unload_module(struct module *mod, struct codetag *start, struct codetag *stop)
>>> +{
>>> +     struct alloc_tag *start_tag = ct_to_alloc_tag(start);
>>> +     struct alloc_tag *stop_tag = ct_to_alloc_tag(stop);
>>> +     struct alloc_tag *tag;
>>> +
>>> +     /*
>>> +      * Turn tracing off for the tags of the module being unloaded. Without
>>> +      * this, `alloc_tag_trace_cnt` would never reach zero and tracing would
>>> +      * stay enabled forever.
>>> +      *
>>> +      * `alloc_tag_trace_mutex` is not needed here as this code path is
>>> +      * protected by a `down_write(&cttype->mod_lock)`.
>>> +      */
>>> +     for (tag = start_tag; tag < stop_tag; tag++)
>>> +             alloc_tag_trace_toggle(tag, false);
>>> +}
>>> +
>>>   static void replace_module(struct module *mod, struct module *new_mod)
>>>   {
>>>        MA_STATE(mas, &mod_area_mt, 0, module_tags.size);
>>> @@ -1357,6 +1466,7 @@ static int __init alloc_tag_init(void)
>>>                .alloc_section_mem      = reserve_module_tags,
>>>                .free_section_mem       = release_module_tags,
>>>                .module_load            = load_module,
>>> +             .module_unload          = unload_module,
>>>                .module_replaced        = replace_module,
>>>   #endif
>>>        };

  reply	other threads:[~2026-10-15 15:54 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-07 22:39 [PATCH v2 0/4] alloc_tag: Introduce selective tracing for MAP Abhishek Bapat
2026-10-07 22:39 ` [PATCH v2 1/4] alloc_tag: Add trace events for tracing allocations Abhishek Bapat
2026-10-13  6:13   ` Hao Ge
2026-10-07 22:40 ` [PATCH v2 2/4] alloc_tag: Introduce IOCTLs to toggle allocation tracepoints Abhishek Bapat
2026-10-07 22:51   ` sashiko-bot
2026-10-13  7:42   ` Hao Ge
2026-10-14 17:18     ` Abhishek Bapat
2026-10-15 15:54       ` Hao Ge [this message]
2026-10-07 22:40 ` [PATCH v2 3/4] alloc_tag: Extend allocinfo_filter to support tracing queries Abhishek Bapat
2026-10-07 22:47   ` sashiko-bot
2026-10-07 22:40 ` [PATCH v2 4/4] alloc_tag: Add a test for trace state toggle and filtering Abhishek Bapat

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=7473d6e6-cbcd-40f5-9b48-e2cac9c926f1@linux.dev \
    --to=hao.ge@linux.dev \
    --cc=abhishekbapat@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --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