[PATCH 1/2] drm/amdgpu: Warn when bad pages approaches 90% threshold

[PATCH 1/2] drm/amdgpu: Warn when bad pages approaches 90% threshold

[Kent Russell]

dmesg doesn't warn when the number of bad pages approaches the
threshold for page retirement. WARN when the number of bad pages
is at 90% or greater for easier checks and planning, instead of waiting
until the GPU is full of bad pages.

Cc: Luben Tuikov <luben.tuikov@amd.com>
Cc: Mukul Joshi <Mukul.Joshi@amd.com>
Signed-off-by: Kent Russell <kent.russell@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
index f4c05ff4b26c..ce5089216474 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
@@ -1077,6 +1077,12 @@ int amdgpu_ras_eeprom_init(struct amdgpu_ras_eeprom_control *control,
 		if (res)
 			DRM_ERROR("RAS table incorrect checksum or error:%d\n",
 				  res);
+
+		/* Warn if we are at 90% of the threshold or above */
+		if ((10 * control->ras_num_recs) >= (ras->bad_page_cnt_threshold * 9))
+			DRM_WARN("RAS records:%u exceeds 90%% of threshold:%d",
+					control->ras_num_recs,
+					ras->bad_page_cnt_threshold);
 	} else if (hdr->header == RAS_TABLE_HDR_BAD &&
 		   amdgpu_bad_page_threshold != 0) {
 		res = __verify_ras_table_checksum(control);
-- 
2.25.1

[PATCH 2/2] drm/amdgpu: Add kernel parameter support for ignoring bad page threshold

[Kent Russell]

When a GPU hits the bad_page_threshold, it will not be initialized by
the amdgpu driver. This means that the table cannot be cleared, nor can
information gathering be performed (getting serial number, BDF, etc).

If the bad_page_threshold kernel parameter is set to -2,
continue to initialize the GPU, while printing a warning to dmesg that
this action has been done

Cc: Luben Tuikov <luben.tuikov@amd.com>
Cc: Mukul Joshi <Mukul.Joshi@amd.com>
Signed-off-by: Kent Russell <kent.russell@amd.com>
Acked-by: Felix Kuehling <Felix.Kuehling@amd.com>
Reviewed-by: Luben Tuikov <luben.tuikov@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h            |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c        |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c | 12 ++++++++----
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index d58e37fd01f4..b85b67a88a3d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -205,6 +205,7 @@ extern struct amdgpu_mgpu_info mgpu_info;
 extern int amdgpu_ras_enable;
 extern uint amdgpu_ras_mask;
 extern int amdgpu_bad_page_threshold;
+extern bool amdgpu_ignore_bad_page_threshold;
 extern struct amdgpu_watchdog_timer amdgpu_watchdog_timer;
 extern int amdgpu_async_gfx_ring;
 extern int amdgpu_mcbp;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 96bd63aeeddd..eee3cf874e7a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -877,7 +877,7 @@ module_param_named(reset_method, amdgpu_reset_method, int, 0444);
  * result in the GPU entering bad status when the number of total
  * faulty pages by ECC exceeds the threshold value.
  */
-MODULE_PARM_DESC(bad_page_threshold, "Bad page threshold(-1 = auto(default value), 0 = disable bad page retirement)");
+MODULE_PARM_DESC(bad_page_threshold, "Bad page threshold(-1 = auto(default value), 0 = disable bad page retirement, -2 = ignore bad page threshold)");
 module_param_named(bad_page_threshold, amdgpu_bad_page_threshold, int, 0444);
 
 MODULE_PARM_DESC(num_kcq, "number of kernel compute queue user want to setup (8 if set to greater than 8 or less than 0, only affect gfx 8+)");
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
index ce5089216474..bd6ed43b0df2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
@@ -1104,11 +1104,15 @@ int amdgpu_ras_eeprom_init(struct amdgpu_ras_eeprom_control *control,
 			res = amdgpu_ras_eeprom_correct_header_tag(control,
 								   RAS_TABLE_HDR_VAL);
 		} else {
-			*exceed_err_limit = true;
-			dev_err(adev->dev,
-				"RAS records:%d exceed threshold:%d, "
-				"GPU will not be initialized. Replace this GPU or increase the threshold",
+			dev_err(adev->dev, "RAS records:%d exceed threshold:%d",
 				control->ras_num_recs, ras->bad_page_cnt_threshold);
+			if (amdgpu_bad_page_threshold == -2) {
+				dev_warn(adev->dev, "GPU will be initialized due to bad_page_threshold = -2.");
+				res = 0;
+			} else {
+				*exceed_err_limit = true;
+				dev_err(adev->dev, "GPU will not be initialized. Replace this GPU or increase the threshold.");
+			}
 		}
 	} else {
 		DRM_INFO("Creating a new EEPROM table");
-- 
2.25.1

Re: [PATCH 2/2] drm/amdgpu: Add kernel parameter support for ignoring bad page threshold

[Luben Tuikov]

On 2021-10-21 11:57, Kent Russell wrote:
> When a GPU hits the bad_page_threshold, it will not be initialized by
> the amdgpu driver. This means that the table cannot be cleared, nor can
> information gathering be performed (getting serial number, BDF, etc).
>
> If the bad_page_threshold kernel parameter is set to -2,
> continue to initialize the GPU, while printing a warning to dmesg that
> this action has been done
>
> Cc: Luben Tuikov <luben.tuikov@amd.com>
> Cc: Mukul Joshi <Mukul.Joshi@amd.com>
> Signed-off-by: Kent Russell <kent.russell@amd.com>
> Acked-by: Felix Kuehling <Felix.Kuehling@amd.com>
> Reviewed-by: Luben Tuikov <luben.tuikov@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h            |  1 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c        |  2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c | 12 ++++++++----
>  3 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index d58e37fd01f4..b85b67a88a3d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -205,6 +205,7 @@ extern struct amdgpu_mgpu_info mgpu_info;
>  extern int amdgpu_ras_enable;
>  extern uint amdgpu_ras_mask;
>  extern int amdgpu_bad_page_threshold;
> +extern bool amdgpu_ignore_bad_page_threshold;
>  extern struct amdgpu_watchdog_timer amdgpu_watchdog_timer;
>  extern int amdgpu_async_gfx_ring;
>  extern int amdgpu_mcbp;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 96bd63aeeddd..eee3cf874e7a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -877,7 +877,7 @@ module_param_named(reset_method, amdgpu_reset_method, int, 0444);
>   * result in the GPU entering bad status when the number of total
>   * faulty pages by ECC exceeds the threshold value.
>   */
> -MODULE_PARM_DESC(bad_page_threshold, "Bad page threshold(-1 = auto(default value), 0 = disable bad page retirement)");
> +MODULE_PARM_DESC(bad_page_threshold, "Bad page threshold(-1 = auto(default value), 0 = disable bad page retirement, -2 = ignore bad page threshold)");
>  module_param_named(bad_page_threshold, amdgpu_bad_page_threshold, int, 0444);
>  
>  MODULE_PARM_DESC(num_kcq, "number of kernel compute queue user want to setup (8 if set to greater than 8 or less than 0, only affect gfx 8+)");
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
> index ce5089216474..bd6ed43b0df2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
> @@ -1104,11 +1104,15 @@ int amdgpu_ras_eeprom_init(struct amdgpu_ras_eeprom_control *control,
>  			res = amdgpu_ras_eeprom_correct_header_tag(control,
>  								   RAS_TABLE_HDR_VAL);
>  		} else {
> -			*exceed_err_limit = true;
> -			dev_err(adev->dev,
> -				"RAS records:%d exceed threshold:%d, "
> -				"GPU will not be initialized. Replace this GPU or increase the threshold",
> +			dev_err(adev->dev, "RAS records:%d exceed threshold:%d",
>  				control->ras_num_recs, ras->bad_page_cnt_threshold); 

I thought this would all go in a single set of patches. I wasn't aware a singleton patch went in already which changed just this line--this change was always a part of a patch set.

Regards,
Luben

> +			if (amdgpu_bad_page_threshold == -2) {
> +				dev_warn(adev->dev, "GPU will be initialized due to bad_page_threshold = -2.");
> +				res = 0;
> +			} else {
> +				*exceed_err_limit = true;
> +				dev_err(adev->dev, "GPU will not be initialized. Replace this GPU or increase the threshold.");
> +			}
>  		}
>  	} else {
>  		DRM_INFO("Creating a new EEPROM table");

RE: [PATCH 2/2] drm/amdgpu: Add kernel parameter support for ignoring bad page threshold

[Russell, Kent]

[AMD Official Use Only]



> -----Original Message-----
> From: Tuikov, Luben <Luben.Tuikov@amd.com>
> Sent: Thursday, October 21, 2021 12:21 PM
> To: Russell, Kent <Kent.Russell@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Joshi, Mukul <Mukul.Joshi@amd.com>; Kuehling, Felix <Felix.Kuehling@amd.com>;
> Tuikov, Luben <Luben.Tuikov@amd.com>
> Subject: Re: [PATCH 2/2] drm/amdgpu: Add kernel parameter support for ignoring bad page
> threshold
> 
> On 2021-10-21 11:57, Kent Russell wrote:
> > When a GPU hits the bad_page_threshold, it will not be initialized by
> > the amdgpu driver. This means that the table cannot be cleared, nor can
> > information gathering be performed (getting serial number, BDF, etc).
> >
> > If the bad_page_threshold kernel parameter is set to -2,
> > continue to initialize the GPU, while printing a warning to dmesg that
> > this action has been done
> >
> > Cc: Luben Tuikov <luben.tuikov@amd.com>
> > Cc: Mukul Joshi <Mukul.Joshi@amd.com>
> > Signed-off-by: Kent Russell <kent.russell@amd.com>
> > Acked-by: Felix Kuehling <Felix.Kuehling@amd.com>
> > Reviewed-by: Luben Tuikov <luben.tuikov@amd.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu.h            |  1 +
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c        |  2 +-
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c | 12 ++++++++----
> >  3 files changed, 10 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > index d58e37fd01f4..b85b67a88a3d 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > @@ -205,6 +205,7 @@ extern struct amdgpu_mgpu_info mgpu_info;
> >  extern int amdgpu_ras_enable;
> >  extern uint amdgpu_ras_mask;
> >  extern int amdgpu_bad_page_threshold;
> > +extern bool amdgpu_ignore_bad_page_threshold;
> >  extern struct amdgpu_watchdog_timer amdgpu_watchdog_timer;
> >  extern int amdgpu_async_gfx_ring;
> >  extern int amdgpu_mcbp;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > index 96bd63aeeddd..eee3cf874e7a 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > @@ -877,7 +877,7 @@ module_param_named(reset_method, amdgpu_reset_method,
> int, 0444);
> >   * result in the GPU entering bad status when the number of total
> >   * faulty pages by ECC exceeds the threshold value.
> >   */
> > -MODULE_PARM_DESC(bad_page_threshold, "Bad page threshold(-1 = auto(default
> value), 0 = disable bad page retirement)");
> > +MODULE_PARM_DESC(bad_page_threshold, "Bad page threshold(-1 = auto(default
> value), 0 = disable bad page retirement, -2 = ignore bad page threshold)");
> >  module_param_named(bad_page_threshold, amdgpu_bad_page_threshold, int, 0444);
> >
> >  MODULE_PARM_DESC(num_kcq, "number of kernel compute queue user want to setup
> (8 if set to greater than 8 or less than 0, only affect gfx 8+)");
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
> > index ce5089216474..bd6ed43b0df2 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
> > @@ -1104,11 +1104,15 @@ int amdgpu_ras_eeprom_init(struct
> amdgpu_ras_eeprom_control *control,
> >  			res = amdgpu_ras_eeprom_correct_header_tag(control,
> >  								   RAS_TABLE_HDR_VAL);
> >  		} else {
> > -			*exceed_err_limit = true;
> > -			dev_err(adev->dev,
> > -				"RAS records:%d exceed threshold:%d, "
> > -				"GPU will not be initialized. Replace this GPU or increase the
> threshold",
> > +			dev_err(adev->dev, "RAS records:%d exceed threshold:%d",
> >  				control->ras_num_recs, ras->bad_page_cnt_threshold);
> 
> I thought this would all go in a single set of patches. I wasn't aware a singleton patch went
> in already which changed just this line--this change was always a part of a patch set.
> 

Ah sorry. When you reviewed the original patch2 clarifying the message, I merged it and then re-submitted the remaining 3 (which pared down to 2) for review. Sorry for the confusion, I was trying to minimize the number of moving parts.

 Kent

> Regards,
> Luben
> 
> > +			if (amdgpu_bad_page_threshold == -2) {
> > +				dev_warn(adev->dev, "GPU will be initialized due to
> bad_page_threshold = -2.");
> > +				res = 0;
> > +			} else {
> > +				*exceed_err_limit = true;
> > +				dev_err(adev->dev, "GPU will not be initialized. Replace this
> GPU or increase the threshold.");
> > +			}
> >  		}
> >  	} else {
> >  		DRM_INFO("Creating a new EEPROM table");

Re: [PATCH 2/2] drm/amdgpu: Add kernel parameter support for ignoring bad page threshold

[Luben Tuikov]

On 2021-10-21 12:42, Russell, Kent wrote:
> [AMD Official Use Only]
>
>
>
>> -----Original Message-----
>> From: Tuikov, Luben <Luben.Tuikov@amd.com>
>> Sent: Thursday, October 21, 2021 12:21 PM
>> To: Russell, Kent <Kent.Russell@amd.com>; amd-gfx@lists.freedesktop.org
>> Cc: Joshi, Mukul <Mukul.Joshi@amd.com>; Kuehling, Felix <Felix.Kuehling@amd.com>;
>> Tuikov, Luben <Luben.Tuikov@amd.com>
>> Subject: Re: [PATCH 2/2] drm/amdgpu: Add kernel parameter support for ignoring bad page
>> threshold
>>
>> On 2021-10-21 11:57, Kent Russell wrote:
>>> When a GPU hits the bad_page_threshold, it will not be initialized by
>>> the amdgpu driver. This means that the table cannot be cleared, nor can
>>> information gathering be performed (getting serial number, BDF, etc).
>>>
>>> If the bad_page_threshold kernel parameter is set to -2,
>>> continue to initialize the GPU, while printing a warning to dmesg that
>>> this action has been done
>>>
>>> Cc: Luben Tuikov <luben.tuikov@amd.com>
>>> Cc: Mukul Joshi <Mukul.Joshi@amd.com>
>>> Signed-off-by: Kent Russell <kent.russell@amd.com>
>>> Acked-by: Felix Kuehling <Felix.Kuehling@amd.com>
>>> Reviewed-by: Luben Tuikov <luben.tuikov@amd.com>
>>> ---
>>>  drivers/gpu/drm/amd/amdgpu/amdgpu.h            |  1 +
>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c        |  2 +-
>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c | 12 ++++++++----
>>>  3 files changed, 10 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> index d58e37fd01f4..b85b67a88a3d 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> @@ -205,6 +205,7 @@ extern struct amdgpu_mgpu_info mgpu_info;
>>>  extern int amdgpu_ras_enable;
>>>  extern uint amdgpu_ras_mask;
>>>  extern int amdgpu_bad_page_threshold;
>>> +extern bool amdgpu_ignore_bad_page_threshold;
>>>  extern struct amdgpu_watchdog_timer amdgpu_watchdog_timer;
>>>  extern int amdgpu_async_gfx_ring;
>>>  extern int amdgpu_mcbp;
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> index 96bd63aeeddd..eee3cf874e7a 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> @@ -877,7 +877,7 @@ module_param_named(reset_method, amdgpu_reset_method,
>> int, 0444);
>>>   * result in the GPU entering bad status when the number of total
>>>   * faulty pages by ECC exceeds the threshold value.
>>>   */
>>> -MODULE_PARM_DESC(bad_page_threshold, "Bad page threshold(-1 = auto(default
>> value), 0 = disable bad page retirement)");
>>> +MODULE_PARM_DESC(bad_page_threshold, "Bad page threshold(-1 = auto(default
>> value), 0 = disable bad page retirement, -2 = ignore bad page threshold)");
>>>  module_param_named(bad_page_threshold, amdgpu_bad_page_threshold, int, 0444);
>>>
>>>  MODULE_PARM_DESC(num_kcq, "number of kernel compute queue user want to setup
>> (8 if set to greater than 8 or less than 0, only affect gfx 8+)");
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
>>> index ce5089216474..bd6ed43b0df2 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
>>> @@ -1104,11 +1104,15 @@ int amdgpu_ras_eeprom_init(struct
>> amdgpu_ras_eeprom_control *control,
>>>  			res = amdgpu_ras_eeprom_correct_header_tag(control,
>>>  								   RAS_TABLE_HDR_VAL);
>>>  		} else {
>>> -			*exceed_err_limit = true;
>>> -			dev_err(adev->dev,
>>> -				"RAS records:%d exceed threshold:%d, "
>>> -				"GPU will not be initialized. Replace this GPU or increase the
>> threshold",
>>> +			dev_err(adev->dev, "RAS records:%d exceed threshold:%d",
>>>  				control->ras_num_recs, ras->bad_page_cnt_threshold);
>> I thought this would all go in a single set of patches. I wasn't aware a singleton patch went
>> in already which changed just this line--this change was always a part of a patch set.
>>
> Ah sorry. When you reviewed the original patch2 clarifying the message, I merged it and then re-submitted the remaining 3 (which pared down to 2) for review. Sorry for the confusion, I was trying to minimize the number of moving parts.

Admittedly, now you have 3 patches, one singleton and two coming in. Would've probably be best to submit only the current two.

No worries for now--for the future.

Regards,
Luben

>
>  Kent
>
>> Regards,
>> Luben
>>
>>> +			if (amdgpu_bad_page_threshold == -2) {
>>> +				dev_warn(adev->dev, "GPU will be initialized due to
>> bad_page_threshold = -2.");
>>> +				res = 0;
>>> +			} else {
>>> +				*exceed_err_limit = true;
>>> +				dev_err(adev->dev, "GPU will not be initialized. Replace this
>> GPU or increase the threshold.");
>>> +			}
>>>  		}
>>>  	} else {
>>>  		DRM_INFO("Creating a new EEPROM table");

RE: [PATCH 2/2] drm/amdgpu: Add kernel parameter support for ignoring bad page threshold

[Russell, Kent]

[AMD Official Use Only]



> -----Original Message-----
> From: Tuikov, Luben <Luben.Tuikov@amd.com>
> Sent: Thursday, October 21, 2021 12:47 PM
> To: Russell, Kent <Kent.Russell@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Joshi, Mukul <Mukul.Joshi@amd.com>; Kuehling, Felix <Felix.Kuehling@amd.com>
> Subject: Re: [PATCH 2/2] drm/amdgpu: Add kernel parameter support for ignoring bad page
> threshold
> 
> On 2021-10-21 12:42, Russell, Kent wrote:
> > [AMD Official Use Only]
> >
> >
> >
> >> -----Original Message-----
> >> From: Tuikov, Luben <Luben.Tuikov@amd.com>
> >> Sent: Thursday, October 21, 2021 12:21 PM
> >> To: Russell, Kent <Kent.Russell@amd.com>; amd-gfx@lists.freedesktop.org
> >> Cc: Joshi, Mukul <Mukul.Joshi@amd.com>; Kuehling, Felix <Felix.Kuehling@amd.com>;
> >> Tuikov, Luben <Luben.Tuikov@amd.com>
> >> Subject: Re: [PATCH 2/2] drm/amdgpu: Add kernel parameter support for ignoring bad
> page
> >> threshold
> >>
> >> On 2021-10-21 11:57, Kent Russell wrote:
> >>> When a GPU hits the bad_page_threshold, it will not be initialized by
> >>> the amdgpu driver. This means that the table cannot be cleared, nor can
> >>> information gathering be performed (getting serial number, BDF, etc).
> >>>
> >>> If the bad_page_threshold kernel parameter is set to -2,
> >>> continue to initialize the GPU, while printing a warning to dmesg that
> >>> this action has been done
> >>>
> >>> Cc: Luben Tuikov <luben.tuikov@amd.com>
> >>> Cc: Mukul Joshi <Mukul.Joshi@amd.com>
> >>> Signed-off-by: Kent Russell <kent.russell@amd.com>
> >>> Acked-by: Felix Kuehling <Felix.Kuehling@amd.com>
> >>> Reviewed-by: Luben Tuikov <luben.tuikov@amd.com>
> >>> ---
> >>>  drivers/gpu/drm/amd/amdgpu/amdgpu.h            |  1 +
> >>>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c        |  2 +-
> >>>  drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c | 12 ++++++++----
> >>>  3 files changed, 10 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >>> index d58e37fd01f4..b85b67a88a3d 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >>> @@ -205,6 +205,7 @@ extern struct amdgpu_mgpu_info mgpu_info;
> >>>  extern int amdgpu_ras_enable;
> >>>  extern uint amdgpu_ras_mask;
> >>>  extern int amdgpu_bad_page_threshold;
> >>> +extern bool amdgpu_ignore_bad_page_threshold;
> >>>  extern struct amdgpu_watchdog_timer amdgpu_watchdog_timer;
> >>>  extern int amdgpu_async_gfx_ring;
> >>>  extern int amdgpu_mcbp;
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> >>> index 96bd63aeeddd..eee3cf874e7a 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> >>> @@ -877,7 +877,7 @@ module_param_named(reset_method,
> amdgpu_reset_method,
> >> int, 0444);
> >>>   * result in the GPU entering bad status when the number of total
> >>>   * faulty pages by ECC exceeds the threshold value.
> >>>   */
> >>> -MODULE_PARM_DESC(bad_page_threshold, "Bad page threshold(-1 = auto(default
> >> value), 0 = disable bad page retirement)");
> >>> +MODULE_PARM_DESC(bad_page_threshold, "Bad page threshold(-1 = auto(default
> >> value), 0 = disable bad page retirement, -2 = ignore bad page threshold)");
> >>>  module_param_named(bad_page_threshold, amdgpu_bad_page_threshold, int,
> 0444);
> >>>
> >>>  MODULE_PARM_DESC(num_kcq, "number of kernel compute queue user want to
> setup
> >> (8 if set to greater than 8 or less than 0, only affect gfx 8+)");
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
> >>> index ce5089216474..bd6ed43b0df2 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
> >>> @@ -1104,11 +1104,15 @@ int amdgpu_ras_eeprom_init(struct
> >> amdgpu_ras_eeprom_control *control,
> >>>  			res = amdgpu_ras_eeprom_correct_header_tag(control,
> >>>  								   RAS_TABLE_HDR_VAL);
> >>>  		} else {
> >>> -			*exceed_err_limit = true;
> >>> -			dev_err(adev->dev,
> >>> -				"RAS records:%d exceed threshold:%d, "
> >>> -				"GPU will not be initialized. Replace this GPU or increase the
> >> threshold",
> >>> +			dev_err(adev->dev, "RAS records:%d exceed threshold:%d",
> >>>  				control->ras_num_recs, ras->bad_page_cnt_threshold);
> >> I thought this would all go in a single set of patches. I wasn't aware a singleton patch
> went
> >> in already which changed just this line--this change was always a part of a patch set.
> >>
> > Ah sorry. When you reviewed the original patch2 clarifying the message, I merged it and
> then re-submitted the remaining 3 (which pared down to 2) for review. Sorry for the
> confusion, I was trying to minimize the number of moving parts.
> 
> Admittedly, now you have 3 patches, one singleton and two coming in. Would've probably
> be best to submit only the current two.
> 
> No worries for now--for the future.

Thanks. For the most part, all of my previous multi-patch sets have been all-or-nothing. This is the first time that a patch in the middle got an RB before the rest, so I did the wrong thing and merged it while the rest was still moving. Thanks for bearing with me!

 Kent

> 
> Regards,
> Luben
> 
> >
> >  Kent
> >
> >> Regards,
> >> Luben
> >>
> >>> +			if (amdgpu_bad_page_threshold == -2) {
> >>> +				dev_warn(adev->dev, "GPU will be initialized due to
> >> bad_page_threshold = -2.");
> >>> +				res = 0;
> >>> +			} else {
> >>> +				*exceed_err_limit = true;
> >>> +				dev_err(adev->dev, "GPU will not be initialized. Replace this
> >> GPU or increase the threshold.");
> >>> +			}
> >>>  		}
> >>>  	} else {
> >>>  		DRM_INFO("Creating a new EEPROM table");

Re: [PATCH 2/2] drm/amdgpu: Add kernel parameter support for ignoring bad page threshold

[Luben Tuikov]

[-- Attachment #1: Type: text/html, Size: 11681 bytes --]

Re: [PATCH 1/2] drm/amdgpu: Warn when bad pages approaches 90% threshold

[Lazar, Lijo]

[-- Attachment #1: Type: text/plain, Size: 2096 bytes --]

[Public]

Nit pick - suggest to use dev_warn for easy identification of the device.

Thanks,
Lijo
________________________________
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Kent Russell <kent.russell@amd.com>
Sent: Thursday, October 21, 2021 9:27:10 PM
To: amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Cc: Russell, Kent <Kent.Russell@amd.com>; Tuikov, Luben <Luben.Tuikov@amd.com>; Joshi, Mukul <Mukul.Joshi@amd.com>
Subject: [PATCH 1/2] drm/amdgpu: Warn when bad pages approaches 90% threshold

dmesg doesn't warn when the number of bad pages approaches the
threshold for page retirement. WARN when the number of bad pages
is at 90% or greater for easier checks and planning, instead of waiting
until the GPU is full of bad pages.

Cc: Luben Tuikov <luben.tuikov@amd.com>
Cc: Mukul Joshi <Mukul.Joshi@amd.com>
Signed-off-by: Kent Russell <kent.russell@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
index f4c05ff4b26c..ce5089216474 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
@@ -1077,6 +1077,12 @@ int amdgpu_ras_eeprom_init(struct amdgpu_ras_eeprom_control *control,
                 if (res)
                         DRM_ERROR("RAS table incorrect checksum or error:%d\n",
                                   res);
+
+               /* Warn if we are at 90% of the threshold or above */
+               if ((10 * control->ras_num_recs) >= (ras->bad_page_cnt_threshold * 9))
+                       DRM_WARN("RAS records:%u exceeds 90%% of threshold:%d",
+                                       control->ras_num_recs,
+                                       ras->bad_page_cnt_threshold);
         } else if (hdr->header == RAS_TABLE_HDR_BAD &&
                    amdgpu_bad_page_threshold != 0) {
                 res = __verify_ras_table_checksum(control);
--
2.25.1


[-- Attachment #2: Type: text/html, Size: 4289 bytes --]

RE: [PATCH 1/2] drm/amdgpu: Warn when bad pages approaches 90% threshold

[Russell, Kent]

[-- Attachment #1: Type: text/plain, Size: 3659 bytes --]

[Public]

I had noticed that all of these RAS messages use DRM instead of dev_warn. I wasn't sure if there was a reason for that or not. It's definitely inconsistent.

DRM_ERROR("Partial read for checksum, res:%d\n", res);
DRM_DEBUG_DRIVER("Found existing EEPROM table with %d records",
DRM_ERROR("RAS table incorrect checksum or error:%d\n",
DRM_WARN("RAS records:%u exceeds 90%% of threshold:%d",
DRM_ERROR("RAS Table incorrect checksum or error:%d\n",
dev_info(adev->dev,  "records:%d threshold:%d, resetting RAS table header signature",
dev_err(adev->dev, "RAS records:%d exceed threshold:%d",
dev_warn(adev->dev, "GPU will be initialized due to bad_page_threshold = -2.");
DRM_INFO("Creating a new EEPROM table");

Might be worth making a separate patch to handle those inconsistencies. I agree that device is useful for this kind of error/warning/info

Kent

From: Lazar, Lijo <Lijo.Lazar@amd.com>
Sent: Thursday, October 21, 2021 12:31 PM
To: Russell, Kent <Kent.Russell@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Russell, Kent <Kent.Russell@amd.com>; Tuikov, Luben <Luben.Tuikov@amd.com>; Joshi, Mukul <Mukul.Joshi@amd.com>
Subject: Re: [PATCH 1/2] drm/amdgpu: Warn when bad pages approaches 90% threshold


[Public]

Nit pick - suggest to use dev_warn for easy identification of the device.

Thanks,
Lijo
________________________________
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org<mailto:amd-gfx-bounces@lists.freedesktop.org>> on behalf of Kent Russell <kent.russell@amd.com<mailto:kent.russell@amd.com>>
Sent: Thursday, October 21, 2021 9:27:10 PM
To: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Cc: Russell, Kent <Kent.Russell@amd.com<mailto:Kent.Russell@amd.com>>; Tuikov, Luben <Luben.Tuikov@amd.com<mailto:Luben.Tuikov@amd.com>>; Joshi, Mukul <Mukul.Joshi@amd.com<mailto:Mukul.Joshi@amd.com>>
Subject: [PATCH 1/2] drm/amdgpu: Warn when bad pages approaches 90% threshold

dmesg doesn't warn when the number of bad pages approaches the
threshold for page retirement. WARN when the number of bad pages
is at 90% or greater for easier checks and planning, instead of waiting
until the GPU is full of bad pages.

Cc: Luben Tuikov <luben.tuikov@amd.com<mailto:luben.tuikov@amd.com>>
Cc: Mukul Joshi <Mukul.Joshi@amd.com<mailto:Mukul.Joshi@amd.com>>
Signed-off-by: Kent Russell <kent.russell@amd.com<mailto:kent.russell@amd.com>>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
index f4c05ff4b26c..ce5089216474 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
@@ -1077,6 +1077,12 @@ int amdgpu_ras_eeprom_init(struct amdgpu_ras_eeprom_control *control,
                 if (res)
                         DRM_ERROR("RAS table incorrect checksum or error:%d\n",
                                   res);
+
+               /* Warn if we are at 90% of the threshold or above */
+               if ((10 * control->ras_num_recs) >= (ras->bad_page_cnt_threshold * 9))
+                       DRM_WARN("RAS records:%u exceeds 90%% of threshold:%d",
+                                       control->ras_num_recs,
+                                       ras->bad_page_cnt_threshold);
         } else if (hdr->header == RAS_TABLE_HDR_BAD &&
                    amdgpu_bad_page_threshold != 0) {
                 res = __verify_ras_table_checksum(control);
--
2.25.1

[-- Attachment #2: Type: text/html, Size: 47293 bytes --]

Re: [PATCH 1/2] drm/amdgpu: Warn when bad pages approaches 90% threshold

[Luben Tuikov]

On 2021-10-21 11:57, Kent Russell wrote:
> dmesg doesn't warn when the number of bad pages approaches the
> threshold for page retirement. WARN when the number of bad pages
> is at 90% or greater for easier checks and planning, instead of waiting
> until the GPU is full of bad pages.
>
> Cc: Luben Tuikov <luben.tuikov@amd.com>
> Cc: Mukul Joshi <Mukul.Joshi@amd.com>
> Signed-off-by: Kent Russell <kent.russell@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
> index f4c05ff4b26c..ce5089216474 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
> @@ -1077,6 +1077,12 @@ int amdgpu_ras_eeprom_init(struct amdgpu_ras_eeprom_control *control,
>  		if (res)
>  			DRM_ERROR("RAS table incorrect checksum or error:%d\n",
>  				  res);
> +
> +		/* Warn if we are at 90% of the threshold or above */

The kernel uses a couple of styles, this is one of them:

/* Warn ...
 */
if (...)

Please use this style as it is used extensively in the amdgpu_ras_eeprom.c file.

> +		if ((10 * control->ras_num_recs) >= (ras->bad_page_cnt_threshold * 9))

You don't need the extra parenthesis around multiplication--it has higher precedence than relational operators--drop the extra parenthesis.

Regards,
Luben

> +			DRM_WARN("RAS records:%u exceeds 90%% of threshold:%d",
> +					control->ras_num_recs,
> +					ras->bad_page_cnt_threshold);
>  	} else if (hdr->header == RAS_TABLE_HDR_BAD &&
>  		   amdgpu_bad_page_threshold != 0) {
>  		res = __verify_ras_table_checksum(control);

Re: [PATCH 1/2] drm/amdgpu: Warn when bad pages approaches 90% threshold

[Luben Tuikov]

On 2021-10-21 12:35, Luben Tuikov wrote:
> On 2021-10-21 11:57, Kent Russell wrote:
>> dmesg doesn't warn when the number of bad pages approaches the
>> threshold for page retirement. WARN when the number of bad pages
>> is at 90% or greater for easier checks and planning, instead of waiting
>> until the GPU is full of bad pages.
>>
>> Cc: Luben Tuikov <luben.tuikov@amd.com>
>> Cc: Mukul Joshi <Mukul.Joshi@amd.com>
>> Signed-off-by: Kent Russell <kent.russell@amd.com>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
>> index f4c05ff4b26c..ce5089216474 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
>> @@ -1077,6 +1077,12 @@ int amdgpu_ras_eeprom_init(struct amdgpu_ras_eeprom_control *control,
>>  		if (res)
>>  			DRM_ERROR("RAS table incorrect checksum or error:%d\n",
>>  				  res);
>> +
>> +		/* Warn if we are at 90% of the threshold or above */
> The kernel uses a couple of styles, this is one of them:
>
> /* Warn ...
>  */
> if (...)
>
> Please use this style as it is used extensively in the amdgpu_ras_eeprom.c file.
>
>> +		if ((10 * control->ras_num_recs) >= (ras->bad_page_cnt_threshold * 9))
> You don't need the extra parenthesis around multiplication--it has higher precedence than relational operators--drop the extra parenthesis.
>
> Regards,
> Luben
>
>> +			DRM_WARN("RAS records:%u exceeds 90%% of threshold:%d",
>> +					control->ras_num_recs,
>> +					ras->bad_page_cnt_threshold);

One more note: The code uses "dev_err()" for this very similar message:

            dev_err(adev->dev,
                "RAS records:%d exceed threshold:%d, "
                "GPU will not be initialized. Replace this GPU or increase the threshold",
                control->ras_num_recs, ras->bad_page_cnt_threshold);

Since your message is essentially the same, sans the "90% of threshold", perhaps you want to use dev_warn(), instead of "DRM_WARN()".

Regards,
Luben

>>  	} else if (hdr->header == RAS_TABLE_HDR_BAD &&
>>  		   amdgpu_bad_page_threshold != 0) {
>>  		res = __verify_ras_table_checksum(control);

RE: [PATCH 1/2] drm/amdgpu: Warn when bad pages approaches 90% threshold

[Russell, Kent]

[AMD Official Use Only]



> -----Original Message-----
> From: Tuikov, Luben <Luben.Tuikov@amd.com>
> Sent: Thursday, October 21, 2021 12:45 PM
> To: Russell, Kent <Kent.Russell@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Joshi, Mukul <Mukul.Joshi@amd.com>
> Subject: Re: [PATCH 1/2] drm/amdgpu: Warn when bad pages approaches 90% threshold
> 
> On 2021-10-21 12:35, Luben Tuikov wrote:
> > On 2021-10-21 11:57, Kent Russell wrote:
> >> dmesg doesn't warn when the number of bad pages approaches the
> >> threshold for page retirement. WARN when the number of bad pages
> >> is at 90% or greater for easier checks and planning, instead of waiting
> >> until the GPU is full of bad pages.
> >>
> >> Cc: Luben Tuikov <luben.tuikov@amd.com>
> >> Cc: Mukul Joshi <Mukul.Joshi@amd.com>
> >> Signed-off-by: Kent Russell <kent.russell@amd.com>
> >> ---
> >>  drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c | 6 ++++++
> >>  1 file changed, 6 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
> >> index f4c05ff4b26c..ce5089216474 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
> >> @@ -1077,6 +1077,12 @@ int amdgpu_ras_eeprom_init(struct
> amdgpu_ras_eeprom_control *control,
> >>  		if (res)
> >>  			DRM_ERROR("RAS table incorrect checksum or error:%d\n",
> >>  				  res);
> >> +
> >> +		/* Warn if we are at 90% of the threshold or above */
> > The kernel uses a couple of styles, this is one of them:
> >
> > /* Warn ...
> >  */
> > if (...)
> >
> > Please use this style as it is used extensively in the amdgpu_ras_eeprom.c file.
> >
> >> +		if ((10 * control->ras_num_recs) >= (ras->bad_page_cnt_threshold * 9))
> > You don't need the extra parenthesis around multiplication--it has higher precedence
> than relational operators--drop the extra parenthesis.
> >
> > Regards,
> > Luben
> >
> >> +			DRM_WARN("RAS records:%u exceeds 90%% of threshold:%d",
> >> +					control->ras_num_recs,
> >> +					ras->bad_page_cnt_threshold);
> 
> One more note: The code uses "dev_err()" for this very similar message:
> 
>             dev_err(adev->dev,
>                 "RAS records:%d exceed threshold:%d, "
>                 "GPU will not be initialized. Replace this GPU or increase the threshold",
>                 control->ras_num_recs, ras->bad_page_cnt_threshold);
> 
> Since your message is essentially the same, sans the "90% of threshold", perhaps you want
> to use dev_warn(), instead of "DRM_WARN()".

Agreed. Lijo had a similar comment. I may follow up with another patch to change all of these table-specific DRM_* messages to dev_*

 Kent

> 
> Regards,
> Luben
> 
> >>  	} else if (hdr->header == RAS_TABLE_HDR_BAD &&
> >>  		   amdgpu_bad_page_threshold != 0) {
> >>  		res = __verify_ras_table_checksum(control);

Re: [PATCH 1/2] drm/amdgpu: Warn when bad pages approaches 90% threshold

[Luben Tuikov]

On 2021-10-21 12:47, Russell, Kent wrote:
> [AMD Official Use Only]
>
>
>
>> -----Original Message-----
>> From: Tuikov, Luben <Luben.Tuikov@amd.com>
>> Sent: Thursday, October 21, 2021 12:45 PM
>> To: Russell, Kent <Kent.Russell@amd.com>; amd-gfx@lists.freedesktop.org
>> Cc: Joshi, Mukul <Mukul.Joshi@amd.com>
>> Subject: Re: [PATCH 1/2] drm/amdgpu: Warn when bad pages approaches 90% threshold
>>
>> On 2021-10-21 12:35, Luben Tuikov wrote:
>>> On 2021-10-21 11:57, Kent Russell wrote:
>>>> dmesg doesn't warn when the number of bad pages approaches the
>>>> threshold for page retirement. WARN when the number of bad pages
>>>> is at 90% or greater for easier checks and planning, instead of waiting
>>>> until the GPU is full of bad pages.
>>>>
>>>> Cc: Luben Tuikov <luben.tuikov@amd.com>
>>>> Cc: Mukul Joshi <Mukul.Joshi@amd.com>
>>>> Signed-off-by: Kent Russell <kent.russell@amd.com>
>>>> ---
>>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c | 6 ++++++
>>>>  1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
>>>> index f4c05ff4b26c..ce5089216474 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
>>>> @@ -1077,6 +1077,12 @@ int amdgpu_ras_eeprom_init(struct
>> amdgpu_ras_eeprom_control *control,
>>>>  		if (res)
>>>>  			DRM_ERROR("RAS table incorrect checksum or error:%d\n",
>>>>  				  res);
>>>> +
>>>> +		/* Warn if we are at 90% of the threshold or above */
>>> The kernel uses a couple of styles, this is one of them:
>>>
>>> /* Warn ...
>>>  */
>>> if (...)
>>>
>>> Please use this style as it is used extensively in the amdgpu_ras_eeprom.c file.
>>>
>>>> +		if ((10 * control->ras_num_recs) >= (ras->bad_page_cnt_threshold * 9))
>>> You don't need the extra parenthesis around multiplication--it has higher precedence
>> than relational operators--drop the extra parenthesis.
>>> Regards,
>>> Luben
>>>
>>>> +			DRM_WARN("RAS records:%u exceeds 90%% of threshold:%d",
>>>> +					control->ras_num_recs,
>>>> +					ras->bad_page_cnt_threshold);
>> One more note: The code uses "dev_err()" for this very similar message:
>>
>>             dev_err(adev->dev,
>>                 "RAS records:%d exceed threshold:%d, "
>>                 "GPU will not be initialized. Replace this GPU or increase the threshold",
>>                 control->ras_num_recs, ras->bad_page_cnt_threshold);
>>
>> Since your message is essentially the same, sans the "90% of threshold", perhaps you want
>> to use dev_warn(), instead of "DRM_WARN()".
> Agreed. Lijo had a similar comment. I may follow up with another patch to change all of these table-specific DRM_* messages to dev_*

You can still do that, but for this patch, make the changes I requested and change this to dev_warn().

Regards,
Luben

>
>  Kent
>
>> Regards,
>> Luben
>>
>>>>  	} else if (hdr->header == RAS_TABLE_HDR_BAD &&
>>>>  		   amdgpu_bad_page_threshold != 0) {
>>>>  		res = __verify_ras_table_checksum(control);

Re: [PATCH 1/2] drm/amdgpu: Warn when bad pages approaches 90% threshold

[Felix Kuehling]

Am 2021-10-21 um 12:35 p.m. schrieb Luben Tuikov:
> On 2021-10-21 11:57, Kent Russell wrote:
>> dmesg doesn't warn when the number of bad pages approaches the
>> threshold for page retirement. WARN when the number of bad pages
>> is at 90% or greater for easier checks and planning, instead of waiting
>> until the GPU is full of bad pages.
>>
>> Cc: Luben Tuikov <luben.tuikov@amd.com>
>> Cc: Mukul Joshi <Mukul.Joshi@amd.com>
>> Signed-off-by: Kent Russell <kent.russell@amd.com>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
>> index f4c05ff4b26c..ce5089216474 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
>> @@ -1077,6 +1077,12 @@ int amdgpu_ras_eeprom_init(struct amdgpu_ras_eeprom_control *control,
>>  		if (res)
>>  			DRM_ERROR("RAS table incorrect checksum or error:%d\n",
>>  				  res);
>> +
>> +		/* Warn if we are at 90% of the threshold or above */
> The kernel uses a couple of styles, this is one of them:
>
> /* Warn ...
>  */

That's a waste of space. That means there can be no single-line comments?

checkpatch.pl complains about multi-line comments that don't have the
final */ on their own line. But a single line comment is OK. Let's stick
with the coding style recommended by checkpatch.pl. If we make up our
own arbitrary rules for different reviewers and different parts of the
code, I think it becomes a mine-field of pointless cosmetic fixes for
anyone trespassing on unfamiliar code.


> if (...)
>
> Please use this style as it is used extensively in the amdgpu_ras_eeprom.c file.
>
>> +		if ((10 * control->ras_num_recs) >= (ras->bad_page_cnt_threshold * 9))
> You don't need the extra parenthesis around multiplication--it has higher precedence than relational operators--drop the extra parenthesis.

I agree. With that fixed, the patch is

Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>


>
> Regards,
> Luben
>
>> +			DRM_WARN("RAS records:%u exceeds 90%% of threshold:%d",
>> +					control->ras_num_recs,
>> +					ras->bad_page_cnt_threshold);
>>  	} else if (hdr->header == RAS_TABLE_HDR_BAD &&
>>  		   amdgpu_bad_page_threshold != 0) {
>>  		res = __verify_ras_table_checksum(control);