Attachment 13743 Details for Bug 12853 – git-am *TEST* patch for 4.7.next, 4.6.next.

[patch] git-am *TEST* patch for 4.7.next, 4.6.next.

look (text/plain), 10.56 KB, created by Jeremy Allison on 2017-10-30 22:37:20 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeremy Allison

Created: 2017-10-30 22:37:20 UTC

Size: 10.56 KB

patch

obsolete

>From 6fb7dabcc306452588535d090e2e80e0a161e26e Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Fri, 27 Oct 2017 12:48:14 -0700
>Subject: [PATCH 1/3] WIP. Always use O_NONBLOCK for kernel oplocks.
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> source3/smbd/open.c | 13 +++----------
> 1 file changed, 3 insertions(+), 10 deletions(-)
>
>diff --git a/source3/smbd/open.c b/source3/smbd/open.c
>index 7781a6f86a7..8e3227aa3b4 100644
>--- a/source3/smbd/open.c
>+++ b/source3/smbd/open.c
>@@ -3253,20 +3253,13 @@ 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.
>-		 *
>-		 * 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.
>+		 * fcntl" on Linux).
> 		 */
> 		flags2 |= O_NONBLOCK;
> 	}
>-- 
>2.15.0.rc2.357.g7e34df9404-goog
>
>
>From fb6f05720ce8be339feab1b7801ff983eb858984 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Mon, 30 Oct 2017 15:05:00 -0700
>Subject: [PATCH 2/3] WIP: Revert "s3/smbd: fix deferred open with streams and
> kernel oplocks"
>
>This reverts commit b35a296a27a0807c780f2a9e7af2f2e93feefaa8.
>---
> source3/smbd/open.c | 115 +++++-----------------------------------------------
> 1 file changed, 11 insertions(+), 104 deletions(-)
>
>diff --git a/source3/smbd/open.c b/source3/smbd/open.c
>index 8e3227aa3b4..8dc41fd7180 100644
>--- a/source3/smbd/open.c
>+++ b/source3/smbd/open.c
>@@ -1896,23 +1896,6 @@ static bool delay_for_oplock(files_struct *fsp,
> 	return delay;
> }
> 
>-/**
>- * Return lease or oplock state from a share mode
>- **/
>-static uint32_t get_lease_type_from_share_mode(const struct share_mode_data *d)
>-{
>-	uint32_t e_lease_type = 0;
>-	uint32_t i;
>-
>-	for (i=0; i < d->num_share_modes; i++) {
>-		struct share_mode_entry *e = &d->share_modes[i];
>-
>-		e_lease_type |= get_lease_type(d, e);
>-	}
>-
>-	return e_lease_type;
>-}
>-
> static bool file_has_brlocks(files_struct *fsp)
> {
> 	struct byte_range_lock *br_lck;
>@@ -2325,11 +2308,6 @@ static struct deferred_open_record *deferred_open_record_create(
> struct defer_open_state {
> 	struct smbXsrv_connection *xconn;
> 	uint64_t mid;
>-	struct file_id file_id;
>-	struct timeval request_time;
>-	struct timeval timeout;
>-	bool kernel_oplock;
>-	uint32_t lease_type;
> };
> 
> static void defer_open_done(struct tevent_req *req);
>@@ -2348,7 +2326,6 @@ static void defer_open(struct share_mode_lock *lck,
> 		       struct timeval timeout,
> 		       struct smb_request *req,
> 		       bool delayed_for_oplocks,
>-		       bool kernel_oplock,
> 		       struct file_id id)
> {
> 	struct deferred_open_record *open_rec = NULL;
>@@ -2360,12 +2337,11 @@ static void defer_open(struct share_mode_lock *lck,
> 	abs_timeout = timeval_sum(&request_time, &timeout);
> 
> 	DBG_DEBUG("request time [%s] timeout [%s] mid [%" PRIu64 "] "
>-		  "delayed_for_oplocks [%s] kernel_oplock [%s] file_id [%s]\n",
>+		  "delayed_for_oplocks [%s] file_id [%s]\n",
> 		  timeval_string(talloc_tos(), &request_time, false),
> 		  timeval_string(talloc_tos(), &abs_timeout, false),
> 		  req->mid,
> 		  delayed_for_oplocks ? "yes" : "no",
>-		  kernel_oplock ? "yes" : "no",
> 		  file_id_string_tos(&id));
> 
> 	open_rec = deferred_open_record_create(delayed_for_oplocks,
>@@ -2382,11 +2358,6 @@ static void defer_open(struct share_mode_lock *lck,
> 	}
> 	watch_state->xconn = req->xconn;
> 	watch_state->mid = req->mid;
>-	watch_state->file_id = lck->data->id;
>-	watch_state->request_time = request_time;
>-	watch_state->timeout = timeout;
>-	watch_state->kernel_oplock = kernel_oplock;
>-	watch_state->lease_type = get_lease_type_from_share_mode(lck->data);
> 
> 	DBG_DEBUG("defering mid %" PRIu64 "\n", req->mid);
> 
>@@ -2416,12 +2387,8 @@ static void defer_open_done(struct tevent_req *req)
> {
> 	struct defer_open_state *state = tevent_req_callback_data(
> 		req, struct defer_open_state);
>-	struct tevent_req *watch_req = NULL;
>-	struct share_mode_lock *lck = NULL;
>-	bool schedule_req = true;
>-	struct timeval timeout;
> 	NTSTATUS status;
>-	bool ok;
>+	bool ret;
> 
> 	status = dbwrap_watched_watch_recv(req, talloc_tos(), NULL, NULL,
> 					  NULL);
>@@ -2433,72 +2400,13 @@ static void defer_open_done(struct tevent_req *req)
> 		 * Even if it failed, retry anyway. TODO: We need a way to
> 		 * tell a re-scheduled open about that error.
> 		 */
>-		if (NT_STATUS_EQUAL(status, NT_STATUS_IO_TIMEOUT) &&
>-		    state->kernel_oplock)
>-		{
>-			/*
>-			 * If we reschedule but the kernel oplock is still hold
>-			 * we would block in the second open as that will be a
>-			 * blocking open attempt.
>-			 */
>-			exit_server("Kernel oplock holder didn't "
>-				    "respond to break message");
>-		}
>-	}
>-
>-	if (state->kernel_oplock) {
>-		lck = get_existing_share_mode_lock(talloc_tos(), state->file_id);
>-		if (lck != NULL) {
>-			uint32_t lease_type;
>-
>-			lease_type = get_lease_type_from_share_mode(lck->data);
>-
>-			if ((lease_type != 0) &&
>-			    (lease_type == state->lease_type))
>-			{
>-				DBG_DEBUG("Unchanged lease: %" PRIu32 "\n",
>-					  lease_type);
>-				schedule_req = false;
>-			}
>-		}
>-	}
>-
>-	if (schedule_req) {
>-		DBG_DEBUG("scheduling mid %" PRIu64 "\n", state->mid);
>-
>-		ok = schedule_deferred_open_message_smb(state->xconn,
>-							state->mid);
>-		if (!ok) {
>-			exit_server("schedule_deferred_open_message_smb failed");
>-		}
>-		TALLOC_FREE(lck);
>-		TALLOC_FREE(state);
>-		return;
>-	}
>-
>-	DBG_DEBUG("Keep waiting for oplock release for [%s/%s%s] "
>-		  "mid: %" PRIu64 "\n",
>-		  lck->data->servicepath,
>-		  lck->data->base_name,
>-		  lck->data->stream_name ? lck->data->stream_name : "",
>-		  state->mid);
>-
>-	watch_req = dbwrap_watched_watch_send(state,
>-					      state->xconn->ev_ctx,
>-					      lck->data->record,
>-					      (struct server_id){0});
>-	if (watch_req == NULL) {
>-		exit_server("Could not watch share mode record");
> 	}
>-	tevent_req_set_callback(watch_req, defer_open_done, state);
> 
>-	timeout = timeval_sum(&state->request_time, &state->timeout);
>-	ok = tevent_req_set_endtime(watch_req, state->xconn->ev_ctx, timeout);
>-	if (!ok) {
>-		exit_server("tevent_req_set_endtime failed");
>-	}
>+	DEBUG(10, ("scheduling mid %llu\n", (unsigned long long)state->mid));
> 
>-	TALLOC_FREE(lck);
>+	ret = schedule_deferred_open_message_smb(state->xconn, state->mid);
>+	SMB_ASSERT(ret);
>+	TALLOC_FREE(state);
> }
> 
> /**
>@@ -2649,8 +2557,7 @@ static NTSTATUS fcb_or_dos_open(struct smb_request *req,
> static void schedule_defer_open(struct share_mode_lock *lck,
> 				struct file_id id,
> 				struct timeval request_time,
>-				struct smb_request *req,
>-				bool kernel_oplock)
>+				struct smb_request *req)
> {
> 	/* This is a relative time, added to the absolute
> 	   request_time value to get the absolute timeout time.
>@@ -2674,7 +2581,7 @@ static void schedule_defer_open(struct share_mode_lock *lck,
> 		return;
> 	}
> 
>-	defer_open(lck, request_time, timeout, req, true, kernel_oplock, id);
>+	defer_open(lck, request_time, timeout, req, true, id);
> }
> 
> /****************************************************************************
>@@ -3353,7 +3260,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
> 					 first_open_attempt);
> 		if (delay) {
> 			schedule_defer_open(lck, fsp->file_id, request_time,
>-					    req, true);
>+					    req);
> 			TALLOC_FREE(lck);
> 			DEBUG(10, ("Sent oplock break request to kernel "
> 				   "oplock holder\n"));
>@@ -3486,7 +3393,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
> 					 first_open_attempt);
> 		if (delay) {
> 			schedule_defer_open(lck, fsp->file_id,
>-					    request_time, req, false);
>+					    request_time, req);
> 			TALLOC_FREE(lck);
> 			fd_close(fsp);
> 			return NT_STATUS_SHARING_VIOLATION;
>@@ -3590,7 +3497,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
> 
> 			if (!request_timed_out(request_time, timeout)) {
> 				defer_open(lck, request_time, timeout, req,
>-					   false, false, id);
>+					   false, id);
> 			}
> 		}
> 
>-- 
>2.15.0.rc2.357.g7e34df9404-goog
>
>
>From 67f7d1852fabee994212e4c6fd171228a39167a2 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Mon, 30 Oct 2017 15:22:17 -0700
>Subject: [PATCH 3/3] WIP: Real fix for problem
> b35a296a27a0807c780f2a9e7af2f2e93feefaa8 tried to fix.
>
>If we're opening a stream file and have fsp->base_fsp open, ensure
>we close this handle under the share mode lock *before* setting a watchpoint
>on the share mode record. Otherwise the cleanup close of base_fsp in
>create_file_unixpath() triggers the record to fire when we don't
>have any real change to examine.
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> source3/smbd/open.c | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
>diff --git a/source3/smbd/open.c b/source3/smbd/open.c
>index 8dc41fd7180..f0916def54c 100644
>--- a/source3/smbd/open.c
>+++ b/source3/smbd/open.c
>@@ -3259,6 +3259,17 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
> 					 create_disposition,
> 					 first_open_attempt);
> 		if (delay) {
>+			if (fsp->base_fsp) {
>+				/*
>+				 * We're opening a stream here.
>+				 * Ensure we remove any base_fsp
>+				 * record under the lock *before*
>+				 * setting the watch.
>+				 */
>+				close_file(req, fsp->base_fsp, ERROR_CLOSE);
>+				fsp->base_fsp = NULL;
>+			}
>+
> 			schedule_defer_open(lck, fsp->file_id, request_time,
> 					    req);
> 			TALLOC_FREE(lck);
>@@ -3392,6 +3403,16 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
> 					 create_disposition,
> 					 first_open_attempt);
> 		if (delay) {
>+			if (fsp->base_fsp) {
>+				/*
>+				 * We're opening a stream here.
>+				 * Ensure we remove any base_fsp
>+				 * record under the lock *before*
>+				 * setting the watch.
>+				 */
>+				close_file(req, fsp->base_fsp, ERROR_CLOSE);
>+				fsp->base_fsp = NULL;
>+			}
> 			schedule_defer_open(lck, fsp->file_id,
> 					    request_time, req);
> 			TALLOC_FREE(lck);
>@@ -5176,6 +5197,13 @@ static NTSTATUS create_file_unixpath(connection_struct *conn,
> 					    fsp);
> 
> 		if(!NT_STATUS_IS_OK(status)) {
>+			if (base_fsp != NULL && fsp->base_fsp == NULL) {
>+				/*
>+				 * open_file_ntcreate() already closed
>+				 * base_fsp as part of erroring out.
>+				 */
>+				base_fsp = NULL;
>+			}
> 			file_free(req, fsp);
> 			fsp = NULL;
> 		}
>-- 
>2.15.0.rc2.357.g7e34df9404-goog
>

Actions: View

Attachments on bug 12853: 13296 | 13564 | 13743