Virtualization - Nov 2021 - [PATH v1 2/2] vdpa/mlx5: Add support for reading descriptor statistics

On Thu, Nov 25, 2021 at 3:59 PM Eli Cohen <elic at nvidia.com>
wrote:
>
> On Thu, Nov 25, 2021 at 12:57:53PM +0800, Jason Wang wrote:
> > On Thu, Nov 25, 2021 at 12:56 AM Eli Cohen <elic at nvidia.com>
wrote:
> > >
> > > Implement the get_vq_stats calback of vdpa_config_ops to return
the
> > > statistics for a virtqueue.
> > >
> > > Signed-off-by: Eli Cohen <elic at nvidia.com>
> > > ---
> > > V0 -> V1:
> > > Use mutex to sync stats query with change of number of queues
> > >
> >
> > [...]
> >
> > > +static int mlx5_get_vq_stats(struct vdpa_device *vdev, u16 *idx,
> > > +                            struct vdpa_vq_stats *stats)
> > > +{
> > > +       struct mlx5_vdpa_dev *mvdev = to_mvdev(vdev);
> > > +       struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);
> > > +       struct mlx5_vdpa_virtqueue *mvq =
&ndev->vqs[*idx];
> > > +       struct mlx5_control_vq *cvq;
> > > +       int err;
> > > +
> > > +       mutex_lock(&ndev->numq_lock);
> > > +       if ((!ctrl_vq_active(mvdev) && *idx >=
ndev->cur_num_vqs) ||
> > > +           (*idx != ctrl_vq_idx(mvdev) && *idx >=
ndev->cur_num_vqs)) {
> > > +               err = -EINVAL;
> > > +               goto out;
> >
> > Interesting, I wonder if it's simpler that we just allow stats up
to
> > the max vqs. It's sometimes useful to dump the stats of all the
vqs so
> > we can let that policy to userspace. Then we can get rid of the mutex.
> >
> If a VQ is not active then I don't have stats for it. The hardware
> object is not available to be queried.
I wonder if it's ok that we just return 0 in this case?

Btw, the cvq counters:

+
+ if (*idx == ctrl_vq_idx(mvdev)) {
+ cvq = &mvdev->cvq;
+ stats->received_desc = cvq->head;
+ stats->completed_desc = cvq->head;
+ stats->ctrl_vq = true;
+ *idx = VDPA_INVAL_QUEUE_INDEX;
+ err = 0;
+ goto out;
+ }

Seems not to consider the case that the head can wrap around.

Thanks

>
> > Thanks
> >
>