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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox