svn commit: samba r14596 - in branches/SAMBA_3_0/source/smbd: .
jra at samba.org
jra at samba.org
Mon Mar 20 23:40:44 GMT 2006
Author: jra
Date: 2006-03-20 23:40:43 +0000 (Mon, 20 Mar 2006)
New Revision: 14596
WebSVN: http://websvn.samba.org/cgi-bin/viewcvs.cgi?view=rev&root=samba&rev=14596
Log:
Fix a logic bug with multiple oplock contention.
The sad thing is the core of this bug fix is just
removing a paranoia "exit_server" call, as the
rest of the logic was already correct :-).
Lots of comments to explain the logic added.
I will look at adding tests to exercise this,
might be possible.
Jeremy.
Modified:
branches/SAMBA_3_0/source/smbd/open.c
Changeset:
Modified: branches/SAMBA_3_0/source/smbd/open.c
===================================================================
--- branches/SAMBA_3_0/source/smbd/open.c 2006-03-20 23:40:23 UTC (rev 14595)
+++ branches/SAMBA_3_0/source/smbd/open.c 2006-03-20 23:40:43 UTC (rev 14596)
@@ -1091,20 +1091,14 @@
struct deferred_open_record *state =
(struct deferred_open_record *)pml->private_data.data;
+ /* Remember the absolute time of the original
+ request with this mid. We'll use it later to
+ see if this has timed out. */
+
request_time = pml->request_time;
delayed_for_oplocks = state->delayed_for_oplocks;
- /* There could be a race condition where the dev/inode pair
- has changed since we deferred the message. If so, just
- remove the deferred open entry and return sharing
- violation. */
-
- /* If the timeout value is non-zero, we need to just return
- sharing violation. Don't retry the open as we were not
- notified of a close and we don't want to trigger another
- spurious oplock break. */
-
- /* Now remove the deferred open entry under lock. */
+ /* Remove the deferred open entry under lock. */
lck = get_share_mode_lock(NULL, state->dev, state->inode, NULL, NULL);
if (lck == NULL) {
DEBUG(0, ("could not get share mode lock\n"));
@@ -1327,15 +1321,16 @@
if (delay_for_oplocks(lck, fsp)) {
struct deferred_open_record state;
- struct timeval timeout;
- if (delayed_for_oplocks) {
- DEBUG(0, ("Trying to delay for oplocks "
- "twice\n"));
- exit_server("exiting");
- }
+ /* This is a relative time, added to the absolute
+ request_time value to get the absolute timeout time.
+ Note that if this is the second or greater time we enter
+ this codepath for this particular request mid then
+ request_time is left as the absolute time of the *first*
+ time this request mid was processed. This is what allows
+ the request to eventually time out. */
- timeout = timeval_set(OPLOCK_BREAK_TIMEOUT*2, 0);
+ struct timeval timeout;
/* Normally the smbd we asked should respond within
* OPLOCK_BREAK_TIMEOUT seconds regardless of whether
@@ -1343,6 +1338,13 @@
* measure here in case the other smbd is stuck
* somewhere else. */
+ timeout = timeval_set(OPLOCK_BREAK_TIMEOUT*2, 0);
+
+ /* Nothing actually uses state.delayed_for_oplocks
+ but it's handy to differentiate in debug messages
+ between a 30 second delay due to oplock break, and
+ a 1 second delay for share mode conflicts. */
+
state.delayed_for_oplocks = True;
state.dev = dev;
state.inode = inode;
@@ -1434,8 +1436,21 @@
struct timeval timeout;
struct deferred_open_record state;
+ /* This is a relative time, added to the absolute
+ request_time value to get the absolute timeout time.
+ Note that if this is the second or greater time we enter
+ this codepath for this particular request mid then
+ request_time is left as the absolute time of the *first*
+ time this request mid was processed. This is what allows
+ the request to eventually time out. */
+
timeout = timeval_set(0, SHARING_VIOLATION_USEC_WAIT);
+ /* Nothing actually uses state.delayed_for_oplocks
+ but it's handy to differentiate in debug messages
+ between a 30 second delay due to oplock break, and
+ a 1 second delay for share mode conflicts. */
+
state.delayed_for_oplocks = False;
state.dev = dev;
state.inode = inode;
More information about the samba-cvs
mailing list