linux-kernel - Re: [RFC PATCH] i2c: busses: i2c-omap: Increase timeout for i2c interrupt

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Fri, 10 Jul 2015 11:09:09 +0200
From:	Wolfram Sang <wsa@...-dreams.de>
To:	Alexander Sverdlin <alexander.sverdlin@...ia.com>,
	Felipe Balbi <balbi@...com>
Cc:	ext Vignesh R <vigneshr@...com>, Tony Lindgren <tony@...mide.com>,
	linux-omap@...r.kernel.org, linux-i2c@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH] i2c: busses: i2c-omap: Increase timeout for i2c
 interrupt


> 60 s sounds way too much and actually I simply don't believe this is
> the root cause. If I take a look into the driver, then I see, that

I agree, this is just a workaround.

> the design is not really the best. The whole IRQ handling could be
> actually performed in hard IRQ handler, without threading overhead.
> Putting even 2 bytes in the controller FIFO should not be too heavy
> for the hard IRQ handler. Then these ridiculous spin_lock()s. What
> is the reason behind? The IRQ is flagged with ONESHOT, so thread and
> hardirq handler are anyway mutually excluded. But if this thread
> ever runs longer than it's allowed in IRQ context, then it anyway
> produces this IRQ latency because it locks spin_lock_irqsave() for
> the whole time! So the whole point of threaded interrupt is missing.

Furthermore, this combination of threaded_irq and struct completion seems
bogus to me. If you just want to ensure the irq happened before timeout,
you just complete when the irq happened and do the "bottom half" after the
completion returned?

> I would propose you to throw away spinlocks. Convert threaded IRQ to
> just one hardirq handler. And continue debugging. You will reduce the
> load of the system with the above measures, maybe it will not happen
> any more, maybe you'll figure out that problem is somewhere else.

Or this.

Felipe converted it to threaded_irq, maybe he has something to add?


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