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

Reply via email to