linux-kernel - Re: Regression: spi: core: avoid waking pump thread from spi

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: <b6d6585b-86ce-5eae-842e-83561beefa61@nvidia.com>
Date:   Wed, 16 Jan 2019 11:01:37 +0000
From:   Jon Hunter <jonathanh@...dia.com>
To:     Mark Brown <broonie@...nel.org>,
        Martin Sperl <kernel@...tin.sperl.org>
CC:     linux-tegra <linux-tegra@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        <linux-spi@...r.kernel.org>
Subject: Re: Regression: spi: core: avoid waking pump thread from spi_sync
 instead run teardown delayed


On 15/01/2019 21:25, Mark Brown wrote:
> On Tue, Jan 15, 2019 at 09:58:55PM +0100, Martin Sperl wrote:
> 
>> Maybe a bigger change to the reduce the complexity of
>> the state machine would solve that problem and also
>> reduce code complexity... 
> 
> Yeah, that's where I was getting to with that test patch I posted.
> 
>> I may find some time over the weekend if no solution
>> has been found until then.
> 
> Thanks for volunteering :)
> 
>> The way I would envision it it would have a “state”
>> as a level (0=shutdown, 1=hw enabled, 2=in pump, 
>> 3=in transfer, 4=in hw-mode,...) and a complete
>> to allow waking the shutdown thread (and by this
>> avoiding the busy wait loop we have now).
>> This would replace those idling, busy, and running flags.
> 
> That's a good idea, yes - a single enum much more reflects what we can
> actually do in terms of transitions.
> 
>> Drawback: it is invasive, but let us see what it
>> really looks like...
> 
> I think we need to either drop your change (which would be bad since it
> is a big performance improvement, I'd punted it for later when I did the
> original refactoring to push the work into the caller threads then never
> got around to it) or have a invasive changes to make the new situation
> clearer.  Right now things are just far too complex to reason about
> which isn't helping anyone.

What I am seeing on Tegra is that the SPI controller is no longer
runtime-pm suspended and so while this may improve performance, I think
that this change is going to have a negative impact on idle power.

I would be surprised if this problem is specific to Tegra because I
don't see how the controller's busy state can ever be cleared :-(

Cheers
Jon

-- 
nvpublic