include/linux/trace_events.h | 3 --- kernel/trace/trace_events.c | 24 ++++++++++++------------ 2 files changed, 12 insertions(+), 15 deletions(-)
From: Steven Rostedt <rostedt@goodmis.org>
When soft disabling of trace events was first created, it needed to have a
way to know if a file had a user that was using it with soft disabled (for
triggers that need to enable or disable events from a context that can not
really enable or disable the event, it would set SOFT_DISABLED to state it
is disabled). The flag SOFT_MODE was used to denote that an event had a
user that would enable or disable it via the SOFT_DISABLED flag.
Commit 1cf4c0732db3c ("tracing: Modify soft-mode only if there's no other
referrer") fixed a bug where if two users were using the SOFT_DISABLED
flag the accounting would get messed up as the SOFT_MODE flag could only
handle one user. That commit added the sm_ref counter which kept track of
how many users were using the event in "soft mode". This made the
SOFT_MODE flag redundant as it should only be set if the sm_ref counter is
non zero.
Remove the SOFT_MODE flag and just use the sm_ref counter to know the
event is in soft mode or not. This makes the code a bit simpler.
Link: https://lore.kernel.org/all/20250702111908.03759998@batman.local.home/
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
include/linux/trace_events.h | 3 ---
kernel/trace/trace_events.c | 24 ++++++++++++------------
2 files changed, 12 insertions(+), 15 deletions(-)
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index fa9cf4292dff..04307a19cde3 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -480,7 +480,6 @@ enum {
EVENT_FILE_FL_RECORDED_TGID_BIT,
EVENT_FILE_FL_FILTERED_BIT,
EVENT_FILE_FL_NO_SET_FILTER_BIT,
- EVENT_FILE_FL_SOFT_MODE_BIT,
EVENT_FILE_FL_SOFT_DISABLED_BIT,
EVENT_FILE_FL_TRIGGER_MODE_BIT,
EVENT_FILE_FL_TRIGGER_COND_BIT,
@@ -618,7 +617,6 @@ extern int __kprobe_event_add_fields(struct dynevent_cmd *cmd, ...);
* RECORDED_TGID - The tgids should be recorded at sched_switch
* FILTERED - The event has a filter attached
* NO_SET_FILTER - Set when filter has error and is to be ignored
- * SOFT_MODE - The event is enabled/disabled by SOFT_DISABLED
* SOFT_DISABLED - When set, do not trace the event (even though its
* tracepoint may be enabled)
* TRIGGER_MODE - When set, invoke the triggers associated with the event
@@ -633,7 +631,6 @@ enum {
EVENT_FILE_FL_RECORDED_TGID = (1 << EVENT_FILE_FL_RECORDED_TGID_BIT),
EVENT_FILE_FL_FILTERED = (1 << EVENT_FILE_FL_FILTERED_BIT),
EVENT_FILE_FL_NO_SET_FILTER = (1 << EVENT_FILE_FL_NO_SET_FILTER_BIT),
- EVENT_FILE_FL_SOFT_MODE = (1 << EVENT_FILE_FL_SOFT_MODE_BIT),
EVENT_FILE_FL_SOFT_DISABLED = (1 << EVENT_FILE_FL_SOFT_DISABLED_BIT),
EVENT_FILE_FL_TRIGGER_MODE = (1 << EVENT_FILE_FL_TRIGGER_MODE_BIT),
EVENT_FILE_FL_TRIGGER_COND = (1 << EVENT_FILE_FL_TRIGGER_COND_BIT),
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 120531268abf..0980f4def360 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -768,6 +768,7 @@ static int __ftrace_event_enable_disable(struct trace_event_file *file,
{
struct trace_event_call *call = file->event_call;
struct trace_array *tr = file->tr;
+ bool soft_mode = atomic_read(&file->sm_ref) != 0;
int ret = 0;
int disable;
@@ -782,7 +783,7 @@ static int __ftrace_event_enable_disable(struct trace_event_file *file,
* is set we do not want the event to be enabled before we
* clear the bit.
*
- * When soft_disable is not set but the SOFT_MODE flag is,
+ * When soft_disable is not set but the soft_mode is,
* we do nothing. Do not disable the tracepoint, otherwise
* "soft enable"s (clearing the SOFT_DISABLED bit) wont work.
*/
@@ -790,11 +791,11 @@ static int __ftrace_event_enable_disable(struct trace_event_file *file,
if (atomic_dec_return(&file->sm_ref) > 0)
break;
disable = file->flags & EVENT_FILE_FL_SOFT_DISABLED;
- clear_bit(EVENT_FILE_FL_SOFT_MODE_BIT, &file->flags);
+ soft_mode = false;
/* Disable use of trace_buffered_event */
trace_buffered_event_disable();
} else
- disable = !(file->flags & EVENT_FILE_FL_SOFT_MODE);
+ disable = !soft_mode;
if (disable && (file->flags & EVENT_FILE_FL_ENABLED)) {
clear_bit(EVENT_FILE_FL_ENABLED_BIT, &file->flags);
@@ -812,8 +813,8 @@ static int __ftrace_event_enable_disable(struct trace_event_file *file,
WARN_ON_ONCE(ret);
}
- /* If in SOFT_MODE, just set the SOFT_DISABLE_BIT, else clear it */
- if (file->flags & EVENT_FILE_FL_SOFT_MODE)
+ /* If in soft mode, just set the SOFT_DISABLE_BIT, else clear it */
+ if (soft_mode)
set_bit(EVENT_FILE_FL_SOFT_DISABLED_BIT, &file->flags);
else
clear_bit(EVENT_FILE_FL_SOFT_DISABLED_BIT, &file->flags);
@@ -823,7 +824,7 @@ static int __ftrace_event_enable_disable(struct trace_event_file *file,
* When soft_disable is set and enable is set, we want to
* register the tracepoint for the event, but leave the event
* as is. That means, if the event was already enabled, we do
- * nothing (but set SOFT_MODE). If the event is disabled, we
+ * nothing (but set soft_mode). If the event is disabled, we
* set SOFT_DISABLED before enabling the event tracepoint, so
* it still seems to be disabled.
*/
@@ -832,7 +833,7 @@ static int __ftrace_event_enable_disable(struct trace_event_file *file,
else {
if (atomic_inc_return(&file->sm_ref) > 1)
break;
- set_bit(EVENT_FILE_FL_SOFT_MODE_BIT, &file->flags);
+ soft_mode = true;
/* Enable use of trace_buffered_event */
trace_buffered_event_enable();
}
@@ -840,7 +841,7 @@ static int __ftrace_event_enable_disable(struct trace_event_file *file,
if (!(file->flags & EVENT_FILE_FL_ENABLED)) {
bool cmd = false, tgid = false;
- /* Keep the event disabled, when going to SOFT_MODE. */
+ /* Keep the event disabled, when going to soft mode. */
if (soft_disable)
set_bit(EVENT_FILE_FL_SOFT_DISABLED_BIT, &file->flags);
@@ -1792,8 +1793,7 @@ event_enable_read(struct file *filp, char __user *ubuf, size_t cnt,
!(flags & EVENT_FILE_FL_SOFT_DISABLED))
strcpy(buf, "1");
- if (flags & EVENT_FILE_FL_SOFT_DISABLED ||
- flags & EVENT_FILE_FL_SOFT_MODE)
+ if (atomic_read(&file->sm_ref) != 0)
strcat(buf, "*");
strcat(buf, "\n");
@@ -3584,7 +3584,7 @@ static int probe_remove_event_call(struct trace_event_call *call)
continue;
/*
* We can't rely on ftrace_event_enable_disable(enable => 0)
- * we are going to do, EVENT_FILE_FL_SOFT_MODE can suppress
+ * we are going to do, soft mode can suppress
* TRACE_REG_UNREGISTER.
*/
if (file->flags & EVENT_FILE_FL_ENABLED)
@@ -3997,7 +3997,7 @@ static int free_probe_data(void *data)
edata->ref--;
if (!edata->ref) {
- /* Remove the SOFT_MODE flag */
+ /* Remove soft mode */
__ftrace_event_enable_disable(edata->file, 0, 1);
trace_event_put_ref(edata->file->event_call);
kfree(edata);
--
2.47.2On Wed, Jul 2, 2025 at 8:37 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> From: Steven Rostedt <rostedt@goodmis.org>
>
> When soft disabling of trace events was first created, it needed to have a
> way to know if a file had a user that was using it with soft disabled (for
> triggers that need to enable or disable events from a context that can not
> really enable or disable the event, it would set SOFT_DISABLED to state it
> is disabled). The flag SOFT_MODE was used to denote that an event had a
> user that would enable or disable it via the SOFT_DISABLED flag.
>
> Commit 1cf4c0732db3c ("tracing: Modify soft-mode only if there's no other
> referrer") fixed a bug where if two users were using the SOFT_DISABLED
> flag the accounting would get messed up as the SOFT_MODE flag could only
> handle one user. That commit added the sm_ref counter which kept track of
> how many users were using the event in "soft mode". This made the
> SOFT_MODE flag redundant as it should only be set if the sm_ref counter is
> non zero.
>
> Remove the SOFT_MODE flag and just use the sm_ref counter to know the
> event is in soft mode or not. This makes the code a bit simpler.
>
> Link: https://lore.kernel.org/all/20250702111908.03759998@batman.local.home/
>
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
> include/linux/trace_events.h | 3 ---
> kernel/trace/trace_events.c | 24 ++++++++++++------------
> 2 files changed, 12 insertions(+), 15 deletions(-)
>
> diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
> index fa9cf4292dff..04307a19cde3 100644
> --- a/include/linux/trace_events.h
> +++ b/include/linux/trace_events.h
> @@ -480,7 +480,6 @@ enum {
> EVENT_FILE_FL_RECORDED_TGID_BIT,
> EVENT_FILE_FL_FILTERED_BIT,
> EVENT_FILE_FL_NO_SET_FILTER_BIT,
> - EVENT_FILE_FL_SOFT_MODE_BIT,
> EVENT_FILE_FL_SOFT_DISABLED_BIT,
> EVENT_FILE_FL_TRIGGER_MODE_BIT,
> EVENT_FILE_FL_TRIGGER_COND_BIT,
> @@ -618,7 +617,6 @@ extern int __kprobe_event_add_fields(struct dynevent_cmd *cmd, ...);
> * RECORDED_TGID - The tgids should be recorded at sched_switch
> * FILTERED - The event has a filter attached
> * NO_SET_FILTER - Set when filter has error and is to be ignored
> - * SOFT_MODE - The event is enabled/disabled by SOFT_DISABLED
> * SOFT_DISABLED - When set, do not trace the event (even though its
> * tracepoint may be enabled)
> * TRIGGER_MODE - When set, invoke the triggers associated with the event
> @@ -633,7 +631,6 @@ enum {
> EVENT_FILE_FL_RECORDED_TGID = (1 << EVENT_FILE_FL_RECORDED_TGID_BIT),
> EVENT_FILE_FL_FILTERED = (1 << EVENT_FILE_FL_FILTERED_BIT),
> EVENT_FILE_FL_NO_SET_FILTER = (1 << EVENT_FILE_FL_NO_SET_FILTER_BIT),
> - EVENT_FILE_FL_SOFT_MODE = (1 << EVENT_FILE_FL_SOFT_MODE_BIT),
> EVENT_FILE_FL_SOFT_DISABLED = (1 << EVENT_FILE_FL_SOFT_DISABLED_BIT),
> EVENT_FILE_FL_TRIGGER_MODE = (1 << EVENT_FILE_FL_TRIGGER_MODE_BIT),
> EVENT_FILE_FL_TRIGGER_COND = (1 << EVENT_FILE_FL_TRIGGER_COND_BIT),
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index 120531268abf..0980f4def360 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -768,6 +768,7 @@ static int __ftrace_event_enable_disable(struct trace_event_file *file,
> {
> struct trace_event_call *call = file->event_call;
> struct trace_array *tr = file->tr;
> + bool soft_mode = atomic_read(&file->sm_ref) != 0;
I was checking why an atomic counter is needed here, and, since this
function should be called with
event_mutex locked, I think the atomic counter is not needed, so maybe
it should be removed...
On a different angle I also checked the callers of this function and
it seems that in some cases
event_mutex is not held when calling this function. E.g.:
trace_event_trigger_enable_disable() -> trace_event_enable_disable()
-> __ftrace_event_enable_disable()
In general it seems that all the callers of
trace_event_enable_disable() are outside trace_events.c, so maybe
the modification below could solve this issue and concurrently we
could replace the atomic sm_ref with a
standard one...?
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -880,7 +880,9 @@ static int __ftrace_event_enable_disable(struct
trace_event_file *file,
int trace_event_enable_disable(struct trace_event_file *file,
int enable, int soft_disable)
{
+ mutex_lock(&event_mutex);
return __ftrace_event_enable_disable(file, enable, soft_disable);
+ mutex_unlock(&event_mutex);
}
Thanks
Gab
> int ret = 0;
> int disable;
>
> @@ -782,7 +783,7 @@ static int __ftrace_event_enable_disable(struct trace_event_file *file,
> * is set we do not want the event to be enabled before we
> * clear the bit.
> *
> - * When soft_disable is not set but the SOFT_MODE flag is,
> + * When soft_disable is not set but the soft_mode is,
> * we do nothing. Do not disable the tracepoint, otherwise
> * "soft enable"s (clearing the SOFT_DISABLED bit) wont work.
> */
> @@ -790,11 +791,11 @@ static int __ftrace_event_enable_disable(struct trace_event_file *file,
> if (atomic_dec_return(&file->sm_ref) > 0)
> break;
> disable = file->flags & EVENT_FILE_FL_SOFT_DISABLED;
> - clear_bit(EVENT_FILE_FL_SOFT_MODE_BIT, &file->flags);
> + soft_mode = false;
> /* Disable use of trace_buffered_event */
> trace_buffered_event_disable();
> } else
> - disable = !(file->flags & EVENT_FILE_FL_SOFT_MODE);
> + disable = !soft_mode;
>
> if (disable && (file->flags & EVENT_FILE_FL_ENABLED)) {
> clear_bit(EVENT_FILE_FL_ENABLED_BIT, &file->flags);
> @@ -812,8 +813,8 @@ static int __ftrace_event_enable_disable(struct trace_event_file *file,
>
> WARN_ON_ONCE(ret);
> }
> - /* If in SOFT_MODE, just set the SOFT_DISABLE_BIT, else clear it */
> - if (file->flags & EVENT_FILE_FL_SOFT_MODE)
> + /* If in soft mode, just set the SOFT_DISABLE_BIT, else clear it */
> + if (soft_mode)
> set_bit(EVENT_FILE_FL_SOFT_DISABLED_BIT, &file->flags);
> else
> clear_bit(EVENT_FILE_FL_SOFT_DISABLED_BIT, &file->flags);
> @@ -823,7 +824,7 @@ static int __ftrace_event_enable_disable(struct trace_event_file *file,
> * When soft_disable is set and enable is set, we want to
> * register the tracepoint for the event, but leave the event
> * as is. That means, if the event was already enabled, we do
> - * nothing (but set SOFT_MODE). If the event is disabled, we
> + * nothing (but set soft_mode). If the event is disabled, we
> * set SOFT_DISABLED before enabling the event tracepoint, so
> * it still seems to be disabled.
> */
> @@ -832,7 +833,7 @@ static int __ftrace_event_enable_disable(struct trace_event_file *file,
> else {
> if (atomic_inc_return(&file->sm_ref) > 1)
> break;
> - set_bit(EVENT_FILE_FL_SOFT_MODE_BIT, &file->flags);
> + soft_mode = true;
> /* Enable use of trace_buffered_event */
> trace_buffered_event_enable();
> }
> @@ -840,7 +841,7 @@ static int __ftrace_event_enable_disable(struct trace_event_file *file,
> if (!(file->flags & EVENT_FILE_FL_ENABLED)) {
> bool cmd = false, tgid = false;
>
> - /* Keep the event disabled, when going to SOFT_MODE. */
> + /* Keep the event disabled, when going to soft mode. */
> if (soft_disable)
> set_bit(EVENT_FILE_FL_SOFT_DISABLED_BIT, &file->flags);
>
> @@ -1792,8 +1793,7 @@ event_enable_read(struct file *filp, char __user *ubuf, size_t cnt,
> !(flags & EVENT_FILE_FL_SOFT_DISABLED))
> strcpy(buf, "1");
>
> - if (flags & EVENT_FILE_FL_SOFT_DISABLED ||
> - flags & EVENT_FILE_FL_SOFT_MODE)
> + if (atomic_read(&file->sm_ref) != 0)
> strcat(buf, "*");
>
> strcat(buf, "\n");
> @@ -3584,7 +3584,7 @@ static int probe_remove_event_call(struct trace_event_call *call)
> continue;
> /*
> * We can't rely on ftrace_event_enable_disable(enable => 0)
> - * we are going to do, EVENT_FILE_FL_SOFT_MODE can suppress
> + * we are going to do, soft mode can suppress
> * TRACE_REG_UNREGISTER.
> */
> if (file->flags & EVENT_FILE_FL_ENABLED)
> @@ -3997,7 +3997,7 @@ static int free_probe_data(void *data)
>
> edata->ref--;
> if (!edata->ref) {
> - /* Remove the SOFT_MODE flag */
> + /* Remove soft mode */
> __ftrace_event_enable_disable(edata->file, 0, 1);
> trace_event_put_ref(edata->file->event_call);
> kfree(edata);
> --
> 2.47.2
>
>
On Thu, Jul 3, 2025 at 6:18 PM Gabriele Paoloni <gpaoloni@redhat.com> wrote:
>
> On Wed, Jul 2, 2025 at 8:37 PM Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > From: Steven Rostedt <rostedt@goodmis.org>
> >
> > When soft disabling of trace events was first created, it needed to have a
> > way to know if a file had a user that was using it with soft disabled (for
> > triggers that need to enable or disable events from a context that can not
> > really enable or disable the event, it would set SOFT_DISABLED to state it
> > is disabled). The flag SOFT_MODE was used to denote that an event had a
> > user that would enable or disable it via the SOFT_DISABLED flag.
> >
> > Commit 1cf4c0732db3c ("tracing: Modify soft-mode only if there's no other
> > referrer") fixed a bug where if two users were using the SOFT_DISABLED
> > flag the accounting would get messed up as the SOFT_MODE flag could only
> > handle one user. That commit added the sm_ref counter which kept track of
> > how many users were using the event in "soft mode". This made the
> > SOFT_MODE flag redundant as it should only be set if the sm_ref counter is
> > non zero.
> >
> > Remove the SOFT_MODE flag and just use the sm_ref counter to know the
> > event is in soft mode or not. This makes the code a bit simpler.
> >
> > Link: https://lore.kernel.org/all/20250702111908.03759998@batman.local.home/
> >
> > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> > ---
> > include/linux/trace_events.h | 3 ---
> > kernel/trace/trace_events.c | 24 ++++++++++++------------
> > 2 files changed, 12 insertions(+), 15 deletions(-)
> >
> > diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
> > index fa9cf4292dff..04307a19cde3 100644
> > --- a/include/linux/trace_events.h
> > +++ b/include/linux/trace_events.h
> > @@ -480,7 +480,6 @@ enum {
> > EVENT_FILE_FL_RECORDED_TGID_BIT,
> > EVENT_FILE_FL_FILTERED_BIT,
> > EVENT_FILE_FL_NO_SET_FILTER_BIT,
> > - EVENT_FILE_FL_SOFT_MODE_BIT,
> > EVENT_FILE_FL_SOFT_DISABLED_BIT,
> > EVENT_FILE_FL_TRIGGER_MODE_BIT,
> > EVENT_FILE_FL_TRIGGER_COND_BIT,
> > @@ -618,7 +617,6 @@ extern int __kprobe_event_add_fields(struct dynevent_cmd *cmd, ...);
> > * RECORDED_TGID - The tgids should be recorded at sched_switch
> > * FILTERED - The event has a filter attached
> > * NO_SET_FILTER - Set when filter has error and is to be ignored
> > - * SOFT_MODE - The event is enabled/disabled by SOFT_DISABLED
> > * SOFT_DISABLED - When set, do not trace the event (even though its
> > * tracepoint may be enabled)
> > * TRIGGER_MODE - When set, invoke the triggers associated with the event
> > @@ -633,7 +631,6 @@ enum {
> > EVENT_FILE_FL_RECORDED_TGID = (1 << EVENT_FILE_FL_RECORDED_TGID_BIT),
> > EVENT_FILE_FL_FILTERED = (1 << EVENT_FILE_FL_FILTERED_BIT),
> > EVENT_FILE_FL_NO_SET_FILTER = (1 << EVENT_FILE_FL_NO_SET_FILTER_BIT),
> > - EVENT_FILE_FL_SOFT_MODE = (1 << EVENT_FILE_FL_SOFT_MODE_BIT),
> > EVENT_FILE_FL_SOFT_DISABLED = (1 << EVENT_FILE_FL_SOFT_DISABLED_BIT),
> > EVENT_FILE_FL_TRIGGER_MODE = (1 << EVENT_FILE_FL_TRIGGER_MODE_BIT),
> > EVENT_FILE_FL_TRIGGER_COND = (1 << EVENT_FILE_FL_TRIGGER_COND_BIT),
> > diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> > index 120531268abf..0980f4def360 100644
> > --- a/kernel/trace/trace_events.c
> > +++ b/kernel/trace/trace_events.c
> > @@ -768,6 +768,7 @@ static int __ftrace_event_enable_disable(struct trace_event_file *file,
> > {
> > struct trace_event_call *call = file->event_call;
> > struct trace_array *tr = file->tr;
> > + bool soft_mode = atomic_read(&file->sm_ref) != 0;
>
> I was checking why an atomic counter is needed here, and, since this
> function should be called with
> event_mutex locked, I think the atomic counter is not needed, so maybe
> it should be removed...
> On a different angle I also checked the callers of this function and
> it seems that in some cases
> event_mutex is not held when calling this function. E.g.:
> trace_event_trigger_enable_disable() -> trace_event_enable_disable()
> -> __ftrace_event_enable_disable()
>
> In general it seems that all the callers of
> trace_event_enable_disable() are outside trace_events.c, so maybe
> the modification below could solve this issue and concurrently we
> could replace the atomic sm_ref with a
> standard one...?
Sorry I had a better look at the code and event_mutex is locked/unlocked
also outside of trace_events.c so the modification below is not needed.
However I think we could still replace the atomic counter.
Gab
>
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -880,7 +880,9 @@ static int __ftrace_event_enable_disable(struct
> trace_event_file *file,
> int trace_event_enable_disable(struct trace_event_file *file,
> int enable, int soft_disable)
> {
> + mutex_lock(&event_mutex);
> return __ftrace_event_enable_disable(file, enable, soft_disable);
> + mutex_unlock(&event_mutex);
> }
>
> Thanks
> Gab
>
> > int ret = 0;
> > int disable;
> >
> > @@ -782,7 +783,7 @@ static int __ftrace_event_enable_disable(struct trace_event_file *file,
> > * is set we do not want the event to be enabled before we
> > * clear the bit.
> > *
> > - * When soft_disable is not set but the SOFT_MODE flag is,
> > + * When soft_disable is not set but the soft_mode is,
> > * we do nothing. Do not disable the tracepoint, otherwise
> > * "soft enable"s (clearing the SOFT_DISABLED bit) wont work.
> > */
> > @@ -790,11 +791,11 @@ static int __ftrace_event_enable_disable(struct trace_event_file *file,
> > if (atomic_dec_return(&file->sm_ref) > 0)
> > break;
> > disable = file->flags & EVENT_FILE_FL_SOFT_DISABLED;
> > - clear_bit(EVENT_FILE_FL_SOFT_MODE_BIT, &file->flags);
> > + soft_mode = false;
> > /* Disable use of trace_buffered_event */
> > trace_buffered_event_disable();
> > } else
> > - disable = !(file->flags & EVENT_FILE_FL_SOFT_MODE);
> > + disable = !soft_mode;
> >
> > if (disable && (file->flags & EVENT_FILE_FL_ENABLED)) {
> > clear_bit(EVENT_FILE_FL_ENABLED_BIT, &file->flags);
> > @@ -812,8 +813,8 @@ static int __ftrace_event_enable_disable(struct trace_event_file *file,
> >
> > WARN_ON_ONCE(ret);
> > }
> > - /* If in SOFT_MODE, just set the SOFT_DISABLE_BIT, else clear it */
> > - if (file->flags & EVENT_FILE_FL_SOFT_MODE)
> > + /* If in soft mode, just set the SOFT_DISABLE_BIT, else clear it */
> > + if (soft_mode)
> > set_bit(EVENT_FILE_FL_SOFT_DISABLED_BIT, &file->flags);
> > else
> > clear_bit(EVENT_FILE_FL_SOFT_DISABLED_BIT, &file->flags);
> > @@ -823,7 +824,7 @@ static int __ftrace_event_enable_disable(struct trace_event_file *file,
> > * When soft_disable is set and enable is set, we want to
> > * register the tracepoint for the event, but leave the event
> > * as is. That means, if the event was already enabled, we do
> > - * nothing (but set SOFT_MODE). If the event is disabled, we
> > + * nothing (but set soft_mode). If the event is disabled, we
> > * set SOFT_DISABLED before enabling the event tracepoint, so
> > * it still seems to be disabled.
> > */
> > @@ -832,7 +833,7 @@ static int __ftrace_event_enable_disable(struct trace_event_file *file,
> > else {
> > if (atomic_inc_return(&file->sm_ref) > 1)
> > break;
> > - set_bit(EVENT_FILE_FL_SOFT_MODE_BIT, &file->flags);
> > + soft_mode = true;
> > /* Enable use of trace_buffered_event */
> > trace_buffered_event_enable();
> > }
> > @@ -840,7 +841,7 @@ static int __ftrace_event_enable_disable(struct trace_event_file *file,
> > if (!(file->flags & EVENT_FILE_FL_ENABLED)) {
> > bool cmd = false, tgid = false;
> >
> > - /* Keep the event disabled, when going to SOFT_MODE. */
> > + /* Keep the event disabled, when going to soft mode. */
> > if (soft_disable)
> > set_bit(EVENT_FILE_FL_SOFT_DISABLED_BIT, &file->flags);
> >
> > @@ -1792,8 +1793,7 @@ event_enable_read(struct file *filp, char __user *ubuf, size_t cnt,
> > !(flags & EVENT_FILE_FL_SOFT_DISABLED))
> > strcpy(buf, "1");
> >
> > - if (flags & EVENT_FILE_FL_SOFT_DISABLED ||
> > - flags & EVENT_FILE_FL_SOFT_MODE)
> > + if (atomic_read(&file->sm_ref) != 0)
> > strcat(buf, "*");
> >
> > strcat(buf, "\n");
> > @@ -3584,7 +3584,7 @@ static int probe_remove_event_call(struct trace_event_call *call)
> > continue;
> > /*
> > * We can't rely on ftrace_event_enable_disable(enable => 0)
> > - * we are going to do, EVENT_FILE_FL_SOFT_MODE can suppress
> > + * we are going to do, soft mode can suppress
> > * TRACE_REG_UNREGISTER.
> > */
> > if (file->flags & EVENT_FILE_FL_ENABLED)
> > @@ -3997,7 +3997,7 @@ static int free_probe_data(void *data)
> >
> > edata->ref--;
> > if (!edata->ref) {
> > - /* Remove the SOFT_MODE flag */
> > + /* Remove soft mode */
> > __ftrace_event_enable_disable(edata->file, 0, 1);
> > trace_event_put_ref(edata->file->event_call);
> > kfree(edata);
> > --
> > 2.47.2
> >
> >
On Thu, 3 Jul 2025 18:39:18 +0200 Gabriele Paoloni <gpaoloni@redhat.com> wrote: > Sorry I had a better look at the code and event_mutex is locked/unlocked > also outside of trace_events.c so the modification below is not needed. > However I think we could still replace the atomic counter. Maybe, but that's unrelated to the current patch. I'm going to take this as is. -- Steve
© 2016 - 2025 Red Hat, Inc.