Re: [PATCH] accel/rocket: Fix job submit error handling

MoGGuU <[email protected]>
Newsgroups dev.linux.lists.sashiko-reviews
Message-ID <[email protected]>
Thanks for the review.

I checked the reported partial-submission restart path.  I do not believe
the new issue is reachable as described.  The interruptible operation in
this path is drm_gem_lock_reservations().  Its
dma_resv_lock_interruptible() call returns -EINTR when interrupted, as
documented by ww_mutex_lock_interruptible(), and drm_ioctl() propagates
that value unchanged.  It does not return an -ERESTART* value, so the
kernel will not automatically restart this ioctl from job zero.  The other
failure paths in rocket_ioctl_submit_job() also do not return an
-ERESTART* value.  Userspace could still choose to retry an EINTR manually;
the ordered partial-submit semantics documented in the commit message mean
that it must not blindly retry the whole batch.

The duplicate-BO report does not cause a double unlock either.  Trying to
lock the same WW mutex twice with the same acquisition context returns
-EALREADY.  drm_gem_lock_reservations() then unlocks the reservations it
already acquired and finalizes the context before returning the error.
rocket_job_push() takes its err path in that case, not err_unlock.  I also
verified this on RK3588: a submission containing the same input BO handle
twice returned EALREADY without a new warning or error in the kernel log.

The two dma-resv fence observations are valid pre-existing issues.  The
completion fence should be attached with DMA_RESV_USAGE_READ to input BOs
and DMA_RESV_USAGE_WRITE to output BOs after drm_sched_job_arm() creates
the finished fence, but before drm_sched_entity_push_job() makes the job
runnable.  I will address those in a separate follow-up patch so this patch
remains focused on its stated error-path fixes.

-- 
MoGGuU
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.