Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incorrect error code passed to structured logging and redundant error formatting.
--
--- Patch [11]: [PATCH 11/12] drm/xe/lmtt: Report invalidation errors using structured xe_log API ---
commit 923aca6a5a5574864c744c6654055fb24cc4ad4d
Author: Dnyaneshwar Bhadane <[email protected]>
drm/xe/lmtt: Report invalidation errors using structured xe_log API
This commit replaces xe_tile_sriov_err() with xe_log_err() using the LMTT
component tag for LMTT and MERT invalidation failures. It attempts to
preserve the SR-IOV prefix in the log messages.
> diff --git a/drivers/gpu/drm/xe/xe_lmtt.c b/drivers/gpu/drm/xe/xe_lmtt.c
> --- a/drivers/gpu/drm/xe/xe_lmtt.c
> +++ b/drivers/gpu/drm/xe/xe_lmtt.c
[ ... ]
> @@ -302,14 +303,16 @@ void xe_lmtt_invalidate_hw(struct xe_lmtt *lmtt)
>
> err = lmtt_invalidate_hw(lmtt);
> if (err)
> - xe_tile_sriov_err(tile, "LMTT invalidation failed (%pe)",
> - ERR_PTR(err));
> + xe_log_err(tile, LMTT, -EIO, "%s invalidation failed (%pe)",
> + xe_sriov_printk_prefix(xe),
> + ERR_PTR(err));
[Severity: Medium]
Since err holds the actual error code returned by lmtt_invalidate_hw(),
should this pass err to xe_log_err instead of hardcoding -EIO?
Additionally, does explicitly including (%pe) in the format string here
result in redundant and conflicting error codes in the log output, since
xe_log_err() automatically records the passed error code?
>
> if (xe_device_has_mert(xe) && xe_tile_is_root(tile)) {
> err = xe_mert_invalidate_lmtt(xe);
> if (err)
> - xe_tile_sriov_err(tile, "MERT LMTT invalidation failed (%pe)",
> - ERR_PTR(err));
> + xe_log_err(tile, LMTT, -EIO, "%s MERT invalidation failed (%pe)",
> + xe_sriov_printk_prefix(xe),
> + ERR_PTR(err));
[Severity: Medium]
Similar to the above, xe_mert_invalidate_lmtt() can return -ETIMEDOUT. Does
hardcoding -EIO here result in incorrect structured telemetry logs?
Also, would removing the (%pe) simplify the format string and avoid
redundant logging?
--
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=11
lmpx.com only provides a reader for public news (NNTP) servers. It is not
affiliated with the servers or forums shown here and is not responsible for
the content of articles, which is written by their respective authors.