Re: [PATCH 3/6] accel/qaic: Isolate hw submission

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] qaic: redundant req_submit_ts assignment causes incorrect stats
--

commit 832ae6a6a52a6bd91f900e3b436da51c50e676e9
Author: Carl Vanderlip <carl.vanderlip@oss.qualcomm.com>

accel/qaic: Isolate hw submission

This commit merges two pathways used to submit a request to the DBC FIFO
into a single function. This simplifies the job submission logic in
preparation for using the DRM scheduler.

> diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
> index 3326d9da0e1e4..0208400d2636f 100644
> --- a/drivers/accel/qaic/qaic_data.c
> +++ b/drivers/accel/qaic/qaic_data.c
[ ... ]
> +/* Caller should be holding dbc->req_lock */
> +static inline int qaic_submit_reqs_to_hw(struct bo_slice *slice,
> +					 unsigned int num_req,
> +					 bool is_partial, u32 partial_size)
> +{
[ ... ]
> +	tail = (tail + num_req) % dbc->nelem;
> +
> +	/* Submit_ts will be taken for the last job of this BO */
> +	bo->perf_stats.req_submit_ts = ktime_get_ns();
> +
> +	/* Finalize commit to hardware */
> +	dma_sync_sgtable_for_device(&dbc->qdev->pdev->dev, bo->sgt, bo->dir);
> +	writel(tail, dbc->dbc_base + REQTP_OFF);
>  	return 0;
>  }

[Severity: Medium]
Does update_profiling_data() unconditionally overwrite this new timestamp?

In the IOCTL path, __qaic_execute_bo_ioctl() calls send_bo_list_to_device(),
which eventually reaches this code and records the per-job timestamp.
However, immediately after send_bo_list_to_device() returns,
__qaic_execute_bo_ioctl() calls update_profiling_data(&exec):

static void update_profiling_data(struct execute_info *exec)
{
        ...
        for (i = 0; i < exec->count; i++) {
                bo = exec->bo_arr[i];
                ...
                bo->perf_stats.req_submit_ts = exec->perf_stats.submit_ts;
        ...
}

Could this render the newly added per-job timestamp dead code? Since this
refactoring prepares for the DRM scheduler, might this overwrite also cause
a data race or incorrect profiling statistics when submissions become
asynchronous?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261003023818.1161666-1-carl.vanderlip@oss.qualcomm.com?part=3