diff mbox series

[net-next,5/5] mlxsw: spectrum_kvdl: combine devlink resource occupation getters

Message ID 20240806143307.14839-6-przemyslaw.kitszel@intel.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series devlink: embed driver's priv data callback param into devlink_resource | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 29 this patch: 29
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 6 of 6 maintainers
netdev/build_clang success Errors and warnings before: 29 this patch: 29
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 29 this patch: 29
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 142 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Przemek Kitszel Aug. 6, 2024, 2:33 p.m. UTC 
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@intel.com>
Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.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(-)

Comments

Jiri Pirko Aug. 7, 2024, 6:47 a.m. UTC | #1 
Tue, Aug 06, 2024 at 04:33:07PM CEST, przemyslaw.kitszel@intel.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@intel.com>
>Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.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
>
Przemek Kitszel Aug. 12, 2024, 11:23 a.m. UTC | #2 
On 8/7/24 08:47, Jiri Pirko wrote:
> Tue, Aug 06, 2024 at 04:33:07PM CEST, przemyslaw.kitszel@intel.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@intel.com>
>> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.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.

You mean in general or this particular function?

Anyway, a part of motivation is to avoid dumb (even if easy to read in
isolation) getters and replace it with a one that exposes the logic.
(Now you have 2+ functions that reader needs to manually compare).

Re general oddities:
sizeof(void *) stuff just follows from the main idea, and is a temporary
solution (see this patch, it removes such).

> 
> 
>> }
>>
>> -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
>>
Jiri Pirko Aug. 12, 2024, 3:01 p.m. UTC | #3 
Mon, Aug 12, 2024 at 01:23:20PM CEST, przemyslaw.kitszel@intel.com wrote:
>On 8/7/24 08:47, Jiri Pirko wrote:
>> Tue, Aug 06, 2024 at 04:33:07PM CEST, przemyslaw.kitszel@intel.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@intel.com>
>> > Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.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.
>
>You mean in general or this particular function?

In general.

>
>Anyway, a part of motivation is to avoid dumb (even if easy to read in
>isolation) getters and replace it with a one that exposes the logic.
>(Now you have 2+ functions that reader needs to manually compare).

I don't follow. Are you not able to implement what you need using the
existing api, or you just don't like it?


>
>Re general oddities:
>sizeof(void *) stuff just follows from the main idea, and is a temporary
>solution (see this patch, it removes such).
>
>> 
>> 
>> > }
>> > 
>> > -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
>> > 
>
diff mbox series

Patch

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;
 }
 
-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;
 }