linux-kernel - Re: spidev: fix hang when transfer_one

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]

Message-ID: <20140124130135.GX11727@sirena.org.uk>
Date:	Fri, 24 Jan 2014 13:01:35 +0000
From:	Mark Brown <broonie@...nel.org>
To:	Daniel Santos <danielfsantos@....net>
Cc:	Geert Uytterhoeven <geert@...ux-m68k.org>,
	Daniel Santos <daniel.santos@...ox.com>,
	linux-spi <linux-spi@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: spidev: fix hang when transfer_one_message fails

On Thu, Jan 23, 2014 at 08:21:39PM -0600, Daniel Santos wrote:
> On 01/23/2014 12:17 PM, Mark Brown wrote:
> >On Thu, Jan 23, 2014 at 05:47:02PM +0100, Geert Uytterhoeven wrote:

Please don't write enormous walls of text, it really doesn't make it
easy to read your messages or encourage doing so.  Use blank lines
between paragraphs (including within lists) and try to either split or
condense your ideas so that what you're trying to say comes over more
clearly.

> The only reason I'm using transfer_one_message() at all is because
> transfer() is being deprecated. My driver (currently out-of-tree)
> supports both but will prefer transfer() as long as it hasn't been
> removed or become broken ( which I'm managing via a #if
> LINUX_VERSION_CODE >= KERNEL_VERSION(4,99,99) check: https://github.com/daniel-santos/mcp2210-linux/blob/master/mcp2210-spi.c#L143).

No, don't do that - it's not sensible.  If there's something you need
work upstream to get it implemented or understand how to use the
framework better.  Don't code around the frameworks, talk to people
instead.

> of other spi drivers in the mainline, I can see that at least two of
> them do this as well: spi-pxa2xx and spi-bfin-v3. So perhaps we need
> a non-deprecated mechanism to do our own queuing and avoid the

No, that's not what those drivers are doing (nor the others doing
similar things) - they have done some optimisation on the code that
pushes messages to hardware so they don't defer to task context when
they don't have to.  There's very little hardware specific about what
they're doing, it's all about how we work with the scheduler to minimise
the idle time for the hardware.  A major goal of factoring out the loops
that traverse the messages from the drivers is to allow us to move that
code out of the drivers and into the framework where it belongs.

> overhead of the spi core providing a thread & queue which we'll just
> ignore. Then, the core can take care of setting status and
> finalizing when calls to transfer() fail (since there should be no
> ambiguity about this here), but leave that up to the driver when
> calling transfer_one_message()?

When the core refactoring is finished popping up into the thread will be
mostly optional.  Things like PIO, clock reprogramming and delays will
need to be pushed up into task context as do some of the DMA operations
and the completions - you don't want to be doing anything slow in
interrupt context.

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