Thanks for your comments and will fix it on V3.
Regards,
Li Zhang
> -----Original Message-----
> From: Maxime Coquelin <maxime.coque...@redhat.com>
> Sent: Friday, June 17, 2022 11:54 PM
> To: Li Zhang <l...@nvidia.com>; Ori Kam <or...@nvidia.com>; Slava
> Ovsiienko <viachesl...@nvidia.com>; Matan Azrad <ma...@nvidia.com>;
> Shahaf Shuler <shah...@nvidia.com>
> Cc: dev@dpdk.org; NBU-Contact-Thomas Monjalon (EXTERNAL)
> <tho...@monjalon.net>; Raslan Darawsheh <rasl...@nvidia.com>; Roni
> Bar Yanai <ron...@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 <l...@nvidia.com>
> > Acked-by: Matan Azrad <ma...@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.coque...@redhat.com>
>
> Thanks,
> Maxime
