| Age | Commit message (Collapse) | Author |
|---|
|
Use kmalloc_array() to replace kmalloc() with multiplication.
kmalloc_array() has multiply overflow check, which will be safer.
In once case change kcalloc() as we don't need to clear the memory
since it's all being reinitialised just immediately after that.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Link: https://patch.msgid.link/20241121123433.4180133-1-andriy.shevchenko@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
In some cases when using the maple tree register cache, the lockdep
validator might complain about invalid deadlocks:
[7.131886] Possible interrupt unsafe locking scenario:
[7.131890] CPU0 CPU1
[7.131893] ---- ----
[7.131896] lock(&mt->ma_lock);
[7.131904] local_irq_disable();
[7.131907] lock(rockchip_drm_vop2:3114:(&vop2_regmap_config)->lock);
[7.131916] lock(&mt->ma_lock);
[7.131925] <Interrupt>
[7.131928] lock(rockchip_drm_vop2:3114:(&vop2_regmap_config)->lock);
[7.131936]
*** DEADLOCK ***
[7.131939] no locks held by swapper/0/0.
[7.131944]
the shortest dependencies between 2nd lock and 1st lock:
[7.131950] -> (&mt->ma_lock){+.+.}-{2:2} {
[7.131966] HARDIRQ-ON-W at:
[7.131973] lock_acquire+0x200/0x330
[7.131986] _raw_spin_lock+0x50/0x70
[7.131998] regcache_maple_write+0x68/0xe0
[7.132010] regcache_write+0x6c/0x90
[7.132019] _regmap_read+0x19c/0x1d0
[7.132029] _regmap_update_bits+0xc0/0x148
[7.132038] regmap_update_bits_base+0x6c/0xa8
[7.132048] rk8xx_probe+0x22c/0x3d8
[7.132057] rk8xx_spi_probe+0x74/0x88
[7.132065] spi_probe+0xa8/0xe0
[...]
[7.132675] }
[7.132678] ... key at: [<ffff800082943c20>] __key.0+0x0/0x10
[7.132691] ... acquired at:
[7.132695] _raw_spin_lock+0x50/0x70
[7.132704] regcache_maple_write+0x68/0xe0
[7.132714] regcache_write+0x6c/0x90
[7.132724] _regmap_read+0x19c/0x1d0
[7.132732] _regmap_update_bits+0xc0/0x148
[7.132741] regmap_field_update_bits_base+0x74/0xb8
[7.132751] vop2_plane_atomic_update+0x480/0x14d8 [rockchipdrm]
[7.132820] drm_atomic_helper_commit_planes+0x1a0/0x320 [drm_kms_helper]
[...]
[7.135112] -> (rockchip_drm_vop2:3114:(&vop2_regmap_config)->lock){-...}-{2:2} {
[7.135130] IN-HARDIRQ-W at:
[7.135136] lock_acquire+0x200/0x330
[7.135147] _raw_spin_lock_irqsave+0x6c/0x98
[7.135157] regmap_lock_spinlock+0x20/0x40
[7.135166] regmap_read+0x44/0x90
[7.135175] vop2_isr+0x90/0x290 [rockchipdrm]
[7.135225] __handle_irq_event_percpu+0x124/0x2d0
In the example above, the validator seems to get the scope of
dependencies wrong, since the regmap instance used in rk8xx-spi driver
has nothing to do with the instance from vop2.
Improve validation by sharing the regmap's lockdep class with the maple
tree's internal lock, while also providing a subclass for the latter.
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
Link: https://patch.msgid.link/20241031-regmap-maple-lockdep-fix-v2-1-06a3710f3623@collabora.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Commit fd4ebc07b4df ("regmap: Hold the regmap lock when allocating and
freeing the cache") introduced a locking around the allocating and
freeing a regmap cache, so adjust the memory allocation flags to the ones
given in the regmap configuration instead of the hardcoded GFP_KERNEL.
This fixes the "BUG: sleeping function called from invalid context"
introduced by the mentioned commit.
Fixes: fd4ebc07b4df ("regmap: Hold the regmap lock when allocating and freeing the cache")
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Link: https://patch.msgid.link/20240828122834.3778031-1-m.szyprowski@samsung.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
With gcc-14.1, there is a false-postive -Wuninitialized warning in
regcache_maple_drop:
drivers/base/regmap/regcache-maple.c: In function 'regcache_maple_drop':
drivers/base/regmap/regcache-maple.c:113:23: error: 'lower_index' is used uninitialized [-Werror=uninitialized]
113 | unsigned long lower_index, lower_last;
| ^~~~~~~~~~~
drivers/base/regmap/regcache-maple.c:113:36: error: 'lower_last' is used uninitialized [-Werror=uninitialized]
113 | unsigned long lower_index, lower_last;
| ^~~~~~~~~~
I've created a reduced test case to see if this needs to be reported
as a gcc, but it appears that the gcc-14.x branch already has a change
that turns this into a more sensible -Wmaybe-uninitialized warning, so
I ended up not reporting it so far.
The reduced test case also produces a warning for gcc-13 and gcc-12
but I don't see that with the version in the kernel.
Link: https://godbolt.org/z/oKbohKqd3
Link: https://lore.kernel.org/all/CAMuHMdWj=FLmkazPbYKPevDrcym2_HDb_U7Mb9YE9ovrP0jJfA@mail.gmail.com/
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Link: https://patch.msgid.link/20240719104030.1382465-1-arnd@kernel.org
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Let the kememdup_array() take care about multiplication and possible
overflows.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Link: https://lore.kernel.org/r/20240606164717.3031107-5-andriy.shevchenko@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Merge series from Richard Fitzgerald <rf@opensource.cirrus.com>:
This series adds some more test cases, mainly for testing:
commit eaa03486d932 ("regmap: maple: Fix uninitialized symbol 'ret' warnings")
commit 00bb549d7d63 ("regmap: maple: Fix cache corruption in regcache_maple_drop()")
And the pending patch ("regmap: Add regmap_read_bypassed()")
There are also a few small improvements to the KUnit implementation.
|
|
Fix warnings reported by smatch by initializing local 'ret' variable
to 0.
drivers/base/regmap/regcache-maple.c:186 regcache_maple_drop()
error: uninitialized symbol 'ret'.
drivers/base/regmap/regcache-maple.c:290 regcache_maple_sync()
error: uninitialized symbol 'ret'.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Fixes: f033c26de5a5 ("regmap: Add maple tree based register cache")
Link: https://lore.kernel.org/r/20240329144630.1965159-1-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
When keeping the upper end of a cache block entry, the entry[] array
must be indexed by the offset from the base register of the block,
i.e. max - mas.index.
The code was indexing entry[] by only the register address, leading
to an out-of-bounds access that copied some part of the kernel
memory over the cache contents.
This bug was not detected by the regmap KUnit test because it only
tests with a block of registers starting at 0, so mas.index == 0.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Fixes: f033c26de5a5 ("regmap: Add maple tree based register cache")
Link: https://msgid.link/r/20240327114406.976986-1-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
There is a statement with two semicolons. Remove the second one, it
is redundant.
Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
Link: https://msgid.link/r/20240315084417.2427797-1-colin.i.king@gmail.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
REGCACHE_MAPLE needs to allocate memory for regmap operations.
This results in lockdep splats if used with fast_io since fast_io uses
spinlocks for locking.
BUG: sleeping function called from invalid context at include/linux/sched/mm.h:306
in_atomic(): 1, irqs_disabled(): 128, non_block: 0, pid: 167, name: kunit_try_catch
preempt_count: 1, expected: 0
1 lock held by kunit_try_catch/167:
#0: 838e9c10 (regmap_kunit:86:(config)->lock){....}-{2:2}, at: regmap_lock_spinlock+0x14/0x1c
irq event stamp: 146
hardirqs last enabled at (145): [<8078bfa8>] crng_make_state+0x1a0/0x294
hardirqs last disabled at (146): [<80c5f62c>] _raw_spin_lock_irqsave+0x7c/0x80
softirqs last enabled at (0): [<80110cc4>] copy_process+0x810/0x216c
softirqs last disabled at (0): [<00000000>] 0x0
CPU: 0 PID: 167 Comm: kunit_try_catch Tainted: G N 6.5.0-rc1-00028-gc4be22597a36-dirty #6
Hardware name: Generic DT based system
unwind_backtrace from show_stack+0x18/0x1c
show_stack from dump_stack_lvl+0x38/0x5c
dump_stack_lvl from __might_resched+0x188/0x2d0
__might_resched from __kmem_cache_alloc_node+0x1f4/0x258
__kmem_cache_alloc_node from __kmalloc+0x48/0x170
__kmalloc from regcache_maple_write+0x194/0x248
regcache_maple_write from _regmap_write+0x88/0x140
_regmap_write from regmap_write+0x44/0x68
regmap_write from basic_read_write+0x8c/0x27c
basic_read_write from kunit_generic_run_threadfn_adapter+0x1c/0x28
kunit_generic_run_threadfn_adapter from kthread+0xf8/0x120
kthread from ret_from_fork+0x14/0x3c
Exception stack(0x881a5fb0 to 0x881a5ff8)
5fa0: 00000000 00000000 00000000 00000000
5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
5fe0: 00000000 00000000 00000000 00000000 00000013 00000000
Use map->alloc_flags instead of GFP_KERNEL for memory allocations to fix
the problem.
Fixes: f033c26de5a5 ("regmap: Add maple tree based register cache")
Cc: Dan Carpenter <dan.carpenter@linaro.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Link: https://lore.kernel.org/r/20230720172021.2617326-1-linux@roeck-us.net
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
For register maps where we can write multiple values in a single bus
operation it is generally much faster to do so. Improve the performance of
maple tree cache syncs on such devices by identifying blocks of adjacent
registers that need to be written out and combining them into a single
operation.
Combining writes does mean that we need to allocate a scratch buffer and
format the data into it but it is expected that for most cases where caches
are in use the cost of I/O will be much greater than the cost of doing the
allocation and format.
Signed-off-by: Mark Brown <broonie@kernel.org>
Link: https://lore.kernel.org/r/20230609-regcache-maple-sync-raw-v1-1-8ddeb4e2b9ab@kernel.org
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
The fix for maple tree RCU locking on sync is a dependency for the
block sync code for the maple tree.
|
|
Currently we use the normal single register write function to load the
default values into the cache, resulting in a large number of reallocations
when there are blocks of registers as we extend the memory region we are
using to store the values. Instead scan through the list of defaults for
blocks of adjacent registers and do a single allocation and insert for each
such block. No functional change.
We do not take advantage of the maple tree preallocation, this is purely at
the regcache level. It is not clear to me yet if the maple tree level would
help much here or if we'd have more overhead from overallocating and then
freeing maple tree data.
Signed-off-by: Mark Brown <broonie@kernel.org>
Link: https://lore.kernel.org/r/20230523-regcache-maple-load-defaults-v1-1-0c04336f005d@kernel.org
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Unfortunately the maple tree requires us to explicitly lock it so we need
to take the RCU read lock while iterating. When syncing this means that we
end up trying to write out register values while holding the RCU read lock
which triggers lockdep issues since that is an atomic context but most
buses can't be used in atomic context. Pause the iteration and drop the
lock for each register we check to avoid this.
Reported-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Tested-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
Link: https://lore.kernel.org/r/20230523-regcache-maple-sync-lock-v1-1-530e4d68dfab@kernel.org
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Liam recommends using mas_walk() instead of mas_find() for our use case so
let's do that, it avoids some minor overhead associated with being able to
restart the operation which we don't need since we do a simple search.
Suggested-by: Liam R. Howlett <Liam.Howlett@Oracle.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
Link: https://lore.kernel.org/r/20230403-regmap-maple-walk-fine-v2-1-c07371c8a867@kernel.org
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Doing the dance to drop the maple tree's internal spinlock means we need
multiple exit paths in our error handling.
Reported-by: Liam R. Howlett <Liam.Howlett@Oracle.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
Link: https://lore.kernel.org/r/20230403-regmap-maple-unlock-v1-1-89998991b16c@kernel.org
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
The current state of the art for sparse register maps is the
rbtree cache. This works well for most applications but isn't
always ideal for sparser register maps since the rbtree can get
deep, requiring a lot of walking. Fortunately the kernel has a
data structure intended to address this very problem, the maple
tree. Provide an initial implementation of a register cache
based on the maple tree to start taking advantage of it.
The entries stored in the maple tree are arrays of register
values, with the maple tree keys holding the register addresses.
We store data in host native format rather than device native
format as we do for rbtree, this will be a benefit for devices
where we don't marshal data within regmap and simplifies the code
but will result in additional CPU overhead when syncing the cache
on devices where we do marshal data in regmap.
This should work well for a lot of devices, though there's some
additional areas that could be looked at such as caching the
last accessed entry like we do for rbtree and trying to minimise
the maple tree level locking. We should also use bulk writes
rather than single register writes when resyncing the cache where
possible, even if we don't store in device native format.
Very small register maps may continue to to better with rbtree
longer term.
Signed-off-by: Mark Brown <broonie@kernel.org>
Link: https://lore.kernel.org/r/20230325-regcache-maple-v3-2-23e271f93dc7@kernel.org
Signed-off-by: Mark Brown <broonie@kernel.org>
|
