Skip to content

Commit

Permalink
net/smc: Fix pos miscalculation in statistics
Browse files Browse the repository at this point in the history
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: e0e4b8f ("net/smc: Add SMC statistics support")
Suggested-by: Halil Pasic <[email protected]>
Signed-off-by: Nils Hoppmann <[email protected]>
Reviewed-by: Halil Pasic <[email protected]>
Reviewed-by: Wenjia Zhang <[email protected]>
Reviewed-by: Dust Li <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
  • Loading branch information
Nils Hoppmann authored and davem330 committed Oct 11, 2023
1 parent 1469099 commit a950a59
Showing 1 changed file with 9 additions and 5 deletions.
14 changes: 9 additions & 5 deletions net/smc/smc_stats.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,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); \
} \
Expand Down Expand Up @@ -138,9 +139,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)
Expand Down

0 comments on commit a950a59

Please sign in to comment.