* [PATCH 1/2] tracing: use the more proper parameter. @ 2009-02-13 1:39 Wenji Huang 2009-02-13 1:42 ` [PATCH 2/2] tracing: add checks for printing pci devices in mmiotrace Wenji Huang 0 siblings, 1 reply; 5+ messages in thread From: Wenji Huang @ 2009-02-13 1:39 UTC (permalink / raw) To: linux-kernel; +Cc: rostedt, fweisbec, mingo, Wenji Huang Impact: clean up. Rename tsk to next as Ingo suggested and pass it to tracing_record_cmdline. Signed-off-by: Wenji Huang <wenji.huang@oracle.com> --- kernel/trace/trace.c | 18 +++++++++--------- 1 files changed, 9 insertions(+), 9 deletions(-) diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c index 95f99a7..648df63 100644 --- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c @@ -318,7 +318,7 @@ static raw_spinlock_t ftrace_max_lock = * for later retrieval via /debugfs/tracing/latency_trace) */ static void -__update_max_tr(struct trace_array *tr, struct task_struct *tsk, int cpu) +__update_max_tr(struct trace_array *tr, struct task_struct *next, int cpu) { struct trace_array_cpu *data = tr->data[cpu]; @@ -328,15 +328,15 @@ __update_max_tr(struct trace_array *tr, struct task_struct *tsk, int cpu) data = max_tr.data[cpu]; data->saved_latency = tracing_max_latency; - memcpy(data->comm, tsk->comm, TASK_COMM_LEN); - data->pid = tsk->pid; - data->uid = task_uid(tsk); - data->nice = tsk->static_prio - 20 - MAX_RT_PRIO; - data->policy = tsk->policy; - data->rt_priority = tsk->rt_priority; + memcpy(data->comm, next->comm, TASK_COMM_LEN); + data->pid = next->pid; + data->uid = task_uid(next); + data->nice = next->static_prio - 20 - MAX_RT_PRIO; + data->policy = next->policy; + data->rt_priority = next->rt_priority; - /* record this tasks comm */ - tracing_record_cmdline(current); + /* record this task comm */ + tracing_record_cmdline(next); } static void -- 1.5.6 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] tracing: add checks for printing pci devices in mmiotrace. 2009-02-13 1:39 [PATCH 1/2] tracing: use the more proper parameter Wenji Huang @ 2009-02-13 1:42 ` Wenji Huang 2009-02-13 6:04 ` Steven Rostedt 0 siblings, 1 reply; 5+ messages in thread From: Wenji Huang @ 2009-02-13 1:42 UTC (permalink / raw) To: linux-kernel; +Cc: rostedt, pq, mingo, Wenji Huang This patch is to provide checking return value of trace_seq_printf so as to keep completeness of mmio_print_pcidev. Signed-off-by: Wenji Huang <wenji.huang@oracle.com> --- kernel/trace/trace_mmiotrace.c | 26 ++++++++++++++++---------- 1 files changed, 16 insertions(+), 10 deletions(-) diff --git a/kernel/trace/trace_mmiotrace.c b/kernel/trace/trace_mmiotrace.c index c401b90..ebb0b61 100644 --- a/kernel/trace/trace_mmiotrace.c +++ b/kernel/trace/trace_mmiotrace.c @@ -56,17 +56,19 @@ static void mmio_trace_start(struct trace_array *tr) mmio_reset_data(tr); } -static int mmio_print_pcidev(struct trace_seq *s, const struct pci_dev *dev) +static enum print_line_t mmio_print_pcidev(struct trace_seq *s, + const struct pci_dev *dev) { int ret = 0; int i; resource_size_t start, end; const struct pci_driver *drv = pci_dev_driver(dev); - /* XXX: incomplete checks for trace_seq_printf() return value */ - ret += trace_seq_printf(s, "PCIDEV %02x%02x %04x%04x %x", + ret = trace_seq_printf(s, "PCIDEV %02x%02x %04x%04x %x", dev->bus->number, dev->devfn, dev->vendor, dev->device, dev->irq); + if (!ret) + return TRACE_TYPE_PARTIAL_LINE; /* * XXX: is pci_resource_to_user() appropriate, since we are * supposed to interpret the __ioremap() phys_addr argument based on @@ -74,21 +76,25 @@ static int mmio_print_pcidev(struct trace_seq *s, const struct pci_dev *dev) */ for (i = 0; i < 7; i++) { pci_resource_to_user(dev, i, &dev->resource[i], &start, &end); - ret += trace_seq_printf(s, " %llx", + ret = trace_seq_printf(s, " %llx", (unsigned long long)(start | (dev->resource[i].flags & PCI_REGION_FLAG_MASK))); - } - for (i = 0; i < 7; i++) { + if (!ret) + return TRACE_TYPE_PARTIAL_LINE; pci_resource_to_user(dev, i, &dev->resource[i], &start, &end); - ret += trace_seq_printf(s, " %llx", + ret = trace_seq_printf(s, " %llx", dev->resource[i].start < dev->resource[i].end ? (unsigned long long)(end - start) + 1 : 0); + if (!ret) + return TRACE_TYPE_PARTIAL_LINE; } if (drv) - ret += trace_seq_printf(s, " %s\n", drv->name); + ret = trace_seq_printf(s, " %s\n", drv->name); else - ret += trace_seq_printf(s, " \n"); - return ret; + ret = trace_seq_printf(s, " \n"); + if (!ret) + return TRACE_TYPE_PARTIAL_LINE; + return TRACE_TYPE_HANDLED; } static void destroy_header_iter(struct header_iter *hiter) -- 1.5.6 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] tracing: add checks for printing pci devices in mmiotrace. 2009-02-13 1:42 ` [PATCH 2/2] tracing: add checks for printing pci devices in mmiotrace Wenji Huang @ 2009-02-13 6:04 ` Steven Rostedt 2009-02-13 6:09 ` Wenji Huang 0 siblings, 1 reply; 5+ messages in thread From: Steven Rostedt @ 2009-02-13 6:04 UTC (permalink / raw) To: Wenji Huang; +Cc: linux-kernel, pq, mingo On Thu, 12 Feb 2009, Wenji Huang wrote: > This patch is to provide checking return value of trace_seq_printf > so as to keep completeness of mmio_print_pcidev. > > Signed-off-by: Wenji Huang <wenji.huang@oracle.com> > --- > kernel/trace/trace_mmiotrace.c | 26 ++++++++++++++++---------- > 1 files changed, 16 insertions(+), 10 deletions(-) > > diff --git a/kernel/trace/trace_mmiotrace.c b/kernel/trace/trace_mmiotrace.c > index c401b90..ebb0b61 100644 > --- a/kernel/trace/trace_mmiotrace.c > +++ b/kernel/trace/trace_mmiotrace.c > @@ -56,17 +56,19 @@ static void mmio_trace_start(struct trace_array *tr) > mmio_reset_data(tr); > } > > -static int mmio_print_pcidev(struct trace_seq *s, const struct pci_dev *dev) > +static enum print_line_t mmio_print_pcidev(struct trace_seq *s, > + const struct pci_dev *dev) > { > int ret = 0; > int i; > resource_size_t start, end; > const struct pci_driver *drv = pci_dev_driver(dev); > > - /* XXX: incomplete checks for trace_seq_printf() return value */ > - ret += trace_seq_printf(s, "PCIDEV %02x%02x %04x%04x %x", > + ret = trace_seq_printf(s, "PCIDEV %02x%02x %04x%04x %x", > dev->bus->number, dev->devfn, > dev->vendor, dev->device, dev->irq); > + if (!ret) > + return TRACE_TYPE_PARTIAL_LINE; > /* > * XXX: is pci_resource_to_user() appropriate, since we are > * supposed to interpret the __ioremap() phys_addr argument based on > @@ -74,21 +76,25 @@ static int mmio_print_pcidev(struct trace_seq *s, const struct pci_dev *dev) > */ > for (i = 0; i < 7; i++) { > pci_resource_to_user(dev, i, &dev->resource[i], &start, &end); > - ret += trace_seq_printf(s, " %llx", > + ret = trace_seq_printf(s, " %llx", > (unsigned long long)(start | > (dev->resource[i].flags & PCI_REGION_FLAG_MASK))); > - } > - for (i = 0; i < 7; i++) { > + if (!ret) > + return TRACE_TYPE_PARTIAL_LINE; The above looks like a change of logic to me. I'm a bit tired so I can be wrong. But it looks like the original was: for (i = 0; i < 7; i++) { pci_resource_to_user(dev, i, &dev->resource[i], &start, &end); ret += trace_seq_printf(s, " %llx", (unsigned long long)(start | (dev->resource[i].flags & PCI_REGION_FLAG_MASK))); } for (i = 0; i < 7; i++) { pci_resource_to_user(dev, i, &dev->resource[i], &start, &end); ret += trace_seq_printf(s, " %llx", dev->resource[i].start < dev->resource[i].end ? (unsigned long long)(end - start) + 1 : 0); } And the new is: for (i = 0; i < 7; i++) { pci_resource_to_user(dev, i, &dev->resource[i], &start, &end); ret = trace_seq_printf(s, " %llx", (unsigned long long)(start | (dev->resource[i].flags & PCI_REGION_FLAG_MASK))); if (!ret) return TRACE_TYPE_PARTIAL_LINE; pci_resource_to_user(dev, i, &dev->resource[i], &start, &end); ret = trace_seq_printf(s, " %llx", dev->resource[i].start < dev->resource[i].end ? (unsigned long long)(end - start) + 1 : 0); if (!ret) return TRACE_TYPE_PARTIAL_LINE; } This changes the output format. Pekka, Is this OK? -- Steve > pci_resource_to_user(dev, i, &dev->resource[i], &start, &end); > - ret += trace_seq_printf(s, " %llx", > + ret = trace_seq_printf(s, " %llx", > dev->resource[i].start < dev->resource[i].end ? > (unsigned long long)(end - start) + 1 : 0); > + if (!ret) > + return TRACE_TYPE_PARTIAL_LINE; > } > if (drv) > - ret += trace_seq_printf(s, " %s\n", drv->name); > + ret = trace_seq_printf(s, " %s\n", drv->name); > else > - ret += trace_seq_printf(s, " \n"); > - return ret; > + ret = trace_seq_printf(s, " \n"); > + if (!ret) > + return TRACE_TYPE_PARTIAL_LINE; > + return TRACE_TYPE_HANDLED; > } > > static void destroy_header_iter(struct header_iter *hiter) > -- > 1.5.6 > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] tracing: add checks for printing pci devices in mmiotrace. 2009-02-13 6:04 ` Steven Rostedt @ 2009-02-13 6:09 ` Wenji Huang 2009-02-13 18:19 ` Pekka Paalanen 0 siblings, 1 reply; 5+ messages in thread From: Wenji Huang @ 2009-02-13 6:09 UTC (permalink / raw) To: Steven Rostedt; +Cc: linux-kernel, pq, mingo Steven Rostedt wrote: > On Thu, 12 Feb 2009, Wenji Huang wrote: > >> This patch is to provide checking return value of trace_seq_printf >> so as to keep completeness of mmio_print_pcidev. >> >> Signed-off-by: Wenji Huang <wenji.huang@oracle.com> >> --- >> kernel/trace/trace_mmiotrace.c | 26 ++++++++++++++++---------- >> 1 files changed, 16 insertions(+), 10 deletions(-) >> >> diff --git a/kernel/trace/trace_mmiotrace.c b/kernel/trace/trace_mmiotrace.c >> index c401b90..ebb0b61 100644 >> --- a/kernel/trace/trace_mmiotrace.c >> +++ b/kernel/trace/trace_mmiotrace.c >> @@ -56,17 +56,19 @@ static void mmio_trace_start(struct trace_array *tr) >> mmio_reset_data(tr); >> } >> >> -static int mmio_print_pcidev(struct trace_seq *s, const struct pci_dev *dev) >> +static enum print_line_t mmio_print_pcidev(struct trace_seq *s, >> + const struct pci_dev *dev) >> { >> int ret = 0; >> int i; >> resource_size_t start, end; >> const struct pci_driver *drv = pci_dev_driver(dev); >> >> - /* XXX: incomplete checks for trace_seq_printf() return value */ >> - ret += trace_seq_printf(s, "PCIDEV %02x%02x %04x%04x %x", >> + ret = trace_seq_printf(s, "PCIDEV %02x%02x %04x%04x %x", >> dev->bus->number, dev->devfn, >> dev->vendor, dev->device, dev->irq); >> + if (!ret) >> + return TRACE_TYPE_PARTIAL_LINE; >> /* >> * XXX: is pci_resource_to_user() appropriate, since we are >> * supposed to interpret the __ioremap() phys_addr argument based on >> @@ -74,21 +76,25 @@ static int mmio_print_pcidev(struct trace_seq *s, const struct pci_dev *dev) >> */ >> for (i = 0; i < 7; i++) { >> pci_resource_to_user(dev, i, &dev->resource[i], &start, &end); >> - ret += trace_seq_printf(s, " %llx", >> + ret = trace_seq_printf(s, " %llx", >> (unsigned long long)(start | >> (dev->resource[i].flags & PCI_REGION_FLAG_MASK))); >> - } >> - for (i = 0; i < 7; i++) { >> + if (!ret) >> + return TRACE_TYPE_PARTIAL_LINE; > > The above looks like a change of logic to me. I'm a bit tired so I can be > wrong. But it looks like the original was: > > for (i = 0; i < 7; i++) { > pci_resource_to_user(dev, i, &dev->resource[i], &start, &end); > ret += trace_seq_printf(s, " %llx", > (unsigned long long)(start | > (dev->resource[i].flags & PCI_REGION_FLAG_MASK))); > } > for (i = 0; i < 7; i++) { > pci_resource_to_user(dev, i, &dev->resource[i], &start, &end); > ret += trace_seq_printf(s, " %llx", > dev->resource[i].start < dev->resource[i].end ? > (unsigned long long)(end - start) + 1 : 0); > } > > And the new is: > > for (i = 0; i < 7; i++) { > pci_resource_to_user(dev, i, &dev->resource[i], &start, &end); > ret = trace_seq_printf(s, " %llx", > (unsigned long long)(start | > (dev->resource[i].flags & PCI_REGION_FLAG_MASK))); > if (!ret) > return TRACE_TYPE_PARTIAL_LINE; > pci_resource_to_user(dev, i, &dev->resource[i], &start, &end); > ret = trace_seq_printf(s, " %llx", > dev->resource[i].start < dev->resource[i].end ? > (unsigned long long)(end - start) + 1 : 0); > if (!ret) > return TRACE_TYPE_PARTIAL_LINE; > } > > This changes the output format. > > Pekka, > > Is this OK? > > -- Steve oops, output format is changed. Need to update it. Regards, wenji ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] tracing: add checks for printing pci devices in mmiotrace. 2009-02-13 6:09 ` Wenji Huang @ 2009-02-13 18:19 ` Pekka Paalanen 0 siblings, 0 replies; 5+ messages in thread From: Pekka Paalanen @ 2009-02-13 18:19 UTC (permalink / raw) To: wenji.huang; +Cc: Steven Rostedt, linux-kernel, mingo On Fri, 13 Feb 2009 14:09:52 +0800 Wenji Huang <wenji.huang@oracle.com> wrote: > Steven Rostedt wrote: > > On Thu, 12 Feb 2009, Wenji Huang wrote: > > > >> This patch is to provide checking return value of trace_seq_printf > >> so as to keep completeness of mmio_print_pcidev. > >> > >> Signed-off-by: Wenji Huang <wenji.huang@oracle.com> > >> --- > >> kernel/trace/trace_mmiotrace.c | 26 ++++++++++++++++---------- > >> 1 files changed, 16 insertions(+), 10 deletions(-) > >> > >> diff --git a/kernel/trace/trace_mmiotrace.c b/kernel/trace/trace_mmiotrace.c > >> index c401b90..ebb0b61 100644 > >> --- a/kernel/trace/trace_mmiotrace.c > >> +++ b/kernel/trace/trace_mmiotrace.c > >> @@ -56,17 +56,19 @@ static void mmio_trace_start(struct trace_array *tr) > >> mmio_reset_data(tr); > >> } > >> > >> -static int mmio_print_pcidev(struct trace_seq *s, const struct pci_dev *dev) > >> +static enum print_line_t mmio_print_pcidev(struct trace_seq *s, > >> + const struct pci_dev *dev) > >> { > >> int ret = 0; > >> int i; > >> resource_size_t start, end; > >> const struct pci_driver *drv = pci_dev_driver(dev); > >> > >> - /* XXX: incomplete checks for trace_seq_printf() return value */ > >> - ret += trace_seq_printf(s, "PCIDEV %02x%02x %04x%04x %x", > >> + ret = trace_seq_printf(s, "PCIDEV %02x%02x %04x%04x %x", > >> dev->bus->number, dev->devfn, > >> dev->vendor, dev->device, dev->irq); > >> + if (!ret) > >> + return TRACE_TYPE_PARTIAL_LINE; > >> /* > >> * XXX: is pci_resource_to_user() appropriate, since we are > >> * supposed to interpret the __ioremap() phys_addr argument based on > >> @@ -74,21 +76,25 @@ static int mmio_print_pcidev(struct trace_seq *s, const struct pci_dev *dev) > >> */ > >> for (i = 0; i < 7; i++) { > >> pci_resource_to_user(dev, i, &dev->resource[i], &start, &end); > >> - ret += trace_seq_printf(s, " %llx", > >> + ret = trace_seq_printf(s, " %llx", > >> (unsigned long long)(start | > >> (dev->resource[i].flags & PCI_REGION_FLAG_MASK))); > >> - } > >> - for (i = 0; i < 7; i++) { > >> + if (!ret) > >> + return TRACE_TYPE_PARTIAL_LINE; > > > > The above looks like a change of logic to me. I'm a bit tired so I can be > > wrong. But it looks like the original was: > > > > for (i = 0; i < 7; i++) { > > pci_resource_to_user(dev, i, &dev->resource[i], &start, &end); > > ret += trace_seq_printf(s, " %llx", > > (unsigned long long)(start | > > (dev->resource[i].flags & PCI_REGION_FLAG_MASK))); > > } > > for (i = 0; i < 7; i++) { > > pci_resource_to_user(dev, i, &dev->resource[i], &start, &end); > > ret += trace_seq_printf(s, " %llx", > > dev->resource[i].start < dev->resource[i].end ? > > (unsigned long long)(end - start) + 1 : 0); > > } > > > > And the new is: > > > > for (i = 0; i < 7; i++) { > > pci_resource_to_user(dev, i, &dev->resource[i], &start, &end); > > ret = trace_seq_printf(s, " %llx", > > (unsigned long long)(start | > > (dev->resource[i].flags & PCI_REGION_FLAG_MASK))); > > if (!ret) > > return TRACE_TYPE_PARTIAL_LINE; > > pci_resource_to_user(dev, i, &dev->resource[i], &start, &end); > > ret = trace_seq_printf(s, " %llx", > > dev->resource[i].start < dev->resource[i].end ? > > (unsigned long long)(end - start) + 1 : 0); > > if (!ret) > > return TRACE_TYPE_PARTIAL_LINE; > > } > > > > This changes the output format. > > > > Pekka, > > > > Is this OK? > > > > -- Steve > > oops, output format is changed. Need to update it. Indeed, don't change the output format, please. These checks are not enough to actually handle the case, mmio_read() needs to be updated, too, to do the right thing (I'm looking at Ingo's tip/master). I.e. it should skip to print_out to flush out old data. Shouldn't also *ppos be updated in mmio_read()? Thanks for looking into this. -- Pekka Paalanen http://www.iki.fi/pq/ ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2009-02-13 18:19 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-02-13 1:39 [PATCH 1/2] tracing: use the more proper parameter Wenji Huang 2009-02-13 1:42 ` [PATCH 2/2] tracing: add checks for printing pci devices in mmiotrace Wenji Huang 2009-02-13 6:04 ` Steven Rostedt 2009-02-13 6:09 ` Wenji Huang 2009-02-13 18:19 ` Pekka Paalanen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox