smbd: Separate aio_pthread indicator from normal EINTR
authorVolker Lendecke <vl@samba.org>
Thu, 20 Feb 2020 13:13:35 +0000 (14:13 +0100)
committerKarolin Seeger <kseeger@samba.org>
Tue, 25 Feb 2020 19:32:28 +0000 (19:32 +0000)
According to Posix and the Linux open(2) manpage, the open-syscall can
return EINTR. If that happens, core smbd saw this as an indication
that aio_pthread's open function was doing its job. With a real EINTR
without aio_pthread this meant we ended up in a server_exit after 20
seconds, because there was nobody to do the retry.

EINTR is mapped to NT_STATUS_RETRY. Handle this by just retrying after
a second.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=14285
Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Autobuild-User(master): Jeremy Allison <jra@samba.org>
Autobuild-Date(master): Thu Feb 20 22:14:25 UTC 2020 on sn-devel-184

(cherry picked from commit aebe427b77b5315eb5d2b05b8c72824ca0389723)

selftest/knownfail.d/open_eintr [deleted file]
source3/modules/vfs_aio_pthread.c
source3/smbd/open.c

diff --git a/selftest/knownfail.d/open_eintr b/selftest/knownfail.d/open_eintr
deleted file mode 100644 (file)
index e5b42ed..0000000
+++ /dev/null
@@ -1 +0,0 @@
-samba3.blackbox.open-eintr.*\(simpleserver:local\)
index a7d97223dbdf1dc5a128243a13763f9e39f25081..d13ce2fdc63ba87c77418bbc61973e5f7c9b1315 100644 (file)
@@ -318,7 +318,7 @@ static int open_async(const files_struct *fsp,
                opd->fname));
 
        /* Cause the calling code to reschedule us. */
-       errno = EINTR; /* Maps to NT_STATUS_RETRY. */
+       errno = EINPROGRESS; /* Maps to NT_STATUS_MORE_PROCESSING_REQUIRED. */
        return -1;
 }
 
index e64e22581e0060ee5e512ab8740ed2fa4a88d5a1..98770358cf1e1226036348203cdee4d4d56e323e 100644 (file)
@@ -3301,6 +3301,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
        SMB_STRUCT_STAT saved_stat = smb_fname->st;
        struct timespec old_write_time;
        struct file_id id;
+       bool setup_poll = false;
        bool ok;
 
        if (conn->printer) {
@@ -3645,6 +3646,14 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
                             open_access_mask, &new_file_created);
 
        if (NT_STATUS_EQUAL(fsp_open, NT_STATUS_NETWORK_BUSY)) {
+               if (file_existed && S_ISFIFO(fsp->fsp_name->st.st_ex_mode)) {
+                       DEBUG(10, ("FIFO busy\n"));
+                       return NT_STATUS_NETWORK_BUSY;
+               }
+               if (req == NULL) {
+                       DEBUG(10, ("Internal open busy\n"));
+                       return NT_STATUS_NETWORK_BUSY;
+               }
                /*
                 * This handles the kernel oplock case:
                 *
@@ -3654,15 +3663,20 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
                 * "Samba locking.tdb oplocks" are handled below after acquiring
                 * the sharemode lock with get_share_mode_lock().
                 */
-               if (file_existed && S_ISFIFO(fsp->fsp_name->st.st_ex_mode)) {
-                       DEBUG(10, ("FIFO busy\n"));
-                       return NT_STATUS_NETWORK_BUSY;
-               }
-               if (req == NULL) {
-                       DEBUG(10, ("Internal open busy\n"));
-                       return NT_STATUS_NETWORK_BUSY;
-               }
+               setup_poll = true;
+       }
+
+       if (NT_STATUS_EQUAL(fsp_open, NT_STATUS_RETRY)) {
+               /*
+                * EINTR from the open(2) syscall. Just setup a retry
+                * in a bit. We can't use the sys_write() tight retry
+                * loop here, as we might have to actually deal with
+                * lease-break signals to avoid a deadlock.
+                */
+               setup_poll = true;
+       }
 
+       if (setup_poll) {
                /*
                 * From here on we assume this is an oplock break triggered
                 */
@@ -3693,7 +3707,9 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
        }
 
        if (!NT_STATUS_IS_OK(fsp_open)) {
-               if (NT_STATUS_EQUAL(fsp_open, NT_STATUS_RETRY)) {
+               bool wait_for_aio = NT_STATUS_EQUAL(
+                       fsp_open, NT_STATUS_MORE_PROCESSING_REQUIRED);
+               if (wait_for_aio) {
                        schedule_async_open(req);
                }
                return fsp_open;