diff mbox series

compress/mlx5: fix level translation in xform API

Message ID 20210729141108.20908-1-rzidane@nvidia.com (mailing list archive)
State Accepted, archived
Delegated to: akhil goyal
Headers show
Series compress/mlx5: fix level translation in xform API | expand

Checks

Context Check Description
ci/iol-mellanox-Functional fail Functional Testing issues
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-testing success Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/intel-Testing success Testing PASS
ci/Intel-compilation success Compilation OK
ci/github-robot success github build: passed
ci/checkpatch success coding style OK

Commit Message

Raja Zidane July 29, 2021, 2:11 p.m. UTC
Compression Level is interpreted by each PMD differently.
However, lower numbers give faster compression
at the expense of compression ratio, while higher numbers
may give better compression ratios but are likely slower.
The level affects the block size, which affects performance,
the bigger the block, the faster the compression is.

The problem was that higher levels caused bigger blocks:
  size = min_block_size - 1 + level.

the solution is to reverse the above:
  size = max_block_size + 1 - level.

Fixes: 39a2c8715f8f ("compress/mlx5: add transformation operations")
Cc: matan@nvidia.com
Cc: stable@dpdk.org

Signed-off-by: Raja Zidane <rzidane@nvidia.com>
---
 drivers/compress/mlx5/mlx5_compress.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

Comments

Akhil Goyal July 31, 2021, 5:57 p.m. UTC | #1
> Compression Level is interpreted by each PMD differently.
> However, lower numbers give faster compression
> at the expense of compression ratio, while higher numbers
> may give better compression ratios but are likely slower.
> The level affects the block size, which affects performance,
> the bigger the block, the faster the compression is.
> 
> The problem was that higher levels caused bigger blocks:
>   size = min_block_size - 1 + level.
> 
> the solution is to reverse the above:
>   size = max_block_size + 1 - level.
> 
> Fixes: 39a2c8715f8f ("compress/mlx5: add transformation operations")
> Cc: matan@nvidia.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Raja Zidane <rzidane@nvidia.com>
> ---
This patch is not acked, hence not applied for RC3.
@Thomas Monjalon can you pick it directly if it gets acked?
Matan Azrad Aug. 1, 2021, 6:13 a.m. UTC | #2
From: Raja Zidane
> Compression Level is interpreted by each PMD differently.
> However, lower numbers give faster compression at the expense of
> compression ratio, while higher numbers may give better compression ratios
> but are likely slower.
> The level affects the block size, which affects performance, the bigger the
> block, the faster the compression is.
> 
> The problem was that higher levels caused bigger blocks:
>   size = min_block_size - 1 + level.
> 
> the solution is to reverse the above:
>   size = max_block_size + 1 - level.
> 
> Fixes: 39a2c8715f8f ("compress/mlx5: add transformation operations")
> Cc: matan@nvidia.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Raja Zidane <rzidane@nvidia.com>

Congrats on your first patch, Raja!

Acked-by: Matan Azrad <matan@nvidia.com>
Thomas Monjalon Aug. 3, 2021, 12:11 p.m. UTC | #3
01/08/2021 08:13, Matan Azrad:
> From: Raja Zidane
> > Compression Level is interpreted by each PMD differently.
> > However, lower numbers give faster compression at the expense of
> > compression ratio, while higher numbers may give better compression ratios
> > but are likely slower.
> > The level affects the block size, which affects performance, the bigger the
> > block, the faster the compression is.
> > 
> > The problem was that higher levels caused bigger blocks:
> >   size = min_block_size - 1 + level.
> > 
> > the solution is to reverse the above:
> >   size = max_block_size + 1 - level.
> > 
> > Fixes: 39a2c8715f8f ("compress/mlx5: add transformation operations")
> > Cc: matan@nvidia.com
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Raja Zidane <rzidane@nvidia.com>
> 
> Congrats on your first patch, Raja!

The explanation above is very clear, thank you and
congratulations!

> Acked-by: Matan Azrad <matan@nvidia.com>

Applied, thanks.
diff mbox series

Patch

diff --git a/drivers/compress/mlx5/mlx5_compress.c b/drivers/compress/mlx5/mlx5_compress.c
index 5c2b9dc859..883e720ec1 100644
--- a/drivers/compress/mlx5/mlx5_compress.c
+++ b/drivers/compress/mlx5/mlx5_compress.c
@@ -316,12 +316,19 @@  mlx5_compress_xform_create(struct rte_compressdev *dev,
 			size /= MLX5_GGA_COMP_WIN_SIZE_UNITS;
 			xfrm->gga_ctrl1 += RTE_MIN(rte_log2_u32(size),
 					 MLX5_COMP_MAX_WIN_SIZE_CONF) <<
-					   WQE_GGA_COMP_WIN_SIZE_OFFSET;
-			if (xform->compress.level == RTE_COMP_LEVEL_PMD_DEFAULT)
+						WQE_GGA_COMP_WIN_SIZE_OFFSET;
+			switch (xform->compress.level) {
+			case RTE_COMP_LEVEL_PMD_DEFAULT:
 				size = MLX5_GGA_COMP_LOG_BLOCK_SIZE_MAX;
-			else
-				size = priv->min_block_size - 1 +
-							  xform->compress.level;
+				break;
+			case RTE_COMP_LEVEL_MAX:
+				size = priv->min_block_size;
+				break;
+			default:
+				size = RTE_MAX(MLX5_GGA_COMP_LOG_BLOCK_SIZE_MAX
+					+ 1 - xform->compress.level,
+					priv->min_block_size);
+			}
 			xfrm->gga_ctrl1 += RTE_MIN(size,
 					    MLX5_GGA_COMP_LOG_BLOCK_SIZE_MAX) <<
 						 WQE_GGA_COMP_BLOCK_SIZE_OFFSET;