Bug #13227
closed
SMB server serializes writes where it should not
100%
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.
Updated by Adam Stylinski about 3 years ago
Maybe this is at least indirectly related to incident #13138?
Updated by Toomas Soome about 3 years 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:)
Updated by Gordon Ross almost 3 years ago
Testing:
iometer with four workers, 100% write, on a VM with disks over NFS.
Before got about 35MB/sec, and after shows about 48MB/sec.
Previous tests on real hardware show nearly 2:1 improvement with parallel writes to the same file.
Updated by Gordon Ross almost 3 years ago
- Status changed from In Progress to Pending RTI
Updated by Electric Monk almost 3 years ago
- Status changed from Pending RTI to Closed
- % Done changed from 0 to 100
git commit f74a127feb6111d3b6890ae374a142eda1fd4e3e
commit f74a127feb6111d3b6890ae374a142eda1fd4e3e
Author: Gordon Ross <gordon.ross@tintri.com>
Date: 2021-02-08T20:21:58.000Z
13227 SMB server serializes writes where it should not
Reviewed by: Suresh Jayaraman <suresh.jayaraman@tegile.com>
Reviewed by: Prashanth Badari <prbadari@tintri.com>
Reviewed by: Andy Giles <andy@tegile.com>
Approved by: Dan McDonald <danmcd@joyent.com>