Export the cached pid / tgid mappings to userspace. This allows user
apps to translate the pids from a trace to their respective thread
group.
Example saved_tgids file with pid / tgid values separated by ' ':
# cat saved_tgids
1048 1048
1047 1047
7 7
1049 1047
1054 1047
1053 1047
[ Impact: let userspace apps reading binary buffer know tgid's ]
Signed-off-by: Michael Sartain <[email protected]>
---
kernel/trace/trace.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 59 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 68c214b..ca84c97 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -4692,6 +4692,7 @@ static const struct file_operations tracing_readme_fops = {
static void *saved_cmdlines_next(struct seq_file *m, void *v, loff_t *pos)
{
unsigned int *ptr = v;
+ long tgid_check = (long) m->private;
if (*pos || m->count)
ptr++;
@@ -4702,6 +4703,8 @@ static void *saved_cmdlines_next(struct seq_file *m, void *v, loff_t *pos)
ptr++) {
if (*ptr == -1 || *ptr == NO_CMDLINE_MAP)
continue;
+ if (tgid_check && !trace_find_tgid(*ptr))
+ continue;
return ptr;
}
@@ -4713,6 +4716,10 @@ static void *saved_cmdlines_start(struct seq_file *m, loff_t *pos)
{
void *v;
loff_t l = 0;
+ long tgid_check = (long) m->private;
+
+ if (tgid_check && !tgid_map)
+ return NULL;
preempt_disable();
arch_spin_lock(&trace_cmdline_lock);
@@ -4743,6 +4750,14 @@ static int saved_cmdlines_show(struct seq_file *m, void *v)
return 0;
}
+static int saved_tgids_show(struct seq_file *m, void *v)
+{
+ unsigned int *pid = v;
+
+ seq_printf(m, "%d %d\n", *pid, trace_find_tgid(*pid));
+ return 0;
+}
+
static const struct seq_operations tracing_saved_cmdlines_seq_ops = {
.start = saved_cmdlines_start,
.next = saved_cmdlines_next,
@@ -4750,12 +4765,45 @@ static const struct seq_operations tracing_saved_cmdlines_seq_ops = {
.show = saved_cmdlines_show,
};
+static const struct seq_operations tracing_saved_tgids_seq_ops = {
+ .start = saved_cmdlines_start,
+ .next = saved_cmdlines_next,
+ .stop = saved_cmdlines_stop,
+ .show = saved_tgids_show,
+};
+
static int tracing_saved_cmdlines_open(struct inode *inode, struct file *filp)
{
+ int ret;
+
+ if (tracing_disabled)
+ return -ENODEV;
+
+ ret = seq_open(filp, &tracing_saved_cmdlines_seq_ops);
+ if (!ret) {
+ struct seq_file *m = filp->private_data;
+
+ m->private = (void *) 0; /* Do not check for valid tgids */
+ }
+
+ return ret;
+}
+
+static int tracing_saved_tgids_open(struct inode *inode, struct file *filp)
+{
+ int ret;
+
if (tracing_disabled)
return -ENODEV;
- return seq_open(filp, &tracing_saved_cmdlines_seq_ops);
+ ret = seq_open(filp, &tracing_saved_tgids_seq_ops);
+ if (!ret) {
+ struct seq_file *m = filp->private_data;
+
+ m->private = (void *) 1; /* Check for valid tgids */
+ }
+
+ return ret;
}
static const struct file_operations tracing_saved_cmdlines_fops = {
@@ -4765,6 +4813,13 @@ static const struct file_operations tracing_saved_cmdlines_fops = {
.release = seq_release,
};
+static const struct file_operations tracing_saved_tgids_fops = {
+ .open = tracing_saved_tgids_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = seq_release,
+};
+
static ssize_t
tracing_saved_cmdlines_size_read(struct file *filp, char __user *ubuf,
size_t cnt, loff_t *ppos)
@@ -7933,6 +7988,9 @@ static __init int tracer_init_tracefs(void)
trace_create_file("saved_cmdlines_size", 0644, d_tracer,
NULL, &tracing_saved_cmdlines_size_fops);
+ trace_create_file("saved_tgids", 0444, d_tracer,
+ NULL, &tracing_saved_tgids_fops);
+
trace_eval_init();
trace_create_eval_file(d_tracer);
--
2.11.0
On Fri, 30 Jun 2017 11:17:50 -0600
Michael Sartain <[email protected]> wrote:
> Export the cached pid / tgid mappings to userspace. This allows user
> apps to translate the pids from a trace to their respective thread
> group.
>
> Example saved_tgids file with pid / tgid values separated by ' ':
>
> # cat saved_tgids
> 1048 1048
> 1047 1047
> 7 7
> 1049 1047
> 1054 1047
> 1053 1047
>
> [ Impact: let userspace apps reading binary buffer know tgid's ]
Impact line? Linus put a kibosh on this a while ago...
https://lkml.org/lkml/2009/4/15/296
Although, I will admit that this line is actually useful. Just remove
the brackets.
>
> Signed-off-by: Michael Sartain <[email protected]>
> ---
> kernel/trace/trace.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 59 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 68c214b..ca84c97 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -4692,6 +4692,7 @@ static const struct file_operations tracing_readme_fops = {
> static void *saved_cmdlines_next(struct seq_file *m, void *v, loff_t *pos)
> {
> unsigned int *ptr = v;
> + long tgid_check = (long) m->private;
I really don't like the subtle use of having m->private != NULL mean
this is for tgid listing.
In fact, I don't see the purpose of reusing the seq code. The cmdlines
and tgid map are quite different. Just create its own functions. I
don't see the benefit of trying to reuse this except for making the
code more complex.
-- Steve
>
> if (*pos || m->count)
> ptr++;
> @@ -4702,6 +4703,8 @@ static void *saved_cmdlines_next(struct seq_file *m, void *v, loff_t *pos)
> ptr++) {
> if (*ptr == -1 || *ptr == NO_CMDLINE_MAP)
> continue;
> + if (tgid_check && !trace_find_tgid(*ptr))
> + continue;
>
> return ptr;
> }
> @@ -4713,6 +4716,10 @@ static void *saved_cmdlines_start(struct seq_file *m, loff_t *pos)
> {
> void *v;
> loff_t l = 0;
> + long tgid_check = (long) m->private;
> +
> + if (tgid_check && !tgid_map)
> + return NULL;
>
> preempt_disable();
> arch_spin_lock(&trace_cmdline_lock);
> @@ -4743,6 +4750,14 @@ static int saved_cmdlines_show(struct seq_file *m, void *v)
> return 0;
> }
>
On Fri, Jun 30, 2017 at 2:50 PM, Steven Rostedt <[email protected]> wrote:
> On Fri, 30 Jun 2017 11:17:50 -0600
[..]
>>
>> Signed-off-by: Michael Sartain <[email protected]>
>> ---
>> kernel/trace/trace.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 59 insertions(+), 1 deletion(-)
>>
>> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
>> index 68c214b..ca84c97 100644
>> --- a/kernel/trace/trace.c
>> +++ b/kernel/trace/trace.c
>> @@ -4692,6 +4692,7 @@ static const struct file_operations tracing_readme_fops = {
>> static void *saved_cmdlines_next(struct seq_file *m, void *v, loff_t *pos)
>> {
>> unsigned int *ptr = v;
>> + long tgid_check = (long) m->private;
>
> I really don't like the subtle use of having m->private != NULL mean
> this is for tgid listing.
>
> In fact, I don't see the purpose of reusing the seq code. The cmdlines
> and tgid map are quite different. Just create its own functions. I
> don't see the benefit of trying to reuse this except for making the
> code more complex.
I agree with Steven, this patch should be rewritten to use separate
functions and not reuse the other ones as its otherwise really
confusing and not good design.
Thanks,
Joel
On Fri, Jun 30, 2017 at 05:50:57PM -0400, Steven Rostedt wrote:
> On Fri, 30 Jun 2017 11:17:50 -0600
> Michael Sartain <[email protected]> wrote:
>
> > Export the cached pid / tgid mappings to userspace. This allows user
> > apps to translate the pids from a trace to their respective thread
> > group.
> >
> > Example saved_tgids file with pid / tgid values separated by ' ':
> >
> > # cat saved_tgids
> > 1048 1048
> > 1047 1047
> > 7 7
> > 1049 1047
> > 1054 1047
> > 1053 1047
> >
> > [ Impact: let userspace apps reading binary buffer know tgid's ]
>
> Impact line? Linus put a kibosh on this a while ago...
>
> https://lkml.org/lkml/2009/4/15/296
>
> Although, I will admit that this line is actually useful. Just remove
> the brackets.
Ah, thank you. I saw it used in the previous cmdline patch and used it
as a reference.
> > Signed-off-by: Michael Sartain <[email protected]>
> > ---
> > kernel/trace/trace.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 59 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index 68c214b..ca84c97 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c
> > @@ -4692,6 +4692,7 @@ static const struct file_operations tracing_readme_fops = {
> > static void *saved_cmdlines_next(struct seq_file *m, void *v, loff_t *pos)
> > {
> > unsigned int *ptr = v;
> > + long tgid_check = (long) m->private;
>
> I really don't like the subtle use of having m->private != NULL mean
> this is for tgid listing.
>
> In fact, I don't see the purpose of reusing the seq code. The cmdlines
> and tgid map are quite different. Just create its own functions. I
> don't see the benefit of trying to reuse this except for making the
> code more complex.
Will do. Joel was also kind enough to send me feedback about RFCs and
merge windows, etc. so I apologize - wasn't trying to get anything by
anyone, just that this is my first real patch and I've obviously got a
lot to learn here. Thank you for the comments. I'll fix and resubmit
next week.
-Mike
Hi Michael,
[auto build test ERROR on trace/for-next]
[also build test ERROR on next-20170630]
[cannot apply to tip/perf/core linus/master v4.12-rc7]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Michael-Sartain/tracing-Add-saved_tgids-file-to-show-cached-pid-to-tgid-mappings/20170702-092448
base: https://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git for-next
config: i386-randconfig-x070-07010433 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
kernel//trace/trace.c: In function 'saved_cmdlines_next':
>> kernel//trace/trace.c:4612:22: error: implicit declaration of function 'trace_find_tgid' [-Werror=implicit-function-declaration]
if (tgid_check && !trace_find_tgid(*ptr))
^~~~~~~~~~~~~~~
kernel//trace/trace.c: In function 'saved_cmdlines_start':
>> kernel//trace/trace.c:4627:21: error: 'tgid_map' undeclared (first use in this function)
if (tgid_check && !tgid_map)
^~~~~~~~
kernel//trace/trace.c:4627:21: note: each undeclared identifier is reported only once for each function it appears in
cc1: some warnings being treated as errors
vim +/trace_find_tgid +4612 kernel//trace/trace.c
4606 (*pos)++;
4607
4608 for (; ptr < &savedcmd->map_cmdline_to_pid[savedcmd->cmdline_num];
4609 ptr++) {
4610 if (*ptr == -1 || *ptr == NO_CMDLINE_MAP)
4611 continue;
> 4612 if (tgid_check && !trace_find_tgid(*ptr))
4613 continue;
4614
4615 return ptr;
4616 }
4617
4618 return NULL;
4619 }
4620
4621 static void *saved_cmdlines_start(struct seq_file *m, loff_t *pos)
4622 {
4623 void *v;
4624 loff_t l = 0;
4625 long tgid_check = (long) m->private;
4626
> 4627 if (tgid_check && !tgid_map)
4628 return NULL;
4629
4630 preempt_disable();
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Michael,
[auto build test WARNING on trace/for-next]
[also build test WARNING on next-20170630]
[cannot apply to tip/perf/core linus/master v4.12-rc7]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Michael-Sartain/tracing-Add-saved_tgids-file-to-show-cached-pid-to-tgid-mappings/20170702-092448
base: https://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git for-next
config: i386-randconfig-x072-07010433 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All warnings (new ones prefixed by >>):
In file included from include/uapi/linux/stddef.h:1:0,
from include/linux/stddef.h:4,
from include/uapi/linux/posix_types.h:4,
from include/uapi/linux/types.h:13,
from include/linux/types.h:5,
from include/linux/mm_types_task.h:10,
from include/linux/mm_types.h:4,
from include/linux/kmemcheck.h:4,
from include/linux/ring_buffer.h:4,
from kernel/trace/trace.c:14:
kernel/trace/trace.c: In function 'saved_cmdlines_next':
kernel/trace/trace.c:4612:22: error: implicit declaration of function 'trace_find_tgid' [-Werror=implicit-function-declaration]
if (tgid_check && !trace_find_tgid(*ptr))
^
include/linux/compiler.h:160:30: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
>> kernel/trace/trace.c:4612:3: note: in expansion of macro 'if'
if (tgid_check && !trace_find_tgid(*ptr))
^~
kernel/trace/trace.c: In function 'saved_cmdlines_start':
kernel/trace/trace.c:4627:21: error: 'tgid_map' undeclared (first use in this function)
if (tgid_check && !tgid_map)
^
include/linux/compiler.h:160:30: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
kernel/trace/trace.c:4627:2: note: in expansion of macro 'if'
if (tgid_check && !tgid_map)
^~
kernel/trace/trace.c:4627:21: note: each undeclared identifier is reported only once for each function it appears in
if (tgid_check && !tgid_map)
^
include/linux/compiler.h:160:30: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
kernel/trace/trace.c:4627:2: note: in expansion of macro 'if'
if (tgid_check && !tgid_map)
^~
cc1: some warnings being treated as errors
vim +/if +4612 kernel/trace/trace.c
4596 };
4597
4598 static void *saved_cmdlines_next(struct seq_file *m, void *v, loff_t *pos)
4599 {
4600 unsigned int *ptr = v;
4601 long tgid_check = (long) m->private;
4602
4603 if (*pos || m->count)
4604 ptr++;
4605
4606 (*pos)++;
4607
4608 for (; ptr < &savedcmd->map_cmdline_to_pid[savedcmd->cmdline_num];
4609 ptr++) {
4610 if (*ptr == -1 || *ptr == NO_CMDLINE_MAP)
4611 continue;
> 4612 if (tgid_check && !trace_find_tgid(*ptr))
4613 continue;
4614
4615 return ptr;
4616 }
4617
4618 return NULL;
4619 }
4620
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation