On Thu, Nov 25, 2021 at 3:59 PM Eli Cohen <elic@xxxxxxxxxx> 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@xxxxxxxxxx> wrote:
> > >
> > > Implement the get_vq_stats calback of vdpa_config_ops to return the
> > > statistics for a virtqueue.
> > >
> > > Signed-off-by: Eli Cohen <elic@xxxxxxxxxx>
> > > ---
> > > 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
> >
>
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
