AMD-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Jack.Xiao@amd.com
Cc: amd-gfx@lists.freedesktop.org
Subject: [bug report] drm/amdgpu/mes: use ring for kernel queue submission
Date: Mon, 9 May 2022 10:41:25 +0300	[thread overview]
Message-ID: <YnjFpdmDZex0MhpI@kili> (raw)

Hello Jack Xiao,

The patch d0c423b64765: "drm/amdgpu/mes: use ring for kernel queue
submission" from Mar 27, 2020, leads to the following Smatch static
checker warning:

	drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c:941 amdgpu_mes_add_ring()
	error: double unlocked '&adev->mes.mutex_hidden' (orig line 914)

drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
    848 int amdgpu_mes_add_ring(struct amdgpu_device *adev, int gang_id,
    849                         int queue_type, int idx,
    850                         struct amdgpu_mes_ctx_data *ctx_data,
    851                         struct amdgpu_ring **out)
    852 {
    853         struct amdgpu_ring *ring;
    854         struct amdgpu_mes_gang *gang;
    855         struct amdgpu_mes_queue_properties qprops = {0};
    856         int r, queue_id, pasid;
    857 
    858         /*
    859          * Avoid taking any other locks under MES lock to avoid circular
    860          * lock dependencies.
    861          */
    862         amdgpu_mes_lock(&adev->mes);
    863         gang = idr_find(&adev->mes.gang_id_idr, gang_id);
    864         if (!gang) {
    865                 DRM_ERROR("gang id %d doesn't exist\n", gang_id);
    866                 amdgpu_mes_unlock(&adev->mes);
    867                 return -EINVAL;
    868         }
    869         pasid = gang->process->pasid;
    870 
    871         ring = kzalloc(sizeof(struct amdgpu_ring), GFP_KERNEL);
    872         if (!ring) {
    873                 amdgpu_mes_unlock(&adev->mes);
    874                 return -ENOMEM;
    875         }
    876 
    877         ring->ring_obj = NULL;
    878         ring->use_doorbell = true;
    879         ring->is_mes_queue = true;
    880         ring->mes_ctx = ctx_data;
    881         ring->idx = idx;
    882         ring->no_scheduler = true;
    883 
    884         if (queue_type == AMDGPU_RING_TYPE_COMPUTE) {
    885                 int offset = offsetof(struct amdgpu_mes_ctx_meta_data,
    886                                       compute[ring->idx].mec_hpd);
    887                 ring->eop_gpu_addr =
    888                         amdgpu_mes_ctx_get_offs_gpu_addr(ring, offset);
    889         }
    890 
    891         switch (queue_type) {
    892         case AMDGPU_RING_TYPE_GFX:
    893                 ring->funcs = adev->gfx.gfx_ring[0].funcs;
    894                 break;
    895         case AMDGPU_RING_TYPE_COMPUTE:
    896                 ring->funcs = adev->gfx.compute_ring[0].funcs;
    897                 break;
    898         case AMDGPU_RING_TYPE_SDMA:
    899                 ring->funcs = adev->sdma.instance[0].ring.funcs;
    900                 break;
    901         default:
    902                 BUG();
    903         }
    904 
    905         r = amdgpu_ring_init(adev, ring, 1024, NULL, 0,
    906                              AMDGPU_RING_PRIO_DEFAULT, NULL);
    907         if (r)
    908                 goto clean_up_memory;
    909 
    910         amdgpu_mes_ring_to_queue_props(adev, ring, &qprops);
    911 
    912         dma_fence_wait(gang->process->vm->last_update, false);
    913         dma_fence_wait(ctx_data->meta_data_va->last_pt_update, false);
    914         amdgpu_mes_unlock(&adev->mes);

Unlocked

    915 
    916         r = amdgpu_mes_add_hw_queue(adev, gang_id, &qprops, &queue_id);
    917         if (r)
    918                 goto clean_up_ring;

Double unlocked by goto.  It's weird that this warning is only showing
up now but my guess is that this code was only just released to the
public?

    919 
    920         ring->hw_queue_id = queue_id;
    921         ring->doorbell_index = qprops.doorbell_off;
    922 
    923         if (queue_type == AMDGPU_RING_TYPE_GFX)
    924                 sprintf(ring->name, "gfx_%d.%d.%d", pasid, gang_id, queue_id);
    925         else if (queue_type == AMDGPU_RING_TYPE_COMPUTE)
    926                 sprintf(ring->name, "compute_%d.%d.%d", pasid, gang_id,
    927                         queue_id);
    928         else if (queue_type == AMDGPU_RING_TYPE_SDMA)
    929                 sprintf(ring->name, "sdma_%d.%d.%d", pasid, gang_id,
    930                         queue_id);
    931         else
    932                 BUG();
    933 
    934         *out = ring;
    935         return 0;
    936 
    937 clean_up_ring:
    938         amdgpu_ring_fini(ring);
    939 clean_up_memory:
    940         kfree(ring);
--> 941         amdgpu_mes_unlock(&adev->mes);
    942         return r;
    943 }

regards,
dan carpenter

             reply	other threads:[~2022-05-09  7:41 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-09  7:41 Dan Carpenter [this message]
  -- strict thread matches above, loose matches on Subject: below --
2022-05-09  9:07 [bug report] drm/amdgpu/mes: use ring for kernel queue submission Dan Carpenter
2022-10-26  9:35 Dan Carpenter
2024-10-04 17:22 Dan Carpenter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YnjFpdmDZex0MhpI@kili \
    --to=dan.carpenter@oracle.com \
    --cc=Jack.Xiao@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox