[1/1] ml/cnxk: fix name of TVM model with single layer
Checks
Commit Message
Name field of TVM model with single MRVL layer is
currently set to empty string. Update the name with
the field from metadata.
Fixes: 5cea2c67edfc ("ml/cnxk: update internal TVM model info structure")
Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
---
drivers/ml/cnxk/mvtvm_ml_model.c | 4 ++++
1 file changed, 4 insertions(+)
Comments
On Wed, Nov 22, 2023 at 08:36:40AM -0800, Srikanth Yalavarthi wrote:
> Name field of TVM model with single MRVL layer is
> currently set to empty string. Update the name with
> the field from metadata.
>
> Fixes: 5cea2c67edfc ("ml/cnxk: update internal TVM model info structure")
>
> Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
> ---
> drivers/ml/cnxk/mvtvm_ml_model.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/ml/cnxk/mvtvm_ml_model.c b/drivers/ml/cnxk/mvtvm_ml_model.c
> index 3e06ea658bb..102d30e5f4f 100644
> --- a/drivers/ml/cnxk/mvtvm_ml_model.c
> +++ b/drivers/ml/cnxk/mvtvm_ml_model.c
> @@ -352,6 +352,10 @@ mvtvm_ml_model_info_set(struct cnxk_ml_dev *cnxk_mldev, struct cnxk_ml_model *mo
> tvm_mrvl_model:
> cn10k_ml_model_info_set(cnxk_mldev, model, &model->mvtvm.info,
> &model->layer[0].glow.metadata);
> +
> + metadata = &model->mvtvm.metadata;
> + rte_memcpy(info->name, metadata->model.name, TVMDP_NAME_STRLEN);
> +
Minor nit, but you probably don't need a high-performance copy here, so I'd
recommend using regular memcpy rather than rte_memcpy.
/Bruce
On Wed, Nov 22, 2023 at 04:48:50PM +0000, Bruce Richardson wrote:
> On Wed, Nov 22, 2023 at 08:36:40AM -0800, Srikanth Yalavarthi wrote:
> > Name field of TVM model with single MRVL layer is
> > currently set to empty string. Update the name with
> > the field from metadata.
> >
> > Fixes: 5cea2c67edfc ("ml/cnxk: update internal TVM model info structure")
> >
> > Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > ---
> > drivers/ml/cnxk/mvtvm_ml_model.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/ml/cnxk/mvtvm_ml_model.c b/drivers/ml/cnxk/mvtvm_ml_model.c
> > index 3e06ea658bb..102d30e5f4f 100644
> > --- a/drivers/ml/cnxk/mvtvm_ml_model.c
> > +++ b/drivers/ml/cnxk/mvtvm_ml_model.c
> > @@ -352,6 +352,10 @@ mvtvm_ml_model_info_set(struct cnxk_ml_dev *cnxk_mldev, struct cnxk_ml_model *mo
> > tvm_mrvl_model:
> > cn10k_ml_model_info_set(cnxk_mldev, model, &model->mvtvm.info,
> > &model->layer[0].glow.metadata);
> > +
> > + metadata = &model->mvtvm.metadata;
> > + rte_memcpy(info->name, metadata->model.name, TVMDP_NAME_STRLEN);
> > +
>
> Minor nit, but you probably don't need a high-performance copy here, so I'd
> recommend using regular memcpy rather than rte_memcpy.
>
Actually, if this is a name value, would a strcpy function, e.g. strlcpy,
not be more appropriate?
> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: 22 November 2023 22:21
> To: Srikanth Yalavarthi <syalavarthi@marvell.com>
> Cc: Prince Takkar <ptakkar@marvell.com>; dev@dpdk.org; Shivah Shankar
> Shankar Narayan Rao <sshankarnara@marvell.com>; Anup Prabhu
> <aprabhu@marvell.com>; Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> Subject: [EXT] Re: [PATCH 1/1] ml/cnxk: fix name of TVM model with single
> layer
>
> External Email
>
> ----------------------------------------------------------------------
> On Wed, Nov 22, 2023 at 04:48:50PM +0000, Bruce Richardson wrote:
> > On Wed, Nov 22, 2023 at 08:36:40AM -0800, Srikanth Yalavarthi wrote:
> > > Name field of TVM model with single MRVL layer is currently set to
> > > empty string. Update the name with the field from metadata.
> > >
> > > Fixes: 5cea2c67edfc ("ml/cnxk: update internal TVM model info
> > > structure")
> > >
> > > Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > > ---
> > > drivers/ml/cnxk/mvtvm_ml_model.c | 4 ++++
> > > 1 file changed, 4 insertions(+)
> > >
> > > diff --git a/drivers/ml/cnxk/mvtvm_ml_model.c
> > > b/drivers/ml/cnxk/mvtvm_ml_model.c
> > > index 3e06ea658bb..102d30e5f4f 100644
> > > --- a/drivers/ml/cnxk/mvtvm_ml_model.c
> > > +++ b/drivers/ml/cnxk/mvtvm_ml_model.c
> > > @@ -352,6 +352,10 @@ mvtvm_ml_model_info_set(struct cnxk_ml_dev
> > > *cnxk_mldev, struct cnxk_ml_model *mo
> > > tvm_mrvl_model:
> > > cn10k_ml_model_info_set(cnxk_mldev, model, &model-
> >mvtvm.info,
> > > &model->layer[0].glow.metadata);
> > > +
> > > + metadata = &model->mvtvm.metadata;
> > > + rte_memcpy(info->name, metadata->model.name,
> TVMDP_NAME_STRLEN);
> > > +
> >
> > Minor nit, but you probably don't need a high-performance copy here,
> > so I'd recommend using regular memcpy rather than rte_memcpy.
> >
> Actually, if this is a name value, would a strcpy function, e.g. strlcpy, not be
> more appropriate?
Thanks. Updated in v2.
We have few other locations unrelated to the current patch, where rte_memcpy can be replaced with strlcpy. Will submit a separate patch for this.
@@ -352,6 +352,10 @@ mvtvm_ml_model_info_set(struct cnxk_ml_dev *cnxk_mldev, struct cnxk_ml_model *mo
tvm_mrvl_model:
cn10k_ml_model_info_set(cnxk_mldev, model, &model->mvtvm.info,
&model->layer[0].glow.metadata);
+
+ metadata = &model->mvtvm.metadata;
+ rte_memcpy(info->name, metadata->model.name, TVMDP_NAME_STRLEN);
+
info->io_layout = RTE_ML_IO_LAYOUT_SPLIT;
}