[PATCHES] Issue fsync for SMB2 FLUSH asynchronously

[Christof Schmitt]

On Fri, Nov 13, 2015 at 09:06:50AM -0800, Jeremy Allison wrote:
> On Fri, Nov 13, 2015 at 09:44:06AM -0700, Christof Schmitt wrote:
> > > > > After some private conversations with Volker,
> > > > > here is a follow-up patch that will apply
> > > > > on top of the previous one and removes one
> > > > > of the global varables completely.
> > > > > 
> > > > > Volker pointed out that we need to rethink
> > > > > the aio_pending_size limit on outstanding
> > > > > aio requests.
> > > > > 
> > > > > What happens when we reach this limit is
> > > > > that we start processing SMB requests
> > > > > synchronously, which slows down the system
> > > > > further and we never get to see the replies
> > > > > from the asys_results write to the internal
> > > > > pipe as we're too busy reading and processing
> > > > > incoming SMB requests.
> > > > > 
> > > > > So here is a patchset that does a few
> > > > > things on top of the one you pushed !
> > > > > 
> > > > > a). Removes --with-aio-support. We no longer
> > > > > need to run on systems that don't have
> > > > > pthread support but use POSIX-RT aio_read()
> > > > > aio_write() and friends. These days we
> > > > > require pthreads.
> > > > > 
> > > > > This has the advantage that it removes
> > > > > vfs_aio_posix as it's no longer used
> > > > > (and indeed it was never even documented !).
> > > > > 
> > > > > Plus is removes a boatload of code and
> > > > > options processing (as we no longer have
> > > > > to check for POSIX-RT io calls).
> > > > > 
> > > > > b). Removes the fallback to processing SMB requests
> > > > > synchronously which as Volker pointed out makes
> > > > > the problem worse. Instead always put them onto
> > > > > the internal pthreadpool queue, which will
> > > > > dispatch them as a thread becomes free.
> > > > > 
> > > > > c). Change the internal aio_pending_size static variable
> > > > > which is hard-coded at compile time to 100 to a
> > > > > proper smb.conf global variable "aio max threads"
> > > > > which is set to 100 by default but is now a
> > > > > system-tunable.
> > > > > 
> > > > > It passes make test.
> > > > 
> > > > Reviewed-by: Me.
> > 
> > Also
> > Reviewed-by: me
> > 
> > > > I have only one minor question: Should we explain in the
> > > > manpage entry that the parameter is per smbd, not global?
> > > 
> > > Sounds like a good idea.
> > > 
> > > Like this?
> > > 
> > > s/threads Samba will create/threads one smbd process will create/
> > 
> > Yes, i think that would clarify it.
> OK, I'll fixup the man page and push.
> 
> Thanks everyone !

Probably we should also update the documentation for 'strict sync' and
mention the changes in WHATSNEW. See attached patches.

Christof
-------------- next part --------------
From 9f2cee9674e46dc0f0f375d137ce5e4eacf82f5f Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Fri, 13 Nov 2015 11:46:33 -0700
Subject: [PATCH 1/2] docs: Update doc for 'strict sync' parameter for async SMB2 flush

Signed-off-by: Christof Schmitt <cs at samba.org>
---
 docs-xml/smbdotconf/tuning/strictsync.xml |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/docs-xml/smbdotconf/tuning/strictsync.xml b/docs-xml/smbdotconf/tuning/strictsync.xml
index 0d33845..5cfd388 100644
--- a/docs-xml/smbdotconf/tuning/strictsync.xml
+++ b/docs-xml/smbdotconf/tuning/strictsync.xml
@@ -5,7 +5,7 @@
  <description>
     <para>Many Windows applications (including the Windows 98 explorer
     shell) seem to confuse flushing buffer contents to disk with doing
-    a sync to disk. Under UNIX, a sync call forces the process to be
+    a sync to disk. Under UNIX, a sync call forces the thread to be
     suspended until the kernel has ensured that all outstanding data in
     kernel disk buffers has been safely stored onto stable storage.
     This is very slow and should only be done rarely. Setting this
@@ -17,6 +17,10 @@
     on crashes, so there is little danger in this default setting. In
     addition, this fixes many performance problems that people have
     reported with the new Windows98 explorer shell file copies.</para>
+    <para>The flush request from SMB2/3 clients is handled
+    asynchronously, so for these clients setting the parameter
+    to <constant>yes</constant> does not block the processing of other
+    requests in the smbd process.</para>
 </description>
 
 <related>sync always</related>
-- 
1.7.1


From 56a82195b5d4f944e9e759c3e6cab95eef4a9fd2 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Fri, 13 Nov 2015 11:47:32 -0700
Subject: [PATCH 2/2] WHATSNEW: Add async SMB2 flush and new aio parameter

Signed-off-by: Christof Schmitt <cs at samba.org>
---
 WHATSNEW.txt |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/WHATSNEW.txt b/WHATSNEW.txt
index d76d8fc..f1a1c07 100644
--- a/WHATSNEW.txt
+++ b/WHATSNEW.txt
@@ -17,6 +17,11 @@ Nothing special.
 NEW FEATURES
 ============
 
+Flush requests from SMB2/3 clients are handled asynchronously and do
+not block the processing of other requests. Note that 'strict sync'
+has to be set to 'yes' for Samba to honor flush requests from SMB
+clients.
+
 WINS nsswitch module
 ====================
 
@@ -34,6 +39,7 @@ smb.conf changes
 
   Parameter Name		Description		Default
   --------------		-----------		-------
+  aio max threads               New                     100
 
 KNOWN ISSUES
 ============
-- 
1.7.1