> On 09-May-2023, at 10:15 PM, Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
>
> !! External Email
>
> On Tue, 9 May 2023 12:29:23 +0000
> Ajay Kaher <akaher@xxxxxxxxxx> wrote:
>
>> > On 02/05/23, 11:42 PM, "kernel test robot" <lkp@xxxxxxxxx> wrote:
>>>>> kernel/trace/trace_events.c:2424:17: warning: variable 'd_events' set but not used [-Wunused-but-set-variable]
>>> struct dentry *d_events;
>>> ^
>>> 1 warning generated.
>>>
>>
>> Steve, with-in event_create_dir(), do we have any scenario when file->event_call->class->system
>> doesn't have TRACE_SYSTEM? And need to execute following:
>>
>> ae63b31e4d0e2e Steven Rostedt 2012-05-03 2437 d_events = parent;
>>
>> looking for your input if we could remove d_events from event_create_dir().
>>
>
> I have hit this in the beginning, but I don't think it's an issue
> anymore. Perhaps just have it be:
>
> if (WARN_ON_ONCE(strcmp(call->class->system, TRACE_SYSTEM) == 0))
> return -ENODEV;
>
> ef_subsystem = event_subsystem_dir(tr, call->class->system, file, parent);
>
> Hmm, how about just add this patch before your patch set:
>
Sounds good. Thanks Steve :)
Once you will merge below patch, I will rebase this patch in v3.
- Ajay
>
> From: "Steven Rostedt (Google)" <rostedt@xxxxxxxxxxx>
> Subject: [PATCH] tracing: Require all trace events to have a TRACE_SYSTEM
>
> The creation of the trace event directory requires that a TRACE_SYSTEM is
> defined that the trace event directory is added within the system it was
> defined in.
>
> The code handled the case where a TRACE_SYSTEM was not added, and would
> then add the event at the events directory. But nothing should be doing
> this. This code also prevents the implementation of creating dynamic
> dentrys for the eventfs system.
>
> As this path has never been hit on correct code, remove it. If it does get
> hit, issues a WARN_ON_ONCE() and return ENODEV.
>
> Signed-off-by: Steven Rostedt (Google) <rostedt@xxxxxxxxxxx>
> ---
>
> (lightly tested)
>
> kernel/trace/trace_events.c | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index 654ffa40457a..16bc5ba45507 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -2424,14 +2424,15 @@ event_create_dir(struct dentry *parent, struct trace_event_file *file)
>
> /*
> * If the trace point header did not define TRACE_SYSTEM
> - * then the system would be called "TRACE_SYSTEM".
> + * then the system would be called "TRACE_SYSTEM". This should
> + * never happen.
> */
> - if (strcmp(call->class->system, TRACE_SYSTEM) != 0) {
> - d_events = event_subsystem_dir(tr, call->class->system, file, parent);
> - if (!d_events)
> - return -ENOMEM;
> - } else
> - d_events = parent;
> + if (WARN_ON_ONCE(strcmp(call->class->system, TRACE_SYSTEM) == 0))
> + return -ENODEV;
> +
> + d_events = event_subsystem_dir(tr, call->class->system, file, parent);
> + if (!d_events)
> + return -ENOMEM;
>
> name = trace_event_name(call);
> file->dir = tracefs_create_dir(name, d_events);
> --
> 2.39.2
>
>
> !! External Email: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender.
