On 05/02/2024 10:10, Tian, Kevin wrote:
From: Yishai Hadas <yishaih@xxxxxxxxxx>
Sent: Wednesday, January 31, 2024 1:02 AM
+static void set_tracker_event(struct mlx5vf_pci_core_device *mvdev)
+{
+ mvdev->tracker.event_occur = true;
+ complete(&mvdev->tracker_comp);
it's slightly clearer to call it 'object_changed'.
Do you refer to the 'event_occur' field ? I can rename it, if you think
that it's clearer.
@@ -1634,6 +1671,11 @@ int mlx5vf_tracker_read_and_clear(struct
vfio_device *vdev, unsigned long iova,
goto end;
}
+ if (tracker->is_err) {
+ err = -EIO;
+ goto end;
+ }
+
this sounds like a separate improvement? i.e. if the tracker is already
in an error state then exit early. if yes better put it in a separate patch.
As it's just an early exit, I don't think that it worth a separate patch.
However, I can add one statement about that in the commit log.
@@ -1652,6 +1694,12 @@ int mlx5vf_tracker_read_and_clear(struct
vfio_device *vdev, unsigned long iova,
dirty, &tracker->status);
if (poll_err == CQ_EMPTY) {
wait_for_completion(&mvdev-
tracker_comp);
+ if (tracker->event_occur) {
+ tracker->event_occur = false;
+ err =
mlx5vf_cmd_query_tracker(mdev, tracker);
+ if (err)
+ goto end;
+ }
this implies that the error notified by tracker event cannot be queried
by mlx5vf_cq_poll_one() otherwise the next iteration will get the error
state anyway.
Right, in that case, no CQE will be delivered, so, this is the way to
detect that firmware moved the object to an error state, following the
device specification.
possibly add a comment to clarify.
OK, I'll add a note in the commit log.
and why not setting state->is_err too?
No need for an extra code here.
Upon mlx5vf_cmd_query_tracker() the tracker->status field will be
updated to an error, the while loop will detect that, and do the job.
Yishai
