| Message ID | 930c4ef71c7bcb1158d2a8cad905f4de425b8d1e.1675181734.git.bristot@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | rtla: Add hwnoise tool | expand |
On Tue, 31 Jan 2023 17:30:02 +0100 Daniel Bristot de Oliveira <bristot@kernel.org> wrote: > Add some helper functions to read and set the on/off osnoise/options. > No usage in this patch. > > In preparation for hwnoise tool. Honestly, I don't see why patches 1-5 isn't a single patch. It's not that big of a change, and everything in 1-5 is to do what 5 does. Breaking it up this fine grain isn't helpful in reviewing, as I found that I had to apply 1-5 and then do a diff from where I started to make sense of any of it. -- Steve > > Signed-off-by: Daniel Bristot de Oliveira <bristot@kernel.org> > Cc: Daniel Bristot de Oliveira <bristot@kernel.org> > Cc: Steven Rostedt <rostedt@goodmis.org> > Cc: Jonathan Corbet <corbet@lwn.net> > --- >
On Tue, 31 Jan 2023 17:30:02 +0100 Daniel Bristot de Oliveira <bristot@kernel.org> wrote: > +#define OSNOISE_OPTION(name, option_str) \ > +static int osnoise_get_##name(struct osnoise_context *context) \ > +{ \ > + if (context->opt_##name != OSNOISE_OPTION_INIT_VAL) \ > + return context->opt_##name; \ > + \ > + if (context->orig_opt_##name != OSNOISE_OPTION_INIT_VAL) \ > + return context->orig_opt_##name; \ > + \ > + context->orig_opt_##name = osnoise_options_get_option(option_str); \ > + \ > + return context->orig_opt_##name; \ > +} \ > + \ What you could have done is not make this into a super macro (as there's only one instance of it). And then add a patch that turns it into this macro as the first patch of a series that adds another user. Because I don't understand why this exists when it only has one user. -- Steve
On 2/1/23 20:23, Steven Rostedt wrote: > On Tue, 31 Jan 2023 17:30:02 +0100 > Daniel Bristot de Oliveira <bristot@kernel.org> wrote: > >> Add some helper functions to read and set the on/off osnoise/options. >> No usage in this patch. >> >> In preparation for hwnoise tool. > > Honestly, I don't see why patches 1-5 isn't a single patch. It's not that > big of a change, and everything in 1-5 is to do what 5 does. Breaking it up > this fine grain isn't helpful in reviewing, as I found that I had to apply > 1-5 and then do a diff from where I started to make sense of any of it. Maybe what is missing is a clear: In preparation for hwnoise tool. IMHO, it is easier to understand by using small "logical" pieces in preparation for the "conclusion." But I see your point, and it does not hurt :-). I will reduce the number of patches. -- Daniel > > -- Steve > > >> >> Signed-off-by: Daniel Bristot de Oliveira <bristot@kernel.org> >> Cc: Daniel Bristot de Oliveira <bristot@kernel.org> >> Cc: Steven Rostedt <rostedt@goodmis.org> >> Cc: Jonathan Corbet <corbet@lwn.net> >> --- >>
diff --git a/tools/tracing/rtla/src/osnoise.c b/tools/tracing/rtla/src/osnoise.c index 4dee343909b1..050a9997191c 100644 --- a/tools/tracing/rtla/src/osnoise.c +++ b/tools/tracing/rtla/src/osnoise.c @@ -734,6 +734,114 @@ void osnoise_put_tracing_thresh(struct osnoise_context *context) context->orig_tracing_thresh = OSNOISE_OPTION_INIT_VAL; } +static int osnoise_options_get_option(char *option) +{ + char *options = tracefs_instance_file_read(NULL, "osnoise/options", NULL); + char no_option[128]; + int retval = 0; + char *opt; + + if (!options) + return OSNOISE_OPTION_INIT_VAL; + + /* + * Check first if the option is disabled. + */ + snprintf(no_option, sizeof(no_option), "NO_%s", option); + + opt = strstr(options, no_option); + if (opt) + goto out_free; + + /* + * Now that it is not disabled, if the string is there, it is + * enabled. If the string is not there, the option does not exist. + */ + opt = strstr(options, option); + if (opt) + retval = 1; + else + retval = OSNOISE_OPTION_INIT_VAL; + +out_free: + free(options); + return retval; +} + +static int osnoise_options_set_option(char *option, bool onoff) +{ + char no_option[128]; + + if (onoff) + return tracefs_instance_file_write(NULL, "osnoise/options", option); + + snprintf(no_option, sizeof(no_option), "NO_%s", option); + + return tracefs_instance_file_write(NULL, "osnoise/options", no_option); +} + +#define OSNOISE_OPTION(name, option_str) \ +static int osnoise_get_##name(struct osnoise_context *context) \ +{ \ + if (context->opt_##name != OSNOISE_OPTION_INIT_VAL) \ + return context->opt_##name; \ + \ + if (context->orig_opt_##name != OSNOISE_OPTION_INIT_VAL) \ + return context->orig_opt_##name; \ + \ + context->orig_opt_##name = osnoise_options_get_option(option_str); \ + \ + return context->orig_opt_##name; \ +} \ + \ +int osnoise_set_##name(struct osnoise_context *context, bool onoff) \ +{ \ + int opt_##name = osnoise_get_##name(context); \ + int retval; \ + \ + if (opt_##name == OSNOISE_OPTION_INIT_VAL) \ + return -1; \ + \ + if (opt_##name == onoff) \ + return 0; \ + \ + retval = osnoise_options_set_option(option_str, onoff); \ + if (retval < 0) \ + return -1; \ + \ + context->opt_##name = onoff; \ + \ + return 0; \ +} \ + \ +static void osnoise_restore_##name(struct osnoise_context *context) \ +{ \ + int retval; \ + \ + if (context->orig_opt_##name == OSNOISE_OPTION_INIT_VAL) \ + return; \ + \ + if (context->orig_opt_##name == context->opt_##name) \ + goto out_done; \ + \ + retval = osnoise_options_set_option(option_str, context->orig_opt_##name); \ + if (retval < 0) \ + err_msg("Could not restore original osnoise " #option_str " option\n"); \ + \ +out_done: \ + context->orig_opt_##name = OSNOISE_OPTION_INIT_VAL; \ +} \ + \ +static void osnoise_put_##name(struct osnoise_context *context) \ +{ \ + osnoise_restore_##name(context); \ + \ + if (context->orig_opt_##name == OSNOISE_OPTION_INIT_VAL) \ + return; \ + \ + context->orig_opt_##name = OSNOISE_OPTION_INIT_VAL; \ +} + /* * enable_osnoise - enable osnoise tracer in the trace_instance */
Add some helper functions to read and set the on/off osnoise/options. No usage in this patch. In preparation for hwnoise tool. Signed-off-by: Daniel Bristot de Oliveira <bristot@kernel.org> Cc: Daniel Bristot de Oliveira <bristot@kernel.org> Cc: Steven Rostedt <rostedt@goodmis.org> Cc: Jonathan Corbet <corbet@lwn.net> --- tools/tracing/rtla/src/osnoise.c | 108 +++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+)