2024-05-06 19:43:20

by Christophe JAILLET

[permalink] [raw]
Subject: [PATCH v1] regmap: Reorder fields in 'struct regmap_config' to save some memory

On x86_64 and allmodconfig, this shrinks the size of 'struct regmap_config'
from 328 to 312 bytes.

This is usually a win, because this structure is used as a static global
variable.

When moving the kerneldoc fields, I've tried to keep the layout as
consistent as possible, which is not really easy!

Before:
/* size: 328, cachelines: 6, members: 55 */
/* sum members: 296, holes: 6, sum holes: 25 */
/* padding: 7 */
/* last cacheline: 8 bytes */

After:
/* size: 312, cachelines: 5, members: 55 */
/* sum members: 296, holes: 5, sum holes: 16 */
/* last cacheline: 56 bytes */

For the records, this is also widely used:
$git grep static.*regmap_config | wc -l
1327

Signed-off-by: Christophe JAILLET <[email protected]>
---
Built-tested only on several dozens of modified files.

Example of size reduction on drivers/clk/clk-si544.o (randomly chosen)

Before:
text data bss dec hex filename
9917 700 16 10633 2989 drivers/clk/clk-si544.o

After:
text data bss dec hex filename
9885 700 16 10601 2969 drivers/clk/clk-si544.o
---
include/linux/regmap.h | 62 +++++++++++++++++++++---------------------
1 file changed, 31 insertions(+), 31 deletions(-)

diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index d470303b1bbb..a6bc2980a98b 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -297,15 +297,6 @@ typedef void (*regmap_unlock)(void *);
* performed on such table (a register is no increment
* readable if it belongs to one of the ranges specified
* by rd_noinc_table).
- * @disable_locking: This regmap is either protected by external means or
- * is guaranteed not to be accessed from multiple threads.
- * Don't use any locking mechanisms.
- * @lock: Optional lock callback (overrides regmap's default lock
- * function, based on spinlock or mutex).
- * @unlock: As above for unlocking.
- * @lock_arg: this field is passed as the only argument of lock/unlock
- * functions (ignored in case regular lock/unlock functions
- * are not overridden).
* @reg_read: Optional callback that if filled will be used to perform
* all the reads from the registers. Should only be provided for
* devices whose read operation cannot be represented as a simple
@@ -323,6 +314,7 @@ typedef void (*regmap_unlock)(void *);
* @write: Same as above for writing.
* @max_raw_read: Max raw read size that can be used on the device.
* @max_raw_write: Max raw write size that can be used on the device.
+ * @can_sleep: Optional, specifies whether regmap operations can sleep.
* @fast_io: Register IO is fast. Use a spinlock instead of a mutex
* to perform locking. This field is ignored if custom lock/unlock
* functions are used (see fields lock/unlock of struct regmap_config).
@@ -331,6 +323,15 @@ typedef void (*regmap_unlock)(void *);
* Use it only for "no-bus" cases.
* @io_port: Support IO port accessors. Makes sense only when MMIO vs. IO port
* access can be distinguished.
+ * @disable_locking: This regmap is either protected by external means or
+ * is guaranteed not to be accessed from multiple threads.
+ * Don't use any locking mechanisms.
+ * @lock: Optional lock callback (overrides regmap's default lock
+ * function, based on spinlock or mutex).
+ * @unlock: As above for unlocking.
+ * @lock_arg: This field is passed as the only argument of lock/unlock
+ * functions (ignored in case regular lock/unlock functions
+ * are not overridden).
* @max_register: Optional, specifies the maximum valid register address.
* @max_register_is_0: Optional, specifies that zero value in @max_register
* should be taken into account. This is a workaround to
@@ -373,21 +374,20 @@ typedef void (*regmap_unlock)(void *);
* @reg_defaults_raw: Power on reset values for registers (for use with
* register cache support).
* @num_reg_defaults_raw: Number of elements in reg_defaults_raw.
- * @reg_format_endian: Endianness for formatted register addresses. If this is
- * DEFAULT, the @reg_format_endian_default value from the
- * regmap bus is used.
- * @val_format_endian: Endianness for formatted register values. If this is
- * DEFAULT, the @reg_format_endian_default value from the
- * regmap bus is used.
- *
- * @ranges: Array of configuration entries for virtual address ranges.
- * @num_ranges: Number of range configuration entries.
* @use_hwlock: Indicate if a hardware spinlock should be used.
* @use_raw_spinlock: Indicate if a raw spinlock should be used.
* @hwlock_id: Specify the hardware spinlock id.
* @hwlock_mode: The hardware spinlock mode, should be HWLOCK_IRQSTATE,
* HWLOCK_IRQ or 0.
- * @can_sleep: Optional, specifies whether regmap operations can sleep.
+ * @reg_format_endian: Endianness for formatted register addresses. If this is
+ * DEFAULT, the @reg_format_endian_default value from the
+ * regmap bus is used.
+ * @val_format_endian: Endianness for formatted register values. If this is
+ * DEFAULT, the @reg_format_endian_default value from the
+ * regmap bus is used.
+ *
+ * @ranges: Array of configuration entries for virtual address ranges.
+ * @num_ranges: Number of range configuration entries.
*/
struct regmap_config {
const char *name;
@@ -406,11 +406,6 @@ struct regmap_config {
bool (*writeable_noinc_reg)(struct device *dev, unsigned int reg);
bool (*readable_noinc_reg)(struct device *dev, unsigned int reg);

- bool disable_locking;
- regmap_lock lock;
- regmap_unlock unlock;
- void *lock_arg;
-
int (*reg_read)(void *context, unsigned int reg, unsigned int *val);
int (*reg_write)(void *context, unsigned int reg, unsigned int val);
int (*reg_update_bits)(void *context, unsigned int reg,
@@ -422,9 +417,16 @@ struct regmap_config {
size_t max_raw_read;
size_t max_raw_write;

+ bool can_sleep;
+
bool fast_io;
bool io_port;

+ bool disable_locking;
+ regmap_lock lock;
+ regmap_unlock unlock;
+ void *lock_arg;
+
unsigned int max_register;
bool max_register_is_0;
const struct regmap_access_table *wr_table;
@@ -448,18 +450,16 @@ struct regmap_config {
bool use_relaxed_mmio;
bool can_multi_write;

- enum regmap_endian reg_format_endian;
- enum regmap_endian val_format_endian;
-
- const struct regmap_range_cfg *ranges;
- unsigned int num_ranges;
-
bool use_hwlock;
bool use_raw_spinlock;
unsigned int hwlock_id;
unsigned int hwlock_mode;

- bool can_sleep;
+ enum regmap_endian reg_format_endian;
+ enum regmap_endian val_format_endian;
+
+ const struct regmap_range_cfg *ranges;
+ unsigned int num_ranges;
};

/**
--
2.45.0



2024-05-07 15:28:39

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v1] regmap: Reorder fields in 'struct regmap_config' to save some memory

On Mon, 06 May 2024 21:33:33 +0200, Christophe JAILLET wrote:
> On x86_64 and allmodconfig, this shrinks the size of 'struct regmap_config'
> from 328 to 312 bytes.
>
> This is usually a win, because this structure is used as a static global
> variable.
>
> When moving the kerneldoc fields, I've tried to keep the layout as
> consistent as possible, which is not really easy!
>
> [...]

Applied to

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git for-next

Thanks!

[1/1] regmap: Reorder fields in 'struct regmap_config' to save some memory
commit: 9b1fe0510494c989ab6a131ce8b97cdd02a1c869

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark