[PATCH] smb: client: require CONFIG_CIFS_DEBUG2=y for accounting tcon bytes stats
Enzo Matsumiya <[email protected]>
| Newsgroups | org.kernel.vger.linux-cifs |
|---|---|
| Message-ID | <[email protected]> |
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 <[email protected]>
---
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