lkml.org 
[lkml]   [2012]   [Jun]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH 1/9] PCI: Remove redundant debug output in pci_do_fixups
    On Thu, Jun 21, 2012 at 2:24 PM, Myron Stowe <myron.stowe@redhat.com> wrote:
    > When the boot argument 'initcall_debug' is specified, redundant debug
    > output occurs for each device as a quirk is applied:
    >  ...
    >  pci 0000:00:1a.0: calling quirk_usb_early_handoff+0x0/0x620
    >  calling  quirk_usb_early_handoff+0x0/0x620 @ 1 for 0000:00:1a.0
    >  pci fixup quirk_usb_early_handoff+0x0/0x620 returned after 32 usecs for 0000:00: 1a.0
    >  ...
    >
    > This patch removes the redundancy by eliminating the first debug output
    > occurence in the sequence shown above when 'initcall_debug' is specified.

    Here's what I don't like about this: adding "initcall_debug" *removes*
    some output. My expectation is that it would only *add* output.

    > Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
    > ---
    >
    >  drivers/pci/quirks.c |    5 +++--
    >  1 files changed, 3 insertions(+), 2 deletions(-)
    >
    > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
    > index a2d9d33..9c93558 100644
    > --- a/drivers/pci/quirks.c
    > +++ b/drivers/pci/quirks.c
    > @@ -2953,11 +2953,12 @@ static void pci_do_fixups(struct pci_dev *dev, struct pci_fixup *f,
    >                     f->vendor == (u16) PCI_ANY_ID) &&
    >                    (f->device == dev->device ||
    >                     f->device == (u16) PCI_ANY_ID)) {
    > -                       dev_dbg(&dev->dev, "calling %pF\n", f->hook);
    >                        if (initcall_debug)
    >                                do_one_fixup_debug(f->hook, dev);
    > -                       else
    > +                       else {
    > +                               dev_dbg(&dev->dev, "calling %pF\n", f->hook);
    >                                f->hook(dev);

    This part isn't something you changed, but I also think it's a bit
    ugly that we have two possible call sites for the quirk: either inside
    do_one_fixup_debug() or directly in pci_do_fixups(). I wonder if this
    could be restructured a bit in the style of initcall_debug_start() and
    initcall_debug_report(), so we could have this:

    ktime_t calltime;

    calltime = initcall_debug_start(dev);
    f->hook(dev);
    initcall_debug_report(dev, calltime);

    where initcall_debug_report() would only print something when
    initcall_debug is enabled.

    > +                       }
    >                }
    >  }
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2012-06-27 01:01    [W:0.053 / U:31.792 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site