Author: jra Date: 2006-03-20 23:40:23 +0000 (Mon, 20 Mar 2006) New Revision: 14595
WebSVN: http://websvn.samba.org/cgi-bin/viewcvs.cgi?view=rev&root=samba&rev=14595 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: trunk/source/smbd/open.c Changeset: Modified: trunk/source/smbd/open.c =================================================================== --- trunk/source/smbd/open.c 2006-03-20 23:35:08 UTC (rev 14594) +++ trunk/source/smbd/open.c 2006-03-20 23:40:23 UTC (rev 14595) @@ -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;
