[PATCHES] Issue fsync for SMB2 FLUSH asynchronously
Jeremy Allison
jra at samba.org
Wed Nov 11 22:37:06 UTC 2015
On Wed, Nov 11, 2015 at 03:32:59PM -0700, Christof Schmitt wrote:
> On Wed, Nov 11, 2015 at 01:27:50PM -0800, Jeremy Allison wrote:
> > On Wed, Nov 11, 2015 at 01:36:32PM -0700, Christof Schmitt wrote:
> > >
> > > From 4762b67740770f1ba43e78349c8d1f48857b55dd Mon Sep 17 00:00:00 2001
> > > From: Christof Schmitt <cs at samba.org>
> > > Date: Wed, 11 Nov 2015 13:31:15 -0700
> > > Subject: [PATCH 2/2] smbd: Issue fsync for SMB2 FLUSH asynchronously
> > >
> > > SMB2 FLUSH mainly calls fsync and there is already code in place to
> > > handle fsync asynchronously, so use the asynchronous code path for SMB2
> > > FLUSH. This avoids a SMB2 FLUSH stalling other requests processing.
> >
> > This code isn't quite right I'm afraid.
> >
> > It would end up calling tevent_req_done()
> > twice, once in at the end of smbd_smb2_flush_send()
> > and then again in smbd_smb2_flush_done() when
> > when the subreq completes.
>
> Yes, sorry. I meant to handle this correctly, but then forgot again.
>
> >
> > The correct thing to do in the:
> >
> > if (lp_strict_sync(SNUM(smbreq->conn))) {
> >
> > block is to add a cancellation function
> > to the outstanding req using tevent_req_set_cancel_fn()
> > (see smbd_smb2_write_send() for details) to allow
> > the client to send a cancel on this outstanding request,
> > then add the req to the pending aio queue
> > using aio_add_req_to_fsp() (see smbd_smb2_setinfo_send()
> > for details) to ensure that if the file gets
> > closed whilst this flush request is outstanding
> > then when the flush completes it doesn't
> > crash smbd.
> >
> > I'll code this up for you if you like as
> > it is a little trickier than it looks.
>
> Looking at smbd_smb2_write_cancel this does not seem to be too
> difficult. Let me try to add this.
Errr. Sorry, I already did it :-(.
> > Thanks a *LOT* for doing this though !
> > It's something we've needed for a while.
>
> Agreed, it is needed. When looking through the code, i started wondering
> why this has not been done before, given that there is already an async
> FSYNC available.
Turns out we don't need the cancellation
function (not really cancellable).
Here is the version I have and am
testing right now.
Let me know if this works for you.
Cheers,
Jeremy
-------------- next part --------------
From 01ad975c265982dc2ae04f4207e849587c883409 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Wed, 11 Nov 2015 13:28:09 -0700
Subject: [PATCH 1/2] selftest: Use strict sync = yes
This enables the codepaths calling fsync for FLUSH requests during
selftest.
Signed-off-by: Christof Schmitt <cs at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
selftest/target/Samba3.pm | 1 +
1 file changed, 1 insertion(+)
diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index 4b7882b..1c54dae 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -1331,6 +1331,7 @@ sub provision($$$$$$$$)
create mask = 755
dos filemode = yes
strict rename = yes
+ strict sync = yes
vfs objects = acl_xattr fake_acls xattr_tdb streams_depot
printing = vlp
--
2.6.0.rc2.230.g3dd15c0
From 11d01eaab76dd05cb4c1c7e02bacfcf354da9a9e Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Wed, 11 Nov 2015 13:31:15 -0700
Subject: [PATCH 2/2] smbd: Issue fsync for SMB2 FLUSH asynchronously
SMB2 FLUSH mainly calls fsync and there is already code in place to
handle fsync asynchronously, so use the asynchronous code path for SMB2
FLUSH. This avoids a SMB2 FLUSH stalling other requests processing.
Signed-off-by: Christof Schmitt <cs at samba.org>
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/smb2_flush.c | 57 +++++++++++++++++++++++++++++++++++++++++------
1 file changed, 50 insertions(+), 7 deletions(-)
diff --git a/source3/smbd/smb2_flush.c b/source3/smbd/smb2_flush.c
index 04a8710..15dc538 100644
--- a/source3/smbd/smb2_flush.c
+++ b/source3/smbd/smb2_flush.c
@@ -110,15 +110,18 @@ struct smbd_smb2_flush_state {
struct smbd_smb2_request *smb2req;
};
+static void smbd_smb2_flush_done(struct tevent_req *subreq);
+
static struct tevent_req *smbd_smb2_flush_send(TALLOC_CTX *mem_ctx,
struct tevent_context *ev,
struct smbd_smb2_request *smb2req,
struct files_struct *fsp)
{
struct tevent_req *req;
+ struct tevent_req *subreq;
struct smbd_smb2_flush_state *state;
- NTSTATUS status;
struct smb_request *smbreq;
+ int ret;
req = tevent_req_create(mem_ctx, &state,
struct smbd_smb2_flush_state);
@@ -145,16 +148,56 @@ static struct tevent_req *smbd_smb2_flush_send(TALLOC_CTX *mem_ctx,
return tevent_req_post(req, ev);
}
- status = sync_file(smbreq->conn, fsp, true);
- if (!NT_STATUS_IS_OK(status)) {
- DEBUG(5,("smbd_smb2_flush: sync_file for %s returned %s\n",
- fsp_str_dbg(fsp), nt_errstr(status)));
- tevent_req_nterror(req, status);
+ if (fsp->fh->fd == -1) {
+ tevent_req_nterror(req, NT_STATUS_INVALID_HANDLE);
+ return tevent_req_post(req, ev);
+ }
+
+ if (!lp_strict_sync(SNUM(smbreq->conn))) {
+ /*
+ * No strict sync. Don't really do
+ * anything here.
+ */
+ tevent_req_done(req);
+ return tevent_req_post(req, ev);
+ }
+
+ ret = flush_write_cache(fsp, SAMBA_SYNC_FLUSH);
+ if (ret == -1) {
+ tevent_req_nterror(req, map_nt_error_from_unix(errno));
+ return tevent_req_post(req, ev);
+ }
+
+ subreq = SMB_VFS_FSYNC_SEND(state, ev, fsp);
+ if (tevent_req_nomem(subreq, req)) {
return tevent_req_post(req, ev);
}
+ tevent_req_set_callback(subreq, smbd_smb2_flush_done, req);
+
+ /* Ensure any close request knows about this outstanding IO. */
+ if (!aio_add_req_to_fsp(fsp, req)) {
+ tevent_req_nterror(req, NT_STATUS_NO_MEMORY);
+ return tevent_req_post(req, ev);
+ }
+
+ return req;
+
+}
+
+static void smbd_smb2_flush_done(struct tevent_req *subreq)
+{
+ struct tevent_req *req = tevent_req_callback_data(
+ subreq, struct tevent_req);
+ int ret, err;
+
+ ret = SMB_VFS_FSYNC_RECV(subreq, &err);
+ TALLOC_FREE(subreq);
+ if (ret == -1) {
+ tevent_req_error(req, err);
+ return;
+ }
tevent_req_done(req);
- return tevent_req_post(req, ev);
}
static NTSTATUS smbd_smb2_flush_recv(struct tevent_req *req)
--
2.6.0.rc2.230.g3dd15c0
More information about the samba-technical
mailing list