linux-kernel - Re: [PATCH] regmap: Allocate buffers with GFP_ATOMIC when fast

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

Message-ID: <20150911172443.GN20922@sirena.org.uk>
Date:	Fri, 11 Sep 2015 18:24:44 +0100
From:	Mark Brown <broonie@...nel.org>
To:	Stephen Boyd <sboyd@...eaurora.org>
Cc:	linux-kernel@...r.kernel.org,
	Abhijeet Dharmapurikar <adharmap@...eaurora.org>
Subject: Re: [PATCH] regmap: Allocate buffers with GFP_ATOMIC when fast_io ==
 true

On Fri, Sep 11, 2015 at 09:16:47AM -0700, Stephen Boyd wrote:
> On 09/11, Mark Brown wrote:

> > Why is this needed?  If something needs fast I/O it probably doesn't
> > want to be going down any of the code paths that result in us doing
> > allocations.  I'd expect either no cache, a flat cache or setting up
> > defaults at initialisation time.

> We tripped over this with regmap_bulk_write() users on the SPMI
> bus. How about going down the same paths as !map->can_multi_write
> and map->use_single_rw if fast_io == true? Something like this
> untested patch.

Hrm, OK.  Bulk writes are a reasonable use case here but we should be
able to do better...  if we had a cache we could bulk write directly
from the cache but I don't think the SPMI devices tend to have caches so
that won't help.

> -	map->use_single_rw = config->use_single_rw;
> -	map->can_multi_write = config->can_multi_write;
> +	map->use_single_rw = config->use_single_rw || config->fast_io;
> +	map->can_multi_write = config->can_multi_write && !config->fast_io;

The trouble with this is that you end up doing register at a time writes
which isn't ideal for performance.  Unless we can figure out a way to
reliably allocate a bigger buffer ahead of time which I can't think of a
way to do reliably without just putting a random number in the config
(that we then end up either hitting or making too big) I think this is
actually a reasonable use case.  

Can you redo your patch to touch only the allocations in your data paths
(I'm thinking particularly of the async changes as being unneeded, now I
look again you didn't touch the cache code anyway) and put a comment in
there about the alloc flags being used in fast path cases please?

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