* [PATCH v2] drm/amdgpu: Skip the coredump collection on reset during driver reload
@ 2024-04-17 4:13 Ahmad Rehman
2024-04-17 4:31 ` Lazar, Lijo
2024-04-17 7:36 ` Khatri, Sunil
0 siblings, 2 replies; 6+ messages in thread
From: Ahmad Rehman @ 2024-04-17 4:13 UTC (permalink / raw)
To: amd-gfx; +Cc: lijo.lazar, Ahmad Rehman
In passthrough environment, the driver triggers the mode-1 reset on
reload. The reset causes the core dump collection which is delayed task
and prevents driver from unloading until it is completed. Since we do
not need to collect data on "reset on reload" case, we can skip core
dump collection.
v2: Use the same flag to avoid calling amdgpu_reset_reg_dumps as well.
Signed-off-by: Ahmad Rehman <Ahmad.Rehman@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 7 +++++--
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h | 1 +
3 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 1b2e177bc2d6..c718982cffa8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -5357,7 +5357,9 @@ int amdgpu_do_asic_reset(struct list_head *device_list_handle,
/* Try reset handler method first */
tmp_adev = list_first_entry(device_list_handle, struct amdgpu_device,
reset_list);
- amdgpu_reset_reg_dumps(tmp_adev);
+
+ if (!test_bit(AMDGPU_SKIP_COREDUMP, &reset_context->flags))
+ amdgpu_reset_reg_dumps(tmp_adev);
reset_context->reset_device_list = device_list_handle;
r = amdgpu_reset_perform_reset(tmp_adev, reset_context);
@@ -5430,7 +5432,8 @@ int amdgpu_do_asic_reset(struct list_head *device_list_handle,
vram_lost = amdgpu_device_check_vram_lost(tmp_adev);
- amdgpu_coredump(tmp_adev, vram_lost, reset_context);
+ if (!test_bit(AMDGPU_SKIP_COREDUMP, &reset_context->flags))
+ amdgpu_coredump(tmp_adev, vram_lost, reset_context);
if (vram_lost) {
DRM_INFO("VRAM is lost due to GPU reset!\n");
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 6ea893ad9a36..c512f70b8272 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2481,6 +2481,7 @@ static void amdgpu_drv_delayed_reset_work_handler(struct work_struct *work)
/* Use a common context, just need to make sure full reset is done */
set_bit(AMDGPU_SKIP_HW_RESET, &reset_context.flags);
+ set_bit(AMDGPU_SKIP_COREDUMP, &reset_context.flags);
r = amdgpu_do_asic_reset(&device_list, &reset_context);
if (r) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
index 66125d43cf21..b11d190ece53 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
@@ -32,6 +32,7 @@ enum AMDGPU_RESET_FLAGS {
AMDGPU_NEED_FULL_RESET = 0,
AMDGPU_SKIP_HW_RESET = 1,
+ AMDGPU_SKIP_COREDUMP = 2,
};
struct amdgpu_reset_context {
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] drm/amdgpu: Skip the coredump collection on reset during driver reload
2024-04-17 4:13 [PATCH v2] drm/amdgpu: Skip the coredump collection on reset during driver reload Ahmad Rehman
@ 2024-04-17 4:31 ` Lazar, Lijo
2024-04-17 7:36 ` Khatri, Sunil
1 sibling, 0 replies; 6+ messages in thread
From: Lazar, Lijo @ 2024-04-17 4:31 UTC (permalink / raw)
To: Ahmad Rehman, amd-gfx
On 4/17/2024 9:43 AM, Ahmad Rehman wrote:
> In passthrough environment, the driver triggers the mode-1 reset on
> reload. The reset causes the core dump collection which is delayed task
> and prevents driver from unloading until it is completed. Since we do
> not need to collect data on "reset on reload" case, we can skip core
> dump collection.
>
> v2: Use the same flag to avoid calling amdgpu_reset_reg_dumps as well.
>
> Signed-off-by: Ahmad Rehman <Ahmad.Rehman@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 7 +++++--
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 1 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h | 1 +
> 3 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 1b2e177bc2d6..c718982cffa8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -5357,7 +5357,9 @@ int amdgpu_do_asic_reset(struct list_head *device_list_handle,
> /* Try reset handler method first */
> tmp_adev = list_first_entry(device_list_handle, struct amdgpu_device,
> reset_list);
> - amdgpu_reset_reg_dumps(tmp_adev);
> +
> + if (!test_bit(AMDGPU_SKIP_COREDUMP, &reset_context->flags))
> + amdgpu_reset_reg_dumps(tmp_adev);
This may be saved to some bool. Anyway,
Reviewed-by: Lijo Lazar <lijo.lazar@amd.com>
Thanks,
Lijo
>
> reset_context->reset_device_list = device_list_handle;
> r = amdgpu_reset_perform_reset(tmp_adev, reset_context);
> @@ -5430,7 +5432,8 @@ int amdgpu_do_asic_reset(struct list_head *device_list_handle,
>
> vram_lost = amdgpu_device_check_vram_lost(tmp_adev);
>
> - amdgpu_coredump(tmp_adev, vram_lost, reset_context);
> + if (!test_bit(AMDGPU_SKIP_COREDUMP, &reset_context->flags))
> + amdgpu_coredump(tmp_adev, vram_lost, reset_context);
>
> if (vram_lost) {
> DRM_INFO("VRAM is lost due to GPU reset!\n");
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 6ea893ad9a36..c512f70b8272 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -2481,6 +2481,7 @@ static void amdgpu_drv_delayed_reset_work_handler(struct work_struct *work)
>
> /* Use a common context, just need to make sure full reset is done */
> set_bit(AMDGPU_SKIP_HW_RESET, &reset_context.flags);
> + set_bit(AMDGPU_SKIP_COREDUMP, &reset_context.flags);
> r = amdgpu_do_asic_reset(&device_list, &reset_context);
>
> if (r) {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
> index 66125d43cf21..b11d190ece53 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
> @@ -32,6 +32,7 @@ enum AMDGPU_RESET_FLAGS {
>
> AMDGPU_NEED_FULL_RESET = 0,
> AMDGPU_SKIP_HW_RESET = 1,
> + AMDGPU_SKIP_COREDUMP = 2,
> };
>
> struct amdgpu_reset_context {
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] drm/amdgpu: Skip the coredump collection on reset during driver reload
2024-04-17 4:13 [PATCH v2] drm/amdgpu: Skip the coredump collection on reset during driver reload Ahmad Rehman
2024-04-17 4:31 ` Lazar, Lijo
@ 2024-04-17 7:36 ` Khatri, Sunil
2024-04-17 7:44 ` Khatri, Sunil
1 sibling, 1 reply; 6+ messages in thread
From: Khatri, Sunil @ 2024-04-17 7:36 UTC (permalink / raw)
To: Ahmad Rehman, amd-gfx; +Cc: lijo.lazar
devcoredump is used to debug gpu hangs/resets. So in normal process when
there is a hang due to ring timeout or page fault we are doing a hard
reset as soft reset fail in those cases. How are we making sure that the
devcoredump is triggered in those cases and captured?
Regards
Sunil Khatri
On 4/17/2024 9:43 AM, Ahmad Rehman wrote:
> In passthrough environment, the driver triggers the mode-1 reset on
> reload. The reset causes the core dump collection which is delayed task
> and prevents driver from unloading until it is completed. Since we do
> not need to collect data on "reset on reload" case, we can skip core
> dump collection.
>
> v2: Use the same flag to avoid calling amdgpu_reset_reg_dumps as well.
>
> Signed-off-by: Ahmad Rehman <Ahmad.Rehman@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 7 +++++--
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 1 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h | 1 +
> 3 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 1b2e177bc2d6..c718982cffa8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -5357,7 +5357,9 @@ int amdgpu_do_asic_reset(struct list_head *device_list_handle,
> /* Try reset handler method first */
> tmp_adev = list_first_entry(device_list_handle, struct amdgpu_device,
> reset_list);
> - amdgpu_reset_reg_dumps(tmp_adev);
> +
> + if (!test_bit(AMDGPU_SKIP_COREDUMP, &reset_context->flags))
> + amdgpu_reset_reg_dumps(tmp_adev);
>
> reset_context->reset_device_list = device_list_handle;
> r = amdgpu_reset_perform_reset(tmp_adev, reset_context);
> @@ -5430,7 +5432,8 @@ int amdgpu_do_asic_reset(struct list_head *device_list_handle,
>
> vram_lost = amdgpu_device_check_vram_lost(tmp_adev);
>
> - amdgpu_coredump(tmp_adev, vram_lost, reset_context);
> + if (!test_bit(AMDGPU_SKIP_COREDUMP, &reset_context->flags))
> + amdgpu_coredump(tmp_adev, vram_lost, reset_context);
>
> if (vram_lost) {
> DRM_INFO("VRAM is lost due to GPU reset!\n");
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 6ea893ad9a36..c512f70b8272 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -2481,6 +2481,7 @@ static void amdgpu_drv_delayed_reset_work_handler(struct work_struct *work)
>
> /* Use a common context, just need to make sure full reset is done */
> set_bit(AMDGPU_SKIP_HW_RESET, &reset_context.flags);
> + set_bit(AMDGPU_SKIP_COREDUMP, &reset_context.flags);
> r = amdgpu_do_asic_reset(&device_list, &reset_context);
>
> if (r) {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
> index 66125d43cf21..b11d190ece53 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
> @@ -32,6 +32,7 @@ enum AMDGPU_RESET_FLAGS {
>
> AMDGPU_NEED_FULL_RESET = 0,
> AMDGPU_SKIP_HW_RESET = 1,
> + AMDGPU_SKIP_COREDUMP = 2,
> };
>
> struct amdgpu_reset_context {
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] drm/amdgpu: Skip the coredump collection on reset during driver reload
2024-04-17 7:36 ` Khatri, Sunil
@ 2024-04-17 7:44 ` Khatri, Sunil
2024-04-17 7:49 ` Lazar, Lijo
0 siblings, 1 reply; 6+ messages in thread
From: Khatri, Sunil @ 2024-04-17 7:44 UTC (permalink / raw)
To: Ahmad Rehman, amd-gfx; +Cc: lijo.lazar
On 4/17/2024 1:06 PM, Khatri, Sunil wrote:
> devcoredump is used to debug gpu hangs/resets. So in normal process
> when there is a hang due to ring timeout or page fault we are doing a
> hard reset as soft reset fail in those cases. How are we making sure
> that the devcoredump is triggered in those cases and captured?
>
> Regards
> Sunil Khatri
>
> On 4/17/2024 9:43 AM, Ahmad Rehman wrote:
>> In passthrough environment, the driver triggers the mode-1 reset on
>> reload. The reset causes the core dump collection which is delayed task
>> and prevents driver from unloading until it is completed. Since we do
>> not need to collect data on "reset on reload" case, we can skip core
>> dump collection.
>>
>> v2: Use the same flag to avoid calling amdgpu_reset_reg_dumps as well.
>>
>> Signed-off-by: Ahmad Rehman <Ahmad.Rehman@amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 7 +++++--
>> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 1 +
>> drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h | 1 +
>> 3 files changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index 1b2e177bc2d6..c718982cffa8 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -5357,7 +5357,9 @@ int amdgpu_do_asic_reset(struct list_head
>> *device_list_handle,
>> /* Try reset handler method first */
>> tmp_adev = list_first_entry(device_list_handle, struct
>> amdgpu_device,
>> reset_list);
>> - amdgpu_reset_reg_dumps(tmp_adev);
>> +
>> + if (!test_bit(AMDGPU_SKIP_COREDUMP, &reset_context->flags))
>> + amdgpu_reset_reg_dumps(tmp_adev);
>> reset_context->reset_device_list = device_list_handle;
>> r = amdgpu_reset_perform_reset(tmp_adev, reset_context);
>> @@ -5430,7 +5432,8 @@ int amdgpu_do_asic_reset(struct list_head
>> *device_list_handle,
>> vram_lost = amdgpu_device_check_vram_lost(tmp_adev);
>> - amdgpu_coredump(tmp_adev, vram_lost, reset_context);
>> + if (!test_bit(AMDGPU_SKIP_COREDUMP,
>> &reset_context->flags))
>> + amdgpu_coredump(tmp_adev, vram_lost,
>> reset_context);
>> if (vram_lost) {
>> DRM_INFO("VRAM is lost due to GPU reset!\n");
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> index 6ea893ad9a36..c512f70b8272 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> @@ -2481,6 +2481,7 @@ static void
>> amdgpu_drv_delayed_reset_work_handler(struct work_struct *work)
>> /* Use a common context, just need to make sure full reset is
>> done */
>> set_bit(AMDGPU_SKIP_HW_RESET, &reset_context.flags);
>> + set_bit(AMDGPU_SKIP_COREDUMP, &reset_context.flags);
If this is used for guests only can we better have a flag like
amdgpu_sriov_vf for setting the skip coredump flag ??
Regards
Sunil khatri
>> r = amdgpu_do_asic_reset(&device_list, &reset_context);
>> if (r) {
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
>> index 66125d43cf21..b11d190ece53 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
>> @@ -32,6 +32,7 @@ enum AMDGPU_RESET_FLAGS {
>> AMDGPU_NEED_FULL_RESET = 0,
>> AMDGPU_SKIP_HW_RESET = 1,
>> + AMDGPU_SKIP_COREDUMP = 2,
>> };
>> struct amdgpu_reset_context {
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] drm/amdgpu: Skip the coredump collection on reset during driver reload
2024-04-17 7:44 ` Khatri, Sunil
@ 2024-04-17 7:49 ` Lazar, Lijo
2024-04-17 8:24 ` Khatri, Sunil
0 siblings, 1 reply; 6+ messages in thread
From: Lazar, Lijo @ 2024-04-17 7:49 UTC (permalink / raw)
To: Khatri, Sunil, Ahmad Rehman, amd-gfx
On 4/17/2024 1:14 PM, Khatri, Sunil wrote:
>
> On 4/17/2024 1:06 PM, Khatri, Sunil wrote:
>> devcoredump is used to debug gpu hangs/resets. So in normal process
>> when there is a hang due to ring timeout or page fault we are doing a
>> hard reset as soft reset fail in those cases. How are we making sure
>> that the devcoredump is triggered in those cases and captured?
>>
>> Regards
>> Sunil Khatri
>>
>> On 4/17/2024 9:43 AM, Ahmad Rehman wrote:
>>> In passthrough environment, the driver triggers the mode-1 reset on
>>> reload. The reset causes the core dump collection which is delayed task
>>> and prevents driver from unloading until it is completed. Since we do
>>> not need to collect data on "reset on reload" case, we can skip core
>>> dump collection.
>>>
>>> v2: Use the same flag to avoid calling amdgpu_reset_reg_dumps as well.
>>>
>>> Signed-off-by: Ahmad Rehman <Ahmad.Rehman@amd.com>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 7 +++++--
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 1 +
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h | 1 +
>>> 3 files changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> index 1b2e177bc2d6..c718982cffa8 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> @@ -5357,7 +5357,9 @@ int amdgpu_do_asic_reset(struct list_head
>>> *device_list_handle,
>>> /* Try reset handler method first */
>>> tmp_adev = list_first_entry(device_list_handle, struct
>>> amdgpu_device,
>>> reset_list);
>>> - amdgpu_reset_reg_dumps(tmp_adev);
>>> +
>>> + if (!test_bit(AMDGPU_SKIP_COREDUMP, &reset_context->flags))
>>> + amdgpu_reset_reg_dumps(tmp_adev);
>>> reset_context->reset_device_list = device_list_handle;
>>> r = amdgpu_reset_perform_reset(tmp_adev, reset_context);
>>> @@ -5430,7 +5432,8 @@ int amdgpu_do_asic_reset(struct list_head
>>> *device_list_handle,
>>> vram_lost = amdgpu_device_check_vram_lost(tmp_adev);
>>> - amdgpu_coredump(tmp_adev, vram_lost, reset_context);
>>> + if (!test_bit(AMDGPU_SKIP_COREDUMP,
>>> &reset_context->flags))
>>> + amdgpu_coredump(tmp_adev, vram_lost,
>>> reset_context);
>>> if (vram_lost) {
>>> DRM_INFO("VRAM is lost due to GPU reset!\n");
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> index 6ea893ad9a36..c512f70b8272 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> @@ -2481,6 +2481,7 @@ static void
>>> amdgpu_drv_delayed_reset_work_handler(struct work_struct *work)
>>> /* Use a common context, just need to make sure full reset is
>>> done */
>>> set_bit(AMDGPU_SKIP_HW_RESET, &reset_context.flags);
>>> + set_bit(AMDGPU_SKIP_COREDUMP, &reset_context.flags);
> If this is used for guests only can we better have a flag like
> amdgpu_sriov_vf for setting the skip coredump flag ??
>
A reset is not always triggered just because of hang. There are other
cases like we want to do a reset after a suspend/resume cycle so that
the device starts from a clean state. Those are intentionally triggered
by driver. Also, there are case like RAS errors where we reset and that
also really doesn't need a core dump. In all such cases, this flag is
required, and this is one such case (this patch only addresses passthrough).
Thanks,
Lijo
> Regards
> Sunil khatri
>
>>> r = amdgpu_do_asic_reset(&device_list, &reset_context);
>>> if (r) {
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
>>> index 66125d43cf21..b11d190ece53 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
>>> @@ -32,6 +32,7 @@ enum AMDGPU_RESET_FLAGS {
>>> AMDGPU_NEED_FULL_RESET = 0,
>>> AMDGPU_SKIP_HW_RESET = 1,
>>> + AMDGPU_SKIP_COREDUMP = 2,
>>> };
>>> struct amdgpu_reset_context {
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] drm/amdgpu: Skip the coredump collection on reset during driver reload
2024-04-17 7:49 ` Lazar, Lijo
@ 2024-04-17 8:24 ` Khatri, Sunil
0 siblings, 0 replies; 6+ messages in thread
From: Khatri, Sunil @ 2024-04-17 8:24 UTC (permalink / raw)
To: Lazar, Lijo, Ahmad Rehman, amd-gfx
On 4/17/2024 1:19 PM, Lazar, Lijo wrote:
>
> On 4/17/2024 1:14 PM, Khatri, Sunil wrote:
>> On 4/17/2024 1:06 PM, Khatri, Sunil wrote:
>>> devcoredump is used to debug gpu hangs/resets. So in normal process
>>> when there is a hang due to ring timeout or page fault we are doing a
>>> hard reset as soft reset fail in those cases. How are we making sure
>>> that the devcoredump is triggered in those cases and captured?
>>>
>>> Regards
>>> Sunil Khatri
>>>
>>> On 4/17/2024 9:43 AM, Ahmad Rehman wrote:
>>>> In passthrough environment, the driver triggers the mode-1 reset on
>>>> reload. The reset causes the core dump collection which is delayed task
>>>> and prevents driver from unloading until it is completed. Since we do
>>>> not need to collect data on "reset on reload" case, we can skip core
>>>> dump collection.
>>>>
>>>> v2: Use the same flag to avoid calling amdgpu_reset_reg_dumps as well.
>>>>
>>>> Signed-off-by: Ahmad Rehman <Ahmad.Rehman@amd.com>
>>>> ---
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 7 +++++--
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 1 +
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h | 1 +
>>>> 3 files changed, 7 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> index 1b2e177bc2d6..c718982cffa8 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> @@ -5357,7 +5357,9 @@ int amdgpu_do_asic_reset(struct list_head
>>>> *device_list_handle,
>>>> /* Try reset handler method first */
>>>> tmp_adev = list_first_entry(device_list_handle, struct
>>>> amdgpu_device,
>>>> reset_list);
>>>> - amdgpu_reset_reg_dumps(tmp_adev);
>>>> +
>>>> + if (!test_bit(AMDGPU_SKIP_COREDUMP, &reset_context->flags))
>>>> + amdgpu_reset_reg_dumps(tmp_adev);
>>>> reset_context->reset_device_list = device_list_handle;
>>>> r = amdgpu_reset_perform_reset(tmp_adev, reset_context);
>>>> @@ -5430,7 +5432,8 @@ int amdgpu_do_asic_reset(struct list_head
>>>> *device_list_handle,
>>>> vram_lost = amdgpu_device_check_vram_lost(tmp_adev);
>>>> - amdgpu_coredump(tmp_adev, vram_lost, reset_context);
>>>> + if (!test_bit(AMDGPU_SKIP_COREDUMP,
>>>> &reset_context->flags))
>>>> + amdgpu_coredump(tmp_adev, vram_lost,
>>>> reset_context);
>>>> if (vram_lost) {
>>>> DRM_INFO("VRAM is lost due to GPU reset!\n");
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>>> index 6ea893ad9a36..c512f70b8272 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>>> @@ -2481,6 +2481,7 @@ static void
>>>> amdgpu_drv_delayed_reset_work_handler(struct work_struct *work)
>>>> /* Use a common context, just need to make sure full reset is
>>>> done */
>>>> set_bit(AMDGPU_SKIP_HW_RESET, &reset_context.flags);
>>>> + set_bit(AMDGPU_SKIP_COREDUMP, &reset_context.flags);
>> If this is used for guests only can we better have a flag like
>> amdgpu_sriov_vf for setting the skip coredump flag ??
>>
> A reset is not always triggered just because of hang. There are other
> cases like we want to do a reset after a suspend/resume cycle so that
> the device starts from a clean state. Those are intentionally triggered
> by driver. Also, there are case like RAS errors where we reset and that
> also really doesn't need a core dump. In all such cases, this flag is
> required, and this is one such case (this patch only addresses passthrough).
Thanks Lijo
Able to verify that in normal hangs dump is working.
Regards
Sunil
>
> Thanks,
> Lijo
>
>> Regards
>> Sunil khatri
>>
>>>> r = amdgpu_do_asic_reset(&device_list, &reset_context);
>>>> if (r) {
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
>>>> index 66125d43cf21..b11d190ece53 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
>>>> @@ -32,6 +32,7 @@ enum AMDGPU_RESET_FLAGS {
>>>> AMDGPU_NEED_FULL_RESET = 0,
>>>> AMDGPU_SKIP_HW_RESET = 1,
>>>> + AMDGPU_SKIP_COREDUMP = 2,
>>>> };
>>>> struct amdgpu_reset_context {
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-04-17 8:24 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-17 4:13 [PATCH v2] drm/amdgpu: Skip the coredump collection on reset during driver reload Ahmad Rehman
2024-04-17 4:31 ` Lazar, Lijo
2024-04-17 7:36 ` Khatri, Sunil
2024-04-17 7:44 ` Khatri, Sunil
2024-04-17 7:49 ` Lazar, Lijo
2024-04-17 8:24 ` Khatri, Sunil
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.