REGCACHE_RBTREE and REGCACHE_MAPLE dynamically allocate memory
for regmap operations. This is incompatible with spinlock based locking
which is used for fast_io operations. Disable locking for the associated
unit tests to avoid lockdep splashes.
Fixes: f033c26de5a5 ("regmap: Add maple tree based register cache")
Fixes: 2238959b6ad2 ("regmap: Add some basic kunit tests")
Signed-off-by: Guenter Roeck <[email protected]>
---
drivers/base/regmap/regmap-kunit.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/base/regmap/regmap-kunit.c b/drivers/base/regmap/regmap-kunit.c
index 24257aa9004d..9ff3018a46aa 100644
--- a/drivers/base/regmap/regmap-kunit.c
+++ b/drivers/base/regmap/regmap-kunit.c
@@ -58,6 +58,9 @@ static struct regmap *gen_regmap(struct regmap_config *config,
int i;
struct reg_default *defaults;
+ config->disable_locking = config->cache_type == REGCACHE_RBTREE ||
+ config->cache_type == REGCACHE_MAPLE;
+
buf = kmalloc(size, GFP_KERNEL);
if (!buf)
return ERR_PTR(-ENOMEM);
@@ -889,6 +892,8 @@ static struct regmap *gen_raw_regmap(struct regmap_config *config,
config->cache_type = test_type->cache_type;
config->val_format_endian = test_type->val_endian;
+ config->disable_locking = config->cache_type == REGCACHE_RBTREE ||
+ config->cache_type == REGCACHE_MAPLE;
buf = kmalloc(size, GFP_KERNEL);
if (!buf)
--
2.39.2
REGCACHE_RBTREE and REGCACHE_MAPLE dynamically allocate memory for regmap
operations. This is incompatible with spinlock based locking which is used
for fast_io operations. Reject affected configurations.
Signed-off-by: Guenter Roeck <[email protected]>
---
This seems prudent, given that accesses will be protected by spinlock
but may allocate memory with GFP_KERNEL. Another option might be to use
WARN_ON instead of rejecting the configuration to avoid hard regressions
(and I think both drivers/net/ieee802154/mcr20a.c and
sound/soc/codecs/sti-sas.c may be affected, though I can not test it).
drivers/base/regmap/regmap.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 89a7f1c459c1..b4640285c0b9 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -777,6 +777,15 @@ struct regmap *__regmap_init(struct device *dev,
} else {
if ((bus && bus->fast_io) ||
config->fast_io) {
+ /*
+ * fast_io is incompatible with REGCACHE_RBTREE and REGCACHE_MAPLE
+ * since both need to dynamically allocate memory.
+ */
+ if (config->cache_type == REGCACHE_RBTREE ||
+ config->cache_type == REGCACHE_MAPLE) {
+ ret = -EINVAL;
+ goto err_name;
+ }
if (config->use_raw_spinlock) {
raw_spin_lock_init(&map->raw_spinlock);
map->lock = regmap_lock_raw_spinlock;
--
2.39.2
On Wed, 19 Jul 2023 20:28:47 -0700, Guenter Roeck wrote:
> REGCACHE_RBTREE and REGCACHE_MAPLE dynamically allocate memory
> for regmap operations. This is incompatible with spinlock based locking
> which is used for fast_io operations. Disable locking for the associated
> unit tests to avoid lockdep splashes.
>
>
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git for-next
Thanks!
[1/2] regmap: Disable locking for RBTREE and MAPLE unit tests
(no commit info)
[2/2] regmap: Reject fast_io regmap configurations with RBTREE and MAPLE caches
commit: ee43f5bb23340c27603c3ad8ef94f677ad7cb9ad
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
On Fri, Jul 21, 2023 at 04:53:42PM +0200, Marek Szyprowski wrote:
> This patch, which landed in today's linux-next, breaks operation of the
> RockChip's VOP2 DRM driver
> (drivers/gpu/drm/rockchip/rockchip_drm_vop2.c). I'm not sure what is the
> proper fix in this case. Should one change the cache type to REGCACHE_FLAT?
Actually Guenter and Dan have made the required updates to support this
so the warning will be gone soon (hopefully Dan will send his patch
properly shortly).
On 7/21/23 08:03, Mark Brown wrote:
> On Fri, Jul 21, 2023 at 04:53:42PM +0200, Marek Szyprowski wrote:
>
>> This patch, which landed in today's linux-next, breaks operation of the
>> RockChip's VOP2 DRM driver
>> (drivers/gpu/drm/rockchip/rockchip_drm_vop2.c). I'm not sure what is the
>> proper fix in this case. Should one change the cache type to REGCACHE_FLAT?
>
> Actually Guenter and Dan have made the required updates to support this
> so the warning will be gone soon (hopefully Dan will send his patch
> properly shortly).
Do you plan to revert this patch ? If not regmap_init() would still fail
for the affected drivers, even after my and Dan's patches have been applied.
Thanks,
Guenter
On Fri, Jul 21, 2023 at 08:07:28AM -0700, Guenter Roeck wrote:
> On 7/21/23 08:03, Mark Brown wrote:
> > Actually Guenter and Dan have made the required updates to support this
> > so the warning will be gone soon (hopefully Dan will send his patch
> > properly shortly).
> Do you plan to revert this patch ? If not regmap_init() would still fail
> for the affected drivers, even after my and Dan's patches have been applied.
Yeah. You *can* use the dynamically allocating caches safely if you
ensure that no new cache nodes are allocated during I/O. I'd not
realised people were actually doing this.
Hi,
On 20.07.2023 05:28, Guenter Roeck wrote:
> REGCACHE_RBTREE and REGCACHE_MAPLE dynamically allocate memory for regmap
> operations. This is incompatible with spinlock based locking which is used
> for fast_io operations. Reject affected configurations.
>
> Signed-off-by: Guenter Roeck <[email protected]>
> ---
> This seems prudent, given that accesses will be protected by spinlock
> but may allocate memory with GFP_KERNEL. Another option might be to use
> WARN_ON instead of rejecting the configuration to avoid hard regressions
> (and I think both drivers/net/ieee802154/mcr20a.c and
> sound/soc/codecs/sti-sas.c may be affected, though I can not test it).
This patch, which landed in today's linux-next, breaks operation of the
RockChip's VOP2 DRM driver
(drivers/gpu/drm/rockchip/rockchip_drm_vop2.c). I'm not sure what is the
proper fix in this case. Should one change the cache type to REGCACHE_FLAT?
> drivers/base/regmap/regmap.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
> index 89a7f1c459c1..b4640285c0b9 100644
> --- a/drivers/base/regmap/regmap.c
> +++ b/drivers/base/regmap/regmap.c
> @@ -777,6 +777,15 @@ struct regmap *__regmap_init(struct device *dev,
> } else {
> if ((bus && bus->fast_io) ||
> config->fast_io) {
> + /*
> + * fast_io is incompatible with REGCACHE_RBTREE and REGCACHE_MAPLE
> + * since both need to dynamically allocate memory.
> + */
> + if (config->cache_type == REGCACHE_RBTREE ||
> + config->cache_type == REGCACHE_MAPLE) {
> + ret = -EINVAL;
> + goto err_name;
> + }
> if (config->use_raw_spinlock) {
> raw_spin_lock_init(&map->raw_spinlock);
> map->lock = regmap_lock_raw_spinlock;
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
On 7/21/23 07:53, Marek Szyprowski wrote:
> Hi,
>
> On 20.07.2023 05:28, Guenter Roeck wrote:
>> REGCACHE_RBTREE and REGCACHE_MAPLE dynamically allocate memory for regmap
>> operations. This is incompatible with spinlock based locking which is used
>> for fast_io operations. Reject affected configurations.
>>
>> Signed-off-by: Guenter Roeck <[email protected]>
>> ---
>> This seems prudent, given that accesses will be protected by spinlock
>> but may allocate memory with GFP_KERNEL. Another option might be to use
>> WARN_ON instead of rejecting the configuration to avoid hard regressions
>> (and I think both drivers/net/ieee802154/mcr20a.c and
>> sound/soc/codecs/sti-sas.c may be affected, though I can not test it).
>
> This patch, which landed in today's linux-next, breaks operation of the
> RockChip's VOP2 DRM driver
> (drivers/gpu/drm/rockchip/rockchip_drm_vop2.c). I'm not sure what is the
> proper fix in this case. Should one change the cache type to REGCACHE_FLAT?
>
Ah, I missed regcache_init_mmio() when looking for affected drivers.
This affects a larger number of drivers than I thought. In addition
to the drivers mentioned above,
drivers/soc/qcom/icc-bwmon.c
sound/soc/bcm/bcm2835-i2s.c
sound/soc/codecs/jz4740.c
sound/soc/fsl/fsl_aud2htx.c
sound/soc/fsl/fsl_easrc.c
sound/soc/fsl/fsl_micfil.c
all use unsafe locking (spinlock with REGCACHE_RBTREE).
Thanks,
Guenter
>
>> drivers/base/regmap/regmap.c | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
>> index 89a7f1c459c1..b4640285c0b9 100644
>> --- a/drivers/base/regmap/regmap.c
>> +++ b/drivers/base/regmap/regmap.c
>> @@ -777,6 +777,15 @@ struct regmap *__regmap_init(struct device *dev,
>> } else {
>> if ((bus && bus->fast_io) ||
>> config->fast_io) {
>> + /*
>> + * fast_io is incompatible with REGCACHE_RBTREE and REGCACHE_MAPLE
>> + * since both need to dynamically allocate memory.
>> + */
>> + if (config->cache_type == REGCACHE_RBTREE ||
>> + config->cache_type == REGCACHE_MAPLE) {
>> + ret = -EINVAL;
>> + goto err_name;
>> + }
>> if (config->use_raw_spinlock) {
>> raw_spin_lock_init(&map->raw_spinlock);
>> map->lock = regmap_lock_raw_spinlock;
>
> Best regards
On 7/21/23 08:13, Mark Brown wrote:
> On Fri, Jul 21, 2023 at 08:07:28AM -0700, Guenter Roeck wrote:
>> On 7/21/23 08:03, Mark Brown wrote:
>
>>> Actually Guenter and Dan have made the required updates to support this
>>> so the warning will be gone soon (hopefully Dan will send his patch
>>> properly shortly).
>
>> Do you plan to revert this patch ? If not regmap_init() would still fail
>> for the affected drivers, even after my and Dan's patches have been applied.
>
> Yeah. You *can* use the dynamically allocating caches safely if you
> ensure that no new cache nodes are allocated during I/O. I'd not
> realised people were actually doing this.
Ok.
Dan, let me know if you don't have time to send a proper patch.
I have one based on your suggestion prepared that I could send out
if needed.
Thanks,
Guenter
On Fri, Jul 21, 2023 at 09:01:03AM -0700, Guenter Roeck wrote:
> Dan, let me know if you don't have time to send a proper patch.
> I have one based on your suggestion prepared that I could send out
> if needed.
I sent it but, aww crud, I forgot to CC you. Really, get_maintainer.pl
should add everyone from the tag section to the CC list...
https://lore.kernel.org/all/[email protected]/
regards,
dan carpenter
On Fri, Jul 21, 2023 at 09:01:03AM -0700, Guenter Roeck wrote:
> On 7/21/23 08:13, Mark Brown wrote:
> > Yeah. You *can* use the dynamically allocating caches safely if you
> > ensure that no new cache nodes are allocated during I/O. I'd not
> > realised people were actually doing this.
> Ok.
> Dan, let me know if you don't have time to send a proper patch.
> I have one based on your suggestion prepared that I could send out
> if needed.
Dan sent the patch already, assuming my CI doesn't blow up unexpectedly
it should be applied tonight.
On 7/21/23 09:18, Dan Carpenter wrote:
> On Fri, Jul 21, 2023 at 09:01:03AM -0700, Guenter Roeck wrote:
>
>> Dan, let me know if you don't have time to send a proper patch.
>> I have one based on your suggestion prepared that I could send out
>> if needed.
>
> I sent it but, aww crud, I forgot to CC you. Really, get_maintainer.pl
> should add everyone from the tag section to the CC list...
>
> https://lore.kernel.org/all/[email protected]/
>
No worries.
Thanks,
Guenter
On Fri, Jul 21, 2023 at 05:14:42PM +0100, Mark Brown wrote:
> On Fri, Jul 21, 2023 at 09:01:03AM -0700, Guenter Roeck wrote:
> > On 7/21/23 08:13, Mark Brown wrote:
>
> > > Yeah. You *can* use the dynamically allocating caches safely if you
> > > ensure that no new cache nodes are allocated during I/O. I'd not
> > > realised people were actually doing this.
>
> > Ok.
>
> > Dan, let me know if you don't have time to send a proper patch.
> > I have one based on your suggestion prepared that I could send out
> > if needed.
>
> Dan sent the patch already, assuming my CI doesn't blow up unexpectedly
> it should be applied tonight.
Excellent.
Thanks,
Guenter
On Fri, Jul 21, 2023 at 07:18:03PM +0300, Dan Carpenter wrote:
> On Fri, Jul 21, 2023 at 09:01:03AM -0700, Guenter Roeck wrote:
> > Dan, let me know if you don't have time to send a proper patch.
> > I have one based on your suggestion prepared that I could send out
> > if needed.
> I sent it but, aww crud, I forgot to CC you. Really, get_maintainer.pl
> should add everyone from the tag section to the CC list...
b4 prep --auto-to-cc does that IIRC!
On Wed, 19 Jul 2023 20:28:47 -0700, Guenter Roeck wrote:
> REGCACHE_RBTREE and REGCACHE_MAPLE dynamically allocate memory
> for regmap operations. This is incompatible with spinlock based locking
> which is used for fast_io operations. Disable locking for the associated
> unit tests to avoid lockdep splashes.
>
>
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git for-next
Thanks!
[1/2] regmap: Disable locking for RBTREE and MAPLE unit tests
commit: a9e26169cfda651802f88262a315146fbe4bc74c
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