* [PATCH] drm/amdgpu/userq: remove unnecessary NULL check
@ 2025-04-30 8:05 Dan Carpenter
[not found] ` <MW4PR12MB566769E097E394ED607DBD9CF2832@MW4PR12MB5667.namprd12.prod.outlook.com>
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Dan Carpenter @ 2025-04-30 8:05 UTC (permalink / raw)
To: Alex Deucher
Cc: Christian König, David Airlie, Simona Vetter,
Shashank Sharma, Sunil Khatri, Arvind Yadav,
Arunpravin Paneer Selvam, amd-gfx, dri-devel, linux-kernel,
kernel-janitors
The "ticket" pointer points to in the middle of the &exec struct so it
can't be NULL. Remove the check.
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
index b0e8098a3988..7505d920fb3d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
@@ -631,7 +631,7 @@ amdgpu_userq_validate_bos(struct amdgpu_userq_mgr *uq_mgr)
clear = false;
unlock = true;
/* The caller is already holding the reservation lock */
- } else if (ticket && dma_resv_locking_ctx(resv) == ticket) {
+ } else if (dma_resv_locking_ctx(resv) == ticket) {
clear = false;
unlock = false;
/* Somebody else is using the BO right now */
--
2.47.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/amdgpu/userq: remove unnecessary NULL check
[not found] ` <MW4PR12MB566769E097E394ED607DBD9CF2832@MW4PR12MB5667.namprd12.prod.outlook.com>
@ 2025-04-30 9:49 ` Dan Carpenter
2025-04-30 12:50 ` Sharma, Shashank
2025-04-30 12:35 ` Christian König
1 sibling, 1 reply; 7+ messages in thread
From: Dan Carpenter @ 2025-04-30 9:49 UTC (permalink / raw)
To: Sharma, Shashank
Cc: Deucher, Alexander, Koenig, Christian, David Airlie,
Simona Vetter, Khatri, Sunil, Yadav, Arvind, Paneer Selvam,
Arunpravin, amd-gfx, dri-devel, linux-kernel, kernel-janitors
On Wed, Apr 30, 2025 at 09:28:59AM +0000, Sharma, Shashank wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
>
> Hello Dan,
>
> ________________________________
> From: Dan Carpenter
> Sent: Wednesday, April 30, 2025 10:05 AM
> To: Deucher, Alexander
> Cc: Koenig, Christian; David Airlie; Simona Vetter; Sharma, Shashank; Khatri, Sunil; Yadav, Arvind; Paneer Selvam, Arunpravin; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org; kernel-janitors@vger.kernel.org
> Subject: [PATCH] drm/amdgpu/userq: remove unnecessary NULL check
>
> The "ticket" pointer points to in the middle of the &exec struct so it
> can't be NULL. Remove the check.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> index b0e8098a3988..7505d920fb3d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> @@ -631,7 +631,7 @@ amdgpu_userq_validate_bos(struct amdgpu_userq_mgr *uq_mgr)
> clear = false;
> unlock = true;
> /* The caller is already holding the reservation lock */
> - } else if (ticket && dma_resv_locking_ctx(resv) == ticket) {
> + } else if (dma_resv_locking_ctx(resv) == ticket) {
>
> Its a Nack for me, There are a few situations (particularly during the
> first launch of the desktop, and also when eviction fence and new queue
> creation are working in parallel) where this ticket can be NULL, we
> observed it during the stress validation and hence added this check,
>
It shouldn't be NULL. It sounds like you are experiencing stack
corruption and this is just a bandaid.
drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
566 static int
567 amdgpu_userq_validate_bos(struct amdgpu_userq_mgr *uq_mgr)
568 {
569 struct amdgpu_fpriv *fpriv = uq_mgr_to_fpriv(uq_mgr);
570 struct amdgpu_vm *vm = &fpriv->vm;
571 struct amdgpu_device *adev = uq_mgr->adev;
572 struct amdgpu_bo_va *bo_va;
573 struct ww_acquire_ctx *ticket;
574 struct drm_exec exec;
^^^^^^^^^^^^^^^^^^^^^
The "exec" struct is declared on the stack.
575 struct amdgpu_bo *bo;
576 struct dma_resv *resv;
577 bool clear, unlock;
578 int ret = 0;
579
580 drm_exec_init(&exec, DRM_EXEC_IGNORE_DUPLICATES, 0);
581 drm_exec_until_all_locked(&exec) {
582 ret = amdgpu_vm_lock_pd(vm, &exec, 2);
583 drm_exec_retry_on_contention(&exec);
584 if (unlikely(ret)) {
585 DRM_ERROR("Failed to lock PD\n");
586 goto unlock_all;
587 }
588
589 /* Lock the done list */
590 list_for_each_entry(bo_va, &vm->done, base.vm_status) {
591 bo = bo_va->base.bo;
592 if (!bo)
593 continue;
594
595 ret = drm_exec_lock_obj(&exec, &bo->tbo.base);
596 drm_exec_retry_on_contention(&exec);
597 if (unlikely(ret))
598 goto unlock_all;
599 }
600 }
601
602 spin_lock(&vm->status_lock);
603 while (!list_empty(&vm->moved)) {
604 bo_va = list_first_entry(&vm->moved, struct amdgpu_bo_va,
605 base.vm_status);
606 spin_unlock(&vm->status_lock);
607
608 /* Per VM BOs never need to bo cleared in the page tables */
609 ret = amdgpu_vm_bo_update(adev, bo_va, false);
610 if (ret)
611 goto unlock_all;
612 spin_lock(&vm->status_lock);
613 }
614
615 ticket = &exec.ticket;
^^^^^^^^^^^^^^^^^^^^^
ticket is only set here. We know that &exec is non-NULL because it's
declared on the stack. ticket is 4 bytes into the middle of a non-NULL
struct. It is impossible for ticket to be NULL here.
616 while (!list_empty(&vm->invalidated)) {
617 bo_va = list_first_entry(&vm->invalidated, struct amdgpu_bo_va,
618 base.vm_status);
619 resv = bo_va->base.bo->tbo.base.resv;
620 spin_unlock(&vm->status_lock);
621
622 bo = bo_va->base.bo;
623 ret = amdgpu_userq_validate_vm_bo(NULL, bo);
624 if (ret) {
625 DRM_ERROR("Failed to validate BO\n");
626 goto unlock_all;
627 }
628
629 /* Try to reserve the BO to avoid clearing its ptes */
630 if (!adev->debug_vm && dma_resv_trylock(resv)) {
631 clear = false;
632 unlock = true;
633 /* The caller is already holding the reservation lock */
634 } else if (ticket && dma_resv_locking_ctx(resv) == ticket) {
I've included the whole rest of the function so that we can see it is not
set a second time.
regards,
dan carpenter
635 clear = false;
636 unlock = false;
637 /* Somebody else is using the BO right now */
638 } else {
639 clear = true;
640 unlock = false;
641 }
642
643 ret = amdgpu_vm_bo_update(adev, bo_va, clear);
644
645 if (unlock)
646 dma_resv_unlock(resv);
647 if (ret)
648 goto unlock_all;
649
650 spin_lock(&vm->status_lock);
651 }
652 spin_unlock(&vm->status_lock);
653
654 ret = amdgpu_eviction_fence_replace_fence(&fpriv->evf_mgr, &exec);
655 if (ret)
656 DRM_ERROR("Failed to replace eviction fence\n");
657
658 unlock_all:
659 drm_exec_fini(&exec);
660 return ret;
661 }
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/amdgpu/userq: remove unnecessary NULL check
2025-04-30 8:05 [PATCH] drm/amdgpu/userq: remove unnecessary NULL check Dan Carpenter
[not found] ` <MW4PR12MB566769E097E394ED607DBD9CF2832@MW4PR12MB5667.namprd12.prod.outlook.com>
@ 2025-04-30 12:28 ` Christian König
2025-04-30 14:20 ` Alex Deucher
2 siblings, 0 replies; 7+ messages in thread
From: Christian König @ 2025-04-30 12:28 UTC (permalink / raw)
To: Dan Carpenter, Alex Deucher
Cc: David Airlie, Simona Vetter, Shashank Sharma, Sunil Khatri,
Arvind Yadav, Arunpravin Paneer Selvam, amd-gfx, dri-devel,
linux-kernel, kernel-janitors
On 4/30/25 10:05, Dan Carpenter wrote:
> The "ticket" pointer points to in the middle of the &exec struct so it
> can't be NULL. Remove the check.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
Reviewed-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> index b0e8098a3988..7505d920fb3d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> @@ -631,7 +631,7 @@ amdgpu_userq_validate_bos(struct amdgpu_userq_mgr *uq_mgr)
> clear = false;
> unlock = true;
> /* The caller is already holding the reservation lock */
> - } else if (ticket && dma_resv_locking_ctx(resv) == ticket) {
> + } else if (dma_resv_locking_ctx(resv) == ticket) {
> clear = false;
> unlock = false;
> /* Somebody else is using the BO right now */
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/amdgpu/userq: remove unnecessary NULL check
[not found] ` <MW4PR12MB566769E097E394ED607DBD9CF2832@MW4PR12MB5667.namprd12.prod.outlook.com>
2025-04-30 9:49 ` Dan Carpenter
@ 2025-04-30 12:35 ` Christian König
2025-04-30 12:52 ` Sharma, Shashank
1 sibling, 1 reply; 7+ messages in thread
From: Christian König @ 2025-04-30 12:35 UTC (permalink / raw)
To: Sharma, Shashank, Dan Carpenter, Deucher, Alexander
Cc: David Airlie, Simona Vetter, Khatri, Sunil, Yadav, Arvind,
Paneer Selvam, Arunpravin, amd-gfx, dri-devel, linux-kernel,
kernel-janitors
On 4/30/25 11:28, Sharma, Shashank wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
>
>
> Hello Dan,
>
> --------------------------------------------------------------------------------
> *From:* Dan Carpenter
> *Sent:* Wednesday, April 30, 2025 10:05 AM
> *To:* Deucher, Alexander
> *Cc:* Koenig, Christian; David Airlie; Simona Vetter; Sharma, Shashank; Khatri,
> Sunil; Yadav, Arvind; Paneer Selvam, Arunpravin; amd-gfx@lists.freedesktop.org;
> dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org; kernel-
> janitors@vger.kernel.org
> *Subject:* [PATCH] drm/amdgpu/userq: remove unnecessary NULL check
>
> The "ticket" pointer points to in the middle of the &exec struct so it
> can't be NULL. Remove the check.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/
> amdgpu/amdgpu_userq.c
> index b0e8098a3988..7505d920fb3d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> @@ -631,7 +631,7 @@ amdgpu_userq_validate_bos(struct amdgpu_userq_mgr *uq_mgr)
> clear = false;
> unlock = true;
> /* The caller is already holding the reservation lock */
> - } else if (ticket && dma_resv_locking_ctx(resv) == ticket) {
> + } else if (dma_resv_locking_ctx(resv) == ticket) {
>
> Its a Nack for me, There are a few situations (particularly during the first
> launch of the desktop, and also when eviction fence and new queue creation are
> working in parallel) where this ticket can be NULL, we observed it during the
> stress validation and hence added this check,
What that maybe before the code was moved around?
As far as I can see the ticket can't be NULL any more.
Regards,
Christian.
>
> Regards,
> Shashank
>
>
> clear = false;
> unlock = false;
> /* Somebody else is using the BO right now */
> --
> 2.47.2
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/amdgpu/userq: remove unnecessary NULL check
2025-04-30 9:49 ` Dan Carpenter
@ 2025-04-30 12:50 ` Sharma, Shashank
0 siblings, 0 replies; 7+ messages in thread
From: Sharma, Shashank @ 2025-04-30 12:50 UTC (permalink / raw)
To: Dan Carpenter
Cc: Deucher, Alexander, Koenig, Christian, David Airlie,
Simona Vetter, Khatri, Sunil, Yadav, Arvind, Paneer Selvam,
Arunpravin, amd-gfx, dri-devel, linux-kernel, kernel-janitors
On 30/04/2025 11:49, Dan Carpenter wrote:
> On Wed, Apr 30, 2025 at 09:28:59AM +0000, Sharma, Shashank wrote:
>> [AMD Official Use Only - AMD Internal Distribution Only]
>>
>> Hello Dan,
>>
>> ________________________________
>> From: Dan Carpenter
>> Sent: Wednesday, April 30, 2025 10:05 AM
>> To: Deucher, Alexander
>> Cc: Koenig, Christian; David Airlie; Simona Vetter; Sharma, Shashank; Khatri, Sunil; Yadav, Arvind; Paneer Selvam, Arunpravin; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org; kernel-janitors@vger.kernel.org
>> Subject: [PATCH] drm/amdgpu/userq: remove unnecessary NULL check
>>
>> The "ticket" pointer points to in the middle of the &exec struct so it
>> can't be NULL. Remove the check.
>>
>> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>> index b0e8098a3988..7505d920fb3d 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>> @@ -631,7 +631,7 @@ amdgpu_userq_validate_bos(struct amdgpu_userq_mgr *uq_mgr)
>> clear = false;
>> unlock = true;
>> /* The caller is already holding the reservation lock */
>> - } else if (ticket && dma_resv_locking_ctx(resv) == ticket) {
>> + } else if (dma_resv_locking_ctx(resv) == ticket) {
>>
>> Its a Nack for me, There are a few situations (particularly during the
>> first launch of the desktop, and also when eviction fence and new queue
>> creation are working in parallel) where this ticket can be NULL, we
>> observed it during the stress validation and hence added this check,
>>
> It shouldn't be NULL. It sounds like you are experiencing stack
> corruption and this is just a bandaid.
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> 566 static int
> 567 amdgpu_userq_validate_bos(struct amdgpu_userq_mgr *uq_mgr)
> 568 {
> 569 struct amdgpu_fpriv *fpriv = uq_mgr_to_fpriv(uq_mgr);
> 570 struct amdgpu_vm *vm = &fpriv->vm;
> 571 struct amdgpu_device *adev = uq_mgr->adev;
> 572 struct amdgpu_bo_va *bo_va;
> 573 struct ww_acquire_ctx *ticket;
> 574 struct drm_exec exec;
> ^^^^^^^^^^^^^^^^^^^^^
> The "exec" struct is declared on the stack.
>
> 575 struct amdgpu_bo *bo;
> 576 struct dma_resv *resv;
> 577 bool clear, unlock;
> 578 int ret = 0;
> 579
> 580 drm_exec_init(&exec, DRM_EXEC_IGNORE_DUPLICATES, 0);
> 581 drm_exec_until_all_locked(&exec) {
> 582 ret = amdgpu_vm_lock_pd(vm, &exec, 2);
> 583 drm_exec_retry_on_contention(&exec);
> 584 if (unlikely(ret)) {
> 585 DRM_ERROR("Failed to lock PD\n");
> 586 goto unlock_all;
> 587 }
> 588
> 589 /* Lock the done list */
> 590 list_for_each_entry(bo_va, &vm->done, base.vm_status) {
> 591 bo = bo_va->base.bo;
> 592 if (!bo)
> 593 continue;
> 594
> 595 ret = drm_exec_lock_obj(&exec, &bo->tbo.base);
> 596 drm_exec_retry_on_contention(&exec);
> 597 if (unlikely(ret))
> 598 goto unlock_all;
> 599 }
> 600 }
> 601
> 602 spin_lock(&vm->status_lock);
> 603 while (!list_empty(&vm->moved)) {
> 604 bo_va = list_first_entry(&vm->moved, struct amdgpu_bo_va,
> 605 base.vm_status);
> 606 spin_unlock(&vm->status_lock);
> 607
> 608 /* Per VM BOs never need to bo cleared in the page tables */
> 609 ret = amdgpu_vm_bo_update(adev, bo_va, false);
> 610 if (ret)
> 611 goto unlock_all;
> 612 spin_lock(&vm->status_lock);
> 613 }
> 614
> 615 ticket = &exec.ticket;
> ^^^^^^^^^^^^^^^^^^^^^
> ticket is only set here. We know that &exec is non-NULL because it's
> declared on the stack. ticket is 4 bytes into the middle of a non-NULL
> struct. It is impossible for ticket to be NULL here.
Yep, you are right. I just did a code review, and probably we added that
NULL check before we had the right locks in place, and there was a race
between eviction thread and the UQ create thread, causing corruption.
Please feel free to use Acked-by: Shashank Sharma <shashank.sharma@amd.com>
- Shashank
>
> 616 while (!list_empty(&vm->invalidated)) {
> 617 bo_va = list_first_entry(&vm->invalidated, struct amdgpu_bo_va,
> 618 base.vm_status);
> 619 resv = bo_va->base.bo->tbo.base.resv;
> 620 spin_unlock(&vm->status_lock);
> 621
> 622 bo = bo_va->base.bo;
> 623 ret = amdgpu_userq_validate_vm_bo(NULL, bo);
> 624 if (ret) {
> 625 DRM_ERROR("Failed to validate BO\n");
> 626 goto unlock_all;
> 627 }
> 628
> 629 /* Try to reserve the BO to avoid clearing its ptes */
> 630 if (!adev->debug_vm && dma_resv_trylock(resv)) {
> 631 clear = false;
> 632 unlock = true;
> 633 /* The caller is already holding the reservation lock */
> 634 } else if (ticket && dma_resv_locking_ctx(resv) == ticket) {
>
> I've included the whole rest of the function so that we can see it is not
> set a second time.
>
> regards,
> dan carpenter
>
> 635 clear = false;
> 636 unlock = false;
> 637 /* Somebody else is using the BO right now */
> 638 } else {
> 639 clear = true;
> 640 unlock = false;
> 641 }
> 642
> 643 ret = amdgpu_vm_bo_update(adev, bo_va, clear);
> 644
> 645 if (unlock)
> 646 dma_resv_unlock(resv);
> 647 if (ret)
> 648 goto unlock_all;
> 649
> 650 spin_lock(&vm->status_lock);
> 651 }
> 652 spin_unlock(&vm->status_lock);
> 653
> 654 ret = amdgpu_eviction_fence_replace_fence(&fpriv->evf_mgr, &exec);
> 655 if (ret)
> 656 DRM_ERROR("Failed to replace eviction fence\n");
> 657
> 658 unlock_all:
> 659 drm_exec_fini(&exec);
> 660 return ret;
> 661 }
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/amdgpu/userq: remove unnecessary NULL check
2025-04-30 12:35 ` Christian König
@ 2025-04-30 12:52 ` Sharma, Shashank
0 siblings, 0 replies; 7+ messages in thread
From: Sharma, Shashank @ 2025-04-30 12:52 UTC (permalink / raw)
To: Christian König, Dan Carpenter, Deucher, Alexander
Cc: David Airlie, Simona Vetter, Khatri, Sunil, Yadav, Arvind,
Paneer Selvam, Arunpravin, amd-gfx, dri-devel, linux-kernel,
kernel-janitors
On 30/04/2025 14:35, Christian König wrote:
> On 4/30/25 11:28, Sharma, Shashank wrote:
>> [AMD Official Use Only - AMD Internal Distribution Only]
>>
>>
>> Hello Dan,
>>
>> --------------------------------------------------------------------------------
>> *From:* Dan Carpenter
>> *Sent:* Wednesday, April 30, 2025 10:05 AM
>> *To:* Deucher, Alexander
>> *Cc:* Koenig, Christian; David Airlie; Simona Vetter; Sharma, Shashank; Khatri,
>> Sunil; Yadav, Arvind; Paneer Selvam, Arunpravin; amd-gfx@lists.freedesktop.org;
>> dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org; kernel-
>> janitors@vger.kernel.org
>> *Subject:* [PATCH] drm/amdgpu/userq: remove unnecessary NULL check
>>
>> The "ticket" pointer points to in the middle of the &exec struct so it
>> can't be NULL. Remove the check.
>>
>> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/
>> amdgpu/amdgpu_userq.c
>> index b0e8098a3988..7505d920fb3d 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>> @@ -631,7 +631,7 @@ amdgpu_userq_validate_bos(struct amdgpu_userq_mgr *uq_mgr)
>> clear = false;
>> unlock = true;
>> /* The caller is already holding the reservation lock */
>> - } else if (ticket && dma_resv_locking_ctx(resv) == ticket) {
>> + } else if (dma_resv_locking_ctx(resv) == ticket) {
>>
>> Its a Nack for me, There are a few situations (particularly during the first
>> launch of the desktop, and also when eviction fence and new queue creation are
>> working in parallel) where this ticket can be NULL, we observed it during the
>> stress validation and hence added this check,
> What that maybe before the code was moved around?
>
> As far as I can see the ticket can't be NULL any more.
Yes, that was before we sync'ed the locks between the two threads and
moved the code. The NULL check was probably a leftover from the code
carried forward.
- Shashank
> Regards,
> Christian.
>
>
>> Regards,
>> Shashank
>>
>>
>> clear = false;
>> unlock = false;
>> /* Somebody else is using the BO right now */
>> --
>> 2.47.2
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/amdgpu/userq: remove unnecessary NULL check
2025-04-30 8:05 [PATCH] drm/amdgpu/userq: remove unnecessary NULL check Dan Carpenter
[not found] ` <MW4PR12MB566769E097E394ED607DBD9CF2832@MW4PR12MB5667.namprd12.prod.outlook.com>
2025-04-30 12:28 ` Christian König
@ 2025-04-30 14:20 ` Alex Deucher
2 siblings, 0 replies; 7+ messages in thread
From: Alex Deucher @ 2025-04-30 14:20 UTC (permalink / raw)
To: Dan Carpenter
Cc: Alex Deucher, Christian König, David Airlie, Simona Vetter,
Shashank Sharma, Sunil Khatri, Arvind Yadav,
Arunpravin Paneer Selvam, amd-gfx, dri-devel, linux-kernel,
kernel-janitors
On Wed, Apr 30, 2025 at 4:13 AM Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> The "ticket" pointer points to in the middle of the &exec struct so it
> can't be NULL. Remove the check.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
Applied. Thanks!
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> index b0e8098a3988..7505d920fb3d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> @@ -631,7 +631,7 @@ amdgpu_userq_validate_bos(struct amdgpu_userq_mgr *uq_mgr)
> clear = false;
> unlock = true;
> /* The caller is already holding the reservation lock */
> - } else if (ticket && dma_resv_locking_ctx(resv) == ticket) {
> + } else if (dma_resv_locking_ctx(resv) == ticket) {
> clear = false;
> unlock = false;
> /* Somebody else is using the BO right now */
> --
> 2.47.2
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-04-30 14:21 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-04-30 8:05 [PATCH] drm/amdgpu/userq: remove unnecessary NULL check Dan Carpenter
[not found] ` <MW4PR12MB566769E097E394ED607DBD9CF2832@MW4PR12MB5667.namprd12.prod.outlook.com>
2025-04-30 9:49 ` Dan Carpenter
2025-04-30 12:50 ` Sharma, Shashank
2025-04-30 12:35 ` Christian König
2025-04-30 12:52 ` Sharma, Shashank
2025-04-30 12:28 ` Christian König
2025-04-30 14:20 ` Alex Deucher
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®