Messages in this thread | | | Date | Fri, 13 Feb 2009 14:09:52 +0800 | From | Wenji Huang <> | Subject | Re: [PATCH 2/2] tracing: add checks for printing pci devices in mmiotrace. |
| |
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
| |