linux-kernel - Re: [PATCH V2 1/4] regmap: cache: Add "was_reset" argument to regcache_sync

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: <CAJzqFtbTbUG=s7sxNBEYG1Haq5tzaCrMB=j55okZnwdVmzUAYQ@mail.gmail.com>
Date:	Wed, 29 Apr 2015 10:02:03 -0700
From:	Kevin Cernekee <cernekee@...omium.org>
To:	Mark Brown <broonie@...nel.org>
Cc:	Liam Girdwood <lgirdwood@...il.com>,
	Lars-Peter Clausen <lars@...afoo.de>, dgreid@...omium.org,
	Andrew Bresticker <abrestic@...omium.org>,
	Olof Johansson <olofj@...omium.org>,
	alsa-devel@...a-project.org, devicetree@...r.kernel.org,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V2 1/4] regmap: cache: Add "was_reset" argument to regcache_sync_region()

On Wed, Apr 29, 2015 at 9:46 AM, Mark Brown <broonie@...nel.org> wrote:
> On Wed, Apr 29, 2015 at 07:13:27AM -0700, Kevin Cernekee wrote:
>> On Wed, Apr 29, 2015 at 3:40 AM, Mark Brown <broonie@...nel.org> wrote:
>
>> > Like I said above we can tell if the hardware was reset because
>> > mark_dirty() is called.
>
>> That covers the public API, but I do not understand how you intended
>> for this data to be stored in the rbtree if the use of a dirty bitmask
>> is discouraged.
>
> We just need a single boolean?

Right, so if we add a per-regmap bool that tells us whether the device
has been reset, then in the case of "not reset" we will have to write
every regcache entry out to the device.  Even the ones that weren't
touched while in cache_only mode.  This makes the "not reset" case
much less efficient than the "reset" case.

Maybe that's good enough for most purposes.  It's no worse than what
my original patch submission did, anyway.

BTW, any preferences on naming for the bool or for the renamed
mark_dirty function?

>> i.e. regcache_sync() finds a register value marked "present".  How do
>> we know whether we need to write it back to the hardware?  For the
>> special case of "cached non default register values immediately after
>> a HW reset" you can mostly figure this out, but if there was no HW
>> reset how do we know which entries changed while the HW was
>> inaccessible?
>
> In the first instance do we care?

I'm not sure I understand the question.

>> > I'm not suggesting that we do anything based on the presence of a cache
>> > entry, I'm suggesting that we could avoid having to ever cache values
>> > that never get referenced on a system (which can be a lot of them for
>> > common use cases) saving us memory.
>
>> This seems to be solving a different problem.  It sounds like you are
>> more worried about regcache_sync() writing back lots of default values
>> for registers that were never touched, than performing unnecessary
>> writes to a few (actively used) registers that weren't changed while
>> we were in cache_only mode.  Is that accurate?
>
> No.  This is nothing to do with sync, it's just something that might be
> nice.

Thanks, that clarifies things.  I was not aware that other drivers
even had an issue with excessively large regcache rbtrees, as my
reg_defaults list is short.
--
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/