[PATCH v1 1/2] cxl/edac: Fix the min_scrub_cycle of a region miscalculation

[PATCH v1 1/2] cxl/edac: Fix the min_scrub_cycle of a region miscalculation

[Li Ming]

When trying to update the scrub_cycle value of a cxl region, which means
updating the scrub_cycle value of each memdev under a cxl region. cxl
driver needs to guarantee the new scrub_cycle value is greater than the
min_scrub_cycle value of a memdev, otherwise the updating operation will
fail(Per Table 8-223 in CXL r3.2 section 8.2.10.9.11.1).

Current implementation logic of getting the min_scrub_cycle value of a
cxl region is that getting the min_scrub_cycle value of each memdevs
under the cxl region, then using the minimum min_scrub_cycle value as
the region's min_scrub_cycle. Checking if the new scrub_cycle value is
greater than this value. If yes, updating the new scrub_cycle value to
each memdevs. The issue is that the new scrub_cycle value is possibly
greater than the minimum min_scrub_cycle value of all memdevs but less
than the maximum min_scrub_cycle value of all memdevs if memdevs have
a different min_scrub_cycle value. The updating operation will always
fail on these memdevs which have a greater min_scrub_cycle than the new
scrub_cycle.

The correct implementation logic is to get the maximum value of these
memdevs' min_scrub_cycle, check if the new scrub_cycle value is greater
than the value. If yes, the new scrub_cycle value is fit for the region.

The change also impacts the result of
cxl_patrol_scrub_get_min_scrub_cycle(), the interface returned the
minimum min_scrub_cycle value among all memdevs under the region before
the change. The interface will return the maximum min_scrub_cycle value
among all memdevs under the region with the change.

Signed-off-by: Li Ming <ming.li@zohomail.com>
---
Changes from RFC:
1. Add more description about the max scrub cycle. (Alison)
2. Add more description about the min scrub cycle of a region. (Alison)
3. Drop RFC tag.

base-commit: 9f153b7fb5ae45c7d426851f896487927f40e501 cxl/next
---
 drivers/cxl/core/edac.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/cxl/core/edac.c b/drivers/cxl/core/edac.c
index 2cbc664e5d62..0ef245d0bd9f 100644
--- a/drivers/cxl/core/edac.c
+++ b/drivers/cxl/core/edac.c
@@ -103,10 +103,10 @@ static int cxl_scrub_get_attrbs(struct cxl_patrol_scrub_context *cxl_ps_ctx,
 				u8 *cap, u16 *cycle, u8 *flags, u8 *min_cycle)
 {
 	struct cxl_mailbox *cxl_mbox;
-	u8 min_scrub_cycle = U8_MAX;
 	struct cxl_region_params *p;
 	struct cxl_memdev *cxlmd;
 	struct cxl_region *cxlr;
+	u8 min_scrub_cycle = 0;
 	int i, ret;
 
 	if (!cxl_ps_ctx->cxlr) {
@@ -133,8 +133,12 @@ static int cxl_scrub_get_attrbs(struct cxl_patrol_scrub_context *cxl_ps_ctx,
 		if (ret)
 			return ret;
 
+		/*
+		 * The min_scrub_cycle of a region is the max of minimum scrub
+		 * cycles supported by memdevs that back the region.
+		 */
 		if (min_cycle)
-			min_scrub_cycle = min(*min_cycle, min_scrub_cycle);
+			min_scrub_cycle = max(*min_cycle, min_scrub_cycle);
 	}
 
 	if (min_cycle)
-- 
2.34.1

[PATCH v1 2/2] cxl/Documentation: Add more description about min/max scrub cycle

[Li Ming]

user can configurare scrub cycle for a region or a memory device via
sysfs interface. Currently, these interfaces have not enough description
for the return value. So adding return value description to these
interfaces.

Suggested-by: Alison Schofield <alison.schofield@intel.com>
Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
Signed-off-by: Li Ming <ming.li@zohomail.com>
---
 Documentation/ABI/testing/sysfs-edac-scrub | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-edac-scrub b/Documentation/ABI/testing/sysfs-edac-scrub
index c43be90deab4..ab6014743da5 100644
--- a/Documentation/ABI/testing/sysfs-edac-scrub
+++ b/Documentation/ABI/testing/sysfs-edac-scrub
@@ -49,6 +49,12 @@ Description:
 		(RO) Supported minimum scrub cycle duration in seconds
 		by the memory scrubber.
 
+		Device-based scrub: returns the minimum scrub cycle
+		supported by the memory device.
+
+		Region-based scrub: returns the max of minimum scrub cycles
+		supported by individual memory devices that back the region.
+
 What:		/sys/bus/edac/devices/<dev-name>/scrubX/max_cycle_duration
 Date:		March 2025
 KernelVersion:	6.15
@@ -57,6 +63,16 @@ Description:
 		(RO) Supported maximum scrub cycle duration in seconds
 		by the memory scrubber.
 
+		Device-based scrub: returns the maximum scrub cycle supported
+		by the memory device.
+
+		Region-based scrub: returns the min of maximum scrub cycles
+		supported by individual memory devices that back the region.
+
+		If the memory device does not provide maximum scrub cycle
+		information, return the maximum supported value of the scrub
+		cycle field.
+
 What:		/sys/bus/edac/devices/<dev-name>/scrubX/current_cycle_duration
 Date:		March 2025
 KernelVersion:	6.15
-- 
2.34.1

RE: [PATCH v1 1/2] cxl/edac: Fix the min_scrub_cycle of a region miscalculation

[Shiju Jose]

>-----Original Message-----
>From: Li Ming <ming.li@zohomail.com>
>Sent: 03 June 2025 11:43
>To: dave@stgolabs.net; Jonathan Cameron <jonathan.cameron@huawei.com>;
>dave.jiang@intel.com; alison.schofield@intel.com; vishal.l.verma@intel.com;
>ira.weiny@intel.com; dan.j.williams@intel.com; Shiju Jose
><shiju.jose@huawei.com>
>Cc: linux-cxl@vger.kernel.org; linux-kernel@vger.kernel.org; Li Ming
><ming.li@zohomail.com>
>Subject: [PATCH v1 1/2] cxl/edac: Fix the min_scrub_cycle of a region
>miscalculation
>
>When trying to update the scrub_cycle value of a cxl region, which means
>updating the scrub_cycle value of each memdev under a cxl region. cxl driver
>needs to guarantee the new scrub_cycle value is greater than the
>min_scrub_cycle value of a memdev, otherwise the updating operation will
>fail(Per Table 8-223 in CXL r3.2 section 8.2.10.9.11.1).
>
>Current implementation logic of getting the min_scrub_cycle value of a cxl
>region is that getting the min_scrub_cycle value of each memdevs under the cxl
>region, then using the minimum min_scrub_cycle value as the region's
>min_scrub_cycle. Checking if the new scrub_cycle value is greater than this
>value. If yes, updating the new scrub_cycle value to each memdevs. The issue is
>that the new scrub_cycle value is possibly greater than the minimum
>min_scrub_cycle value of all memdevs but less than the maximum
>min_scrub_cycle value of all memdevs if memdevs have a different
>min_scrub_cycle value. The updating operation will always fail on these
>memdevs which have a greater min_scrub_cycle than the new scrub_cycle.
>
>The correct implementation logic is to get the maximum value of these
>memdevs' min_scrub_cycle, check if the new scrub_cycle value is greater than
>the value. If yes, the new scrub_cycle value is fit for the region.
>
>The change also impacts the result of
>cxl_patrol_scrub_get_min_scrub_cycle(), the interface returned the minimum
>min_scrub_cycle value among all memdevs under the region before the change.
>The interface will return the maximum min_scrub_cycle value among all
>memdevs under the region with the change.
>

Reviewed-by: Shiju Jose <shiju.jose@huawei.com>

>Signed-off-by: Li Ming <ming.li@zohomail.com>
>---
>Changes from RFC:
>1. Add more description about the max scrub cycle. (Alison) 2. Add more
>description about the min scrub cycle of a region. (Alison) 3. Drop RFC tag.
>
>base-commit: 9f153b7fb5ae45c7d426851f896487927f40e501 cxl/next
>---
> drivers/cxl/core/edac.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/cxl/core/edac.c b/drivers/cxl/core/edac.c index
>2cbc664e5d62..0ef245d0bd9f 100644
>--- a/drivers/cxl/core/edac.c
>+++ b/drivers/cxl/core/edac.c
>@@ -103,10 +103,10 @@ static int cxl_scrub_get_attrbs(struct
>cxl_patrol_scrub_context *cxl_ps_ctx,
> 				u8 *cap, u16 *cycle, u8 *flags, u8 *min_cycle)
>{
> 	struct cxl_mailbox *cxl_mbox;
>-	u8 min_scrub_cycle = U8_MAX;
> 	struct cxl_region_params *p;
> 	struct cxl_memdev *cxlmd;
> 	struct cxl_region *cxlr;
>+	u8 min_scrub_cycle = 0;
> 	int i, ret;
>
> 	if (!cxl_ps_ctx->cxlr) {
>@@ -133,8 +133,12 @@ static int cxl_scrub_get_attrbs(struct
>cxl_patrol_scrub_context *cxl_ps_ctx,
> 		if (ret)
> 			return ret;
>
>+		/*
>+		 * The min_scrub_cycle of a region is the max of minimum
>scrub
>+		 * cycles supported by memdevs that back the region.
>+		 */
> 		if (min_cycle)
>-			min_scrub_cycle = min(*min_cycle, min_scrub_cycle);
>+			min_scrub_cycle = max(*min_cycle, min_scrub_cycle);
> 	}
>
> 	if (min_cycle)
>--
>2.34.1

Re: [PATCH v1 1/2] cxl/edac: Fix the min_scrub_cycle of a region miscalculation

[Dave Jiang]

On 6/3/25 3:43 AM, Li Ming wrote:
> When trying to update the scrub_cycle value of a cxl region, which means
> updating the scrub_cycle value of each memdev under a cxl region. cxl
> driver needs to guarantee the new scrub_cycle value is greater than the
> min_scrub_cycle value of a memdev, otherwise the updating operation will
> fail(Per Table 8-223 in CXL r3.2 section 8.2.10.9.11.1).
> 
> Current implementation logic of getting the min_scrub_cycle value of a
> cxl region is that getting the min_scrub_cycle value of each memdevs
> under the cxl region, then using the minimum min_scrub_cycle value as
> the region's min_scrub_cycle. Checking if the new scrub_cycle value is
> greater than this value. If yes, updating the new scrub_cycle value to
> each memdevs. The issue is that the new scrub_cycle value is possibly
> greater than the minimum min_scrub_cycle value of all memdevs but less
> than the maximum min_scrub_cycle value of all memdevs if memdevs have
> a different min_scrub_cycle value. The updating operation will always
> fail on these memdevs which have a greater min_scrub_cycle than the new
> scrub_cycle.
> 
> The correct implementation logic is to get the maximum value of these
> memdevs' min_scrub_cycle, check if the new scrub_cycle value is greater
> than the value. If yes, the new scrub_cycle value is fit for the region.
> 
> The change also impacts the result of
> cxl_patrol_scrub_get_min_scrub_cycle(), the interface returned the
> minimum min_scrub_cycle value among all memdevs under the region before
> the change. The interface will return the maximum min_scrub_cycle value
> among all memdevs under the region with the change.
> 
> Signed-off-by: Li Ming <ming.li@zohomail.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
> Changes from RFC:
> 1. Add more description about the max scrub cycle. (Alison)
> 2. Add more description about the min scrub cycle of a region. (Alison)
> 3. Drop RFC tag.
> 
> base-commit: 9f153b7fb5ae45c7d426851f896487927f40e501 cxl/next
> ---
>  drivers/cxl/core/edac.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cxl/core/edac.c b/drivers/cxl/core/edac.c
> index 2cbc664e5d62..0ef245d0bd9f 100644
> --- a/drivers/cxl/core/edac.c
> +++ b/drivers/cxl/core/edac.c
> @@ -103,10 +103,10 @@ static int cxl_scrub_get_attrbs(struct cxl_patrol_scrub_context *cxl_ps_ctx,
>  				u8 *cap, u16 *cycle, u8 *flags, u8 *min_cycle)
>  {
>  	struct cxl_mailbox *cxl_mbox;
> -	u8 min_scrub_cycle = U8_MAX;
>  	struct cxl_region_params *p;
>  	struct cxl_memdev *cxlmd;
>  	struct cxl_region *cxlr;
> +	u8 min_scrub_cycle = 0;
>  	int i, ret;
>  
>  	if (!cxl_ps_ctx->cxlr) {
> @@ -133,8 +133,12 @@ static int cxl_scrub_get_attrbs(struct cxl_patrol_scrub_context *cxl_ps_ctx,
>  		if (ret)
>  			return ret;
>  
> +		/*
> +		 * The min_scrub_cycle of a region is the max of minimum scrub
> +		 * cycles supported by memdevs that back the region.
> +		 */
>  		if (min_cycle)
> -			min_scrub_cycle = min(*min_cycle, min_scrub_cycle);
> +			min_scrub_cycle = max(*min_cycle, min_scrub_cycle);
>  	}
>  
>  	if (min_cycle)

Re: [PATCH v1 2/2] cxl/Documentation: Add more description about min/max scrub cycle

[Dave Jiang]

On 6/3/25 3:43 AM, Li Ming wrote:
> user can configurare scrub cycle for a region or a memory device via

s/configurare/configure/

> sysfs interface. Currently, these interfaces have not enough description
> for the return value. So adding return value description to these
> interfaces.
> 
> Suggested-by: Alison Schofield <alison.schofield@intel.com>
> Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
> Signed-off-by: Li Ming <ming.li@zohomail.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  Documentation/ABI/testing/sysfs-edac-scrub | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-edac-scrub b/Documentation/ABI/testing/sysfs-edac-scrub
> index c43be90deab4..ab6014743da5 100644
> --- a/Documentation/ABI/testing/sysfs-edac-scrub
> +++ b/Documentation/ABI/testing/sysfs-edac-scrub
> @@ -49,6 +49,12 @@ Description:
>  		(RO) Supported minimum scrub cycle duration in seconds
>  		by the memory scrubber.
>  
> +		Device-based scrub: returns the minimum scrub cycle
> +		supported by the memory device.
> +
> +		Region-based scrub: returns the max of minimum scrub cycles
> +		supported by individual memory devices that back the region.
> +
>  What:		/sys/bus/edac/devices/<dev-name>/scrubX/max_cycle_duration
>  Date:		March 2025
>  KernelVersion:	6.15
> @@ -57,6 +63,16 @@ Description:
>  		(RO) Supported maximum scrub cycle duration in seconds
>  		by the memory scrubber.
>  
> +		Device-based scrub: returns the maximum scrub cycle supported
> +		by the memory device.
> +
> +		Region-based scrub: returns the min of maximum scrub cycles
> +		supported by individual memory devices that back the region.
> +
> +		If the memory device does not provide maximum scrub cycle
> +		information, return the maximum supported value of the scrub
> +		cycle field.
> +
>  What:		/sys/bus/edac/devices/<dev-name>/scrubX/current_cycle_duration
>  Date:		March 2025
>  KernelVersion:	6.15

Re: [PATCH v1 2/2] cxl/Documentation: Add more description about min/max scrub cycle

[Jonathan Cameron]

On Tue,  3 Jun 2025 18:43:14 +0800
Li Ming <ming.li@zohomail.com> wrote:

> user can configurare scrub cycle for a region or a memory device via
> sysfs interface. Currently, these interfaces have not enough description
> for the return value. So adding return value description to these
> interfaces.
> 
> Suggested-by: Alison Schofield <alison.schofield@intel.com>
> Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
> Signed-off-by: Li Ming <ming.li@zohomail.com>
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

Re: [PATCH v1 1/2] cxl/edac: Fix the min_scrub_cycle of a region miscalculation

[Jonathan Cameron]

On Tue,  3 Jun 2025 18:43:13 +0800
Li Ming <ming.li@zohomail.com> wrote:

> When trying to update the scrub_cycle value of a cxl region, which means
> updating the scrub_cycle value of each memdev under a cxl region. cxl
> driver needs to guarantee the new scrub_cycle value is greater than the
> min_scrub_cycle value of a memdev, otherwise the updating operation will
> fail(Per Table 8-223 in CXL r3.2 section 8.2.10.9.11.1).
> 
> Current implementation logic of getting the min_scrub_cycle value of a
> cxl region is that getting the min_scrub_cycle value of each memdevs
> under the cxl region, then using the minimum min_scrub_cycle value as
> the region's min_scrub_cycle. Checking if the new scrub_cycle value is
> greater than this value. If yes, updating the new scrub_cycle value to
> each memdevs. The issue is that the new scrub_cycle value is possibly
> greater than the minimum min_scrub_cycle value of all memdevs but less
> than the maximum min_scrub_cycle value of all memdevs if memdevs have
> a different min_scrub_cycle value. The updating operation will always
> fail on these memdevs which have a greater min_scrub_cycle than the new
> scrub_cycle.
> 
> The correct implementation logic is to get the maximum value of these
> memdevs' min_scrub_cycle, check if the new scrub_cycle value is greater
> than the value. If yes, the new scrub_cycle value is fit for the region.
> 
> The change also impacts the result of
> cxl_patrol_scrub_get_min_scrub_cycle(), the interface returned the
> minimum min_scrub_cycle value among all memdevs under the region before
> the change. The interface will return the maximum min_scrub_cycle value
> among all memdevs under the region with the change.
> 
> Signed-off-by: Li Ming <ming.li@zohomail.com>
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

Thanks for fixing this up.

Re: [PATCH v1 1/2] cxl/edac: Fix the min_scrub_cycle of a region miscalculation

[Dave Jiang]

On 6/3/25 3:43 AM, Li Ming wrote:
> When trying to update the scrub_cycle value of a cxl region, which means
> updating the scrub_cycle value of each memdev under a cxl region. cxl
> driver needs to guarantee the new scrub_cycle value is greater than the
> min_scrub_cycle value of a memdev, otherwise the updating operation will
> fail(Per Table 8-223 in CXL r3.2 section 8.2.10.9.11.1).
> 
> Current implementation logic of getting the min_scrub_cycle value of a
> cxl region is that getting the min_scrub_cycle value of each memdevs
> under the cxl region, then using the minimum min_scrub_cycle value as
> the region's min_scrub_cycle. Checking if the new scrub_cycle value is
> greater than this value. If yes, updating the new scrub_cycle value to
> each memdevs. The issue is that the new scrub_cycle value is possibly
> greater than the minimum min_scrub_cycle value of all memdevs but less
> than the maximum min_scrub_cycle value of all memdevs if memdevs have
> a different min_scrub_cycle value. The updating operation will always
> fail on these memdevs which have a greater min_scrub_cycle than the new
> scrub_cycle.
> 
> The correct implementation logic is to get the maximum value of these
> memdevs' min_scrub_cycle, check if the new scrub_cycle value is greater
> than the value. If yes, the new scrub_cycle value is fit for the region.
> 
> The change also impacts the result of
> cxl_patrol_scrub_get_min_scrub_cycle(), the interface returned the
> minimum min_scrub_cycle value among all memdevs under the region before
> the change. The interface will return the maximum min_scrub_cycle value
> among all memdevs under the region with the change.
> 
> Signed-off-by: Li Ming <ming.li@zohomail.com>

Both patches applied to cxl/next

> ---
> Changes from RFC:
> 1. Add more description about the max scrub cycle. (Alison)
> 2. Add more description about the min scrub cycle of a region. (Alison)
> 3. Drop RFC tag.
> 
> base-commit: 9f153b7fb5ae45c7d426851f896487927f40e501 cxl/next
> ---
>  drivers/cxl/core/edac.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cxl/core/edac.c b/drivers/cxl/core/edac.c
> index 2cbc664e5d62..0ef245d0bd9f 100644
> --- a/drivers/cxl/core/edac.c
> +++ b/drivers/cxl/core/edac.c
> @@ -103,10 +103,10 @@ static int cxl_scrub_get_attrbs(struct cxl_patrol_scrub_context *cxl_ps_ctx,
>  				u8 *cap, u16 *cycle, u8 *flags, u8 *min_cycle)
>  {
>  	struct cxl_mailbox *cxl_mbox;
> -	u8 min_scrub_cycle = U8_MAX;
>  	struct cxl_region_params *p;
>  	struct cxl_memdev *cxlmd;
>  	struct cxl_region *cxlr;
> +	u8 min_scrub_cycle = 0;
>  	int i, ret;
>  
>  	if (!cxl_ps_ctx->cxlr) {
> @@ -133,8 +133,12 @@ static int cxl_scrub_get_attrbs(struct cxl_patrol_scrub_context *cxl_ps_ctx,
>  		if (ret)
>  			return ret;
>  
> +		/*
> +		 * The min_scrub_cycle of a region is the max of minimum scrub
> +		 * cycles supported by memdevs that back the region.
> +		 */
>  		if (min_cycle)
> -			min_scrub_cycle = min(*min_cycle, min_scrub_cycle);
> +			min_scrub_cycle = max(*min_cycle, min_scrub_cycle);
>  	}
>  
>  	if (min_cycle)

Re: [PATCH v1 1/2] cxl/edac: Fix the min_scrub_cycle of a region miscalculation

[Davidlohr Bueso]

On Tue, 03 Jun 2025, Li Ming wrote:

>When trying to update the scrub_cycle value of a cxl region, which means
>updating the scrub_cycle value of each memdev under a cxl region. cxl
>driver needs to guarantee the new scrub_cycle value is greater than the
>min_scrub_cycle value of a memdev, otherwise the updating operation will
>fail(Per Table 8-223 in CXL r3.2 section 8.2.10.9.11.1).
>
>Current implementation logic of getting the min_scrub_cycle value of a
>cxl region is that getting the min_scrub_cycle value of each memdevs
>under the cxl region, then using the minimum min_scrub_cycle value as
>the region's min_scrub_cycle. Checking if the new scrub_cycle value is
>greater than this value. If yes, updating the new scrub_cycle value to
>each memdevs. The issue is that the new scrub_cycle value is possibly
>greater than the minimum min_scrub_cycle value of all memdevs but less
>than the maximum min_scrub_cycle value of all memdevs if memdevs have
>a different min_scrub_cycle value. The updating operation will always
>fail on these memdevs which have a greater min_scrub_cycle than the new
>scrub_cycle.
>
>The correct implementation logic is to get the maximum value of these
>memdevs' min_scrub_cycle, check if the new scrub_cycle value is greater
>than the value. If yes, the new scrub_cycle value is fit for the region.
>
>The change also impacts the result of
>cxl_patrol_scrub_get_min_scrub_cycle(), the interface returned the
>minimum min_scrub_cycle value among all memdevs under the region before
>the change. The interface will return the maximum min_scrub_cycle value
>among all memdevs under the region with the change.
>
>Signed-off-by: Li Ming <ming.li@zohomail.com>

Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>

Re: [PATCH v1 2/2] cxl/Documentation: Add more description about min/max scrub cycle

[Davidlohr Bueso]

On Tue, 03 Jun 2025, Li Ming wrote:

>user can configurare scrub cycle for a region or a memory device via
>sysfs interface. Currently, these interfaces have not enough description
>for the return value. So adding return value description to these
>interfaces.
>
>Suggested-by: Alison Schofield <alison.schofield@intel.com>
>Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
>Signed-off-by: Li Ming <ming.li@zohomail.com>

Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>