tracing: More fixes for v6.8
Subsystems (3)
Steven Rostedt
maintainercover tracing: More fixes for v6.8
More fixes for 6.8
- Fix a false positive kmemleak on saved cmdlines
Now that the saved_cmdlines structure is allocated via alloc_page() and not via kmalloc() it has become invisible to kmemleak. The allocation done to one of its pointers was flagged as a dangling allocation leak. Make kmemleak aware of this allocation and free.
- Fix synthetic event dynamic strings.
A update that cleaned up the synthetic event code removed the return value of trace_string(), and had it return zero instead of the length, causing dynamic strings in the synthetic event to always have zero size.
- Clean up documentation and header files for seq_buf
git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git
trace/urgent
Head SHA1: 6efe4d18796934b8ada66c1c446510e7f2d9b972
Andy Shevchenko (2):
seq_buf: Don't use "proxy" headers
seq_buf: Fix kernel documentation
Steven Rostedt (Google) (1):
tracing: Inform kmemleak of saved_cmdlines allocation
Thorsten Blum (1):
tracing/synthetic: Fix trace_string() return value
----
include/linux/seq_buf.h | 17 ++++++++------ kernel/trace/trace.c | 3 +++ kernel/trace/trace_events_synth.c | 3 ++- lib/seq_buf.c | 49 ++++++++++++++++++++++++--------------- 4 files changed, 45 insertions(+), 27 deletions(-)
Steven Rostedt
maintainerPATCH 1/4 tracing: Inform kmemleak of saved_cmdlines allocation
From: "Steven Rostedt (Google)" <[email protected]>
The allocation of the struct saved_cmdlines_buffer structure changed from:
s = kmalloc(sizeof(*s), GFP_KERNEL); s->saved_cmdlines = kmalloc_array(TASK_COMM_LEN, val, GFP_KERNEL);
to:
orig_size = sizeof(*s) + val * TASK_COMM_LEN; order = get_order(orig_size); size = 1 << (order + PAGE_SHIFT); page = alloc_pages(GFP_KERNEL, order); if (!page) return NULL; s = page_address(page); memset(s, 0, sizeof(*s)); s->saved_cmdlines = kmalloc_array(TASK_COMM_LEN, val, GFP_KERNEL);
Where that s->saved_cmdlines allocation looks to be a dangling allocation to kmemleak. That's because kmemleak only keeps track of kmalloc() allocations. For allocations that use page_alloc() directly, the kmemleak needs to be explicitly informed about it.
Add kmemleak_alloc() and kmemleak_free() around the page allocation so that it doesn't give the following false positive:
unreferenced object 0xffff8881010c8000 (size 32760):
comm "swapper", pid 0, jiffies 4294667296
hex dump (first 32 bytes):
ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................
backtrace (crc ae6ec1b9):
[<ffffffff86722405>] kmemleak_alloc+0x45/0x80
[<ffffffff8414028d>] __kmalloc_large_node+0x10d/0x190
[<ffffffff84146ab1>] __kmalloc+0x3b1/0x4c0
[<ffffffff83ed7103>] allocate_cmdlines_buffer+0x113/0x230
[<ffffffff88649c34>] tracer_alloc_buffers.isra.0+0x124/0x460
[<ffffffff8864a174>] early_trace_init+0x14/0xa0
[<ffffffff885dd5ae>] start_kernel+0x12e/0x3c0
[<ffffffff885f5758>] x86_64_start_reservations+0x18/0x30
[<ffffffff885f582b>] x86_64_start_kernel+0x7b/0x80
[<ffffffff83a001c3>] secondary_startup_64_no_verify+0x15e/0x16b--- kernel/trace/trace.c | 3 +++ 1 file changed, 3 insertions(+)
kernel/trace/trace.c +3 -0
@@ -39,6 +39,7 @@#include <linux/ctype.h> #include <linux/init.h> #include <linux/panic_notifier.h> +#include <linux/kmemleak.h> #include <linux/poll.h> #include <linux/nmi.h> #include <linux/fs.h>
@@ -2339,6 +2340,7 @@ static void free_saved_cmdlines_buffer(struct saved_cmdlines_buffer *s)int order =get_order (sizeof (*s) + s->cmdline_num *TASK_COMM_LEN );kfree (s->map_cmdline_to_pid); +kmemleak_free (s);free_pages ((unsigned long )s, order); }
@@ -2358,6 +2360,7 @@ static struct saved_cmdlines_buffer *allocate_cmdlines_buffer(unsigned int val)return NULL ; s =page_address (page); +kmemleak_alloc (s, size,1 ,GFP_KERNEL );memset (s,0 ,sizeof (*s));/* Round up to actual allocation */
Signature · 1 line
2.43.0
Steven Rostedt
maintainerPATCH 2/4 tracing/synthetic: Fix trace_string() return value
In reply to Steven RostedtFrom: Thorsten Blum <[email protected]>
Fix trace_string() by assigning the string length to the return variable which got lost in commit ddeea494a16f ("tracing/synthetic: Use union instead of casts") and caused trace_string() to always return 0.
--- kernel/trace/trace_events_synth.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
kernel/trace/trace_events_synth.c +2 -1
@@ -441,8 +441,9 @@ static unsigned int trace_string(struct synth_trace_event *entry,if (is_dynamic) {union trace_synth_field *data = &entry->fields[*n_u64]; + len =fetch_store_strlen ((unsigned long )str_val); data->as_dynamic.offset =struct_size (entry, fields, event->n_u64) + data_size; - data->as_dynamic.len =fetch_store_strlen ((unsigned long )str_val); + data->as_dynamic.len = len; ret =fetch_store_string ((unsigned long )str_val, &entry->fields[*n_u64], entry);
Signature · 1 line
2.43.0
Steven Rostedt
maintainerPATCH 3/4 seq_buf: Dont use "proxy" headers
In reply to Steven RostedtFrom: Andy Shevchenko <[email protected]>
Update header inclusions to follow IWYU (Include What You Use) principle.
--- include/linux/seq_buf.h | 5 ++++- lib/seq_buf.c | 14 ++++++++++++-- 2 files changed, 16 insertions(+), 3 deletions(-)
include/linux/seq_buf.h +4 -1
@@ -2,7 +2,10 @@#ifndef _LINUX_SEQ_BUF_H#define _LINUX_SEQ_BUF_H -#include <linux/fs.h> +#include <linux/bug.h> +#include <linux/minmax.h> +#include <linux/seq_file.h> +#include <linux/types.h> /* * Trace sequences are used to allow a function to call several other functions
lib/seq_buf.c +12 -2
@@ -13,9 +13,19 @@* seq_buf_init() more than once to reset the seq_buf to start * from scratch. */ -#include <linux/uaccess.h> -#include <linux/seq_file.h> + +#include <linux/bug.h> +#include <linux/err.h> +#include <linux/export.h> +#include <linux/hex.h> +#include <linux/minmax.h> +#include <linux/printk.h> #include <linux/seq_buf.h> +#include <linux/seq_file.h> +#include <linux/sprintf.h> +#include <linux/string.h> +#include <linux/types.h> +#include <linux/uaccess.h> /** * seq_buf_can_fit - can the new data fit in the current buffer?
Signature · 1 line
2.43.0
Steven Rostedt
maintainerPATCH 4/4 seq_buf: Fix kernel documentation
In reply to Steven RostedtFrom: Andy Shevchenko <[email protected]>
There are plenty of issues with the kernel documentation here:
- misspelled word "sequence"
- different style of returned value descriptions
- missed Return sections
- unaligned style of ASCII / NUL-terminated / etc
- wrong function references
Fix all these.
--- include/linux/seq_buf.h | 12 ++++++------ lib/seq_buf.c | 35 ++++++++++++++++++----------------- 2 files changed, 24 insertions(+), 23 deletions(-)
include/linux/seq_buf.h +6 -6
@@ -13,7 +13,7 @@*/ /** -* seq_buf - seq buffer structure +* struct seq_buf - seq buffer structure * @buffer: pointer to the buffer * @size: size of the buffer * @len: the amount of data inside the buffer
@@ -80,10 +80,10 @@ static inline unsigned int seq_buf_used(struct seq_buf *s) }/** -* seq_buf_str - get %NUL-terminated C string from seq_buf +* seq_buf_str - get NUL-terminated C string from seq_buf * @s: the seq_buf handle * -* This makes sure that the buffer in @s is nul terminated and +* This makes sure that the buffer in @s is NUL-terminated and * safe to read as a string. * * Note, if this is called when the buffer has overflowed, then
@@ -93,7 +93,7 @@ static inline unsigned int seq_buf_used(struct seq_buf *s)* After this function is called, s->buffer is safe to use * in string operations. * -* Returns @s->buf after making sure it is terminated. +* Returns: @s->buf after making sure it is terminated. */ static inline const char *seq_buf_str (struct seq_buf *s) {
@@ -113,7 +113,7 @@ static inline const char *seq_buf_str(struct seq_buf *s)* @s: the seq_buf handle * @bufp: the beginning of the buffer is stored here * -* Return the number of bytes available in the buffer, or zero if +* Returns: the number of bytes available in the buffer, or zero if * there's no space. */ static inline size_t seq_buf_get_buf (struct seq_buf *s,char **bufp)
@@ -135,7 +135,7 @@ static inline size_t seq_buf_get_buf(struct seq_buf *s, char **bufp)* @num: the number of bytes to commit * * Commit @num bytes of data written to a buffer previously acquired -* by seq_buf_get. To signal an error condition, or that the data +* by seq_buf_get_buf(). To signal an error condition, or that the data * didn't fit in the available space, pass a negative @num value. */ static inline void seq_buf_commit (struct seq_buf *s,int num)
lib/seq_buf.c +18 -17
@@ -32,7 +32,7 @@* @s: the seq_buf descriptor * @len: The length to see if it can fit in the current buffer * -* Returns true if there's enough unused space in the seq_buf buffer +* Returns: true if there's enough unused space in the seq_buf buffer * to fit the amount of new data according to @len. */ static bool seq_buf_can_fit (struct seq_buf *s,size_t len)
@@ -45,7 +45,7 @@ static bool seq_buf_can_fit(struct seq_buf *s, size_t len)* @m: the seq_file descriptor that is the destination * @s: the seq_buf descriptor that is the source. * -* Returns zero on success, non zero otherwise +* Returns: zero on success, non-zero otherwise. */ int seq_buf_print_seq (struct seq_file *m,struct seq_buf *s) {
@@ -60,9 +60,9 @@ int seq_buf_print_seq(struct seq_file *m, struct seq_buf *s)* @fmt: printf format string * @args: va_list of arguments from a printf() type function * -* Writes a vnprintf() format into the sequencce buffer. +* Writes a vnprintf() format into the sequence buffer. * -* Returns zero on success, -1 on overflow. +* Returns: zero on success, -1 on overflow. */ int seq_buf_vprintf (struct seq_buf *s,const char *fmt,va_list args) {
@@ -88,7 +88,7 @@ int seq_buf_vprintf(struct seq_buf *s, const char *fmt, va_list args)* * Writes a printf() format into the sequence buffer. * -* Returns zero on success, -1 on overflow. +* Returns: zero on success, -1 on overflow. */ int seq_buf_printf (struct seq_buf *s,const char *fmt, ...) {
@@ -104,12 +104,12 @@ int seq_buf_printf(struct seq_buf *s, const char *fmt, ...)EXPORT_SYMBOL_GPL (seq_buf_printf);/** -* seq_buf_do_printk - printk seq_buf line by line +* seq_buf_do_printk - printk() seq_buf line by line * @s: seq_buf descriptor * @lvl: printk level * * printk()-s a multi-line sequential buffer line by line. The function -* makes sure that the buffer in @s is nul terminated and safe to read +* makes sure that the buffer in @s is NUL-terminated and safe to read * as a string. */ void seq_buf_do_printk (struct seq_buf *s,const char *lvl)
@@ -149,7 +149,7 @@ EXPORT_SYMBOL_GPL(seq_buf_do_printk);* This function will take the format and the binary array and finish * the conversion into the ASCII string within the buffer. * -* Returns zero on success, -1 on overflow. +* Returns: zero on success, -1 on overflow. */ int seq_buf_bprintf (struct seq_buf *s,const char *fmt,const u32 *binary) {
@@ -177,7 +177,7 @@ int seq_buf_bprintf(struct seq_buf *s, const char *fmt, const u32 *binary)* * Copy a simple string into the sequence buffer. * -* Returns zero on success, -1 on overflow +* Returns: zero on success, -1 on overflow. */ int seq_buf_puts (struct seq_buf *s,const char *str) {
@@ -206,7 +206,7 @@ EXPORT_SYMBOL_GPL(seq_buf_puts);* * Copy a single character into the sequence buffer. * -* Returns zero on success, -1 on overflow +* Returns: zero on success, -1 on overflow. */ int seq_buf_putc (struct seq_buf *s,unsigned char c) {
@@ -222,7 +222,7 @@ int seq_buf_putc(struct seq_buf *s, unsigned char c)EXPORT_SYMBOL_GPL (seq_buf_putc);/** -* seq_buf_putmem - write raw data into the sequenc buffer +* seq_buf_putmem - write raw data into the sequence buffer * @s: seq_buf descriptor * @mem: The raw memory to copy into the buffer * @len: The length of the raw memory to copy (in bytes)
@@ -231,7 +231,7 @@ EXPORT_SYMBOL_GPL(seq_buf_putc);* buffer and a strcpy() would not work. Using this function allows * for such cases. * -* Returns zero on success, -1 on overflow +* Returns: zero on success, -1 on overflow. */ int seq_buf_putmem (struct seq_buf *s,const void *mem,unsigned int len) {
@@ -259,7 +259,7 @@ int seq_buf_putmem(struct seq_buf *s, const void *mem, unsigned int len)* raw memory into the buffer it writes its ASCII representation of it * in hex characters. * -* Returns zero on success, -1 on overflow +* Returns: zero on success, -1 on overflow. */ int seq_buf_putmem_hex (struct seq_buf *s,const void *mem,unsigned int len)
@@ -307,7 +307,7 @@ int seq_buf_putmem_hex(struct seq_buf *s, const void *mem,* * Write a path name into the sequence buffer. * -* Returns the number of written bytes on success, -1 on overflow +* Returns: the number of written bytes on success, -1 on overflow. */ int seq_buf_path (struct seq_buf *s,const struct path *path,const char *esc) {
@@ -342,6 +342,7 @@ int seq_buf_path(struct seq_buf *s, const struct path *path, const char *esc)* or until it reaches the end of the content in the buffer (@s->len), * whichever comes first. * +* Returns: * On success, it returns a positive number of the number of bytes * it copied. *
@@ -392,11 +393,11 @@ int seq_buf_to_user(struct seq_buf *s, char __user *ubuf, size_t start, int cnt)* linebuf size is maximal length for one line. * 32 * 3 - maximum bytes per line, each printed into 2 chars + 1 for * separating space -* 2 - spaces separating hex dump and ascii representation -* 32 - ascii representation +* 2 - spaces separating hex dump and ASCII representation +* 32 - ASCII representation * 1 - terminating '\0' * -* Returns zero on success, -1 on overflow +* Returns: zero on success, -1 on overflow. */ int seq_buf_hex_dump(struct seq_buf *s, const char *prefix_str, int prefix_type, int rowsize, int groupsize,
Signature · 1 line
2.43.0