Re: [PATCH] regmap: Allocate buffers with GFP_ATOMIC when fast_io == true

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1747 bytes --]

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?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]