linux-kernel - Re: high cpu load on omap3 using musb

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] [day] [month] [year] [list]

Date:	Mon, 21 Jul 2014 20:52:03 +0200
From:	Laurent Pinchart <laurent.pinchart@...asonboard.com>
To:	balbi@...com
Cc:	Adam Wozniak <awozniak@...bot.com>, linux-kernel@...r.kernel.org,
	linux-arm@...ts.infradead.org, linux-usb@...r.kernel.org,
	"Agius, Frank" <fagius@...bot.com>
Subject: Re: high cpu load on omap3 using musb

Hi Felipe and Adam,

On Monday 21 July 2014 10:40:52 Felipe Balbi wrote:
> On Mon, Jul 21, 2014 at 05:28:58PM +0200, Laurent Pinchart wrote:
> > On Wednesday 29 January 2014 08:44:57 Adam Wozniak wrote:
> > > With a USB 2.0 webcam attached to the OTG port on an OMAP3 (applies to
> > > overo gumstix, beagleboard, probably others) we see a high CPU load in a
> > > kworker thread.
> > > 
> > > Between 2.6.33 and 2.6.34 musb_core.c changed.
> > > 
> > > IRQ handlers changed with the result that a worker in musb_core.c got
> > > scheduled far more frequently than needed.
> > > 
> > > I've included a patch below against 3.7, but i think it'll apply against
> > > mainline.
> > > [I apologize for any whitespace mangling.  I've also attached the
> > > patch.]
> > > 
> > > I'd like more eyeballs to tell me if this is right.  I'd also like to
> > > know who I need to talk to to get this pushed into mainline.
> > 
> > Running the scripts/get_maintainer.pl script on your patch produces
> > 
> > Felipe Balbi <balbi@...com> (maintainer:MUSB MULTIPOINT H...)
> > Greg Kroah-Hartman <gregkh@...uxfoundation.org> (supporter:USB SUBSYSTEM)
> > linux-usb@...r.kernel.org (open list:MUSB MULTIPOINT H...)
> > linux-kernel@...r.kernel.org (open list)
> > 
> > Felipe Balbi (CC'ed) is the person who you should talk to.
> > 
> > While we're touching the subject of scripts, you should run the
> > scripts/checkpatch.pl script and fix errors and warnings before submitting
> > patches. Please see Documentation/SubmittingPatches.
> > 
> > Last (but not least) piece of advice, don't give up if you don't receive
> > replies to your patches. People are busy and mails fall to cracks from
> > time to time.
> > 
> > Felipe, apart from the coding style violation and the possibly missing
> > locking, what's your opinion on this ? Does the patch make sense ?
> 
> It's a duplication of the check which is already in musb_irq_work():
> 
> 1742 static void musb_irq_work(struct work_struct *data)
> 1743 {
> 1744         struct musb *musb = container_of(data, struct musb, irq_work);
> 1745
> 1746         if (musb->xceiv->state != musb->xceiv_old_state) {
> 1747                 musb->xceiv_old_state = musb->xceiv->state;
> 1748                 sysfs_notify(&musb->controller->kobj, NULL, "mode");
> 1749         }
> 1750 }
> 
> That does look better, but I'd need the check inside musb_irq_work() to
> be removed and commit log would have to improve a bit.

OK. Adam, could you please modify the patch accordingly and resubmit it ?

> ps: there's no missing locking, musb_stage0_irq() is called within
> musb_interrupt() which is called within a locked IRQ handler.

I hadn't checked that, thank you for the confirmation.

-- 
Regards,

Laurent Pinchart

Download attachment "signature.asc" of type "application/pgp-signature" (474 bytes)