Hello,

On Fri, 6 Dec 2024, David Laight wrote:

The intention of the code seems to be that the minimum table
size should be 256 (1 << min).
However the code uses max = clamp(20, 5, max_avail) which implies

	Actually, it tries to reduce max=20 (max possible) below
max_avail: [8 .. max_avail]. Not sure what 5 is here...

the author thought max_avail could be less than 5.
But clamp(val, min, max) is only well defined for max >= min.
If max < min whether is returns min or max depends on the order of
the comparisons.

	Looks like max_avail goes below 8 ? What value you see
for such small system?

Change to clamp(max_avail, 5, 20) which has the expected behaviour.

	It should be clamp(max_avail, 8, 20)

Replace the clamp_val() on the line below with clamp().
clamp_val() is just 'an accident waiting to happen' and not needed here.

	OK

Fixes: 4f325e26277b6
(Although I actually doubt the code is used on small memory systems.)

Detected by compile time checks added to clamp(), specifically:
minmax.h: use BUILD_BUG_ON_MSG() for the lo < hi test in clamp()

	Existing or new check? Does it happen that max_avail
is a constant, so that a compile check triggers?

Signed-off-by: David Laight <redacted>

	The code below looks ok to me but can you change the
comments above to more correctly specify the values and if the
problem is that max_avail goes below 8 (min).

	More likely we can additionally clamp max_avail here:

	max_avail = max(min, max_avail);

	But your solution solves the problem with less lines.

-	max = clamp(max, min, max_avail);
-	ip_vs_conn_tab_bits = clamp_val(ip_vs_conn_tab_bits, min, max);
+	max = clamp(max_avail, min, max);
+	ip_vs_conn_tab_bits = clamp(ip_vs_conn_tab_bits, min, max);
 	ip_vs_conn_tab_size = 1 << ip_vs_conn_tab_bits;
 	ip_vs_conn_tab_mask = ip_vs_conn_tab_size - 1;
 
-- 
2.17.1

Regards

--
Julian Anastasov [off-list ref]