RE: [PATCH v2 06/15] vdpa/mlx5: pre-create virtq in the prob

[Li Zhang <lizh@nvidia.com>]

Thanks for your comments and will fix it on V3.

Regards,
Li Zhang

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Friday, June 17, 2022 11:54 PM
> To: Li Zhang <lizh@nvidia.com>; Ori Kam <orika@nvidia.com>; Slava
> Ovsiienko <viacheslavo@nvidia.com>; Matan Azrad <matan@nvidia.com>;
> Shahaf Shuler <shahafs@nvidia.com>
> Cc: dev@dpdk.org; NBU-Contact-Thomas Monjalon (EXTERNAL)
> <thomas@monjalon.net>; Raslan Darawsheh <rasland@nvidia.com>; Roni
> Bar Yanai <roniba@nvidia.com>
> Subject: Re: [PATCH v2 06/15] vdpa/mlx5: pre-create virtq in the prob
> 
> External email: Use caution opening links or attachments
> 
> 
> I would rename the title to something like:
> 
> "vdpa/mlx5: pre-create virtq at probe time"
> 
> On 6/16/22 04:30, Li Zhang wrote:
> > dev_config operation is called in LM progress.
> > LM time is very critical because all
> > the VM packets are dropped directly at that time.
> >
> > Move the virtq creation to probe time and only modify the
> > configuration later in the dev_config stage using the new ability to
> > modify virtq.
> >
> > This optimization accelerates the LM process and reduces its time by
> > 70%.
> 
> Nice.
> 
> > Signed-off-by: Li Zhang <lizh@nvidia.com>
> > Acked-by: Matan Azrad <matan@nvidia.com>
> > ---
> >   doc/guides/rel_notes/release_22_07.rst |   4 +
> >   drivers/vdpa/mlx5/mlx5_vdpa.h          |   4 +
> >   drivers/vdpa/mlx5/mlx5_vdpa_lm.c       |  13 +-
> >   drivers/vdpa/mlx5/mlx5_vdpa_virtq.c    | 257 +++++++++++++++----------
> >   4 files changed, 174 insertions(+), 104 deletions(-)
> >
> > diff --git a/doc/guides/rel_notes/release_22_07.rst
> > b/doc/guides/rel_notes/release_22_07.rst
> > index f2cf41def9..2056cd9ee7 100644
> > --- a/doc/guides/rel_notes/release_22_07.rst
> > +++ b/doc/guides/rel_notes/release_22_07.rst
> > @@ -175,6 +175,10 @@ New Features
> >     This is a fall-back implementation for platforms that
> >     don't support vector operations.
> >
> > +* **Updated Nvidia mlx5 vDPA driver.**
> > +
> > +  * Added new devargs ``queue_size`` and ``queues`` to allow prior
> creation of virtq resources.
> > +
> >
> >   Removed Items
> >   -------------
> > diff --git a/drivers/vdpa/mlx5/mlx5_vdpa.h
> > b/drivers/vdpa/mlx5/mlx5_vdpa.h index bf82026e37..e5553079fe 100644
> > --- a/drivers/vdpa/mlx5/mlx5_vdpa.h
> > +++ b/drivers/vdpa/mlx5/mlx5_vdpa.h
> > @@ -80,6 +80,7 @@ struct mlx5_vdpa_virtq {
> >       uint16_t vq_size;
> >       uint8_t notifier_state;
> >       bool stopped;
> > +     uint32_t configured:1;
> >       uint32_t version;
> >       struct mlx5_vdpa_priv *priv;
> >       struct mlx5_devx_obj *virtq;
> > @@ -489,4 +490,7 @@ mlx5_vdpa_virtq_stats_reset(struct
> mlx5_vdpa_priv *priv, int qid);
> >    */
> >   void
> >   mlx5_vdpa_drain_cq(struct mlx5_vdpa_priv *priv);
> > +
> > +bool
> > +mlx5_vdpa_is_modify_virtq_supported(struct mlx5_vdpa_priv *priv);
> >   #endif /* RTE_PMD_MLX5_VDPA_H_ */
> > diff --git a/drivers/vdpa/mlx5/mlx5_vdpa_lm.c
> > b/drivers/vdpa/mlx5/mlx5_vdpa_lm.c
> > index 43a2b98255..a8faf0c116 100644
> > --- a/drivers/vdpa/mlx5/mlx5_vdpa_lm.c
> > +++ b/drivers/vdpa/mlx5/mlx5_vdpa_lm.c
> > @@ -12,14 +12,17 @@ int
> >   mlx5_vdpa_logging_enable(struct mlx5_vdpa_priv *priv, int enable)
> >   {
> >       struct mlx5_devx_virtq_attr attr = {
> > -             .type =
> MLX5_VIRTQ_MODIFY_TYPE_DIRTY_BITMAP_DUMP_ENABLE,
> > +             .mod_fields_bitmap =
> > +                     MLX5_VIRTQ_MODIFY_TYPE_DIRTY_BITMAP_DUMP_ENABLE,
> >               .dirty_bitmap_dump_enable = enable,
> >       };
> > +     struct mlx5_vdpa_virtq *virtq;
> >       int i;
> >
> >       for (i = 0; i < priv->nr_virtqs; ++i) {
> >               attr.queue_index = i;
> > -             if (!priv->virtqs[i].virtq) {
> > +             virtq = &priv->virtqs[i];
> > +             if (!virtq->configured) {
> >                       DRV_LOG(DEBUG, "virtq %d is invalid for dirty bitmap "
> >                               "enabling.", i);
> 
> Please avoid cutting logs, it makes it harder to grep in the code.
> Also, now we can have up to 100 chars, so maybe it would fit anyway.
> 
> 
> Other than that:
> 
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> 
> Thanks,
> Maxime