The Samba-Bugzilla – Attachment 13775 Details for
Bug 13121
Non-smbd processes using kernel oplocks can hang smbd.
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
git-am fix for 4.7.next.
bug-13121-4.7 (text/plain), 13.85 KB, created by
Jeremy Allison
on 2017-11-13 19:54:09 UTC
(
hide
)
Description:
git-am fix for 4.7.next.
Filename:
MIME Type:
Creator:
Jeremy Allison
Created:
2017-11-13 19:54:09 UTC
Size:
13.85 KB
patch
obsolete
>From 68fe50c5ceb4546a6f7fd511dbdf2d808bc04e85 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Thu, 9 Nov 2017 12:48:15 -0800 >Subject: [PATCH 1/2] s3: smbd: kernel oplocks. Replace retry_open() with > setup_kernel_oplock_poll_open(). > >If a O_NONBLOCK open fails with EWOULDBLOCK, this code changes smbd to >do a retry open every second, until either the timeout or we get a successful >open. If we're opening a file that has a kernel lease set by a non-smbd >process, this is the best we can do. > >Prior to this, smbd would block on the second open on such a leased file >(not using O_NONBLOCK) which freezes active clients. > >Regression test to follow. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13121 > >Signed-off-by: Jeremy Allison <jra@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit 47c13fc10a2c9709e9511b2ffcf0e1004497887d) >--- > source3/smbd/open.c | 96 +++++++++++++++++++++++++++++++++++++---------------- > 1 file changed, 68 insertions(+), 28 deletions(-) > >diff --git a/source3/smbd/open.c b/source3/smbd/open.c >index 89a267b0634..8c52f4bba56 100644 >--- a/source3/smbd/open.c >+++ b/source3/smbd/open.c >@@ -2410,19 +2410,40 @@ static void defer_open_done(struct tevent_req *req) > } > > /** >- * Reschedule an open for immediate execution >+ * Actually attempt the kernel oplock polling open. >+ */ >+ >+static void kernel_oplock_poll_open_timer(struct tevent_context *ev, >+ struct tevent_timer *te, >+ struct timeval current_time, >+ void *private_data) >+{ >+ bool ok; >+ struct smb_request *req = (struct smb_request *)private_data; >+ >+ ok = schedule_deferred_open_message_smb(req->xconn, req->mid); >+ if (!ok) { >+ exit_server("schedule_deferred_open_message_smb failed"); >+ } >+ DBG_DEBUG("kernel_oplock_poll_open_timer fired. Retying open !\n"); >+} >+ >+/** >+ * Reschedule an open for 1 second from now, if not timed out. > **/ >-static void retry_open(struct timeval request_time, >+static void setup_kernel_oplock_poll_open(struct timeval request_time, > struct smb_request *req, > struct file_id id) > { >- struct deferred_open_record *open_rec = NULL; >+ > bool ok; >+ struct deferred_open_record *open_rec = NULL; >+ /* Maximum wait time. */ >+ struct timeval timeout = timeval_set(OPLOCK_BREAK_TIMEOUT*2, 0); > >- DBG_DEBUG("request time [%s] mid [%" PRIu64 "] file_id [%s]\n", >- timeval_string(talloc_tos(), &request_time, false), >- req->mid, >- file_id_string_tos(&id)); >+ if (request_timed_out(request_time, timeout)) { >+ return; >+ } > > open_rec = deferred_open_record_create(false, false, id); > if (open_rec == NULL) { >@@ -2431,17 +2452,30 @@ static void retry_open(struct timeval request_time, > > ok = push_deferred_open_message_smb(req, > request_time, >- timeval_set(0, 0), >+ timeout, > id, > open_rec); > if (!ok) { > exit_server("push_deferred_open_message_smb failed"); > } > >- ok = schedule_deferred_open_message_smb(req->xconn, req->mid); >- if (!ok) { >- exit_server("schedule_deferred_open_message_smb failed"); >+ /* >+ * As this timer event is owned by req, it will >+ * disappear if req it talloc_freed. >+ */ >+ open_rec->te = tevent_add_timer(req->sconn->ev_ctx, >+ req, >+ timeval_current_ofs(1, 0), >+ kernel_oplock_poll_open_timer, >+ req); >+ if (open_rec->te == NULL) { >+ exit_server("tevent_add_timer failed"); > } >+ >+ DBG_DEBUG("poll request time [%s] mid [%" PRIu64 "] file_id [%s]\n", >+ timeval_string(talloc_tos(), &request_time, false), >+ req->mid, >+ file_id_string_tos(&id)); > } > > /**************************************************************************** >@@ -3160,20 +3194,18 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, > flags2 &= ~(O_CREAT|O_TRUNC); > } > >- if (first_open_attempt && lp_kernel_oplocks(SNUM(conn))) { >+ if (lp_kernel_oplocks(SNUM(conn))) { > /* > * With kernel oplocks the open breaking an oplock > * blocks until the oplock holder has given up the >- * oplock or closed the file. We prevent this by first >+ * oplock or closed the file. We prevent this by always > * trying to open the file with O_NONBLOCK (see "man >- * fcntl" on Linux). For the second try, triggered by >- * an oplock break response, we do not need this >- * anymore. >+ * fcntl" on Linux). > * >- * This is true under the assumption that only Samba >- * requests kernel oplocks. Once someone else like >- * NFSv4 starts to use that API, we will have to >- * modify this by communicating with the NFSv4 server. >+ * If a process that doesn't use the smbd open files >+ * database or communication methods holds a kernel >+ * oplock we must periodically poll for available open >+ * using O_NONBLOCK. > */ > flags2 |= O_NONBLOCK; > } >@@ -3252,9 +3284,16 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, > > lck = get_existing_share_mode_lock(talloc_tos(), fsp->file_id); > if (lck == NULL) { >- retry_open(request_time, req, fsp->file_id); >- DEBUG(10, ("No share mode lock found after " >- "EWOULDBLOCK, retrying sync\n")); >+ /* >+ * No oplock from Samba around. Set up a poll every 1 >+ * second to retry a non-blocking open until the time >+ * expires. >+ */ >+ setup_kernel_oplock_poll_open(request_time, >+ req, >+ fsp->file_id); >+ DBG_DEBUG("No Samba oplock around after EWOULDBLOCK. " >+ "Retrying with poll\n"); > return NT_STATUS_SHARING_VIOLATION; > } > >@@ -3275,14 +3314,15 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, > } > > /* >- * No oplock from Samba around. Immediately retry with >- * a blocking open. >+ * No oplock from Samba around. Set up a poll every 1 >+ * second to retry a non-blocking open until the time >+ * expires. > */ >- retry_open(request_time, req, fsp->file_id); >+ setup_kernel_oplock_poll_open(request_time, req, fsp->file_id); > > TALLOC_FREE(lck); >- DEBUG(10, ("No Samba oplock around after EWOULDBLOCK. " >- "Retrying sync\n")); >+ DBG_DEBUG("No Samba oplock around after EWOULDBLOCK. " >+ "Retrying with poll\n"); > return NT_STATUS_SHARING_VIOLATION; > } > >-- >2.14.1 > > >From 6237ed906e8986c162d207dc3bec22e71ce52219 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Thu, 9 Nov 2017 09:59:23 -0800 >Subject: [PATCH 2/2] s4: torture: kernel oplocks. Add > smb2.kernel-oplocks.kernel_oplocks8 >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >Test if the server blocks whilst waiting on a kernel lease held by >a non-smbd process. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13121 > >Signed-off-by: Jeremy Allison <jra@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> > >Autobuild-User(master): Ralph Böhme <slow@samba.org> >Autobuild-Date(master): Sat Nov 11 20:12:26 CET 2017 on sn-devel-144 > >(cherry picked from commit ad82557e1355107920ae80fd6a0df0f16d1bdb6c) >--- > source3/selftest/tests.py | 2 +- > source4/torture/smb2/oplock.c | 236 ++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 237 insertions(+), 1 deletion(-) > >diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py >index 56b94c436ce..006451291aa 100755 >--- a/source3/selftest/tests.py >+++ b/source3/selftest/tests.py >@@ -497,7 +497,7 @@ for t in tests: > plansmbtorture4testsuite(t, "simpleserver", '//$SERVER/dosmode -U$USERNAME%$PASSWORD') > elif t == "smb2.kernel-oplocks": > if have_linux_kernel_oplocks: >- plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER/kernel_oplocks -U$USERNAME%$PASSWORD') >+ plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER/kernel_oplocks -U$USERNAME%$PASSWORD --option=torture:localdir=$SELFTEST_PREFIX/nt4_dc/share') > elif t == "smb2.notify-inotify": > if have_inotify: > plansmbtorture4testsuite(t, "fileserver", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD') >diff --git a/source4/torture/smb2/oplock.c b/source4/torture/smb2/oplock.c >index 3290ed42d8c..1830a017e83 100644 >--- a/source4/torture/smb2/oplock.c >+++ b/source4/torture/smb2/oplock.c >@@ -36,6 +36,8 @@ > #include "torture/torture.h" > #include "torture/smb2/proto.h" > >+#include "lib/util/sys_rw.h" >+ > #define CHECK_RANGE(v, min, max) do { \ > if ((v) < (min) || (v) > (max)) { \ > torture_result(tctx, TORTURE_FAIL, "(%s): wrong value for %s " \ >@@ -4790,6 +4792,239 @@ done: > return ret; > } > >+#if HAVE_KERNEL_OPLOCKS_LINUX >+ >+#ifndef F_SETLEASE >+#define F_SETLEASE 1024 >+#endif >+ >+#ifndef RT_SIGNAL_LEASE >+#define RT_SIGNAL_LEASE (SIGRTMIN+1) >+#endif >+ >+#ifndef F_SETSIG >+#define F_SETSIG 10 >+#endif >+ >+static int got_break; >+ >+/* >+ * Signal handler. >+ */ >+ >+static void got_rt_break(int sig) >+{ >+ got_break = 1; >+} >+ >+/* >+ * Child process function. >+ */ >+ >+static int do_child_process(int pipefd, const char *name) >+{ >+ int ret = 0; >+ int fd = -1; >+ char c = 0; >+ struct sigaction act; >+ >+ /* Set up a signal handler for RT_SIGNAL_LEASE. */ >+ ZERO_STRUCT(act); >+ act.sa_handler = got_rt_break; >+ ret = sigaction(RT_SIGNAL_LEASE, &act, NULL); >+ if (ret == -1) { >+ return 1; >+ } >+ /* Open the passed in file and get a kernel oplock. */ >+ fd = open(name, O_RDWR, 0666); >+ if (fd == -1) { >+ return 2; >+ } >+ >+ ret = fcntl(fd, F_SETSIG, RT_SIGNAL_LEASE); >+ if (ret == -1) { >+ return 3; >+ } >+ >+ ret = fcntl(fd, F_SETLEASE, F_WRLCK); >+ if (ret == -1) { >+ return 4; >+ } >+ >+ /* Tell the parent we're ready. */ >+ ret = sys_write(pipefd, &c, 1); >+ if (ret != 1) { >+ return 5; >+ } >+ >+ /* Wait for RT_SIGNAL_LEASE. */ >+ ret = pause(); >+ if (ret != -1 || errno != EINTR) { >+ return 6; >+ } >+ >+ if (got_break != 1) { >+ return 7; >+ } >+ >+ /* Force the server to wait for 3 seconds. */ >+ sleep(3); >+ >+ /* Remove our lease. */ >+ ret = fcntl(fd, F_SETLEASE, F_UNLCK); >+ if (ret == -1) { >+ return 8; >+ } >+ >+ ret = close(fd); >+ if (ret == -1) { >+ return 9; >+ } >+ >+ /* All is well. */ >+ return 0; >+} >+ >+static bool wait_for_child_oplock(struct torture_context *tctx, >+ const char *localdir, >+ const char *fname) >+{ >+ int fds[2]; >+ int ret; >+ pid_t pid; >+ char *name = talloc_asprintf(tctx, >+ "%s/%s", >+ localdir, >+ fname); >+ >+ torture_assert(tctx, name != NULL, "talloc failed"); >+ >+ ret = pipe(fds); >+ torture_assert(tctx, ret != -1, "pipe failed"); >+ >+ pid = fork(); >+ torture_assert(tctx, pid != (pid_t)-1, "fork failed"); >+ >+ if (pid != (pid_t)0) { >+ char c; >+ /* Parent. */ >+ TALLOC_FREE(name); >+ ret = sys_read(fds[0], &c, 1); >+ torture_assert(tctx, ret == 1, "read failed"); >+ return true; >+ } >+ >+ /* Child process. */ >+ ret = do_child_process(fds[1], name); >+ _exit(ret); >+ /* Notreached. */ >+} >+#else >+static bool wait_for_child_oplock(struct torture_context *tctx, >+ const char *localdir, >+ const char *fname) >+{ >+ return false; >+} >+#endif >+ >+/* >+ * Deal with a non-smbd process holding a kernel oplock. >+ */ >+ >+static bool test_smb2_kernel_oplocks8(struct torture_context *tctx, >+ struct smb2_tree *tree) >+{ >+ const char *fname = "test_kernel_oplock8.dat"; >+ const char *fname1 = "tmp_test_kernel_oplock8.dat"; >+ NTSTATUS status; >+ bool ret = true; >+ struct smb2_create io; >+ struct smb2_request *req = NULL; >+ struct smb2_handle h1 = {{0}}; >+ struct smb2_handle h2 = {{0}}; >+ const char *localdir = torture_setting_string(tctx, "localdir", NULL); >+ time_t start; >+ time_t end; >+ >+#ifndef HAVE_KERNEL_OPLOCKS_LINUX >+ torture_skip(tctx, "Need kernel oplocks for test"); >+#endif >+ >+ if (localdir == NULL) { >+ torture_skip(tctx, "Need localdir for test"); >+ } >+ >+ smb2_util_unlink(tree, fname); >+ smb2_util_unlink(tree, fname1); >+ status = torture_smb2_testfile(tree, fname, &h1); >+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, >+ "Error creating testfile\n"); >+ smb2_util_close(tree, h1); >+ ZERO_STRUCT(h1); >+ >+ /* Take the oplock locally in a sub-process. */ >+ ret = wait_for_child_oplock(tctx, localdir, fname); >+ torture_assert_goto(tctx, ret = true, ret, done, >+ "Wait for child process failed.\n"); >+ >+ /* >+ * Now try and open. This should block for 3 seconds. >+ * while the child process is still alive. >+ */ >+ >+ ZERO_STRUCT(io); >+ io.in.desired_access = SEC_FLAG_MAXIMUM_ALLOWED; >+ io.in.file_attributes = FILE_ATTRIBUTE_NORMAL; >+ io.in.create_disposition = NTCREATEX_DISP_OPEN; >+ io.in.share_access = >+ NTCREATEX_SHARE_ACCESS_DELETE| >+ NTCREATEX_SHARE_ACCESS_READ| >+ NTCREATEX_SHARE_ACCESS_WRITE; >+ io.in.create_options = 0; >+ io.in.fname = fname; >+ >+ req = smb2_create_send(tree, &io); >+ torture_assert(tctx, req != NULL, "smb2_create_send"); >+ >+ /* Ensure while the open is blocked the smbd is >+ still serving other requests. */ >+ io.in.fname = fname1; >+ io.in.create_disposition = NTCREATEX_DISP_CREATE; >+ >+ /* Time the start -> end of the request. */ >+ start = time(NULL); >+ status = smb2_create(tree, tctx, &io); >+ end = time(NULL); >+ >+ /* Should succeed. */ >+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, >+ "Error opening the second file\n"); >+ h1 = io.out.file.handle; >+ >+ /* in less than 2 seconds. Otherwise the server blocks. */ >+ torture_assert(tctx, end - start < 2, "server was blocked !"); >+ >+ /* Pick up the return for the initial blocking open. */ >+ status = smb2_create_recv(req, tctx, &io); >+ >+ /* Which should also have succeeded. */ >+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, >+ "Error opening the file\n"); >+ h2 = io.out.file.handle; >+ >+done: >+ if (!smb2_util_handle_empty(h1)) { >+ smb2_util_close(tree, h1); >+ } >+ if (!smb2_util_handle_empty(h2)) { >+ smb2_util_close(tree, h2); >+ } >+ smb2_util_unlink(tree, fname); >+ smb2_util_unlink(tree, fname1); >+ return ret; >+} >+ > struct torture_suite *torture_smb2_kernel_oplocks_init(TALLOC_CTX *ctx) > { > struct torture_suite *suite = >@@ -4802,6 +5037,7 @@ struct torture_suite *torture_smb2_kernel_oplocks_init(TALLOC_CTX *ctx) > torture_suite_add_1smb2_test(suite, "kernel_oplocks5", test_smb2_kernel_oplocks5); > torture_suite_add_2smb2_test(suite, "kernel_oplocks6", test_smb2_kernel_oplocks6); > torture_suite_add_2smb2_test(suite, "kernel_oplocks7", test_smb2_kernel_oplocks7); >+ torture_suite_add_1smb2_test(suite, "kernel_oplocks8", test_smb2_kernel_oplocks8); > > suite->description = talloc_strdup(suite, "SMB2-KERNEL-OPLOCK tests"); > >-- >2.14.1 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Flags:
slow
:
review+
Actions:
View
Attachments on
bug 13121
:
13763
| 13775 |
13776