> From: Yishai Hadas <yishaih@xxxxxxxxxx>
> Sent: Tuesday, February 6, 2024 4:02 PM
>
> On 06/02/2024 9:33, Tian, Kevin wrote:
> >> From: Yishai Hadas <yishaih@xxxxxxxxxx>
> >> Sent: Monday, February 5, 2024 5:21 PM
> >>
> >> 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.
> >
> > yes
> >
> >>
> >>>
> >>> 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.
> >>
> >
> > except below where tracker->status is not updated:
>
> This is the expected behavior in that case, see below.
>
> >
> > + err = mlx5_cmd_exec(mdev, in, sizeof(in), out, sizeof(out));
> > + if (err)
> > + return err;
>
> We can't set unconditionally the tracker status to an error without
> getting that information from the firmware.
>
> Upon an error here, we may just go out from the while loop in the caller
> and userspace will get the returned error.
>
> Any further call to mlx5vf_tracker_read_and_clear() if will come, may
> have the chance to start the regular flow.
>
> In case the tracker can't be moved to MLX5_PAGE_TRACK_STATE_REPORTING
> (e.g. as of some previous error) the call will simply fail as expected.
>
> So, it looks OK.
>
ok then fine to me.
