Re: [PATCH net-next 2/2] net/smc: introduce statistics for ringbufs usage of net namespace

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2024/8/6 18:49, Simon Horman wrote:
On Mon, Aug 05, 2024 at 05:05:51PM +0800, Wen Gu wrote:
The buffer size histograms in smc_stats, namely rx/tx_rmbsize, record
the sizes of ringbufs for all connections that have ever appeared in
the net namespace. They are incremental and we cannot know the actual
ringbufs usage from these. So here introduces statistics for current
ringbufs usage of existing smc connections in the net namespace into
smc_stats, it will be incremented when new connection uses a ringbuf
and decremented when the ringbuf is unused.

Signed-off-by: Wen Gu <guwen@xxxxxxxxxxxxxxxxx>

...

diff --git a/net/smc/smc_stats.h b/net/smc/smc_stats.h

...

@@ -135,38 +137,45 @@ do { \
  } \
  while (0)
-#define SMC_STAT_RMB_SIZE_SUB(_smc_stats, _tech, k, _len) \
+#define SMC_STAT_RMB_SIZE_SUB(_smc_stats, _tech, k, _is_add, _len) \
  do { \
+	typeof(_is_add) is_a = (_is_add); \
  	typeof(_len) _l = (_len); \
  	typeof(_tech) t = (_tech); \
  	int _pos; \
  	int m = SMC_BUF_MAX - 1; \
  	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]); \
+	if (is_a) { \
+		_pos = fls((_l - 1) >> 13); \
+		_pos = (_pos <= m) ? _pos : m; \
+		this_cpu_inc((*(_smc_stats)).smc[t].k ## _rmbsize.buf[_pos]); \
+		this_cpu_add((*(_smc_stats)).smc[t].k ## _rmbuse, _l); \

Nit:

I see that due to the construction of the caller, SMC_STAT_RMB_SIZE(),
it will not occur. But checkpatch warns of possible side effects
from reuse of _smc_stats.

As great care seems to have been taken in these macros to avoid such
problems, even if theoretical, perhaps it is worth doing so here too.

f.e. A macro-local variable could store (*(_smc_stats)).smc[t] which
      I think would both resolve the problem mentioned, and make some
      lines shorter (and maybe easier to read).


It makes sense. I will use a macro-local variable of smc_stats. Thank you!

+	} else { \
+		this_cpu_sub((*(_smc_stats)).smc[t].k ## _rmbuse, _l); \
+	} \
  } \
  while (0)
#define SMC_STAT_RMB_SUB(_smc_stats, type, t, key) \
  	this_cpu_inc((*(_smc_stats)).smc[t].rmb ## _ ## key.type ## _cnt)
-#define SMC_STAT_RMB_SIZE(_smc, _is_smcd, _is_rx, _len) \
+#define SMC_STAT_RMB_SIZE(_smc, _is_smcd, _is_rx, _is_add, _len) \
  do { \
  	struct net *_net = sock_net(&(_smc)->sk); \
  	struct smc_stats __percpu *_smc_stats = _net->smc.smc_stats; \
+	typeof(_is_add) is_add = (_is_add); \
  	typeof(_is_smcd) is_d = (_is_smcd); \
  	typeof(_is_rx) is_r = (_is_rx); \
  	typeof(_len) l = (_len); \
  	if ((is_d) && (is_r)) \
-		SMC_STAT_RMB_SIZE_SUB(_smc_stats, SMC_TYPE_D, rx, l); \
+		SMC_STAT_RMB_SIZE_SUB(_smc_stats, SMC_TYPE_D, rx, is_add, l); \
  	if ((is_d) && !(is_r)) \
-		SMC_STAT_RMB_SIZE_SUB(_smc_stats, SMC_TYPE_D, tx, l); \
+		SMC_STAT_RMB_SIZE_SUB(_smc_stats, SMC_TYPE_D, tx, is_add, l); \
  	if (!(is_d) && (is_r)) \
-		SMC_STAT_RMB_SIZE_SUB(_smc_stats, SMC_TYPE_R, rx, l); \
+		SMC_STAT_RMB_SIZE_SUB(_smc_stats, SMC_TYPE_R, rx, is_add, l); \
  	if (!(is_d) && !(is_r)) \
-		SMC_STAT_RMB_SIZE_SUB(_smc_stats, SMC_TYPE_R, tx, l); \
+		SMC_STAT_RMB_SIZE_SUB(_smc_stats, SMC_TYPE_R, tx, is_add, l); \
  } \
  while (0)
--
2.32.0.3.g01195cf9f






[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Kernel Development]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Info]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Linux Media]     [Device Mapper]

  Powered by Linux