net/smc: Fix pos miscalculation in statistics

[ Upstream commit a950a5921db450c74212327f69950ff03419483a ]

SMC_STAT_PAYLOAD_SUB(_smc_stats, _tech, key, _len, _rc) will calculate
wrong bucket positions for payloads of exactly 4096 bytes and
(1 << (m + 12)) bytes, with m == SMC_BUF_MAX - 1.

Intended bucket distribution:
Assume l == size of payload, m == SMC_BUF_MAX - 1.

Bucket 0                : 0 < l <= 2^13
Bucket n, 1 <= n <= m-1 : 2^(n+12) < l <= 2^(n+13)
Bucket m                : l > 2^(m+12)

Current solution:
_pos = fls64((l) >> 13)
[...]
_pos = (_pos < m) ? ((l == 1 << (_pos + 12)) ? _pos - 1 : _pos) : m

For l == 4096, _pos == -1, but should be _pos == 0.
For l == (1 << (m + 12)), _pos == m, but should be _pos == m - 1.

In order to avoid special treatment of these corner cases, the
calculation is adjusted. The new solution first subtracts the length by
one, and then calculates the correct bucket by shifting accordingly,
i.e. _pos = fls64((l - 1) >> 13), l > 0.
This not only fixes the issues named above, but also makes the whole
bucket assignment easier to follow.

Same is done for SMC_STAT_RMB_SIZE_SUB(_smc_stats, _tech, k, _len),
where the calculation of the bucket position is similar to the one
named above.

Fixes: e0e4b8fa5338 ("net/smc: Add SMC statistics support")
Suggested-by: Halil Pasic <pasic@linux.ibm.com>
Signed-off-by: Nils Hoppmann <niho@linux.ibm.com>
Reviewed-by: Halil Pasic <pasic@linux.ibm.com>
Reviewed-by: Wenjia Zhang <wenjia@linux.ibm.com>
Reviewed-by: Dust Li <dust.li@linux.alibaba.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/net/smc/smc_stats.h b/net/smc/smc_stats.h
index 4dbc237b7c19e74b8093e4a53d10668df398b85e..ee22d6f9a86aae2b6d0759fc64faf0f4a91e036d 100644 (file)
--- a/net/smc/smc_stats.h
+++ b/net/smc/smc_stats.h
@@ -93,13 +93,14 @@ do { \
        typeof(_smc_stats) stats = (_smc_stats); \
        typeof(_tech) t = (_tech); \
        typeof(_len) l = (_len); \
-       int _pos = fls64((l) >> 13); \
+       int _pos; \
        typeof(_rc) r = (_rc); \
        int m = SMC_BUF_MAX - 1; \
        this_cpu_inc((*stats).smc[t].key ## _cnt); \
-       if (r <= 0) \
+       if (r <= 0 || l <= 0) \
                break; \
-       _pos = (_pos < m) ? ((l == 1 << (_pos + 12)) ? _pos - 1 : _pos) : m; \
+       _pos = fls64((l - 1) >> 13); \
+       _pos = (_pos <= m) ? _pos : m; \
        this_cpu_inc((*stats).smc[t].key ## _pd.buf[_pos]); \
        this_cpu_add((*stats).smc[t].key ## _bytes, r); \
 } \
@@ -139,9 +140,12 @@ while (0)
 do { \
        typeof(_len) _l = (_len); \
        typeof(_tech) t = (_tech); \
-       int _pos = fls((_l) >> 13); \
+       int _pos; \
        int m = SMC_BUF_MAX - 1; \
-       _pos = (_pos < m) ? ((_l == 1 << (_pos + 12)) ? _pos - 1 : _pos) : m; \
+       if (_l <= 0) \
+               break; \
+       _pos = fls((_l - 1) >> 13); \
+       _pos = (_pos <= m) ? _pos : m; \
        this_cpu_inc((*(_smc_stats)).smc[t].k ## _rmbsize.buf[_pos]); \
 } \
 while (0)