From: Dan Carpenter <dan.carpenter@linaro.org>
To: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: intel-xe@lists.freedesktop.org
Subject: [bug report] drm/xe/psmi: Add debugfs interface for PSMI
Date: Tue, 26 Aug 2025 10:51:02 +0300 [thread overview]
Message-ID: <aK1nZjyAF0s7bnHg@stanley.mountain> (raw)
Hello Lucas De Marchi,
Commit aaa0c1f50a3d ("drm/xe/psmi: Add debugfs interface for PSMI")
from Aug 21, 2025 (linux-next), leads to the following Smatch static
checker warning:
drivers/gpu/drm/xe/xe_psmi.c:93 psmi_alloc_object()
error: 'bo' dereferencing possible ERR_PTR()
drivers/gpu/drm/xe/xe_psmi.c
68 static struct xe_bo *psmi_alloc_object(struct xe_device *xe,
69 unsigned int id, size_t bo_size)
70 {
71 struct xe_bo *bo = NULL;
72 struct xe_tile *tile;
73 int err;
74
75 if (!id || !bo_size)
76 return NULL;
I really encourage everyone to document why functions return both error
pointers and NULL. Here "bo_size" can never actually be zero so that's
an impossible path. Presumably id can be zero, but what's the point of
storing a NULL in xe->psmi.capture_obj[0]? It feels like it complicates
things...
77
78 tile = &xe->tiles[id - 1];
79
80 /* VRAM: Allocate GEM object for the capture buffer */
81 bo = xe_bo_create_locked(xe, tile, NULL, bo_size,
82 ttm_bo_type_kernel,
83 XE_BO_FLAG_VRAM_IF_DGFX(tile) |
84 XE_BO_FLAG_PINNED |
85 XE_BO_FLAG_PINNED_LATE_RESTORE |
86 XE_BO_FLAG_NEEDS_CPU_ACCESS);
87
88 if (!IS_ERR(bo)) {
^^^^^^^^^^
Better to flip these around. Always do error handling, never success
handling.
89 /* Buffer written by HW, ensure stays resident */
90 err = xe_bo_pin(bo);
91 if (err)
92 bo = ERR_PTR(err);
^^^^^^^^^^^^^^^^^
bo set to error pointer.
--> 93 xe_bo_unlock(bo);
^^
Dead.
Don't we need to call xe_bo_put() or something to free the bo before
returning? Something like this?
if (IS_ERR(bo))
return ERR_CAST(bo);
/* Buffer written by HW, ensure stays resident */
err = xe_bo_pin(bo);
xe_bo_unlock(bo);
if (err) {
xe_bo_put(bo);
return ERR_PTR(err);
}
return bo;
94 }
95
96 return bo;
97 }
regards,
dan carpenter
next reply other threads:[~2025-08-26 7:51 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-08-26 7:51 Dan Carpenter [this message]
2025-09-22 20:52 ` [bug report] drm/xe/psmi: Add debugfs interface for PSMI Lucas De Marchi
2025-09-23 5:39 ` 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=aK1nZjyAF0s7bnHg@stanley.mountain \
--to=dan.carpenter@linaro.org \
--cc=intel-xe@lists.freedesktop.org \
--cc=lucas.demarchi@intel.com \
/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 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.