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

From: Mark Brown
Date: Fri Sep 11 2015 - 13:25:04 EST


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: signature.asc
Description: Digital signature