Linux CIFS filesystem development
 help / color / mirror / Atom feed
* [PATCH] smb: client: require CONFIG_CIFS_DEBUG2=y for accounting tcon bytes stats
@ 2026-08-06 19:05 Enzo Matsumiya
  2026-08-06 22:12 ` Steve French
  0 siblings, 1 reply; 3+ messages in thread
From: Enzo Matsumiya @ 2026-08-06 19:05 UTC (permalink / raw)
  To: linux-cifs
  Cc: smfrench, pc, ronniesahlberg, sprasad, tom, bharathsm,
	henrique.carvalho, Enzo Matsumiya

cifs_stats_bytes_{read,written} uses tcon->stat_lock to account for
R/W bytes.  This is done unconditionally on every read/write since
commit fcabb89299d7 ("cifs: simple stats should always be enabled").

On a highly concurrent workload, a spinlock in the middle of the R/W
codepaths can become heavily contended, which is absurd given its
purpose (protect non-actionable debug/stat data).

Make tcon->{bytes_read,bytes_written,stat_lock} usage depend on
CONFIG_CIFS_DEBUG2=y.

Fixes: fcabb89299d7 ("cifs: simple stats should always be enabled")
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
 fs/smb/client/cifs_debug.c |  4 +++-
 fs/smb/client/cifsglob.h   | 24 ++++++++++++++++++++----
 fs/smb/client/misc.c       |  2 ++
 fs/smb/client/smb1ops.c    | 19 +++++++++++++++----
 fs/smb/client/smb2ops.c    |  7 +++++--
 5 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/fs/smb/client/cifs_debug.c b/fs/smb/client/cifs_debug.c
index 4ed4f55a0bb7..3118174745e0 100644
--- a/fs/smb/client/cifs_debug.c
+++ b/fs/smb/client/cifs_debug.c
@@ -762,11 +762,13 @@ static ssize_t cifs_stats_proc_write(struct file *file,
 					continue;
 				list_for_each_entry(tcon, &ses->tcon_list, tcon_list) {
 					atomic_set(&tcon->num_smbs_sent, 0);
+#ifdef CONFIG_CIFS_DEBUG2
 					spin_lock(&tcon->stat_lock);
 					tcon->bytes_read = 0;
 					tcon->bytes_written = 0;
-					tcon->stats_from_time = ktime_get_real_seconds();
 					spin_unlock(&tcon->stat_lock);
+#endif /* CONFIG_CIFS_DEBUG2 */
+					tcon->stats_from_time = ktime_get_real_seconds();
 					if (server->ops->clear_stats)
 						server->ops->clear_stats(tcon);
 				}
diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
index 79e4e84f8985..e0e945c1de0f 100644
--- a/fs/smb/client/cifsglob.h
+++ b/fs/smb/client/cifsglob.h
@@ -1240,9 +1240,11 @@ struct cifs_tcon {
 			atomic_t smb2_com_failed[NUMBER_OF_SMB2_COMMANDS];
 		} smb2_stats;
 	} stats;
+#ifdef CONFIG_CIFS_DEBUG2
 	__u64    bytes_read;
 	__u64    bytes_written;
 	spinlock_t stat_lock;  /* protects the two fields above */
+#endif /* CONFIG_CIFS_DEBUG2 */
 	time64_t stats_from_time;
 	FILE_SYSTEM_DEVICE_INFO fsDevInfo;
 	FILE_SYSTEM_ATTRIBUTE_INFO fsAttrInfo; /* ok if fs name truncated */
@@ -1655,10 +1657,11 @@ convert_delimiter(char *path, char delim)
 
 #define cifs_stats_inc atomic_inc
 
+#ifdef CONFIG_CIFS_DEBUG2
 static inline void cifs_stats_bytes_written(struct cifs_tcon *tcon,
 					    unsigned int bytes)
 {
-	if (bytes) {
+	if (likely(bytes)) {
 		spin_lock(&tcon->stat_lock);
 		tcon->bytes_written += bytes;
 		spin_unlock(&tcon->stat_lock);
@@ -1668,10 +1671,23 @@ static inline void cifs_stats_bytes_written(struct cifs_tcon *tcon,
 static inline void cifs_stats_bytes_read(struct cifs_tcon *tcon,
 					 unsigned int bytes)
 {
-	spin_lock(&tcon->stat_lock);
-	tcon->bytes_read += bytes;
-	spin_unlock(&tcon->stat_lock);
+	if (likely(bytes)) {
+		spin_lock(&tcon->stat_lock);
+		tcon->bytes_read += bytes;
+		spin_unlock(&tcon->stat_lock);
+	}
+}
+#else /* CONFIG_CIFS_DEBUG2 */
+static inline void cifs_stats_bytes_written(struct cifs_tcon *tcon,
+					    unsigned int bytes)
+{
+}
+
+static inline void cifs_stats_bytes_read(struct cifs_tcon *tcon,
+					 unsigned int bytes)
+{
 }
+#endif /* !CONFIG_CIFS_DEBUG2 */
 
 
 /*
diff --git a/fs/smb/client/misc.c b/fs/smb/client/misc.c
index 46e1382e8e04..597dc3c68cd2 100644
--- a/fs/smb/client/misc.c
+++ b/fs/smb/client/misc.c
@@ -145,7 +145,9 @@ tcon_info_alloc(bool dir_leases_enabled, enum smb3_tcon_ref_trace trace)
 	INIT_LIST_HEAD(&ret_buf->tcon_list);
 	INIT_LIST_HEAD(&ret_buf->cifs_sb_list);
 	spin_lock_init(&ret_buf->open_file_lock);
+#ifdef CONFIG_CIFS_DEBUG2
 	spin_lock_init(&ret_buf->stat_lock);
+#endif /* CONFIG_CIFS_DEBUG2 */
 	spin_lock_init(&ret_buf->sb_list_lock);
 	atomic_set(&ret_buf->num_local_opens, 0);
 	atomic_set(&ret_buf->num_remote_opens, 0);
diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
index dc5a8c1da623..484e12cb9181 100644
--- a/fs/smb/client/smb1ops.c
+++ b/fs/smb/client/smb1ops.c
@@ -817,14 +817,25 @@ cifs_clear_stats(struct cifs_tcon *tcon)
 static void
 cifs_print_stats(struct seq_file *m, struct cifs_tcon *tcon)
 {
+	u64 bytes_read = 0, bytes_written = 0;
+
+#ifdef CONFIG_CIFS_DEBUG2
+	bytes_read = tcon->bytes_read;
+	bytes_written = tcon->bytes_written;
+#endif /* CONFIG_CIFS_DEBUG2 */
+
 	seq_printf(m, " Oplocks breaks: %d",
 		   atomic_read(&tcon->stats.cifs_stats.num_oplock_brks));
 	seq_printf(m, "\nReads:  %d Bytes: %llu",
-		   atomic_read(&tcon->stats.cifs_stats.num_reads),
-		   (long long)(tcon->bytes_read));
+		   atomic_read(&tcon->stats.cifs_stats.num_reads), bytes_read);
+#ifndef CONFIG_CIFS_DEBUG2
+	seq_puts(m, " (CONFIG_CIFS_DEBUG2 is disabled)");
+#endif /* !CONFIG_CIFS_DEBUG2 */
 	seq_printf(m, "\nWrites: %d Bytes: %llu",
-		   atomic_read(&tcon->stats.cifs_stats.num_writes),
-		   (long long)(tcon->bytes_written));
+		   atomic_read(&tcon->stats.cifs_stats.num_writes), bytes_written);
+#ifndef CONFIG_CIFS_DEBUG2
+	seq_puts(m, " (CONFIG_CIFS_DEBUG2 is disabled)");
+#endif /* !CONFIG_CIFS_DEBUG2 */
 	seq_printf(m, "\nFlushes: %d",
 		   atomic_read(&tcon->stats.cifs_stats.num_flushes));
 	seq_printf(m, "\nLocks: %d HardLinks: %d Symlinks: %d",
diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index 192649fec25d..e1e28f202d0c 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -1425,9 +1425,12 @@ smb2_print_stats(struct seq_file *m, struct cifs_tcon *tcon)
 	 *  Can't display SMB2_NEGOTIATE, SESSION_SETUP, LOGOFF, CANCEL and ECHO
 	 *  totals (requests sent) since those SMBs are per-session not per tcon
 	 */
+#ifdef CONFIG_CIFS_DEBUG2
 	seq_printf(m, "\nBytes read: %llu  Bytes written: %llu",
-		   (long long)(tcon->bytes_read),
-		   (long long)(tcon->bytes_written));
+		   tcon->bytes_read, tcon->bytes_written);
+#else /* CONFIG_CIFS_DEBUG2 */
+	seq_puts(m, "\nBytes read: 0 Bytes written: 0 (CONFIG_CIFS_DEBUG2 is disabled)");
+#endif /* !CONFIG_CIFS_DEBUG2 */
 	seq_printf(m, "\nOpen files: %d total (local), %d open on server",
 		   atomic_read(&tcon->num_local_opens),
 		   atomic_read(&tcon->num_remote_opens));
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-08-07 12:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-06 19:05 [PATCH] smb: client: require CONFIG_CIFS_DEBUG2=y for accounting tcon bytes stats Enzo Matsumiya
2026-08-06 22:12 ` Steve French
2026-08-07 12:21   ` Enzo Matsumiya

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox