[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZrMYfIN7cKUpYb8u@nanopsycho.orion>
Date: Wed, 7 Aug 2024 08:47:24 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: Przemek Kitszel <przemyslaw.kitszel@...el.com>
Cc: netdev@...r.kernel.org, Ido Schimmel <idosch@...dia.com>,
Petr Machata <petrm@...dia.com>, Jakub Kicinski <kuba@...nel.org>,
Andrew Lunn <andrew@...n.ch>,
Florian Fainelli <f.fainelli@...il.com>,
Vladimir Oltean <olteanv@...il.com>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
Saeed Mahameed <saeedm@...dia.com>,
Leon Romanovsky <leon@...nel.org>, Tariq Toukan <tariqt@...dia.com>,
Tony Nguyen <anthony.l.nguyen@...el.com>,
nex.sw.ncis.osdt.itp.upstreaming@...el.com,
Wojciech Drewek <wojciech.drewek@...el.com>
Subject: Re: [PATCH net-next 5/5] mlxsw: spectrum_kvdl: combine devlink
resource occupation getters
Tue, Aug 06, 2024 at 04:33:07PM CEST, przemyslaw.kitszel@...el.com wrote:
>Combine spectrum1 kvdl devlink resource occupation getters into one.
>
>Thanks to previous commit of the series we could easily embed more than
>just a single pointer into devlink resource.
>
>Reviewed-by: Wojciech Drewek <wojciech.drewek@...el.com>
>Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>
>---
> .../net/ethernet/mellanox/mlxsw/spectrum.h | 5 ++
> .../net/ethernet/mellanox/mlxsw/spectrum.c | 3 +-
> .../ethernet/mellanox/mlxsw/spectrum1_kvdl.c | 80 ++++++++-----------
> 3 files changed, 41 insertions(+), 47 deletions(-)
>
>diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
>index 8d3c61287696..91fe5fffa675 100644
>--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
>+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
>@@ -836,6 +836,11 @@ int mlxsw_sp_kvdl_alloc_count_query(struct mlxsw_sp *mlxsw_sp,
> unsigned int *p_alloc_count);
>
> /* spectrum1_kvdl.c */
>+struct mlxsw_sp1_kvdl_occ_ctx {
>+ struct mlxsw_sp1_kvdl *kvdl;
>+ int first_part_id;
>+ bool count_all_parts;
>+};
> extern const struct mlxsw_sp_kvdl_ops mlxsw_sp1_kvdl_ops;
> int mlxsw_sp1_kvdl_resources_register(struct mlxsw_core *mlxsw_core);
>
>diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
>index 2730ae3d8fe6..3bda2b2d16f9 100644
>--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
>+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
>@@ -3669,7 +3669,8 @@ static int mlxsw_sp1_resources_kvd_register(struct mlxsw_core *mlxsw_core)
> linear_size,
> MLXSW_SP_RESOURCE_KVD_LINEAR,
> MLXSW_SP_RESOURCE_KVD,
>- &linear_size_params, sizeof(void *));
>+ &linear_size_params,
>+ sizeof(struct mlxsw_sp1_kvdl_occ_ctx));
> if (err)
> return err;
>
>diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum1_kvdl.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum1_kvdl.c
>index ee5f12746371..a8bf052adf31 100644
>--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum1_kvdl.c
>+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum1_kvdl.c
>@@ -292,68 +292,53 @@ static u64 mlxsw_sp1_kvdl_part_occ(struct mlxsw_sp1_kvdl_part *part)
>
> static u64 mlxsw_sp1_kvdl_occ_get(void *priv)
> {
>- const struct mlxsw_sp1_kvdl *kvdl = priv;
>+ struct mlxsw_sp1_kvdl_occ_ctx *ctx = priv;
>+ bool cnt_all = ctx->count_all_parts;
>+ int beg, end;
> u64 occ = 0;
>- int i;
>
>- for (i = 0; i < MLXSW_SP1_KVDL_PARTS_INFO_LEN; i++)
>- occ += mlxsw_sp1_kvdl_part_occ(kvdl->parts[i]);
>+ beg = cnt_all ? 0 : ctx->first_part_id,
>+ end = cnt_all ? MLXSW_SP1_KVDL_PARTS_INFO_LEN : beg + 1;
>+ for (int i = beg; i < end; i++)
>+ occ += mlxsw_sp1_kvdl_part_occ(ctx->kvdl->parts[i]);
>
> return occ;
I don't see the benefit, this just makes code harder to read.
> }
>
>-static u64 mlxsw_sp1_kvdl_single_occ_get(void *priv)
>-{
>- const struct mlxsw_sp1_kvdl *kvdl = priv;
>- struct mlxsw_sp1_kvdl_part *part;
>-
>- part = kvdl->parts[MLXSW_SP1_KVDL_PART_ID_SINGLE];
>- return mlxsw_sp1_kvdl_part_occ(part);
>-}
>-
>-static u64 mlxsw_sp1_kvdl_chunks_occ_get(void *priv)
>-{
>- const struct mlxsw_sp1_kvdl *kvdl = priv;
>- struct mlxsw_sp1_kvdl_part *part;
>-
>- part = kvdl->parts[MLXSW_SP1_KVDL_PART_ID_CHUNKS];
>- return mlxsw_sp1_kvdl_part_occ(part);
>-}
>-
>-static u64 mlxsw_sp1_kvdl_large_chunks_occ_get(void *priv)
>-{
>- const struct mlxsw_sp1_kvdl *kvdl = priv;
>- struct mlxsw_sp1_kvdl_part *part;
>-
>- part = kvdl->parts[MLXSW_SP1_KVDL_PART_ID_LARGE_CHUNKS];
>- return mlxsw_sp1_kvdl_part_occ(part);
>-}
>-
> static int mlxsw_sp1_kvdl_init(struct mlxsw_sp *mlxsw_sp, void *priv)
> {
> struct devlink *devlink = priv_to_devlink(mlxsw_sp->core);
>- struct mlxsw_sp1_kvdl *kvdl = priv;
>+ struct mlxsw_sp1_kvdl_occ_ctx ctx = { priv };
> int err;
>
>- err = mlxsw_sp1_kvdl_parts_init(mlxsw_sp, kvdl);
>+ err = mlxsw_sp1_kvdl_parts_init(mlxsw_sp, ctx.kvdl);
> if (err)
> return err;
>- devl_resource_occ_get_register(devlink,
>- MLXSW_SP_RESOURCE_KVD_LINEAR,
>- mlxsw_sp1_kvdl_occ_get,
>- &kvdl, sizeof(kvdl));
>+
>+ ctx.first_part_id = MLXSW_SP1_KVDL_PART_ID_SINGLE;
> devl_resource_occ_get_register(devlink,
> MLXSW_SP_RESOURCE_KVD_LINEAR_SINGLE,
>- mlxsw_sp1_kvdl_single_occ_get,
>- &kvdl, sizeof(kvdl));
>+ mlxsw_sp1_kvdl_occ_get,
>+ &ctx, sizeof(ctx));
>+
>+ ctx.first_part_id = MLXSW_SP1_KVDL_PART_ID_CHUNKS;
> devl_resource_occ_get_register(devlink,
> MLXSW_SP_RESOURCE_KVD_LINEAR_CHUNKS,
>- mlxsw_sp1_kvdl_chunks_occ_get,
>- &kvdl, sizeof(kvdl));
>+ mlxsw_sp1_kvdl_occ_get,
>+ &ctx, sizeof(ctx));
>+
>+ ctx.first_part_id = MLXSW_SP1_KVDL_PART_ID_LARGE_CHUNKS;
> devl_resource_occ_get_register(devlink,
> MLXSW_SP_RESOURCE_KVD_LINEAR_LARGE_CHUNKS,
>- mlxsw_sp1_kvdl_large_chunks_occ_get,
>- &kvdl, sizeof(kvdl));
>+ mlxsw_sp1_kvdl_occ_get,
>+ &ctx, sizeof(ctx));
>+
>+ ctx.count_all_parts = true;
>+ devl_resource_occ_get_register(devlink,
>+ MLXSW_SP_RESOURCE_KVD_LINEAR,
>+ mlxsw_sp1_kvdl_occ_get,
>+ &ctx, sizeof(ctx));
>+
> return 0;
> }
>
>@@ -400,7 +385,8 @@ int mlxsw_sp1_kvdl_resources_register(struct mlxsw_core *mlxsw_core)
> MLXSW_SP1_KVDL_SINGLE_SIZE,
> MLXSW_SP_RESOURCE_KVD_LINEAR_SINGLE,
> MLXSW_SP_RESOURCE_KVD_LINEAR,
>- &size_params, sizeof(void *));
>+ &size_params,
>+ sizeof(struct mlxsw_sp1_kvdl_occ_ctx));
> if (err)
> return err;
>
>@@ -411,7 +397,8 @@ int mlxsw_sp1_kvdl_resources_register(struct mlxsw_core *mlxsw_core)
> MLXSW_SP1_KVDL_CHUNKS_SIZE,
> MLXSW_SP_RESOURCE_KVD_LINEAR_CHUNKS,
> MLXSW_SP_RESOURCE_KVD_LINEAR,
>- &size_params, sizeof(void *));
>+ &size_params,
>+ sizeof(struct mlxsw_sp1_kvdl_occ_ctx));
> if (err)
> return err;
>
>@@ -422,6 +409,7 @@ int mlxsw_sp1_kvdl_resources_register(struct mlxsw_core *mlxsw_core)
> MLXSW_SP1_KVDL_LARGE_CHUNKS_SIZE,
> MLXSW_SP_RESOURCE_KVD_LINEAR_LARGE_CHUNKS,
> MLXSW_SP_RESOURCE_KVD_LINEAR,
>- &size_params, sizeof(void *));
>+ &size_params,
>+ sizeof(struct mlxsw_sp1_kvdl_occ_ctx));
> return err;
> }
>--
>2.39.3
>
Powered by blists - more mailing lists