[PATCH 0/5] octeontx2-af: Clean-up some bitmap related usage

[PATCH 0/5] octeontx2-af: Clean-up some bitmap related usage

[Christophe JAILLET]

The main point is patch 4.

The other ones are just minor things spoted while looking at the way the
'id_bmap' bitmap was used.

Christophe JAILLET (5):
  octeontx2-af: Fix a potentially spurious error message
  octeontx2-af: Slightly simplify rvu_npc_exact_init()
  octeontx2-af: Use the bitmap API to allocate bitmaps
  octeontx2-af: Fix the size of memory allocated for the 'id_bmap'
    bitmap
  octeontx2-af: Simplify a size computation in rvu_npc_exact_init()

 .../marvell/octeontx2/af/rvu_npc_hash.c         | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

-- 
2.34.1

[PATCH 1/5] octeontx2-af: Fix a potentially spurious error message

[Christophe JAILLET]

When this error message is displayed, we know that the all the bits in the
bitmap are set.

So, bitmap_weight() will return the number of bits of the bitmap, which is
'table->tot_ids'.

It is unlikely that a bit will be cleared between mutex_unlock() and
dev_err(), but, in order to simplify the code and avoid this possibility,
just take 'table->tot_ids'.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
index 594029007f85..5e6c54577a97 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
@@ -490,7 +490,7 @@ static bool rvu_npc_exact_alloc_id(struct rvu *rvu, u32 *seq_id)
 	if (idx == table->tot_ids) {
 		mutex_unlock(&table->lock);
 		dev_err(rvu->dev, "%s: No space in id bitmap (%d)\n",
-			__func__, bitmap_weight(table->id_bmap, table->tot_ids));
+			__func__, table->tot_ids);
 
 		return false;
 	}
-- 
2.34.1

[PATCH 2/5] octeontx2-af: Slightly simplify rvu_npc_exact_init()

[Christophe JAILLET]

Use kzalloc() instead of kmalloc()/memset().

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
index 5e6c54577a97..c584680f2d2b 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
@@ -1870,12 +1870,11 @@ int rvu_npc_exact_init(struct rvu *rvu)
 	/* Set capability to true */
 	rvu->hw->cap.npc_exact_match_enabled = true;
 
-	table = kmalloc(sizeof(*table), GFP_KERNEL);
+	table = kzalloc(sizeof(*table), GFP_KERNEL);
 	if (!table)
 		return -ENOMEM;
 
 	dev_dbg(rvu->dev, "%s: Memory allocation for table success\n", __func__);
-	memset(table, 0, sizeof(*table));
 	rvu->hw->table = table;
 
 	/* Read table size, ways and depth */
-- 
2.34.1

[PATCH 3/5] octeontx2-af: Use the bitmap API to allocate bitmaps

[Christophe JAILLET]

Use devm_bitmap_zalloc() instead of hand-writing it.

This also makes the comment "Allocate bitmap for 32 entry mcam" more
explicit because now 32 is really used in the allocation function, instead
of an obscure 'sizeof(long)'.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
index c584680f2d2b..3f94b620ef5a 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
@@ -1898,15 +1898,15 @@ int rvu_npc_exact_init(struct rvu *rvu)
 	table_size = table->mem_table.depth * table->mem_table.ways;
 
 	/* Allocate bitmap for 4way 2K table */
-	table->mem_table.bmap = devm_kcalloc(rvu->dev, BITS_TO_LONGS(table_size),
-					     sizeof(long), GFP_KERNEL);
+	table->mem_table.bmap = devm_bitmap_zalloc(rvu->dev, table_size,
+						   GFP_KERNEL);
 	if (!table->mem_table.bmap)
 		return -ENOMEM;
 
 	dev_dbg(rvu->dev, "%s: Allocated bitmap for 4way 2K entry table\n", __func__);
 
 	/* Allocate bitmap for 32 entry mcam */
-	table->cam_table.bmap = devm_kcalloc(rvu->dev, 1, sizeof(long), GFP_KERNEL);
+	table->cam_table.bmap = devm_bitmap_zalloc(rvu->dev, 32, GFP_KERNEL);
 
 	if (!table->cam_table.bmap)
 		return -ENOMEM;
-- 
2.34.1

[PATCH 4/5] octeontx2-af: Fix the size of memory allocated for the 'id_bmap' bitmap

[Christophe JAILLET]

This allocation is really spurious.
The size of the bitmap is 'tot_ids' and it is used as such in the driver.

So we could expect something like:
   table->id_bmap = devm_kcalloc(rvu->dev, BITS_TO_LONGS(table->tot_ids),
			         sizeof(long), GFP_KERNEL);

However, when the bitmap is allocated, we allocate:
   BITS_TO_LONGS(table->tot_ids) * table->tot_ids ~=
   table->tot_ids / 32 * table->tot_ids ~=
   table->tot_ids^2 / 32

It is proportional to the square of 'table->tot_ids' which seems to
potentially be big.

Allocate the expected amount of memory, and switch to the bitmap API to
have it more straightforward.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
This patch is speculative.

If I'm right, I'm curious to know if 'able->tot_ids' can really get big
(I'm just guessing) , and if yes, how much.
---
 drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
index 3f94b620ef5a..ae34746341c4 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
@@ -1914,8 +1914,8 @@ int rvu_npc_exact_init(struct rvu *rvu)
 	dev_dbg(rvu->dev, "%s: Allocated bitmap for 32 entry cam\n", __func__);
 
 	table->tot_ids = (table->mem_table.depth * table->mem_table.ways) + table->cam_table.depth;
-	table->id_bmap = devm_kcalloc(rvu->dev, BITS_TO_LONGS(table->tot_ids),
-				      table->tot_ids, GFP_KERNEL);
+	table->id_bmap = devm_bitmap_zalloc(rvu->dev, table->tot_ids,
+					    GFP_KERNEL);
 
 	if (!table->id_bmap)
 		return -ENOMEM;
-- 
2.34.1

[PATCH 5/5] octeontx2-af: Simplify a size computation in rvu_npc_exact_init()

[Christophe JAILLET]

We know that table_size = table->mem_table.depth * table->mem_table.ways,
so use it instead, it is less verbose.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
index ae34746341c4..00aef8f5ac29 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
@@ -1913,7 +1913,7 @@ int rvu_npc_exact_init(struct rvu *rvu)
 
 	dev_dbg(rvu->dev, "%s: Allocated bitmap for 32 entry cam\n", __func__);
 
-	table->tot_ids = (table->mem_table.depth * table->mem_table.ways) + table->cam_table.depth;
+	table->tot_ids = table_size + table->cam_table.depth;
 	table->id_bmap = devm_bitmap_zalloc(rvu->dev, table->tot_ids,
 					    GFP_KERNEL);
 
-- 
2.34.1

Re: [PATCH 1/5] octeontx2-af: Fix a potentially spurious error message

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Fri, 25 Nov 2022 13:23:57 +0100 you wrote:
> When this error message is displayed, we know that the all the bits in the
> bitmap are set.
> 
> So, bitmap_weight() will return the number of bits of the bitmap, which is
> 'table->tot_ids'.
> 
> It is unlikely that a bit will be cleared between mutex_unlock() and
> dev_err(), but, in order to simplify the code and avoid this possibility,
> just take 'table->tot_ids'.
> 
> [...]

Here is the summary with links:
  - [1/5] octeontx2-af: Fix a potentially spurious error message
    https://git.kernel.org/netdev/net-next/c/2450d7d93fd2
  - [2/5] octeontx2-af: Slightly simplify rvu_npc_exact_init()
    https://git.kernel.org/netdev/net-next/c/b6a0ecaee2e6
  - [3/5] octeontx2-af: Use the bitmap API to allocate bitmaps
    https://git.kernel.org/netdev/net-next/c/05a7b52ee5e4
  - [4/5] octeontx2-af: Fix the size of memory allocated for the 'id_bmap' bitmap
    https://git.kernel.org/netdev/net-next/c/6d135d9e2b00
  - [5/5] octeontx2-af: Simplify a size computation in rvu_npc_exact_init()
    https://git.kernel.org/netdev/net-next/c/450f06505396

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html