4.19-stable review patch. If anyone has any objections, please let me know.

------------------

This reverts commit abdbd5f3e8c504d864fdc032dd5a4eb481cb12bf which is commit 91b80cc5b39f00399e8e2d17527cad2c7fa535e2 upstream.

map_hugetlb.c:18:10: fatal error: vm_util.h: No such file or directory 18 | #include "vm_util.h" | ^~~~~~~~~~~ compilation terminated.

vm_util.h is not present in 4.19.y, as commit:642bc52aed9c ("selftests: vm: bring common functions to a new file") is not present in stable kernels <=6.1.y

Signed-off-by: Harshit Mogalapalli harshit.m.mogalapalli@oracle.com Cc: Shuah Khan skhan@linuxfoundation.org Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org --- tools/testing/selftests/vm/map_hugetlb.c | 7 ------- 1 file changed, 7 deletions(-)

--- a/tools/testing/selftests/vm/map_hugetlb.c +++ b/tools/testing/selftests/vm/map_hugetlb.c @@ -15,7 +15,6 @@ #include <unistd.h> #include <sys/mman.h> #include <fcntl.h> -#include "vm_util.h"

#define LENGTH (256UL*1024*1024) #define PROTECTION (PROT_READ | PROT_WRITE) @@ -71,16 +70,10 @@ int main(int argc, char **argv) { void *addr; int ret; - size_t hugepage_size; size_t length = LENGTH; int flags = FLAGS; int shift = 0;

- hugepage_size = default_huge_page_size(); - /* munmap with fail if the length is not page aligned */ - if (hugepage_size > length) - length = hugepage_size; - if (argc > 1) length = atol(argv[1]) << 20; if (argc > 2) {

4.19-stable review patch. If anyone has any objections, please let me know.

------------------

From: Dominique Martinet dominique.martinet@atmark-techno.com

commit 9af503d91298c3f2945e73703f0e00995be08c30 upstream.

The previous patch that replaced BUG_ON by error handling forgot to unlock the mutex in the error path.

Link: https://lore.kernel.org/all/Zh%2fHpAGFqa7YAFuM@duo.ucw.cz Reported-by: Pavel Machek pavel@denx.de Fixes: 7411055db5ce ("btrfs: handle chunk tree lookup error in btrfs_relocate_sys_chunks()") CC: stable@vger.kernel.org Reviewed-by: Pavel Machek pavel@denx.de Signed-off-by: Dominique Martinet dominique.martinet@atmark-techno.com Reviewed-by: David Sterba dsterba@suse.com Signed-off-by: David Sterba dsterba@suse.com Signed-off-by: Dominique Martinet dominique.martinet@atmark-techno.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org --- fs/btrfs/volumes.c | 1 + 1 file changed, 1 insertion(+)

--- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -2957,6 +2957,7 @@ again: * alignment and size). */ ret = -EUCLEAN; + mutex_unlock(&fs_info->delete_unused_bgs_mutex); goto error; }

4.19-stable review patch. If anyone has any objections, please let me know.

------------------

From: Masami Hiramatsu mhiramat@kernel.org

commit 5448d44c38557fc15d1c53b608a9c9f0e1ca8f86 upstream.

Add unified dynamic event framework for ftrace kprobes, uprobes and synthetic events. Those dynamic events can be co-exist on same file because those syntax doesn't overlap.

This introduces a framework part which provides a unified tracefs interface and operations.

Link: http://lkml.kernel.org/r/154140852824.17322.12250362185969352095.stgit@devbo...

Reviewed-by: Tom Zanussi tom.zanussi@linux.intel.com Tested-by: Tom Zanussi tom.zanussi@linux.intel.com Signed-off-by: Masami Hiramatsu mhiramat@kernel.org Signed-off-by: Steven Rostedt (VMware) rostedt@goodmis.org Signed-off-by: George Guo guodongtai@kylinos.cn Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org --- kernel/trace/Kconfig | 3 kernel/trace/Makefile | 1 kernel/trace/trace.c | 4 kernel/trace/trace_dynevent.c | 210 ++++++++++++++++++++++++++++++++++++++++++ kernel/trace/trace_dynevent.h | 119 +++++++++++++++++++++++ 5 files changed, 337 insertions(+) create mode 100644 kernel/trace/trace_dynevent.c create mode 100644 kernel/trace/trace_dynevent.h

--- a/kernel/trace/Kconfig +++ b/kernel/trace/Kconfig @@ -518,6 +518,9 @@ config BPF_EVENTS help This allows the user to attach BPF programs to kprobe events.

+config DYNAMIC_EVENTS + def_bool n + config PROBE_EVENTS def_bool n

--- a/kernel/trace/Makefile +++ b/kernel/trace/Makefile @@ -78,6 +78,7 @@ endif ifeq ($(CONFIG_TRACING),y) obj-$(CONFIG_KGDB_KDB) += trace_kdb.o endif +obj-$(CONFIG_DYNAMIC_EVENTS) += trace_dynevent.o obj-$(CONFIG_PROBE_EVENTS) += trace_probe.o obj-$(CONFIG_UPROBE_EVENTS) += trace_uprobe.o

--- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c @@ -4665,6 +4665,10 @@ static const char readme_msg[] = "\t\t\t traces\n" #endif #endif /* CONFIG_STACK_TRACER */ +#ifdef CONFIG_DYNAMIC_EVENTS + " dynamic_events\t\t- Add/remove/show the generic dynamic events\n" + "\t\t\t Write into this file to define/undefine new trace events.\n" +#endif #ifdef CONFIG_KPROBE_EVENTS " kprobe_events\t\t- Add/remove/show the kernel dynamic events\n" "\t\t\t Write into this file to define/undefine new trace events.\n" --- /dev/null +++ b/kernel/trace/trace_dynevent.c @@ -0,0 +1,210 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Generic dynamic event control interface + * + * Copyright (C) 2018 Masami Hiramatsu mhiramat@kernel.org + */ + +#include <linux/debugfs.h> +#include <linux/kernel.h> +#include <linux/list.h> +#include <linux/mm.h> +#include <linux/mutex.h> +#include <linux/tracefs.h> + +#include "trace.h" +#include "trace_dynevent.h" + +static DEFINE_MUTEX(dyn_event_ops_mutex); +static LIST_HEAD(dyn_event_ops_list); + +int dyn_event_register(struct dyn_event_operations *ops) +{ + if (!ops || !ops->create || !ops->show || !ops->is_busy || + !ops->free || !ops->match) + return -EINVAL; + + INIT_LIST_HEAD(&ops->list); + mutex_lock(&dyn_event_ops_mutex); + list_add_tail(&ops->list, &dyn_event_ops_list); + mutex_unlock(&dyn_event_ops_mutex); + return 0; +} + +int dyn_event_release(int argc, char **argv, struct dyn_event_operations *type) +{ + struct dyn_event *pos, *n; + char *system = NULL, *event, *p; + int ret = -ENOENT; + + if (argv[0][1] != ':') + return -EINVAL; + + event = &argv[0][2]; + p = strchr(event, '/'); + if (p) { + system = event; + event = p + 1; + *p = '\0'; + } + if (event[0] == '\0') + return -EINVAL; + + mutex_lock(&event_mutex); + for_each_dyn_event_safe(pos, n) { + if (type && type != pos->ops) + continue; + if (pos->ops->match(system, event, pos)) { + ret = pos->ops->free(pos); + break; + } + } + mutex_unlock(&event_mutex); + + return ret; +} + +static int create_dyn_event(int argc, char **argv) +{ + struct dyn_event_operations *ops; + int ret; + + if (argv[0][0] == '-') + return dyn_event_release(argc, argv, NULL); + + mutex_lock(&dyn_event_ops_mutex); + list_for_each_entry(ops, &dyn_event_ops_list, list) { + ret = ops->create(argc, (const char **)argv); + if (!ret || ret != -ECANCELED) + break; + } + mutex_unlock(&dyn_event_ops_mutex); + if (ret == -ECANCELED) + ret = -EINVAL; + + return ret; +} + +/* Protected by event_mutex */ +LIST_HEAD(dyn_event_list); + +void *dyn_event_seq_start(struct seq_file *m, loff_t *pos) +{ + mutex_lock(&event_mutex); + return seq_list_start(&dyn_event_list, *pos); +} + +void *dyn_event_seq_next(struct seq_file *m, void *v, loff_t *pos) +{ + return seq_list_next(v, &dyn_event_list, pos); +} + +void dyn_event_seq_stop(struct seq_file *m, void *v) +{ + mutex_unlock(&event_mutex); +} + +static int dyn_event_seq_show(struct seq_file *m, void *v) +{ + struct dyn_event *ev = v; + + if (ev && ev->ops) + return ev->ops->show(m, ev); + + return 0; +} + +static const struct seq_operations dyn_event_seq_op = { + .start = dyn_event_seq_start, + .next = dyn_event_seq_next, + .stop = dyn_event_seq_stop, + .show = dyn_event_seq_show +}; + +/* + * dyn_events_release_all - Release all specific events + * @type: the dyn_event_operations * which filters releasing events + * + * This releases all events which ->ops matches @type. If @type is NULL, + * all events are released. + * Return -EBUSY if any of them are in use, and return other errors when + * it failed to free the given event. Except for -EBUSY, event releasing + * process will be aborted at that point and there may be some other + * releasable events on the list. + */ +int dyn_events_release_all(struct dyn_event_operations *type) +{ + struct dyn_event *ev, *tmp; + int ret = 0; + + mutex_lock(&event_mutex); + for_each_dyn_event(ev) { + if (type && ev->ops != type) + continue; + if (ev->ops->is_busy(ev)) { + ret = -EBUSY; + goto out; + } + } + for_each_dyn_event_safe(ev, tmp) { + if (type && ev->ops != type) + continue; + ret = ev->ops->free(ev); + if (ret) + break; + } +out: + mutex_unlock(&event_mutex); + + return ret; +} + +static int dyn_event_open(struct inode *inode, struct file *file) +{ + int ret; + + if ((file->f_mode & FMODE_WRITE) && (file->f_flags & O_TRUNC)) { + ret = dyn_events_release_all(NULL); + if (ret < 0) + return ret; + } + + return seq_open(file, &dyn_event_seq_op); +} + +static ssize_t dyn_event_write(struct file *file, const char __user *buffer, + size_t count, loff_t *ppos) +{ + return trace_parse_run_command(file, buffer, count, ppos, + create_dyn_event); +} + +static const struct file_operations dynamic_events_ops = { + .owner = THIS_MODULE, + .open = dyn_event_open, + .read = seq_read, + .llseek = seq_lseek, + .release = seq_release, + .write = dyn_event_write, +}; + +/* Make a tracefs interface for controlling dynamic events */ +static __init int init_dynamic_event(void) +{ + struct dentry *d_tracer; + struct dentry *entry; + + d_tracer = tracing_init_dentry(); + if (IS_ERR(d_tracer)) + return 0; + + entry = tracefs_create_file("dynamic_events", 0644, d_tracer, + NULL, &dynamic_events_ops); + + /* Event list interface */ + if (!entry) + pr_warn("Could not create tracefs 'dynamic_events' entry\n"); + + return 0; +} +fs_initcall(init_dynamic_event); --- /dev/null +++ b/kernel/trace/trace_dynevent.h @@ -0,0 +1,119 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Common header file for generic dynamic events. + */ + +#ifndef _TRACE_DYNEVENT_H +#define _TRACE_DYNEVENT_H + +#include <linux/kernel.h> +#include <linux/list.h> +#include <linux/mutex.h> +#include <linux/seq_file.h> + +#include "trace.h" + +struct dyn_event; + +/** + * struct dyn_event_operations - Methods for each type of dynamic events + * + * These methods must be set for each type, since there is no default method. + * Before using this for dyn_event_init(), it must be registered by + * dyn_event_register(). + * + * @create: Parse and create event method. This is invoked when user passes + * a event definition to dynamic_events interface. This must not destruct + * the arguments and return -ECANCELED if given arguments doesn't match its + * command prefix. + * @show: Showing method. This is invoked when user reads the event definitions + * via dynamic_events interface. + * @is_busy: Check whether given event is busy so that it can not be deleted. + * Return true if it is busy, otherwides false. + * @free: Delete the given event. Return 0 if success, otherwides error. + * @match: Check whether given event and system name match this event. + * Return true if it matches, otherwides false. + * + * Except for @create, these methods are called under holding event_mutex. + */ +struct dyn_event_operations { + struct list_head list; + int (*create)(int argc, const char *argv[]); + int (*show)(struct seq_file *m, struct dyn_event *ev); + bool (*is_busy)(struct dyn_event *ev); + int (*free)(struct dyn_event *ev); + bool (*match)(const char *system, const char *event, + struct dyn_event *ev); +}; + +/* Register new dyn_event type -- must be called at first */ +int dyn_event_register(struct dyn_event_operations *ops); + +/** + * struct dyn_event - Dynamic event list header + * + * The dyn_event structure encapsulates a list and a pointer to the operators + * for making a global list of dynamic events. + * User must includes this in each event structure, so that those events can + * be added/removed via dynamic_events interface. + */ +struct dyn_event { + struct list_head list; + struct dyn_event_operations *ops; +}; + +extern struct list_head dyn_event_list; + +static inline +int dyn_event_init(struct dyn_event *ev, struct dyn_event_operations *ops) +{ + if (!ev || !ops) + return -EINVAL; + + INIT_LIST_HEAD(&ev->list); + ev->ops = ops; + return 0; +} + +static inline int dyn_event_add(struct dyn_event *ev) +{ + lockdep_assert_held(&event_mutex); + + if (!ev || !ev->ops) + return -EINVAL; + + list_add_tail(&ev->list, &dyn_event_list); + return 0; +} + +static inline void dyn_event_remove(struct dyn_event *ev) +{ + lockdep_assert_held(&event_mutex); + list_del_init(&ev->list); +} + +void *dyn_event_seq_start(struct seq_file *m, loff_t *pos); +void *dyn_event_seq_next(struct seq_file *m, void *v, loff_t *pos); +void dyn_event_seq_stop(struct seq_file *m, void *v); +int dyn_events_release_all(struct dyn_event_operations *type); +int dyn_event_release(int argc, char **argv, struct dyn_event_operations *type); + +/* + * for_each_dyn_event - iterate over the dyn_event list + * @pos: the struct dyn_event * to use as a loop cursor + * + * This is just a basement of for_each macro. Wrap this for + * each actual event structure with ops filtering. + */ +#define for_each_dyn_event(pos) \ + list_for_each_entry(pos, &dyn_event_list, list) + +/* + * for_each_dyn_event - iterate over the dyn_event list safely + * @pos: the struct dyn_event * to use as a loop cursor + * @n: the struct dyn_event * to use as temporary storage + */ +#define for_each_dyn_event_safe(pos, n) \ + list_for_each_entry_safe(pos, n, &dyn_event_list, list) + +#endif

4.19-stable review patch. If anyone has any objections, please let me know.

------------------

From: Masami Hiramatsu mhiramat@kernel.org

commit 0e2b81f7b52a1c1a8c46986f9ca01eb7b3c421f8 upstream.

Rmove unneeded synth_event_mutex. This mutex protects the reference count in synth_event, however, those operational points are already protected by event_mutex.

1. In __create_synth_event() and create_or_delete_synth_event(), those synth_event_mutex clearly obtained right after event_mutex.

2. event_hist_trigger_func() is trigger_hist_cmd.func() which is called by trigger_process_regex(), which is a part of event_trigger_regex_write() and this function takes event_mutex.

3. hist_unreg_all() is trigger_hist_cmd.unreg_all() which is called by event_trigger_regex_open() and it takes event_mutex.

4. onmatch_destroy() and onmatch_create() have long call tree, but both are finally invoked from event_trigger_regex_write() and event_trace_del_tracer(), former takes event_mutex, and latter ensures called under event_mutex locked.

Finally, I ensured there is no resource conflict. For safety, I added lockdep_assert_held(&event_mutex) for each function.

Link: http://lkml.kernel.org/r/154140864134.17322.4796059721306031894.stgit@devbox

Reviewed-by: Tom Zanussi tom.zanussi@linux.intel.com Tested-by: Tom Zanussi tom.zanussi@linux.intel.com Signed-off-by: Masami Hiramatsu mhiramat@kernel.org Signed-off-by: Steven Rostedt (VMware) rostedt@goodmis.org Signed-off-by: George Guo guodongtai@kylinos.cn Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org --- kernel/trace/trace_events_hist.c | 30 +++++++----------------------- 1 file changed, 7 insertions(+), 23 deletions(-)

--- a/kernel/trace/trace_events_hist.c +++ b/kernel/trace/trace_events_hist.c @@ -443,8 +443,6 @@ static bool have_hist_err(void) return false; }

-static DEFINE_MUTEX(synth_event_mutex); - struct synth_trace_event { struct trace_entry ent; u64 fields[]; @@ -1097,7 +1095,6 @@ static int __create_synth_event(int argc return -EINVAL;

mutex_lock(&event_mutex); - mutex_lock(&synth_event_mutex);

event = find_synth_event(name); if (event) { @@ -1139,7 +1136,6 @@ static int __create_synth_event(int argc else free_synth_event(event); out: - mutex_unlock(&synth_event_mutex); mutex_unlock(&event_mutex);

return ret; @@ -1159,7 +1155,6 @@ static int create_or_delete_synth_event( /* trace_run_command() ensures argc != 0 */ if (name[0] == '!') { mutex_lock(&event_mutex); - mutex_lock(&synth_event_mutex); event = find_synth_event(name + 1); if (event) { if (event->ref) @@ -1173,7 +1168,6 @@ static int create_or_delete_synth_event( } } else ret = -ENOENT; - mutex_unlock(&synth_event_mutex); mutex_unlock(&event_mutex); return ret; } @@ -3660,7 +3654,7 @@ static void onmatch_destroy(struct actio { unsigned int i;

- mutex_lock(&synth_event_mutex); + lockdep_assert_held(&event_mutex);

kfree(data->onmatch.match_event); kfree(data->onmatch.match_event_system); @@ -3673,8 +3667,6 @@ static void onmatch_destroy(struct actio data->onmatch.synth_event->ref--;

kfree(data); - - mutex_unlock(&synth_event_mutex); }

static void destroy_field_var(struct field_var *field_var) @@ -3810,15 +3802,14 @@ static int onmatch_create(struct hist_tr struct synth_event *event; int ret = 0;

- mutex_lock(&synth_event_mutex); + lockdep_assert_held(&event_mutex); + event = find_synth_event(data->onmatch.synth_event_name); if (!event) { hist_err("onmatch: Couldn't find synthetic event: ", data->onmatch.synth_event_name); - mutex_unlock(&synth_event_mutex); return -EINVAL; } event->ref++; - mutex_unlock(&synth_event_mutex);

var_ref_idx = hist_data->n_var_refs;

@@ -3892,9 +3883,7 @@ static int onmatch_create(struct hist_tr out: return ret; err: - mutex_lock(&synth_event_mutex); event->ref--; - mutex_unlock(&synth_event_mutex);

goto out; } @@ -5611,6 +5600,8 @@ static void hist_unreg_all(struct trace_ struct synth_event *se; const char *se_name;

+ lockdep_assert_held(&event_mutex); + if (hist_file_check_refs(file)) return;

@@ -5620,12 +5611,10 @@ static void hist_unreg_all(struct trace_ list_del_rcu(&test->list); trace_event_trigger_enable_disable(file, 0);

- mutex_lock(&synth_event_mutex); se_name = trace_event_name(file->event_call); se = find_synth_event(se_name); if (se) se->ref--; - mutex_unlock(&synth_event_mutex);

update_cond_flag(file); if (hist_data->enable_timestamps) @@ -5651,6 +5640,8 @@ static int event_hist_trigger_func(struc char *trigger, *p; int ret = 0;

+ lockdep_assert_held(&event_mutex); + if (glob && strlen(glob)) { last_cmd_set(param); hist_err_clear(); @@ -5741,14 +5732,10 @@ static int event_hist_trigger_func(struc }

cmd_ops->unreg(glob+1, trigger_ops, trigger_data, file); - - mutex_lock(&synth_event_mutex); se_name = trace_event_name(file->event_call); se = find_synth_event(se_name); if (se) se->ref--; - mutex_unlock(&synth_event_mutex); - ret = 0; goto out_free; } @@ -5787,13 +5774,10 @@ enable: if (ret) goto out_unreg;

- mutex_lock(&synth_event_mutex); se_name = trace_event_name(file->event_call); se = find_synth_event(se_name); if (se) se->ref++; - mutex_unlock(&synth_event_mutex); - /* Just return zero, not the number of registered triggers */ ret = 0; out:

4.19-stable review patch. If anyone has any objections, please let me know.

------------------

From: "Steven Rostedt (VMware)" rostedt@goodmis.org

commit 72921427d46bf9731a1ab7864adc64c43dfae29f upstream.

A discussion came up in the trace triggers thread about converting a bunch of:

strncmp(str, "const", sizeof("const") - 1)

use cases into a helper macro. It started with:

strncmp(str, const, sizeof(const) - 1)

But then Joe Perches mentioned that if a const is not used, the sizeof() will be the size of a pointer, which can be bad. And that gcc will optimize strlen("const") into "sizeof("const") - 1".

Thinking about this more, a quick grep in the kernel tree found several (thousands!) of cases that use this construct. A quick grep also revealed that there's probably several bugs in that use case. Some are that people forgot the "- 1" (which I found) and others could be that the constant for the sizeof is different than the constant (although, I haven't found any of those, but I also didn't look hard).

I figured the best thing to do is to create a helper macro and place it into include/linux/string.h. And go around and fix all the open coded versions of it later.

Note, gcc appears to optimize this when we make it into an always_inline static function, which removes a lot of issues that a macro produces.

Link: http://lkml.kernel.org/r/e3e754f2bd18e56eaa8baf79bee619316ebf4cfc.1545161087... Link: http://lkml.kernel.org/r/20181219211615.2298e781@gandalf.local.home Link: http://lkml.kernel.org/r/CAHk-=wg_sR-UEC1ggmkZpypOUYanL5CMX4R7ceuaV4QMf5jBtg...

Cc: Tom Zanussi zanussi@kernel.org Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Acked-by: Namhyung Kim namhyung@kernel.org Suggestions-by: Linus Torvalds torvalds@linux-foundation.org Suggestions-by: Joe Perches joe@perches.com Suggestions-by: Andreas Schwab schwab@linux-m68k.org Signed-off-by: Steven Rostedt (VMware) rostedt@goodmis.org Signed-off-by: George Guo guodongtai@kylinos.cn Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org --- include/linux/string.h | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)

--- a/include/linux/string.h +++ b/include/linux/string.h @@ -492,4 +492,24 @@ static inline void memcpy_and_pad(void * memcpy(dest, src, dest_len); }

+/** + * str_has_prefix - Test if a string has a given prefix + * @str: The string to test + * @prefix: The string to see if @str starts with + * + * A common way to test a prefix of a string is to do: + * strncmp(str, prefix, sizeof(prefix) - 1) + * + * But this can lead to bugs due to typos, or if prefix is a pointer + * and not a constant. Instead use str_has_prefix(). + * + * Returns: 0 if @str does not start with @prefix + strlen(@prefix) if @str does start with @prefix + */ +static __always_inline size_t str_has_prefix(const char *str, const char *prefix) +{ + size_t len = strlen(prefix); + return strncmp(str, prefix, len) == 0 ? len : 0; +} + #endif /* _LINUX_STRING_H_ */

4.19-stable review patch. If anyone has any objections, please let me know.

------------------

From: "Steven Rostedt (VMware)" rostedt@goodmis.org

commit b6b2735514bcd70ad1556a33892a636b20ece671 upstream.

There are several instances of strncmp(str, "const", 123), where 123 is the strlen of the const string to check if "const" is the prefix of str. But this can be error prone. Use str_has_prefix() instead.

Acked-by: Namhyung Kim namhyung@kernel.org Signed-off-by: Steven Rostedt (VMware) rostedt@goodmis.org Signed-off-by: George Guo guodongtai@kylinos.cn Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org --- kernel/trace/trace.c | 2 +- kernel/trace/trace_events.c | 2 +- kernel/trace/trace_events_hist.c | 2 +- kernel/trace/trace_probe.c | 2 +- kernel/trace/trace_stack.c | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-)

--- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c @@ -4470,7 +4470,7 @@ static int trace_set_options(struct trac

cmp = strstrip(option);

- if (strncmp(cmp, "no", 2) == 0) { + if (str_has_prefix(cmp, "no")) { neg = 1; cmp += 2; } --- a/kernel/trace/trace_events.c +++ b/kernel/trace/trace_events.c @@ -1249,7 +1249,7 @@ static int f_show(struct seq_file *m, vo */ array_descriptor = strchr(field->type, '[');

- if (!strncmp(field->type, "__data_loc", 10)) + if (str_has_prefix(field->type, "__data_loc")) array_descriptor = NULL;

if (!array_descriptor) --- a/kernel/trace/trace_events_hist.c +++ b/kernel/trace/trace_events_hist.c @@ -484,7 +484,7 @@ static int synth_event_define_fields(str

static bool synth_field_signed(char *type) { - if (strncmp(type, "u", 1) == 0) + if (str_has_prefix(type, "u")) return false; if (strcmp(type, "gfp_t") == 0) return false; --- a/kernel/trace/trace_probe.c +++ b/kernel/trace/trace_probe.c @@ -342,7 +342,7 @@ static int parse_probe_vars(char *arg, c f->fn = t->fetch[FETCH_MTD_retval]; else ret = -EINVAL; - } else if (strncmp(arg, "stack", 5) == 0) { + } else if (str_has_prefix(arg, "stack")) { if (arg[5] == '\0') { if (strcmp(t->name, DEFAULT_FETCH_TYPE_STR)) return -EINVAL; --- a/kernel/trace/trace_stack.c +++ b/kernel/trace/trace_stack.c @@ -453,7 +453,7 @@ static char stack_trace_filter_buf[COMMA

static __init int enable_stacktrace(char *str) { - if (strncmp(str, "_filter=", 8) == 0) + if (str_has_prefix(str, "_filter=")) strncpy(stack_trace_filter_buf, str+8, COMMAND_LINE_SIZE);

stack_tracer_enabled = 1;

4.19-stable review patch. If anyone has any objections, please let me know.

------------------

From: Tom Zanussi tom.zanussi@linux.intel.com

commit 7d18a10c316783357fb1b2b649cfcf97c70a7bee upstream.

The hist trigger action code currently implements two essentially hard-coded pairs of 'actions' - onmax(), which tracks a variable and saves some event fields when a max is hit, and onmatch(), which is hard-coded to generate a synthetic event.

These hardcoded pairs (track max/save fields and detect match/generate synthetic event) should really be decoupled into separate components that can then be arbitrarily combined. The first component of each pair (track max/detect match) is called a 'handler' in the new code, while the second component (save fields/generate synthetic event) is called an 'action' in this scheme.

This change refactors the action code to reflect this split by adding two handlers, HANDLER_ONMATCH and HANDLER_ONMAX, along with two actions, ACTION_SAVE and ACTION_TRACE.

The new code combines them to produce the existing ONMATCH/TRACE and ONMAX/SAVE functionality, but doesn't implement the other combinations now possible. Future patches will expand these to further useful cases, such as ONMAX/TRACE, as well as add additional handlers and actions such as ONCHANGE and SNAPSHOT.

Also, add abbreviated documentation for handlers and actions to README.

Link: http://lkml.kernel.org/r/98bfdd48c1b4ff29fc5766442f99f5bc3c34b76b.1550100284...

Signed-off-by: Tom Zanussi tom.zanussi@linux.intel.com Signed-off-by: Steven Rostedt (VMware) rostedt@goodmis.org Signed-off-by: George Guo guodongtai@kylinos.cn Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org --- kernel/trace/trace_events_hist.c | 407 ++++++++++++++++++++++----------------- 1 file changed, 238 insertions(+), 169 deletions(-)

--- a/kernel/trace/trace_events_hist.c +++ b/kernel/trace/trace_events_hist.c @@ -287,9 +287,9 @@ struct hist_trigger_data { struct field_var_hist *field_var_hists[SYNTH_FIELDS_MAX]; unsigned int n_field_var_hists;

- struct field_var *max_vars[SYNTH_FIELDS_MAX]; - unsigned int n_max_vars; - unsigned int n_max_var_str; + struct field_var *save_vars[SYNTH_FIELDS_MAX]; + unsigned int n_save_vars; + unsigned int n_save_var_str; };

static int synth_event_create(int argc, const char **argv); @@ -357,11 +357,25 @@ struct action_data;

typedef void (*action_fn_t) (struct hist_trigger_data *hist_data, struct tracing_map_elt *elt, void *rec, - struct ring_buffer_event *rbe, + struct ring_buffer_event *rbe, void *key, struct action_data *data, u64 *var_ref_vals);

+enum handler_id { + HANDLER_ONMATCH = 1, + HANDLER_ONMAX, +}; + +enum action_id { + ACTION_SAVE = 1, + ACTION_TRACE, +}; + struct action_data { + enum handler_id handler; + enum action_id action; + char *action_name; action_fn_t fn; + unsigned int n_params; char *params[SYNTH_FIELDS_MAX];

@@ -370,13 +384,11 @@ struct action_data { unsigned int var_ref_idx; char *match_event; char *match_event_system; - char *synth_event_name; struct synth_event *synth_event; } onmatch;

struct { char *var_str; - char *fn_name; unsigned int max_var_ref_idx; struct hist_field *max_var; struct hist_field *var; @@ -1065,7 +1077,7 @@ static struct synth_event *alloc_synth_e

static void action_trace(struct hist_trigger_data *hist_data, struct tracing_map_elt *elt, void *rec, - struct ring_buffer_event *rbe, + struct ring_buffer_event *rbe, void *key, struct action_data *data, u64 *var_ref_vals) { struct synth_event *event = data->onmatch.synth_event; @@ -1635,7 +1647,7 @@ find_match_var(struct hist_trigger_data for (i = 0; i < hist_data->n_actions; i++) { struct action_data *data = hist_data->actions[i];

- if (data->fn == action_trace) { + if (data->handler == HANDLER_ONMATCH) { char *system = data->onmatch.match_event_system; char *event_name = data->onmatch.match_event;

@@ -2073,7 +2085,7 @@ static int hist_trigger_elt_data_alloc(s } }

- n_str = hist_data->n_field_var_str + hist_data->n_max_var_str; + n_str = hist_data->n_field_var_str + hist_data->n_save_var_str;

size = STR_VAR_LEN_MAX;

@@ -3115,7 +3127,7 @@ create_field_var_hist(struct hist_trigge int ret;

if (target_hist_data->n_field_var_hists >= SYNTH_FIELDS_MAX) { - hist_err_event("onmatch: Too many field variables defined: ", + hist_err_event("trace action: Too many field variables defined: ", subsys_name, event_name, field_name); return ERR_PTR(-EINVAL); } @@ -3123,7 +3135,7 @@ create_field_var_hist(struct hist_trigge file = event_file(tr, subsys_name, event_name);

if (IS_ERR(file)) { - hist_err_event("onmatch: Event file not found: ", + hist_err_event("trace action: Event file not found: ", subsys_name, event_name, field_name); ret = PTR_ERR(file); return ERR_PTR(ret); @@ -3137,7 +3149,7 @@ create_field_var_hist(struct hist_trigge */ hist_data = find_compatible_hist(target_hist_data, file); if (!hist_data) { - hist_err_event("onmatch: Matching event histogram not found: ", + hist_err_event("trace action: Matching event histogram not found: ", subsys_name, event_name, field_name); return ERR_PTR(-EINVAL); } @@ -3199,7 +3211,7 @@ create_field_var_hist(struct hist_trigge kfree(cmd); kfree(var_hist->cmd); kfree(var_hist); - hist_err_event("onmatch: Couldn't create histogram for field: ", + hist_err_event("trace action: Couldn't create histogram for field: ", subsys_name, event_name, field_name); return ERR_PTR(ret); } @@ -3212,7 +3224,7 @@ create_field_var_hist(struct hist_trigge if (IS_ERR_OR_NULL(event_var)) { kfree(var_hist->cmd); kfree(var_hist); - hist_err_event("onmatch: Couldn't find synthetic variable: ", + hist_err_event("trace action: Couldn't find synthetic variable: ", subsys_name, event_name, field_name); return ERR_PTR(-EINVAL); } @@ -3295,8 +3307,8 @@ static void update_max_vars(struct hist_ struct ring_buffer_event *rbe, void *rec) { - __update_field_vars(elt, rbe, rec, hist_data->max_vars, - hist_data->n_max_vars, hist_data->n_field_var_str); + __update_field_vars(elt, rbe, rec, hist_data->save_vars, + hist_data->n_save_vars, hist_data->n_field_var_str); }

static struct hist_field *create_var(struct hist_trigger_data *hist_data, @@ -3440,9 +3452,9 @@ static void onmax_print(struct seq_file

seq_printf(m, "\n\tmax: %10llu", tracing_map_read_var(elt, max_idx));

- for (i = 0; i < hist_data->n_max_vars; i++) { - struct hist_field *save_val = hist_data->max_vars[i]->val; - struct hist_field *save_var = hist_data->max_vars[i]->var; + for (i = 0; i < hist_data->n_save_vars; i++) { + struct hist_field *save_val = hist_data->save_vars[i]->val; + struct hist_field *save_var = hist_data->save_vars[i]->var; u64 val;

save_var_idx = save_var->var.idx; @@ -3459,7 +3471,7 @@ static void onmax_print(struct seq_file

static void onmax_save(struct hist_trigger_data *hist_data, struct tracing_map_elt *elt, void *rec, - struct ring_buffer_event *rbe, + struct ring_buffer_event *rbe, void *key, struct action_data *data, u64 *var_ref_vals) { unsigned int max_idx = data->onmax.max_var->var.idx; @@ -3486,7 +3498,7 @@ static void onmax_destroy(struct action_ destroy_hist_field(data->onmax.var, 0);

kfree(data->onmax.var_str); - kfree(data->onmax.fn_name); + kfree(data->action_name);

for (i = 0; i < data->n_params; i++) kfree(data->params[i]); @@ -3494,15 +3506,16 @@ static void onmax_destroy(struct action_ kfree(data); }

+static int action_create(struct hist_trigger_data *hist_data, + struct action_data *data); + static int onmax_create(struct hist_trigger_data *hist_data, struct action_data *data) { + struct hist_field *var_field, *ref_field, *max_var = NULL; struct trace_event_file *file = hist_data->event_file; - struct hist_field *var_field, *ref_field, *max_var; unsigned int var_ref_idx = hist_data->n_var_refs; - struct field_var *field_var; - char *onmax_var_str, *param; - unsigned int i; + char *onmax_var_str; int ret = 0;

onmax_var_str = data->onmax.var_str; @@ -3524,8 +3537,8 @@ static int onmax_create(struct hist_trig

data->onmax.var = ref_field;

- data->fn = onmax_save; data->onmax.max_var_ref_idx = var_ref_idx; + max_var = create_var(hist_data, file, "max", sizeof(u64), "u64"); if (IS_ERR(max_var)) { hist_err("onmax: Couldn't create onmax variable: ", "max"); @@ -3534,27 +3547,7 @@ static int onmax_create(struct hist_trig } data->onmax.max_var = max_var;

- for (i = 0; i < data->n_params; i++) { - param = kstrdup(data->params[i], GFP_KERNEL); - if (!param) { - ret = -ENOMEM; - goto out; - } - - field_var = create_target_field_var(hist_data, NULL, NULL, param); - if (IS_ERR(field_var)) { - hist_err("onmax: Couldn't create field variable: ", param); - ret = PTR_ERR(field_var); - kfree(param); - goto out; - } - - hist_data->max_vars[hist_data->n_max_vars++] = field_var; - if (field_var->val->flags & HIST_FIELD_FL_STRING) - hist_data->n_max_var_str++; - - kfree(param); - } + ret = action_create(hist_data, data); out: return ret; } @@ -3565,11 +3558,14 @@ static int parse_action_params(char *par int ret = 0;

while (params) { - if (data->n_params >= SYNTH_FIELDS_MAX) + if (data->n_params >= SYNTH_FIELDS_MAX) { + hist_err("Too many action params", ""); goto out; + }

param = strsep(&params, ","); if (!param) { + hist_err("No action param found", ""); ret = -EINVAL; goto out; } @@ -3593,10 +3589,71 @@ static int parse_action_params(char *par return ret; }

-static struct action_data *onmax_parse(char *str) +static int action_parse(char *str, struct action_data *data, + enum handler_id handler) +{ + char *action_name; + int ret = 0; + + strsep(&str, "."); + if (!str) { + hist_err("action parsing: No action found", ""); + ret = -EINVAL; + goto out; + } + + action_name = strsep(&str, "("); + if (!action_name || !str) { + hist_err("action parsing: No action found", ""); + ret = -EINVAL; + goto out; + } + + if (str_has_prefix(action_name, "save")) { + char *params = strsep(&str, ")"); + + if (!params) { + hist_err("action parsing: No params found for %s", "save"); + ret = -EINVAL; + goto out; + } + + ret = parse_action_params(params, data); + if (ret) + goto out; + + if (handler == HANDLER_ONMAX) + data->fn = onmax_save; + + data->action = ACTION_SAVE; + } else { + char *params = strsep(&str, ")"); + + if (params) { + ret = parse_action_params(params, data); + if (ret) + goto out; + } + + data->fn = action_trace; + data->action = ACTION_TRACE; + } + + data->action_name = kstrdup(action_name, GFP_KERNEL); + if (!data->action_name) { + ret = -ENOMEM; + goto out; + } + + data->handler = handler; + out: + return ret; +} + +static struct action_data *onmax_parse(char *str, enum handler_id handler) { - char *onmax_fn_name, *onmax_var_str; struct action_data *data; + char *onmax_var_str; int ret = -EINVAL;

data = kzalloc(sizeof(*data), GFP_KERNEL); @@ -3615,33 +3672,9 @@ static struct action_data *onmax_parse(c goto free; }

- strsep(&str, "."); - if (!str) - goto free; - - onmax_fn_name = strsep(&str, "("); - if (!onmax_fn_name || !str) - goto free; - - if (str_has_prefix(onmax_fn_name, "save")) { - char *params = strsep(&str, ")"); - - if (!params) { - ret = -EINVAL; - goto free; - } - - ret = parse_action_params(params, data); - if (ret) - goto free; - } else - goto free; - - data->onmax.fn_name = kstrdup(onmax_fn_name, GFP_KERNEL); - if (!data->onmax.fn_name) { - ret = -ENOMEM; + ret = action_parse(str, data, handler); + if (ret) goto free; - } out: return data; free: @@ -3658,7 +3691,7 @@ static void onmatch_destroy(struct actio

kfree(data->onmatch.match_event); kfree(data->onmatch.match_event_system); - kfree(data->onmatch.synth_event_name); + kfree(data->action_name);

for (i = 0; i < data->n_params; i++) kfree(data->params[i]); @@ -3716,8 +3749,9 @@ static int check_synth_field(struct synt }

static struct hist_field * -onmatch_find_var(struct hist_trigger_data *hist_data, struct action_data *data, - char *system, char *event, char *var) +trace_action_find_var(struct hist_trigger_data *hist_data, + struct action_data *data, + char *system, char *event, char *var) { struct hist_field *hist_field;

@@ -3725,7 +3759,7 @@ onmatch_find_var(struct hist_trigger_dat

hist_field = find_target_event_var(hist_data, system, event, var); if (!hist_field) { - if (!system) { + if (!system && data->handler == HANDLER_ONMATCH) { system = data->onmatch.match_event_system; event = data->onmatch.match_event; } @@ -3734,15 +3768,15 @@ onmatch_find_var(struct hist_trigger_dat }

if (!hist_field) - hist_err_event("onmatch: Couldn't find onmatch param: $", system, event, var); + hist_err_event("trace action: Couldn't find param: $", system, event, var);

return hist_field; }

static struct hist_field * -onmatch_create_field_var(struct hist_trigger_data *hist_data, - struct action_data *data, char *system, - char *event, char *var) +trace_action_create_field_var(struct hist_trigger_data *hist_data, + struct action_data *data, char *system, + char *event, char *var) { struct hist_field *hist_field = NULL; struct field_var *field_var; @@ -3765,7 +3799,7 @@ onmatch_create_field_var(struct hist_tri * looking for fields on the onmatch(system.event.xxx) * event. */ - if (!system) { + if (!system && data->handler == HANDLER_ONMATCH) { system = data->onmatch.match_event_system; event = data->onmatch.match_event; } @@ -3791,9 +3825,8 @@ onmatch_create_field_var(struct hist_tri goto out; }

-static int onmatch_create(struct hist_trigger_data *hist_data, - struct trace_event_file *file, - struct action_data *data) +static int trace_action_create(struct hist_trigger_data *hist_data, + struct action_data *data) { char *event_name, *param, *system = NULL; struct hist_field *hist_field, *var_ref; @@ -3804,11 +3837,12 @@ static int onmatch_create(struct hist_tr

lockdep_assert_held(&event_mutex);

- event = find_synth_event(data->onmatch.synth_event_name); + event = find_synth_event(data->action_name); if (!event) { - hist_err("onmatch: Couldn't find synthetic event: ", data->onmatch.synth_event_name); + hist_err("trace action: Couldn't find synthetic event: ", data->action_name); return -EINVAL; } + event->ref++;

var_ref_idx = hist_data->n_var_refs; @@ -3836,13 +3870,15 @@ static int onmatch_create(struct hist_tr }

if (param[0] == '$') - hist_field = onmatch_find_var(hist_data, data, system, - event_name, param); + hist_field = trace_action_find_var(hist_data, data, + system, event_name, + param); else - hist_field = onmatch_create_field_var(hist_data, data, - system, - event_name, - param); + hist_field = trace_action_create_field_var(hist_data, + data, + system, + event_name, + param);

if (!hist_field) { kfree(p); @@ -3864,7 +3900,7 @@ static int onmatch_create(struct hist_tr continue; }

- hist_err_event("onmatch: Param type doesn't match synthetic event field type: ", + hist_err_event("trace action: Param type doesn't match synthetic event field type: ", system, event_name, param); kfree(p); ret = -EINVAL; @@ -3872,12 +3908,11 @@ static int onmatch_create(struct hist_tr }

if (field_pos != event->n_fields) { - hist_err("onmatch: Param count doesn't match synthetic event field count: ", event->name); + hist_err("trace action: Param count doesn't match synthetic event field count: ", event->name); ret = -EINVAL; goto err; }

- data->fn = action_trace; data->onmatch.synth_event = event; data->onmatch.var_ref_idx = var_ref_idx; out: @@ -3888,10 +3923,58 @@ static int onmatch_create(struct hist_tr goto out; }

+static int action_create(struct hist_trigger_data *hist_data, + struct action_data *data) +{ + struct field_var *field_var; + unsigned int i; + char *param; + int ret = 0; + + if (data->action == ACTION_TRACE) + return trace_action_create(hist_data, data); + + if (data->action == ACTION_SAVE) { + if (hist_data->n_save_vars) { + ret = -EEXIST; + hist_err("save action: Can't have more than one save() action per hist", ""); + goto out; + } + + for (i = 0; i < data->n_params; i++) { + param = kstrdup(data->params[i], GFP_KERNEL); + if (!param) { + ret = -ENOMEM; + goto out; + } + + field_var = create_target_field_var(hist_data, NULL, NULL, param); + if (IS_ERR(field_var)) { + hist_err("save action: Couldn't create field variable: ", param); + ret = PTR_ERR(field_var); + kfree(param); + goto out; + } + + hist_data->save_vars[hist_data->n_save_vars++] = field_var; + if (field_var->val->flags & HIST_FIELD_FL_STRING) + hist_data->n_save_var_str++; + kfree(param); + } + } + out: + return ret; +} + +static int onmatch_create(struct hist_trigger_data *hist_data, + struct action_data *data) +{ + return action_create(hist_data, data); +} + static struct action_data *onmatch_parse(struct trace_array *tr, char *str) { char *match_event, *match_event_system; - char *synth_event_name, *params; struct action_data *data; int ret = -EINVAL;

@@ -3929,31 +4012,7 @@ static struct action_data *onmatch_parse goto free; }

- strsep(&str, "."); - if (!str) { - hist_err("onmatch: Missing . after onmatch(): ", str); - goto free; - } - - synth_event_name = strsep(&str, "("); - if (!synth_event_name || !str) { - hist_err("onmatch: Missing opening paramlist paren: ", synth_event_name); - goto free; - } - - data->onmatch.synth_event_name = kstrdup(synth_event_name, GFP_KERNEL); - if (!data->onmatch.synth_event_name) { - ret = -ENOMEM; - goto free; - } - - params = strsep(&str, ")"); - if (!params || !str || (str && strlen(str))) { - hist_err("onmatch: Missing closing paramlist paren: ", params); - goto free; - } - - ret = parse_action_params(params, data); + ret = action_parse(str, data, HANDLER_ONMATCH); if (ret) goto free; out: @@ -4394,9 +4453,9 @@ static void destroy_actions(struct hist_ for (i = 0; i < hist_data->n_actions; i++) { struct action_data *data = hist_data->actions[i];

- if (data->fn == action_trace) + if (data->handler == HANDLER_ONMATCH) onmatch_destroy(data); - else if (data->fn == onmax_save) + else if (data->handler == HANDLER_ONMAX) onmax_destroy(data); else kfree(data); @@ -4423,16 +4482,14 @@ static int parse_actions(struct hist_tri ret = PTR_ERR(data); break; } - data->fn = action_trace; } else if ((len = str_has_prefix(str, "onmax("))) { char *action_str = str + len;

- data = onmax_parse(action_str); + data = onmax_parse(action_str, HANDLER_ONMAX); if (IS_ERR(data)) { ret = PTR_ERR(data); break; } - data->fn = onmax_save; } else { ret = -EINVAL; break; @@ -4444,8 +4501,7 @@ static int parse_actions(struct hist_tri return ret; }

-static int create_actions(struct hist_trigger_data *hist_data, - struct trace_event_file *file) +static int create_actions(struct hist_trigger_data *hist_data) { struct action_data *data; unsigned int i; @@ -4454,14 +4510,17 @@ static int create_actions(struct hist_tr for (i = 0; i < hist_data->attrs->n_actions; i++) { data = hist_data->actions[i];

- if (data->fn == action_trace) { - ret = onmatch_create(hist_data, file, data); + if (data->handler == HANDLER_ONMATCH) { + ret = onmatch_create(hist_data, data); if (ret) - return ret; - } else if (data->fn == onmax_save) { + break; + } else if (data->handler == HANDLER_ONMAX) { ret = onmax_create(hist_data, data); if (ret) - return ret; + break; + } else { + ret = -EINVAL; + break; } }

@@ -4477,26 +4536,42 @@ static void print_actions(struct seq_fil for (i = 0; i < hist_data->n_actions; i++) { struct action_data *data = hist_data->actions[i];

- if (data->fn == onmax_save) + if (data->handler == HANDLER_ONMAX) onmax_print(m, hist_data, elt, data); } }

+static void print_action_spec(struct seq_file *m, + struct hist_trigger_data *hist_data, + struct action_data *data) +{ + unsigned int i; + + if (data->action == ACTION_SAVE) { + for (i = 0; i < hist_data->n_save_vars; i++) { + seq_printf(m, "%s", hist_data->save_vars[i]->var->var.name); + if (i < hist_data->n_save_vars - 1) + seq_puts(m, ","); + } + } else if (data->action == ACTION_TRACE) { + for (i = 0; i < data->n_params; i++) { + if (i) + seq_puts(m, ","); + seq_printf(m, "%s", data->params[i]); + } + } +} + static void print_onmax_spec(struct seq_file *m, struct hist_trigger_data *hist_data, struct action_data *data) { - unsigned int i; - seq_puts(m, ":onmax("); seq_printf(m, "%s", data->onmax.var_str); - seq_printf(m, ").%s(", data->onmax.fn_name); + seq_printf(m, ").%s(", data->action_name); + + print_action_spec(m, hist_data, data);

- for (i = 0; i < hist_data->n_max_vars; i++) { - seq_printf(m, "%s", hist_data->max_vars[i]->var->var.name); - if (i < hist_data->n_max_vars - 1) - seq_puts(m, ","); - } seq_puts(m, ")"); }

@@ -4504,18 +4579,12 @@ static void print_onmatch_spec(struct se struct hist_trigger_data *hist_data, struct action_data *data) { - unsigned int i; - seq_printf(m, ":onmatch(%s.%s).", data->onmatch.match_event_system, data->onmatch.match_event);

- seq_printf(m, "%s(", data->onmatch.synth_event->name); + seq_printf(m, "%s(", data->action_name);

- for (i = 0; i < data->n_params; i++) { - if (i) - seq_puts(m, ","); - seq_printf(m, "%s", data->params[i]); - } + print_action_spec(m, hist_data, data);

seq_puts(m, ")"); } @@ -4532,7 +4601,9 @@ static bool actions_match(struct hist_tr struct action_data *data = hist_data->actions[i]; struct action_data *data_test = hist_data_test->actions[i];

- if (data->fn != data_test->fn) + if (data->handler != data_test->handler) + return false; + if (data->action != data_test->action) return false;

if (data->n_params != data_test->n_params) @@ -4543,23 +4614,20 @@ static bool actions_match(struct hist_tr return false; }

- if (data->fn == action_trace) { - if (strcmp(data->onmatch.synth_event_name, - data_test->onmatch.synth_event_name) != 0) - return false; + if (strcmp(data->action_name, data_test->action_name) != 0) + return false; + + if (data->handler == HANDLER_ONMATCH) { if (strcmp(data->onmatch.match_event_system, data_test->onmatch.match_event_system) != 0) return false; if (strcmp(data->onmatch.match_event, data_test->onmatch.match_event) != 0) return false; - } else if (data->fn == onmax_save) { + } else if (data->handler == HANDLER_ONMAX) { if (strcmp(data->onmax.var_str, data_test->onmax.var_str) != 0) return false; - if (strcmp(data->onmax.fn_name, - data_test->onmax.fn_name) != 0) - return false; } }

@@ -4575,9 +4643,9 @@ static void print_actions_spec(struct se for (i = 0; i < hist_data->n_actions; i++) { struct action_data *data = hist_data->actions[i];

- if (data->fn == action_trace) + if (data->handler == HANDLER_ONMATCH) print_onmatch_spec(m, hist_data, data); - else if (data->fn == onmax_save) + else if (data->handler == HANDLER_ONMAX) print_onmax_spec(m, hist_data, data); } } @@ -4770,14 +4838,15 @@ static inline void add_to_key(char *comp static void hist_trigger_actions(struct hist_trigger_data *hist_data, struct tracing_map_elt *elt, void *rec, - struct ring_buffer_event *rbe, u64 *var_ref_vals) + struct ring_buffer_event *rbe, void *key, + u64 *var_ref_vals) { struct action_data *data; unsigned int i;

for (i = 0; i < hist_data->n_actions; i++) { data = hist_data->actions[i]; - data->fn(hist_data, elt, rec, rbe, data, var_ref_vals); + data->fn(hist_data, elt, rec, rbe, key, data, var_ref_vals); } }

@@ -4838,7 +4907,7 @@ static void event_hist_trigger(struct ev hist_trigger_elt_update(hist_data, elt, rec, rbe, var_ref_vals);

if (resolve_var_refs(hist_data, key, var_ref_vals, true)) - hist_trigger_actions(hist_data, elt, rec, rbe, var_ref_vals); + hist_trigger_actions(hist_data, elt, rec, rbe, key, var_ref_vals); }

static void hist_trigger_stacktrace_print(struct seq_file *m, @@ -5757,7 +5826,7 @@ static int event_hist_trigger_func(struc if (get_named_trigger_data(trigger_data)) goto enable;

- ret = create_actions(hist_data, file); + ret = create_actions(hist_data); if (ret) goto out_unreg;

4.19-stable review patch. If anyone has any objections, please let me know.

------------------

From: Tom Zanussi tom.zanussi@linux.intel.com

commit 466f4528fbc692ea56deca278fa6aeb79e6e8b21 upstream.

The action refactor code allowed actions and handlers to be separated, but the existing onmax handler and save action code is still not flexible enough to handle arbitrary coupling. This change generalizes them and in the process makes additional handlers and actions easier to implement.

The onmax action can be broken up and thought of as two separate components - a variable to be tracked (the parameter given to the onmax($var_to_track) function) and an invisible variable created to save the ongoing result of doing something with that variable, such as saving the max value of that variable so far seen.

Separating it out like this and renaming it appropriately allows us to use the same code for similar tracking functions such as onchange($var_to_track), which would just track the last value seen rather than the max seen so far, which is useful in some situations.

Additionally, because different handlers and actions may want to save and access data differently e.g. save and retrieve tracking values as local variables vs something more global, save_val() and get_val() interface functions are introduced and max-specific implementations are used instead.

The same goes for the code that checks whether a maximum has been hit - a generic check_val() interface and max-checking implementation is used instead, which allows future patches to make use of he same code using their own implemetations of similar functionality.

Link: http://lkml.kernel.org/r/980ea73dd8e3f36db3d646f99652f8fed42b77d4.1550100284...

Signed-off-by: Tom Zanussi tom.zanussi@linux.intel.com Signed-off-by: Steven Rostedt (VMware) rostedt@goodmis.org Signed-off-by: George Guo guodongtai@kylinos.cn Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org --- kernel/trace/trace_events_hist.c | 242 ++++++++++++++++++++++++++------------- 1 file changed, 163 insertions(+), 79 deletions(-)

--- a/kernel/trace/trace_events_hist.c +++ b/kernel/trace/trace_events_hist.c @@ -360,6 +360,8 @@ typedef void (*action_fn_t) (struct hist struct ring_buffer_event *rbe, void *key, struct action_data *data, u64 *var_ref_vals);

+typedef bool (*check_track_val_fn_t) (u64 track_val, u64 var_val); + enum handler_id { HANDLER_ONMATCH = 1, HANDLER_ONMAX, @@ -397,15 +399,35 @@ struct action_data { } match_data;

struct { + /* + * var_str contains the $-unstripped variable + * name referenced by var_ref, and used when + * printing the action. Because var_ref + * creation is deferred to create_actions(), + * we need a per-action way to save it until + * then, thus var_str. + */ char *var_str; - unsigned int max_var_ref_idx; - struct hist_field *max_var; - struct hist_field *var; - } onmax; + + /* + * var_ref refers to the variable being + * tracked e.g onmax($var). + */ + struct hist_field *var_ref; + + /* + * track_var contains the 'invisible' tracking + * variable created to keep the current + * e.g. max value. + */ + struct hist_field *track_var; + + check_track_val_fn_t check_val; + action_fn_t save_data; + } track_data; }; };

- static char last_hist_cmd[MAX_FILTER_STR_VAL]; static char hist_err_str[MAX_FILTER_STR_VAL];

@@ -3311,10 +3333,10 @@ static void update_field_vars(struct his hist_data->n_field_vars, 0); }

-static void update_max_vars(struct hist_trigger_data *hist_data, - struct tracing_map_elt *elt, - struct ring_buffer_event *rbe, - void *rec) +static void save_track_data_vars(struct hist_trigger_data *hist_data, + struct tracing_map_elt *elt, void *rec, + struct ring_buffer_event *rbe, void *key, + struct action_data *data, u64 *var_ref_vals) { __update_field_vars(elt, rbe, rec, hist_data->save_vars, hist_data->n_save_vars, hist_data->n_field_var_str); @@ -3452,14 +3474,67 @@ create_target_field_var(struct hist_trig return create_field_var(target_hist_data, file, var_name); }

-static void onmax_print(struct seq_file *m, - struct hist_trigger_data *hist_data, - struct tracing_map_elt *elt, - struct action_data *data) +static bool check_track_val_max(u64 track_val, u64 var_val) +{ + if (var_val <= track_val) + return false; + + return true; +} + +static u64 get_track_val(struct hist_trigger_data *hist_data, + struct tracing_map_elt *elt, + struct action_data *data) +{ + unsigned int track_var_idx = data->track_data.track_var->var.idx; + u64 track_val; + + track_val = tracing_map_read_var(elt, track_var_idx); + + return track_val; +} + +static void save_track_val(struct hist_trigger_data *hist_data, + struct tracing_map_elt *elt, + struct action_data *data, u64 var_val) +{ + unsigned int track_var_idx = data->track_data.track_var->var.idx; + + tracing_map_set_var(elt, track_var_idx, var_val); +} + +static void save_track_data(struct hist_trigger_data *hist_data, + struct tracing_map_elt *elt, void *rec, + struct ring_buffer_event *rbe, void *key, + struct action_data *data, u64 *var_ref_vals) +{ + if (data->track_data.save_data) + data->track_data.save_data(hist_data, elt, rec, rbe, key, data, var_ref_vals); +} + +static bool check_track_val(struct tracing_map_elt *elt, + struct action_data *data, + u64 var_val) { - unsigned int i, save_var_idx, max_idx = data->onmax.max_var->var.idx; + struct hist_trigger_data *hist_data; + u64 track_val; + + hist_data = data->track_data.track_var->hist_data; + track_val = get_track_val(hist_data, elt, data); + + return data->track_data.check_val(track_val, var_val); +} + +static void track_data_print(struct seq_file *m, + struct hist_trigger_data *hist_data, + struct tracing_map_elt *elt, + struct action_data *data) +{ + u64 track_val = get_track_val(hist_data, elt, data); + unsigned int i, save_var_idx;

- seq_printf(m, "\n\tmax: %10llu", tracing_map_read_var(elt, max_idx)); + if (data->handler == HANDLER_ONMAX) + seq_printf(m, "\n\tmax: %10llu", track_val);

for (i = 0; i < hist_data->n_save_vars; i++) { struct hist_field *save_val = hist_data->save_vars[i]->val; @@ -3478,25 +3553,17 @@ static void onmax_print(struct seq_file } }

-static void onmax_save(struct hist_trigger_data *hist_data, - struct tracing_map_elt *elt, void *rec, - struct ring_buffer_event *rbe, void *key, - struct action_data *data, u64 *var_ref_vals) -{ - unsigned int max_idx = data->onmax.max_var->var.idx; - unsigned int max_var_ref_idx = data->onmax.max_var_ref_idx; - - u64 var_val, max_val; - - var_val = var_ref_vals[max_var_ref_idx]; - max_val = tracing_map_read_var(elt, max_idx); - - if (var_val <= max_val) - return; - - tracing_map_set_var(elt, max_idx, var_val); - - update_max_vars(hist_data, elt, rbe, rec); +static void ontrack_action(struct hist_trigger_data *hist_data, + struct tracing_map_elt *elt, void *rec, + struct ring_buffer_event *rbe, void *key, + struct action_data *data, u64 *var_ref_vals) +{ + u64 var_val = var_ref_vals[data->track_data.var_ref->var_ref_idx]; + + if (check_track_val(elt, data, var_val)) { + save_track_val(hist_data, elt, data, var_val); + save_track_data(hist_data, elt, rec, rbe, key, data, var_ref_vals); + } }

static void action_data_destroy(struct action_data *data) @@ -3516,12 +3583,13 @@ static void action_data_destroy(struct a kfree(data); }

-static void onmax_destroy(struct action_data *data) +static void track_data_destroy(struct hist_trigger_data *hist_data, + struct action_data *data) { - destroy_hist_field(data->onmax.max_var, 0); - destroy_hist_field(data->onmax.var, 0); + destroy_hist_field(data->track_data.track_var, 0); + destroy_hist_field(data->track_data.var_ref, 0);

- kfree(data->onmax.var_str); + kfree(data->track_data.var_str);

action_data_destroy(data); } @@ -3529,25 +3597,24 @@ static void onmax_destroy(struct action_ static int action_create(struct hist_trigger_data *hist_data, struct action_data *data);

-static int onmax_create(struct hist_trigger_data *hist_data, - struct action_data *data) +static int track_data_create(struct hist_trigger_data *hist_data, + struct action_data *data) { - struct hist_field *var_field, *ref_field, *max_var = NULL; + struct hist_field *var_field, *ref_field, *track_var = NULL; struct trace_event_file *file = hist_data->event_file; - unsigned int var_ref_idx = hist_data->n_var_refs; - char *onmax_var_str; + char *track_data_var_str; int ret = 0;

- onmax_var_str = data->onmax.var_str; - if (onmax_var_str[0] != '$') { - hist_err("onmax: For onmax(x), x must be a variable: ", onmax_var_str); + track_data_var_str = data->track_data.var_str; + if (track_data_var_str[0] != '$') { + hist_err("For onmax(x), x must be a variable: ", track_data_var_str); return -EINVAL; } - onmax_var_str++; + track_data_var_str++;

- var_field = find_target_event_var(hist_data, NULL, NULL, onmax_var_str); + var_field = find_target_event_var(hist_data, NULL, NULL, track_data_var_str); if (!var_field) { - hist_err("onmax: Couldn't find onmax variable: ", onmax_var_str); + hist_err("Couldn't find onmax variable: ", track_data_var_str); return -EINVAL; }

@@ -3555,17 +3622,16 @@ static int onmax_create(struct hist_trig if (!ref_field) return -ENOMEM;

- data->onmax.var = ref_field; - - data->onmax.max_var_ref_idx = var_ref_idx; + data->track_data.var_ref = ref_field;

- max_var = create_var(hist_data, file, "max", sizeof(u64), "u64"); - if (IS_ERR(max_var)) { - hist_err("onmax: Couldn't create onmax variable: ", "max"); - ret = PTR_ERR(max_var); + if (data->handler == HANDLER_ONMAX) + track_var = create_var(hist_data, file, "__max", sizeof(u64), "u64"); + if (IS_ERR(track_var)) { + hist_err("Couldn't create onmax variable: ", "__max"); + ret = PTR_ERR(track_var); goto out; } - data->onmax.max_var = max_var; + data->track_data.track_var = track_var;

ret = action_create(hist_data, data); out: @@ -3643,8 +3709,15 @@ static int action_parse(char *str, struc goto out;

if (handler == HANDLER_ONMAX) - data->fn = onmax_save; + data->track_data.check_val = check_track_val_max; + else { + hist_err("action parsing: Handler doesn't support action: ", action_name); + ret = -EINVAL; + goto out; + }

+ data->track_data.save_data = save_track_data_vars; + data->fn = ontrack_action; data->action = ACTION_SAVE; } else { char *params = strsep(&str, ")"); @@ -3655,7 +3728,15 @@ static int action_parse(char *str, struc goto out; }

- data->fn = action_trace; + if (handler == HANDLER_ONMAX) + data->track_data.check_val = check_track_val_max; + + if (handler != HANDLER_ONMATCH) { + data->track_data.save_data = action_trace; + data->fn = ontrack_action; + } else + data->fn = action_trace; + data->action = ACTION_TRACE; }

@@ -3670,24 +3751,25 @@ static int action_parse(char *str, struc return ret; }

-static struct action_data *onmax_parse(char *str, enum handler_id handler) +static struct action_data *track_data_parse(struct hist_trigger_data *hist_data, + char *str, enum handler_id handler) { struct action_data *data; - char *onmax_var_str; int ret = -EINVAL; + char *var_str;

data = kzalloc(sizeof(*data), GFP_KERNEL); if (!data) return ERR_PTR(-ENOMEM);

- onmax_var_str = strsep(&str, ")"); - if (!onmax_var_str || !str) { + var_str = strsep(&str, ")"); + if (!var_str || !str) { ret = -EINVAL; goto free; }

- data->onmax.var_str = kstrdup(onmax_var_str, GFP_KERNEL); - if (!data->onmax.var_str) { + data->track_data.var_str = kstrdup(var_str, GFP_KERNEL); + if (!data->track_data.var_str) { ret = -ENOMEM; goto free; } @@ -3698,7 +3780,7 @@ static struct action_data *onmax_parse(c out: return data; free: - onmax_destroy(data); + track_data_destroy(hist_data, data); data = ERR_PTR(ret); goto out; } @@ -4465,7 +4547,7 @@ static void destroy_actions(struct hist_ if (data->handler == HANDLER_ONMATCH) onmatch_destroy(data); else if (data->handler == HANDLER_ONMAX) - onmax_destroy(data); + track_data_destroy(hist_data, data); else kfree(data); } @@ -4494,7 +4576,8 @@ static int parse_actions(struct hist_tri } else if ((len = str_has_prefix(str, "onmax("))) { char *action_str = str + len;

- data = onmax_parse(action_str, HANDLER_ONMAX); + data = track_data_parse(hist_data, action_str, + HANDLER_ONMAX); if (IS_ERR(data)) { ret = PTR_ERR(data); break; @@ -4524,7 +4607,7 @@ static int create_actions(struct hist_tr if (ret) break; } else if (data->handler == HANDLER_ONMAX) { - ret = onmax_create(hist_data, data); + ret = track_data_create(hist_data, data); if (ret) break; } else { @@ -4546,7 +4629,7 @@ static void print_actions(struct seq_fil struct action_data *data = hist_data->actions[i];

if (data->handler == HANDLER_ONMAX) - onmax_print(m, hist_data, elt, data); + track_data_print(m, hist_data, elt, data); } }

@@ -4571,12 +4654,13 @@ static void print_action_spec(struct seq } }

-static void print_onmax_spec(struct seq_file *m, - struct hist_trigger_data *hist_data, - struct action_data *data) -{ - seq_puts(m, ":onmax("); - seq_printf(m, "%s", data->onmax.var_str); +static void print_track_data_spec(struct seq_file *m, + struct hist_trigger_data *hist_data, + struct action_data *data) +{ + if (data->handler == HANDLER_ONMAX) + seq_puts(m, ":onmax("); + seq_printf(m, "%s", data->track_data.var_str); seq_printf(m, ").%s(", data->action_name);

print_action_spec(m, hist_data, data); @@ -4634,8 +4718,8 @@ static bool actions_match(struct hist_tr data_test->match_data.event) != 0) return false; } else if (data->handler == HANDLER_ONMAX) { - if (strcmp(data->onmax.var_str, - data_test->onmax.var_str) != 0) + if (strcmp(data->track_data.var_str, + data_test->track_data.var_str) != 0) return false; } } @@ -4655,7 +4739,7 @@ static void print_actions_spec(struct se if (data->handler == HANDLER_ONMATCH) print_onmatch_spec(m, hist_data, data); else if (data->handler == HANDLER_ONMAX) - print_onmax_spec(m, hist_data, data); + print_track_data_spec(m, hist_data, data); } }

4.19-stable review patch. If anyone has any objections, please let me know.

------------------

From: Daniel Thompson daniel.thompson@linaro.org

commit b2aba15ad6f908d1a620fd97f6af5620c3639742 upstream.

Currently, when kdb is compiled with keyboard support, then we will use schedule_work() to provoke reset of the keyboard status. Unfortunately schedule_work() gets called from the kgdboc post-debug-exception handler. That risks deadlock since schedule_work() is not NMI-safe and, even on platforms where the NMI is not directly used for debugging, the debug trap can have NMI-like behaviour depending on where breakpoints are placed.

Fix this by using the irq work system, which is NMI-safe, to defer the call to schedule_work() to a point when it is safe to call.

Reported-by: Liuye liu.yeC@h3c.com Closes: https://lore.kernel.org/all/20240228025602.3087748-1-liu.yeC@h3c.com/ Cc: stable@vger.kernel.org Reviewed-by: Douglas Anderson dianders@chromium.org Acked-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Link: https://lore.kernel.org/r/20240424-kgdboc_fix_schedule_work-v2-1-50f5a490aec... Signed-off-by: Daniel Thompson daniel.thompson@linaro.org Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org --- drivers/tty/serial/kgdboc.c | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-)

--- a/drivers/tty/serial/kgdboc.c +++ b/drivers/tty/serial/kgdboc.c @@ -16,6 +16,7 @@ #include <linux/console.h> #include <linux/vt_kern.h> #include <linux/input.h> +#include <linux/irq_work.h> #include <linux/module.h>

#define MAX_CONFIG_LEN 40 @@ -35,6 +36,25 @@ static int kgdboc_use_kms; /* 1 if we u static struct tty_driver *kgdb_tty_driver; static int kgdb_tty_line;

+/* + * When we leave the debug trap handler we need to reset the keyboard status + * (since the original keyboard state gets partially clobbered by kdb use of + * the keyboard). + * + * The path to deliver the reset is somewhat circuitous. + * + * To deliver the reset we register an input handler, reset the keyboard and + * then deregister the input handler. However, to get this done right, we do + * have to carefully manage the calling context because we can only register + * input handlers from task context. + * + * In particular we need to trigger the action from the debug trap handler with + * all its NMI and/or NMI-like oddities. To solve this the kgdboc trap exit code + * (the "post_exception" callback) uses irq_work_queue(), which is NMI-safe, to + * schedule a callback from a hardirq context. From there we have to defer the + * work again, this time using schedule_work(), to get a callback using the + * system workqueue, which runs in task context. + */ #ifdef CONFIG_KDB_KEYBOARD static int kgdboc_reset_connect(struct input_handler *handler, struct input_dev *dev, @@ -86,10 +106,17 @@ static void kgdboc_restore_input_helper(

static DECLARE_WORK(kgdboc_restore_input_work, kgdboc_restore_input_helper);

+static void kgdboc_queue_restore_input_helper(struct irq_work *unused) +{ + schedule_work(&kgdboc_restore_input_work); +} + +static DEFINE_IRQ_WORK(kgdboc_restore_input_irq_work, kgdboc_queue_restore_input_helper); + static void kgdboc_restore_input(void) { if (likely(system_state == SYSTEM_RUNNING)) - schedule_work(&kgdboc_restore_input_work); + irq_work_queue(&kgdboc_restore_input_irq_work); }

static int kgdboc_register_kbd(char **cptr) @@ -120,6 +147,7 @@ static void kgdboc_unregister_kbd(void) i--; } } + irq_work_sync(&kgdboc_restore_input_irq_work); flush_work(&kgdboc_restore_input_work); } #else /* ! CONFIG_KDB_KEYBOARD */