Project

General

Profile

Bug #13227

SMB server serializes writes where it should not

Added by Gordon Ross 13 days ago. Updated 5 days ago.

Status:
In Progress
Priority:
Normal
Assignee:
Category:
-
Start date:
Due date:
% Done:

0%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:

Description

During some performance testing, we discovered that the SMB server is serializing writes on a given file handle where it does not need to do so. This has fairly severe impact in performance tests that issue multiple writes on one file handle. (Sorry, don't have the details handy. Latency reduced about 50%)

The reason for this serialization is simply that some prior work accidentally used RW_WRITER where RW_READER was more suitable.
The fix is a one-liner:

diff -r be29f39333c9 usr/src/uts/common/fs/smbsrv/smb_fsops.c
--- a/usr/src/uts/common/fs/smbsrv/smb_fsops.c    Thu Jul 02 21:48:55 2020 +0000
+++ b/usr/src/uts/common/fs/smbsrv/smb_fsops.c    Fri Jul 03 13:01:50 2020 +0000
@@ -1539,7 +1539,7 @@ smb_fsop_write(
         cr = kcr;
     }

-    smb_node_start_crit(snode, RW_WRITER);
+    smb_node_start_crit(snode, RW_READER);
     rc = nbl_svmand(vp, kcr, &svmand);
     if (rc) {
         smb_node_end_crit(snode);

As per the nbmand locking design, the caller needs to acquire lock on the nbmand state in 'RW_WRITER' mode only if needs to obtain a byte-range lock or share reservation/share mode. For e.g. fs_shrlock() or fs_frlock(). In all other cases including writes/setattr/remove operations, the caller should acquire the lock in 'RW_READER' mode as it is not changing the the byte-range lock/share reservations state.

#1

Updated by Gordon Ross 13 days ago

  • Description updated (diff)
#2

Updated by Adam Stylinski 13 days ago

Maybe this is at least indirectly related to incident #13138?

#3

Updated by Toomas Soome 5 days ago

Gordon Ross wrote:

During some performance testing, we discovered that the SMB server is serializing writes on a given file handle where it does not need to do so. This has fairly severe impact in performance tests that issue multiple writes on one file handle. (Sorry, don't have the details handy. Latency reduced about 50%)

The reason for this serialization is simply that some prior work accidentally used RW_WRITER where RW_READER was more suitable.
The fix is a one-liner:
[...]

As per the nbmand locking design, the caller needs to acquire lock on the nbmand state in 'RW_WRITER' mode only if needs to obtain a byte-range lock or share reservation/share mode. For e.g. fs_shrlock() or fs_frlock(). In all other cases including writes/setattr/remove operations, the caller should acquire the lock in 'RW_READER' mode as it is not changing the the byte-range lock/share reservations state.

Appears to work ok for me:)

Also available in: Atom PDF