Linux ARM-MSM sub-architecture
 help / color / mirror / Atom feed
* [PATCH 0/2] Fix slice accounting and simplify descriptor and locking logic
@ 2026-03-06  3:12 Francisco Munoz Ruiz
  2026-03-06  3:12 ` [PATCH 1/2] soc: qcom: llcc: Add per-slice counter and common llcc slice descriptor Francisco Munoz Ruiz
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Francisco Munoz Ruiz @ 2026-03-06  3:12 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio
  Cc: linux-arm-msm, linux-kernel, Francisco Munoz Ruiz,
	Unnathi Chalicheemala, Konrad Dybcio

Hi all,

This series addresses correctness issues in the LLCC slice
activation logic and simplifies both descriptor management and
locking within the driver.

Patch 1 fixes incorrect slice activation and deactivation
accounting. The current bitmap-based scheme fails when multiple
client drivers vote for the same slice or when
llcc_slice_getd() is called multiple times. This can lead to
mismatched activation and deactivation pairs and incorrect slice
state.

To address this, the patch replaces the bitmap with per-slice
atomic reference counters. This ensures that activation and
deactivation always match, regardless of how many users share a
slice or how often a client requests it. The patch also removes
dynamic allocation from the slice descriptor path. Instead,
llcc_slice_getd() now returns a pointer to a preallocated
descriptor, ensuring consistent lifetime and eliminating repeated
allocation and free cycles.

Patch 2 replaces manual lock and unlock pairs with the guard()
pattern. This removes the need for explicit unlock handling and
ensures the lock is always released, even on early returns. The
control flow becomes simpler and less error-prone.

Together, these changes make LLCC slice management more robust,
simpler, and easier to maintain.

Thanks,
Francisco

Signed-off-by: Francisco Munoz Ruiz <francisco.ruiz@oss.qualcomm.com>
---
Unnathi Chalicheemala (2):
      soc: qcom: llcc: Add per-slice counter and common llcc slice descriptor
      soc: qcom: llcc: Use guards for mutex handling

 drivers/soc/qcom/llcc-qcom.c       | 81 ++++++++++++++++----------------------
 include/linux/soc/qcom/llcc-qcom.h |  8 ++--
 2 files changed, 38 insertions(+), 51 deletions(-)
---
base-commit: 3f9cd19e764b782706dbaacc69e502099cb014ba
change-id: 20260305-external_llcc_changes1set-bae0e71cd913

Best regards,
-- 
Francisco Munoz Ruiz <francisco.ruiz@oss.qualcomm.com>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/2] soc: qcom: llcc: Add per-slice counter and common llcc slice descriptor
  2026-03-06  3:12 [PATCH 0/2] Fix slice accounting and simplify descriptor and locking logic Francisco Munoz Ruiz
@ 2026-03-06  3:12 ` Francisco Munoz Ruiz
  2026-03-06  3:12 ` [PATCH 2/2] soc: qcom: llcc: Use guards for mutex handling Francisco Munoz Ruiz
  2026-03-16  2:02 ` [PATCH 0/2] Fix slice accounting and simplify descriptor and locking logic Bjorn Andersson
  2 siblings, 0 replies; 4+ messages in thread
From: Francisco Munoz Ruiz @ 2026-03-06  3:12 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio
  Cc: linux-arm-msm, linux-kernel, Francisco Munoz Ruiz,
	Unnathi Chalicheemala, Konrad Dybcio

From: Unnathi Chalicheemala <unnathi.chalicheemala@oss.qualcomm.com>

Fix incorrect slice activation/deactivation accounting by replacing the
bitmap-based activation tracking with per-slice atomic reference counters.
This resolves mismatches that occur when multiple client drivers vote for
the same slice or when llcc_slice_getd() is called multiple times.

As part of this fix, simplify slice descriptor handling by eliminating
dynamic allocation. llcc_slice_getd() now returns a pointer to a
preallocated descriptor, removing the need for repeated allocation/free
cycles and ensuring consistent reference tracking across all users.

Signed-off-by: Unnathi Chalicheemala <unnathi.chalicheemala@oss.qualcomm.com>
Signed-off-by: Francisco Munoz Ruiz <francisco.ruiz@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
 drivers/soc/qcom/llcc-qcom.c       | 57 +++++++++++++++++++-------------------
 include/linux/soc/qcom/llcc-qcom.h |  8 +++---
 2 files changed, 32 insertions(+), 33 deletions(-)

diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
index ad5899d083f3..2e7f05df93a6 100644
--- a/drivers/soc/qcom/llcc-qcom.c
+++ b/drivers/soc/qcom/llcc-qcom.c
@@ -5,7 +5,6 @@
  */
 
 #include <linux/bitfield.h>
-#include <linux/bitmap.h>
 #include <linux/bitops.h>
 #include <linux/cleanup.h>
 #include <linux/device.h>
@@ -4431,8 +4430,7 @@ static struct llcc_drv_data *drv_data = (void *) -EPROBE_DEFER;
 struct llcc_slice_desc *llcc_slice_getd(u32 uid)
 {
 	const struct llcc_slice_config *cfg;
-	struct llcc_slice_desc *desc;
-	u32 sz, count;
+	u32 sz, i;
 
 	if (IS_ERR(drv_data))
 		return ERR_CAST(drv_data);
@@ -4440,21 +4438,14 @@ struct llcc_slice_desc *llcc_slice_getd(u32 uid)
 	cfg = drv_data->cfg;
 	sz = drv_data->cfg_size;
 
-	for (count = 0; cfg && count < sz; count++, cfg++)
+	for (i = 0; cfg && i < sz; i++, cfg++)
 		if (cfg->usecase_id == uid)
 			break;
 
-	if (count == sz || !cfg)
+	if (i == sz)
 		return ERR_PTR(-ENODEV);
 
-	desc = kzalloc_obj(*desc);
-	if (!desc)
-		return ERR_PTR(-ENOMEM);
-
-	desc->slice_id = cfg->slice_id;
-	desc->slice_size = cfg->max_cap;
-
-	return desc;
+	return &drv_data->desc[i];
 }
 EXPORT_SYMBOL_GPL(llcc_slice_getd);
 
@@ -4465,7 +4456,7 @@ EXPORT_SYMBOL_GPL(llcc_slice_getd);
 void llcc_slice_putd(struct llcc_slice_desc *desc)
 {
 	if (!IS_ERR_OR_NULL(desc))
-		kfree(desc);
+		return;
 }
 EXPORT_SYMBOL_GPL(llcc_slice_putd);
 
@@ -4541,7 +4532,8 @@ int llcc_slice_activate(struct llcc_slice_desc *desc)
 		return -EINVAL;
 
 	mutex_lock(&drv_data->lock);
-	if (test_bit(desc->slice_id, drv_data->bitmap)) {
+	/* Already active; try to take another reference. */
+	if (refcount_inc_not_zero(&desc->refcount)) {
 		mutex_unlock(&drv_data->lock);
 		return 0;
 	}
@@ -4555,7 +4547,8 @@ int llcc_slice_activate(struct llcc_slice_desc *desc)
 		return ret;
 	}
 
-	__set_bit(desc->slice_id, drv_data->bitmap);
+	/* Set first reference */
+	refcount_set(&desc->refcount, 1);
 	mutex_unlock(&drv_data->lock);
 
 	return ret;
@@ -4581,10 +4574,12 @@ int llcc_slice_deactivate(struct llcc_slice_desc *desc)
 		return -EINVAL;
 
 	mutex_lock(&drv_data->lock);
-	if (!test_bit(desc->slice_id, drv_data->bitmap)) {
+	/* refcount > 1, drop one ref and we’re done. */
+	if (refcount_dec_not_one(&desc->refcount)) {
 		mutex_unlock(&drv_data->lock);
 		return 0;
 	}
+
 	act_ctrl_val = ACT_CTRL_OPCODE_DEACTIVATE << ACT_CTRL_OPCODE_SHIFT;
 
 	ret = llcc_update_act_ctrl(desc->slice_id, act_ctrl_val,
@@ -4594,7 +4589,8 @@ int llcc_slice_deactivate(struct llcc_slice_desc *desc)
 		return ret;
 	}
 
-	__clear_bit(desc->slice_id, drv_data->bitmap);
+	/* Finalize: atomically transition 1 -> 0 */
+	WARN_ON_ONCE(!refcount_dec_if_one(&desc->refcount));
 	mutex_unlock(&drv_data->lock);
 
 	return ret;
@@ -4638,7 +4634,7 @@ static int _qcom_llcc_cfg_program(const struct llcc_slice_config *config,
 	u32 attr1_val;
 	u32 attr0_val;
 	u32 max_cap_cacheline;
-	struct llcc_slice_desc desc;
+	struct llcc_slice_desc *desc;
 
 	attr1_val = config->cache_mode;
 	attr1_val |= config->probe_target_ways << ATTR1_PROBE_TARGET_WAYS_SHIFT;
@@ -4787,8 +4783,11 @@ static int _qcom_llcc_cfg_program(const struct llcc_slice_config *config,
 	}
 
 	if (config->activate_on_init) {
-		desc.slice_id = config->slice_id;
-		ret = llcc_slice_activate(&desc);
+		desc = llcc_slice_getd(config->usecase_id);
+		if (IS_ERR(desc))
+			return PTR_ERR(desc);
+
+		ret = llcc_slice_activate(desc);
 	}
 
 	return ret;
@@ -5101,18 +5100,18 @@ static int qcom_llcc_probe(struct platform_device *pdev)
 
 	llcc_cfg = cfg->sct_data;
 	sz = cfg->size;
-
-	for (i = 0; i < sz; i++)
-		if (llcc_cfg[i].slice_id > drv_data->max_slices)
-			drv_data->max_slices = llcc_cfg[i].slice_id;
-
-	drv_data->bitmap = devm_bitmap_zalloc(dev, drv_data->max_slices,
-					      GFP_KERNEL);
-	if (!drv_data->bitmap) {
+	drv_data->desc = devm_kcalloc(dev, sz, sizeof(struct llcc_slice_desc), GFP_KERNEL);
+	if (!drv_data->desc) {
 		ret = -ENOMEM;
 		goto err;
 	}
 
+	for (i = 0; i < sz; i++) {
+		drv_data->desc[i].slice_id = llcc_cfg[i].slice_id;
+		drv_data->desc[i].slice_size = llcc_cfg[i].max_cap;
+		refcount_set(&drv_data->desc[i].refcount, 0);
+	}
+
 	drv_data->cfg = llcc_cfg;
 	drv_data->cfg_size = sz;
 	drv_data->edac_reg_offset = cfg->edac_reg_offset;
diff --git a/include/linux/soc/qcom/llcc-qcom.h b/include/linux/soc/qcom/llcc-qcom.h
index 8243ab3a12a8..227125d84318 100644
--- a/include/linux/soc/qcom/llcc-qcom.h
+++ b/include/linux/soc/qcom/llcc-qcom.h
@@ -91,10 +91,12 @@
  * struct llcc_slice_desc - Cache slice descriptor
  * @slice_id: llcc slice id
  * @slice_size: Size allocated for the llcc slice
+ * @refcount: Atomic counter to track activate/deactivate calls
  */
 struct llcc_slice_desc {
 	u32 slice_id;
 	size_t slice_size;
+	refcount_t refcount;
 };
 
 /**
@@ -152,11 +154,10 @@ struct llcc_edac_reg_offset {
  * @edac_reg_offset: Offset of the LLCC EDAC registers
  * @lock: mutex associated with each slice
  * @cfg_size: size of the config data table
- * @max_slices: max slices as read from device tree
  * @num_banks: Number of llcc banks
- * @bitmap: Bit map to track the active slice ids
  * @ecc_irq: interrupt for llcc cache error detection and reporting
  * @ecc_irq_configured: 'True' if firmware has already configured the irq propagation
+ * @desc: Array pointer of pre-allocated LLCC slice descriptors
  * @version: Indicates the LLCC version
  */
 struct llcc_drv_data {
@@ -167,12 +168,11 @@ struct llcc_drv_data {
 	const struct llcc_edac_reg_offset *edac_reg_offset;
 	struct mutex lock;
 	u32 cfg_size;
-	u32 max_slices;
 	u32 num_banks;
-	unsigned long *bitmap;
 	int ecc_irq;
 	bool ecc_irq_configured;
 	u32 version;
+	struct llcc_slice_desc *desc;
 };
 
 #if IS_ENABLED(CONFIG_QCOM_LLCC)

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] soc: qcom: llcc: Use guards for mutex handling
  2026-03-06  3:12 [PATCH 0/2] Fix slice accounting and simplify descriptor and locking logic Francisco Munoz Ruiz
  2026-03-06  3:12 ` [PATCH 1/2] soc: qcom: llcc: Add per-slice counter and common llcc slice descriptor Francisco Munoz Ruiz
@ 2026-03-06  3:12 ` Francisco Munoz Ruiz
  2026-03-16  2:02 ` [PATCH 0/2] Fix slice accounting and simplify descriptor and locking logic Bjorn Andersson
  2 siblings, 0 replies; 4+ messages in thread
From: Francisco Munoz Ruiz @ 2026-03-06  3:12 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio
  Cc: linux-arm-msm, linux-kernel, Francisco Munoz Ruiz,
	Unnathi Chalicheemala, Konrad Dybcio

From: Unnathi Chalicheemala <unnathi.chalicheemala@oss.qualcomm.com>

Replacing manual lock/unlock pairs with guard() removes the need to
think about unlocking entirely and keeps the function trivially
structured.

Signed-off-by: Unnathi Chalicheemala <unnathi.chalicheemala@oss.qualcomm.com>
Signed-off-by: Francisco Munoz Ruiz <francisco.ruiz@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
 drivers/soc/qcom/llcc-qcom.c | 28 ++++++++--------------------
 1 file changed, 8 insertions(+), 20 deletions(-)

diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
index 2e7f05df93a6..f859d91460c6 100644
--- a/drivers/soc/qcom/llcc-qcom.c
+++ b/drivers/soc/qcom/llcc-qcom.c
@@ -4531,27 +4531,21 @@ int llcc_slice_activate(struct llcc_slice_desc *desc)
 	if (IS_ERR_OR_NULL(desc))
 		return -EINVAL;
 
-	mutex_lock(&drv_data->lock);
+	guard(mutex)(&drv_data->lock);
 	/* Already active; try to take another reference. */
-	if (refcount_inc_not_zero(&desc->refcount)) {
-		mutex_unlock(&drv_data->lock);
+	if (refcount_inc_not_zero(&desc->refcount))
 		return 0;
-	}
 
 	act_ctrl_val = ACT_CTRL_OPCODE_ACTIVATE << ACT_CTRL_OPCODE_SHIFT;
-
 	ret = llcc_update_act_ctrl(desc->slice_id, act_ctrl_val,
 				  DEACTIVATE);
-	if (ret) {
-		mutex_unlock(&drv_data->lock);
+	if (ret)
 		return ret;
-	}
 
 	/* Set first reference */
 	refcount_set(&desc->refcount, 1);
-	mutex_unlock(&drv_data->lock);
 
-	return ret;
+	return 0;
 }
 EXPORT_SYMBOL_GPL(llcc_slice_activate);
 
@@ -4573,27 +4567,21 @@ int llcc_slice_deactivate(struct llcc_slice_desc *desc)
 	if (IS_ERR_OR_NULL(desc))
 		return -EINVAL;
 
-	mutex_lock(&drv_data->lock);
+	guard(mutex)(&drv_data->lock);
 	/* refcount > 1, drop one ref and we’re done. */
-	if (refcount_dec_not_one(&desc->refcount)) {
-		mutex_unlock(&drv_data->lock);
+	if (refcount_dec_not_one(&desc->refcount))
 		return 0;
-	}
 
 	act_ctrl_val = ACT_CTRL_OPCODE_DEACTIVATE << ACT_CTRL_OPCODE_SHIFT;
-
 	ret = llcc_update_act_ctrl(desc->slice_id, act_ctrl_val,
 				  ACTIVATE);
-	if (ret) {
-		mutex_unlock(&drv_data->lock);
+	if (ret)
 		return ret;
-	}
 
 	/* Finalize: atomically transition 1 -> 0 */
 	WARN_ON_ONCE(!refcount_dec_if_one(&desc->refcount));
-	mutex_unlock(&drv_data->lock);
 
-	return ret;
+	return 0;
 }
 EXPORT_SYMBOL_GPL(llcc_slice_deactivate);
 

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 0/2] Fix slice accounting and simplify descriptor and locking logic
  2026-03-06  3:12 [PATCH 0/2] Fix slice accounting and simplify descriptor and locking logic Francisco Munoz Ruiz
  2026-03-06  3:12 ` [PATCH 1/2] soc: qcom: llcc: Add per-slice counter and common llcc slice descriptor Francisco Munoz Ruiz
  2026-03-06  3:12 ` [PATCH 2/2] soc: qcom: llcc: Use guards for mutex handling Francisco Munoz Ruiz
@ 2026-03-16  2:02 ` Bjorn Andersson
  2 siblings, 0 replies; 4+ messages in thread
From: Bjorn Andersson @ 2026-03-16  2:02 UTC (permalink / raw)
  To: Konrad Dybcio, Francisco Munoz Ruiz
  Cc: linux-arm-msm, linux-kernel, Unnathi Chalicheemala, Konrad Dybcio


On Thu, 05 Mar 2026 19:12:04 -0800, Francisco Munoz Ruiz wrote:
> This series addresses correctness issues in the LLCC slice
> activation logic and simplifies both descriptor management and
> locking within the driver.
> 
> Patch 1 fixes incorrect slice activation and deactivation
> accounting. The current bitmap-based scheme fails when multiple
> client drivers vote for the same slice or when
> llcc_slice_getd() is called multiple times. This can lead to
> mismatched activation and deactivation pairs and incorrect slice
> state.
> 
> [...]

Applied, thanks!

[1/2] soc: qcom: llcc: Add per-slice counter and common llcc slice descriptor
      commit: 45c2a55d13c698ba6a281315676934c44225b034
[2/2] soc: qcom: llcc: Use guards for mutex handling
      commit: eda32f68ce7a3d16071870c7af0803fdfae40401

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-03-16  2:02 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-06  3:12 [PATCH 0/2] Fix slice accounting and simplify descriptor and locking logic Francisco Munoz Ruiz
2026-03-06  3:12 ` [PATCH 1/2] soc: qcom: llcc: Add per-slice counter and common llcc slice descriptor Francisco Munoz Ruiz
2026-03-06  3:12 ` [PATCH 2/2] soc: qcom: llcc: Use guards for mutex handling Francisco Munoz Ruiz
2026-03-16  2:02 ` [PATCH 0/2] Fix slice accounting and simplify descriptor and locking logic Bjorn Andersson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox