* [PATCH 0/2] tracing: Fix document and blktrace
@ 2018-08-16 9:50 Masami Hiramatsu
2018-08-16 9:50 ` [PATCH 1/2] docs: tracing: Add stacktrace filter command Masami Hiramatsu
2018-08-16 9:51 ` [PATCH 2/2] tracing/blktrace: Fix to allow setting same value Masami Hiramatsu
0 siblings, 2 replies; 9+ messages in thread
From: Masami Hiramatsu @ 2018-08-16 9:50 UTC (permalink / raw)
To: Steven Rostedt; +Cc: Ingo Molnar, Masami Hiramatsu, linux-kernel
Hi,
Here are 2 small fixes for ftrace, one is documentation fix to
add stacktrace action and another is blktrace fix so that
user can set the current enablement parameter without an error.
Thank you,
---
Masami Hiramatsu (2):
docs: tracing: Add stacktrace filter command
tracing/blktrace: Fix to allow setting same value
Documentation/trace/ftrace.rst | 3 +++
kernel/trace/blktrace.c | 11 +++++++----
2 files changed, 10 insertions(+), 4 deletions(-)
--
Masami Hiramatsu (Linaro) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/2] docs: tracing: Add stacktrace filter command 2018-08-16 9:50 [PATCH 0/2] tracing: Fix document and blktrace Masami Hiramatsu @ 2018-08-16 9:50 ` Masami Hiramatsu 2018-08-16 12:50 ` Steven Rostedt 2018-08-16 9:51 ` [PATCH 2/2] tracing/blktrace: Fix to allow setting same value Masami Hiramatsu 1 sibling, 1 reply; 9+ messages in thread From: Masami Hiramatsu @ 2018-08-16 9:50 UTC (permalink / raw) To: Steven Rostedt Cc: Ingo Molnar, Masami Hiramatsu, linux-kernel, Jonathan Corbet, linux-doc Add a description of stacktrace filter command. Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> Cc: Jonathan Corbet <corbet@lwn.net> Cc: linux-doc@vger.kernel.org --- Documentation/trace/ftrace.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/trace/ftrace.rst b/Documentation/trace/ftrace.rst index a20d34955333..81378a8b631e 100644 --- a/Documentation/trace/ftrace.rst +++ b/Documentation/trace/ftrace.rst @@ -2987,6 +2987,9 @@ The following commands are supported: command, it only prints out the contents of the ring buffer for the CPU that executed the function that triggered the dump. +- stacktrace: + When the function is hit, a stack trace is recorded. + trace_pipe ---------- ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] docs: tracing: Add stacktrace filter command 2018-08-16 9:50 ` [PATCH 1/2] docs: tracing: Add stacktrace filter command Masami Hiramatsu @ 2018-08-16 12:50 ` Steven Rostedt 2018-08-31 22:35 ` Jonathan Corbet 0 siblings, 1 reply; 9+ messages in thread From: Steven Rostedt @ 2018-08-16 12:50 UTC (permalink / raw) To: Masami Hiramatsu; +Cc: Ingo Molnar, linux-kernel, Jonathan Corbet, linux-doc On Thu, 16 Aug 2018 18:50:47 +0900 Masami Hiramatsu <mhiramat@kernel.org> wrote: > Add a description of stacktrace filter command. > > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> > Cc: Jonathan Corbet <corbet@lwn.net> > Cc: linux-doc@vger.kernel.org Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org> Jon, want to pull this patch into your tree? -- Steve > --- > Documentation/trace/ftrace.rst | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/Documentation/trace/ftrace.rst b/Documentation/trace/ftrace.rst > index a20d34955333..81378a8b631e 100644 > --- a/Documentation/trace/ftrace.rst > +++ b/Documentation/trace/ftrace.rst > @@ -2987,6 +2987,9 @@ The following commands are supported: > command, it only prints out the contents of the ring buffer for the > CPU that executed the function that triggered the dump. > > +- stacktrace: > + When the function is hit, a stack trace is recorded. > + > trace_pipe > ---------- > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] docs: tracing: Add stacktrace filter command 2018-08-16 12:50 ` Steven Rostedt @ 2018-08-31 22:35 ` Jonathan Corbet 0 siblings, 0 replies; 9+ messages in thread From: Jonathan Corbet @ 2018-08-31 22:35 UTC (permalink / raw) To: Steven Rostedt; +Cc: Masami Hiramatsu, Ingo Molnar, linux-kernel, linux-doc On Thu, 16 Aug 2018 08:50:31 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > On Thu, 16 Aug 2018 18:50:47 +0900 > Masami Hiramatsu <mhiramat@kernel.org> wrote: > > > Add a description of stacktrace filter command. > > > > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> > > Cc: Jonathan Corbet <corbet@lwn.net> > > Cc: linux-doc@vger.kernel.org > > Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org> > > Jon, want to pull this patch into your tree? Applied (finally), thanks. jon ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] tracing/blktrace: Fix to allow setting same value 2018-08-16 9:50 [PATCH 0/2] tracing: Fix document and blktrace Masami Hiramatsu 2018-08-16 9:50 ` [PATCH 1/2] docs: tracing: Add stacktrace filter command Masami Hiramatsu @ 2018-08-16 9:51 ` Masami Hiramatsu 2018-08-16 14:38 ` Steven Rostedt 1 sibling, 1 reply; 9+ messages in thread From: Masami Hiramatsu @ 2018-08-16 9:51 UTC (permalink / raw) To: Steven Rostedt Cc: Ingo Molnar, Masami Hiramatsu, linux-kernel, Jens Axboe, linux-block Current trace-enable attribute in sysfs returns an error if user writes the same setting value as current one, e.g. # cat /sys/block/sda/trace/enable 0 # echo 0 > /sys/block/sda/trace/enable bash: echo: write error: Invalid argument # echo 1 > /sys/block/sda/trace/enable # echo 1 > /sys/block/sda/trace/enable bash: echo: write error: Device or resource busy But this is not a preferred behavior, it should ignore if new setting is same as current one. This fixes the problem as below. # cat /sys/block/sda/trace/enable 0 # echo 0 > /sys/block/sda/trace/enable # echo 1 > /sys/block/sda/trace/enable # echo 1 > /sys/block/sda/trace/enable Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> Cc: Jens Axboe <axboe@kernel.dk> Cc: linux-block@vger.kernel.org --- kernel/trace/blktrace.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c index 987d9a9ae283..e67db1e8a000 100644 --- a/kernel/trace/blktrace.c +++ b/kernel/trace/blktrace.c @@ -1602,11 +1602,11 @@ static int blk_trace_remove_queue(struct request_queue *q) struct blk_trace *bt; bt = xchg(&q->blk_trace, NULL); - if (bt == NULL) - return -EINVAL; + if (bt != NULL) { + put_probe_ref(); + blk_trace_free(bt); + } - put_probe_ref(); - blk_trace_free(bt); return 0; } @@ -1619,6 +1619,9 @@ static int blk_trace_setup_queue(struct request_queue *q, struct blk_trace *bt = NULL; int ret = -ENOMEM; + if (q->blk_trace) + return 0; + bt = kzalloc(sizeof(*bt), GFP_KERNEL); if (!bt) return -ENOMEM; ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] tracing/blktrace: Fix to allow setting same value 2018-08-16 9:51 ` [PATCH 2/2] tracing/blktrace: Fix to allow setting same value Masami Hiramatsu @ 2018-08-16 14:38 ` Steven Rostedt 2018-08-16 16:38 ` Masami Hiramatsu 0 siblings, 1 reply; 9+ messages in thread From: Steven Rostedt @ 2018-08-16 14:38 UTC (permalink / raw) To: Masami Hiramatsu; +Cc: Ingo Molnar, linux-kernel, Jens Axboe, linux-block On Thu, 16 Aug 2018 18:51:15 +0900 Masami Hiramatsu <mhiramat@kernel.org> wrote: > Current trace-enable attribute in sysfs returns an error > if user writes the same setting value as current one, > e.g. > > # cat /sys/block/sda/trace/enable > 0 > # echo 0 > /sys/block/sda/trace/enable > bash: echo: write error: Invalid argument > # echo 1 > /sys/block/sda/trace/enable > # echo 1 > /sys/block/sda/trace/enable > bash: echo: write error: Device or resource busy > > But this is not a preferred behavior, it should ignore > if new setting is same as current one. This fixes the > problem as below. > > # cat /sys/block/sda/trace/enable > 0 > # echo 0 > /sys/block/sda/trace/enable > # echo 1 > /sys/block/sda/trace/enable > # echo 1 > /sys/block/sda/trace/enable > > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> > Cc: Jens Axboe <axboe@kernel.dk> > Cc: linux-block@vger.kernel.org > --- > kernel/trace/blktrace.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c > index 987d9a9ae283..e67db1e8a000 100644 > --- a/kernel/trace/blktrace.c > +++ b/kernel/trace/blktrace.c > @@ -1602,11 +1602,11 @@ static int blk_trace_remove_queue(struct request_queue *q) > struct blk_trace *bt; > > bt = xchg(&q->blk_trace, NULL); > - if (bt == NULL) > - return -EINVAL; > + if (bt != NULL) { > + put_probe_ref(); > + blk_trace_free(bt); > + } > > - put_probe_ref(); > - blk_trace_free(bt); > return 0; > } > > @@ -1619,6 +1619,9 @@ static int blk_trace_setup_queue(struct request_queue *q, > struct blk_trace *bt = NULL; > int ret = -ENOMEM; > > + if (q->blk_trace) > + return 0; > + > bt = kzalloc(sizeof(*bt), GFP_KERNEL); > if (!bt) > return -ENOMEM; Hmm, why not just do this? Does this fix it too? diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c index 987d9a9ae283..10b2b9c5fd25 100644 --- a/kernel/trace/blktrace.c +++ b/kernel/trace/blktrace.c @@ -1841,6 +1841,8 @@ static ssize_t sysfs_blk_trace_attr_store(struct device *dev, mutex_lock(&q->blk_trace_mutex); if (attr == &dev_attr_enable) { + if (!!value == !!q->blk_trace) + goto out_unlock_bdev; if (value) ret = blk_trace_setup_queue(q, bdev); else -- Steve ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] tracing/blktrace: Fix to allow setting same value 2018-08-16 14:38 ` Steven Rostedt @ 2018-08-16 16:38 ` Masami Hiramatsu 2018-08-16 16:49 ` Steven Rostedt 0 siblings, 1 reply; 9+ messages in thread From: Masami Hiramatsu @ 2018-08-16 16:38 UTC (permalink / raw) To: Steven Rostedt; +Cc: Ingo Molnar, linux-kernel, Jens Axboe, linux-block On Thu, 16 Aug 2018 10:38:02 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > On Thu, 16 Aug 2018 18:51:15 +0900 > Masami Hiramatsu <mhiramat@kernel.org> wrote: > > > Current trace-enable attribute in sysfs returns an error > > if user writes the same setting value as current one, > > e.g. > > > > # cat /sys/block/sda/trace/enable > > 0 > > # echo 0 > /sys/block/sda/trace/enable > > bash: echo: write error: Invalid argument > > # echo 1 > /sys/block/sda/trace/enable > > # echo 1 > /sys/block/sda/trace/enable > > bash: echo: write error: Device or resource busy > > > > But this is not a preferred behavior, it should ignore > > if new setting is same as current one. This fixes the > > problem as below. > > > > # cat /sys/block/sda/trace/enable > > 0 > > # echo 0 > /sys/block/sda/trace/enable > > # echo 1 > /sys/block/sda/trace/enable > > # echo 1 > /sys/block/sda/trace/enable > > > > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> > > Cc: Jens Axboe <axboe@kernel.dk> > > Cc: linux-block@vger.kernel.org > > --- > > kernel/trace/blktrace.c | 11 +++++++---- > > 1 file changed, 7 insertions(+), 4 deletions(-) > > > > diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c > > index 987d9a9ae283..e67db1e8a000 100644 > > --- a/kernel/trace/blktrace.c > > +++ b/kernel/trace/blktrace.c > > @@ -1602,11 +1602,11 @@ static int blk_trace_remove_queue(struct request_queue *q) > > struct blk_trace *bt; > > > > bt = xchg(&q->blk_trace, NULL); > > - if (bt == NULL) > > - return -EINVAL; > > + if (bt != NULL) { > > + put_probe_ref(); > > + blk_trace_free(bt); > > + } > > > > - put_probe_ref(); > > - blk_trace_free(bt); > > return 0; > > } > > > > @@ -1619,6 +1619,9 @@ static int blk_trace_setup_queue(struct request_queue *q, > > struct blk_trace *bt = NULL; > > int ret = -ENOMEM; > > > > + if (q->blk_trace) > > + return 0; > > + > > bt = kzalloc(sizeof(*bt), GFP_KERNEL); > > if (!bt) > > return -ENOMEM; > > Hmm, why not just do this? Does this fix it too? Ah, yes, but we need to clear "ret" in that case. Thanks! > > diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c > index 987d9a9ae283..10b2b9c5fd25 100644 > --- a/kernel/trace/blktrace.c > +++ b/kernel/trace/blktrace.c > @@ -1841,6 +1841,8 @@ static ssize_t sysfs_blk_trace_attr_store(struct device *dev, > mutex_lock(&q->blk_trace_mutex); > > if (attr == &dev_attr_enable) { > + if (!!value == !!q->blk_trace) > + goto out_unlock_bdev; > if (value) > ret = blk_trace_setup_queue(q, bdev); > else > > -- Steve -- Masami Hiramatsu <mhiramat@kernel.org> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] tracing/blktrace: Fix to allow setting same value 2018-08-16 16:38 ` Masami Hiramatsu @ 2018-08-16 16:49 ` Steven Rostedt 2018-08-16 17:07 ` Masami Hiramatsu 0 siblings, 1 reply; 9+ messages in thread From: Steven Rostedt @ 2018-08-16 16:49 UTC (permalink / raw) To: Masami Hiramatsu; +Cc: Ingo Molnar, linux-kernel, Jens Axboe, linux-block On Fri, 17 Aug 2018 01:38:47 +0900 Masami Hiramatsu <mhiramat@kernel.org> wrote: > On Thu, 16 Aug 2018 10:38:02 -0400 > Steven Rostedt <rostedt@goodmis.org> wrote: > > > On Thu, 16 Aug 2018 18:51:15 +0900 > > Masami Hiramatsu <mhiramat@kernel.org> wrote: > > > > > Current trace-enable attribute in sysfs returns an error > > > if user writes the same setting value as current one, > > > e.g. > > > > > > # cat /sys/block/sda/trace/enable > > > 0 > > > # echo 0 > /sys/block/sda/trace/enable > > > bash: echo: write error: Invalid argument > > > # echo 1 > /sys/block/sda/trace/enable > > > # echo 1 > /sys/block/sda/trace/enable > > > bash: echo: write error: Device or resource busy > > > > > > But this is not a preferred behavior, it should ignore > > > if new setting is same as current one. This fixes the > > > problem as below. > > > > > > # cat /sys/block/sda/trace/enable > > > 0 > > > # echo 0 > /sys/block/sda/trace/enable > > > # echo 1 > /sys/block/sda/trace/enable > > > # echo 1 > /sys/block/sda/trace/enable > > > > > > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> > > > Cc: Jens Axboe <axboe@kernel.dk> > > > Cc: linux-block@vger.kernel.org > > > --- > > > kernel/trace/blktrace.c | 11 +++++++---- > > > 1 file changed, 7 insertions(+), 4 deletions(-) > > > > > > diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c > > > index 987d9a9ae283..e67db1e8a000 100644 > > > --- a/kernel/trace/blktrace.c > > > +++ b/kernel/trace/blktrace.c > > > @@ -1602,11 +1602,11 @@ static int blk_trace_remove_queue(struct request_queue *q) > > > struct blk_trace *bt; > > > > > > bt = xchg(&q->blk_trace, NULL); > > > - if (bt == NULL) > > > - return -EINVAL; > > > + if (bt != NULL) { > > > + put_probe_ref(); > > > + blk_trace_free(bt); > > > + } > > > > > > - put_probe_ref(); > > > - blk_trace_free(bt); > > > return 0; > > > } > > > > > > @@ -1619,6 +1619,9 @@ static int blk_trace_setup_queue(struct request_queue *q, > > > struct blk_trace *bt = NULL; > > > int ret = -ENOMEM; > > > > > > + if (q->blk_trace) > > > + return 0; > > > + > > > bt = kzalloc(sizeof(*bt), GFP_KERNEL); > > > if (!bt) > > > return -ENOMEM; > > > > Hmm, why not just do this? Does this fix it too? > > Ah, yes, but we need to clear "ret" in that case. Sure. > > Thanks! > > > > > diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c > > index 987d9a9ae283..10b2b9c5fd25 100644 > > --- a/kernel/trace/blktrace.c > > +++ b/kernel/trace/blktrace.c > > @@ -1841,6 +1841,8 @@ static ssize_t sysfs_blk_trace_attr_store(struct device *dev, > > mutex_lock(&q->blk_trace_mutex); > > > > if (attr == &dev_attr_enable) { > > + if (!!value == !!q->blk_trace) { + ret = 0; > > + goto out_unlock_bdev; + } -- Steve > > if (value) > > ret = blk_trace_setup_queue(q, bdev); > > else > > > > -- Steve > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] tracing/blktrace: Fix to allow setting same value 2018-08-16 16:49 ` Steven Rostedt @ 2018-08-16 17:07 ` Masami Hiramatsu 0 siblings, 0 replies; 9+ messages in thread From: Masami Hiramatsu @ 2018-08-16 17:07 UTC (permalink / raw) To: Steven Rostedt; +Cc: Ingo Molnar, linux-kernel, Jens Axboe, linux-block On Thu, 16 Aug 2018 12:49:17 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > > > > > > Hmm, why not just do this? Does this fix it too? > > > > Ah, yes, but we need to clear "ret" in that case. > > Sure. > > > > > Thanks! > > > > > > > > diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c > > > index 987d9a9ae283..10b2b9c5fd25 100644 > > > --- a/kernel/trace/blktrace.c > > > +++ b/kernel/trace/blktrace.c > > > @@ -1841,6 +1841,8 @@ static ssize_t sysfs_blk_trace_attr_store(struct device *dev, > > > mutex_lock(&q->blk_trace_mutex); > > > > > > > if (attr == &dev_attr_enable) { > > > + if (!!value == !!q->blk_trace) { > + ret = 0; > > > + goto out_unlock_bdev; > + } > OK, I tested and ensured that this works. Tested-by: Masami Hiramatsu <mhiramat@kernel.org> Thanks! -- Masami Hiramatsu <mhiramat@kernel.org> ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-08-31 22:35 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-08-16 9:50 [PATCH 0/2] tracing: Fix document and blktrace Masami Hiramatsu 2018-08-16 9:50 ` [PATCH 1/2] docs: tracing: Add stacktrace filter command Masami Hiramatsu 2018-08-16 12:50 ` Steven Rostedt 2018-08-31 22:35 ` Jonathan Corbet 2018-08-16 9:51 ` [PATCH 2/2] tracing/blktrace: Fix to allow setting same value Masami Hiramatsu 2018-08-16 14:38 ` Steven Rostedt 2018-08-16 16:38 ` Masami Hiramatsu 2018-08-16 16:49 ` Steven Rostedt 2018-08-16 17:07 ` Masami Hiramatsu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).