linux-kernel - Re: [PATCH 2/8] regmap: Add the indexed cache support

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: <4E613986.4040007@metafoo.de>
Date:	Fri, 02 Sep 2011 22:16:06 +0200
From:	Lars-Peter Clausen <lars@...afoo.de>
To:	Dimitris Papastamos <dp@...nsource.wolfsonmicro.com>
CC:	linux-kernel@...r.kernel.org,
	Mark Brown <broonie@...nsource.wolfsonmicro.com>,
	Liam Girdwood <lrg@...com>,
	Graeme Gregory <gg@...mlogic.co.uk>,
	Samuel Oritz <sameo@...ux.intel.com>
Subject: Re: [PATCH 2/8] regmap: Add the indexed cache support

On 09/02/2011 05:46 PM, Dimitris Papastamos wrote:
> This is the simplest form of a cache available in regcache.  Any
> registers whose default value is 0 are ignored.  If any of those
> registers are modified in the future, they will be placed in the
> cache on demand.  The cache layout is essentially using the provided
> register defaults by the regcache core directly and does not re-map
> it to another representation.
> 
> Signed-off-by: Dimitris Papastamos <dp@...nsource.wolfsonmicro.com>

I wonder if we really still need indexed caches, because ...


> ---
>  drivers/base/regmap/Makefile           |    2 +-
>  drivers/base/regmap/internal.h         |    1 +
>  drivers/base/regmap/regcache-indexed.c |   65 ++++++++++++++++++++++++++++++++
>  drivers/base/regmap/regcache.c         |    3 +
>  include/linux/regmap.h                 |    1 +
>  5 files changed, 71 insertions(+), 1 deletions(-)
>  create mode 100644 drivers/base/regmap/regcache-indexed.c
> 
> [...]
> +static int regcache_indexed_read(struct regmap *map, unsigned int reg,
> +				 unsigned int *value)
> +{
> +	int ret;
> +
> +	ret = regcache_lookup_reg(map, reg);

... this lookup will be slower than an rbtree lookup. And given that we use two
integers per register doesn't give it much size advantage either.

I have some patches for the rbtree register cache code, which reduce some of
it's complexity and also implements the adjacent node merging and allows
smaller holes in the rbnodes register block if the block is smaller then an
rbnode. Currently these patches are based on the ASoC register cache code, but
they should be easy to rework onto of the regmap register cache code.

I did some small statistics on the number of rbnodes without and with the
patches applied:

Without patches:
Nodes:
  0: 4000-4000   1
  1: 4009-400a   2
  2: 400b-400c   2
  3: 400d-4010   4
  4: 4015-4015   1
  5: 4017-4017   1
  6: 4019-401b   3
  7: 401c-401d   2
  8: 401e-4020   3
  9: 4021-4022   2
 10: 4023-4026   4
 11: 4029-402a   2
 12: 4031-4031   1
 13: 4036-4036   1
 14: 40eb-40eb   1
 15: 40f2-40f3   2
 16: 40f8-40f9   2
 17: 40fa-40fa   1

Number of nodes: 18 (504 bytes)
Number of registers: 35 (35 bytes)
Total size: 539 bytes

With patches:
Nodes:
  0: 4000-4036  55
  1: 40eb-40fa  16

Number of nodes: 2 (56 bytes)
Number of registers: 71 (71 bytes)
Total size: 127 bytes

In comparison a ASoC flat cache for this driver would have used 250 bytes and
and regmap indexed cache would use 280 bytes. (Not that it really matters at
those sizes anyway)


> +	if (ret < 0)
> +		*value = 0;
> +	else
> +		*value = map->cache_defaults[ret].def;
> +	return 0;
> +}
> +
> [...]
> diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
> index 90b7e1f..dfefe40 100644
> --- a/drivers/base/regmap/regcache.c
> +++ b/drivers/base/regmap/regcache.c
> @@ -16,6 +16,9 @@
>  #include "internal.h"
>  
>  static const struct regcache_ops *cache_types[] = {
> +#ifdef CONFIG_REGCACHE_INDEXED

This symbol doesn't seem to be defined anywhere.

> +	&regcache_indexed_ops,
> +#endif
>  };
>  
>  int regcache_init(struct regmap *map)
> diff --git a/include/linux/regmap.h b/include/linux/regmap.h
> index b81e86a..4d1ad09 100644
> --- a/include/linux/regmap.h
> +++ b/include/linux/regmap.h
> @@ -23,6 +23,7 @@ struct spi_device;
>  /* An enum of all the supported cache types */
>  enum regcache_type {
>  	REGCACHE_NONE,
> +	REGCACHE_INDEXED,
>  };
>  
>  /**

--
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/