linux-kernel - Re: [PATCH 2/2] xhci:prevent "callbacks suppressed" when debug is not enabled

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130816175009.GE5445@xanatos>
Date:	Fri, 16 Aug 2013 10:50:09 -0700
From:	Sarah Sharp <sarah.a.sharp@...el.com>
To:	Greg KH <gregkh@...uxfoundation.org>
Cc:	Dmitry Kasatkin <dmitry.kasatkin@...il.com>,
	Sarah Sharp <sarah.a.sharp@...ux.intel.com>,
	Dmitry Kasatkin <d.kasatkin@...sung.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] xhci:prevent "callbacks suppressed" when debug is
 not enabled

On Fri, Aug 16, 2013 at 10:45:16AM -0700, Greg KH wrote:
> On Fri, Aug 16, 2013 at 08:38:12PM +0300, Dmitry Kasatkin wrote:
> > On Fri, Aug 16, 2013 at 8:30 PM, Sarah Sharp <sarah.a.sharp@...el.com> wrote:
> > > On Fri, Aug 16, 2013 at 10:26:35AM -0700, Sarah Sharp wrote:
> > >> On Thu, Aug 15, 2013 at 05:17:16PM -0700, Greg KH wrote:
> > >> > On Thu, Aug 15, 2013 at 07:04:55PM +0300, Dmitry Kasatkin wrote:
> > >> > > When debug is not enabled and dev_dbg() will expand to nothing,
> > >> > > log might be flooded with "callbacks suppressed". If it was not
> > >> > > done on purpose, better to use dev_dbg_ratelimited() instead.
> > >> > >
> > >> > > Signed-off-by: Dmitry Kasatkin <d.kasatkin@...sung.com>
> > >> > > ---
> > >> > >  drivers/usb/host/xhci-ring.c | 6 ++----
> > >> > >  1 file changed, 2 insertions(+), 4 deletions(-)
> > >> >
> > >> >
> > >> > Sarah, does this patch conflict with the trace debug patches being
> > >> > worked on?  I'll hold off on applying it for now, let me know if it's ok
> > >> > or not.
> > >>
> > >> It doesn't conflict with the trace debug patches, because those only
> > >> effect debugging with xhci_dbg with the host device, not dev_dbg with
> > >> the USB device.  This should apply fine to usb-next.
> > >
> > > At another glance, the patch removes two if blocks, but doesn't
> > > re-indent the rest of the lines:
> > >
> > >> @@ -3060,8 +3060,7 @@ int xhci_queue_intr_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
> > >>        * to set the polling interval (once the API is added).
> > >>        */
> > >>       if (xhci_interval != ep_interval) {
> > >> -             if (printk_ratelimit())
> > >> -                     dev_dbg(&urb->dev->dev, "Driver uses different interval"
> > >> +             dev_dbg_ratelimited(&urb->dev->dev, "Driver uses different interval"
> > >>                                       " (%d microframe%s) than xHCI "
> > >>                                       "(%d microframe%s)\n",
> > >>                                       ep_interval,
> > >
> > > That should probably be fixed.
> > 
> > It actually looks correct when patch is applied.
> > 
> > But it depends what you mean of course.
> > It looks like it was before:
> > dev_dbg_ratelimited(&urb->dev->dev, "Driver uses different interval"
> >                                        " (%d microframe%s) than xHCI "
> >                                        "(%d microframe%s)\n",
> >                                        ep_interval,
> >                                         ep_interval == 1 ? "" : "s",
> > 
> > Or may be you mean:
> > dev_dbg_ratelimited(&urb->dev->dev, "Driver uses different interval"
> >                                " (%d microframe%s) than xHCI "
> >                                "(%d microframe%s)\n",
> >                                ep_interval,
> >                                ep_interval == 1 ? "" : "s",
> 
> No, it should look like:
> 
> 		dev_dbg_ratelimited(&urb->dev->dev,
> 				    "Driver uses different interval (%d microframe%s) than xHCI (%d microframe%s)\n",
> 				     ep_interval, ep_interval == 1 ? "" : "s",
> 
> and the rest of that call indented the same way.

Or just use the default tabbed indentation in your editor (use '==' if
you're using vim).  I don't mind if the second line is indented with
spaces to align with the parenthesis, and the majority of the xHCI
driver uses cindent vim indentation.  Either way is fine with me though.

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