oplock probs.

Manish Agarwal magarwal at veritas.com
Mon Apr 17 20:52:07 GMT 2000


Hi,

I noticed a few problems with oplocks in samba-2.0.6 and I think I have
it fixed. The problems that the patch tries to fix are:

1. Level2 oplocks should be broken on a LOCKING_ANDX request which
contains record lock request. I don't think the SMB draft mentions this,
but I see the NT server doing this. To me it makes sense to do it.
2. If the client, on being sent an oplock break message (break to
LEVEL2), flushes a write or a lock(i.e. does a write or a lock before it
acknowledges the oplock break), then the server should further break the
oplock to NONE by sending another oplock break message (break to NONE).
Not doing this can lead to some problems which become evident when you
have mandatory locking.
3. The server should not break the client's level2 oplock on a write, if
that client is the only one holding an oplock on the file.

I have confirmed that the NT server does this and if anyone is
interested I
can post some traces for Samba and NT server for the above cases.

Please see the attached patch. Most of it is just rearranges the code,
there is not much new code in it.

Please let me know if there is something I've missed.

Thanks,
-Manish
-------------- next part --------------
diff -btc samba-2.0.6/source/smbd/fileio.c original/samba-2.0.6/source/smbd/fileio.c
*** samba-2.0.6/source/smbd/fileio.c	Sat Apr 15 15:46:45 2000
--- original/samba-2.0.6/source/smbd/fileio.c	Tue Oct 12 22:26:58 1999
***************
*** 139,153 ****
     * other files with a level II lock that they need
     * to flush their read caches. We keep the lock over
     * the shared memory area whilst doing this.
-    *
-    * We need to send another break if this write is in response to
-    * an earlier break-to-level2 message. In that case we break the 
-    * oplock to no oplock. But we do not wait.
-    * Also, if we are the only ones holding a level2 oplock then we need
-    * not send any breaks.
     */
-   do_level2_oplock_break(fsp);
  
    if ((pos != -1) && (seek_file(fsp,pos) == -1))
      return -1;
  
--- 139,219 ----
     * other files with a level II lock that they need
     * to flush their read caches. We keep the lock over
     * the shared memory area whilst doing this.
     */
  
+   if (LEVEL_II_OPLOCK_TYPE(fsp->oplock_type)) {
+     SMB_DEV_T dev = fsp->fd_ptr->dev;
+     SMB_INO_T inode = fsp->fd_ptr->inode;
+     share_mode_entry *share_list = NULL;
+     pid_t pid = getpid();
+     int token = -1;
+     int num_share_modes = 0;
+     int i;
+ 
+     if (lock_share_entry(fsp->conn, dev, inode, &token) == False) {
+       DEBUG(0,("write_file: failed to lock share mode entry for file %s.\n", fsp->fsp_name ));
+     }
+ 
+     num_share_modes = get_share_modes(fsp->conn, token, dev, inode, &share_list);
+ 
+     for(i = 0; i < num_share_modes; i++) {
+       share_mode_entry *share_entry = &share_list[i];
+ 
+       /*
+        * As there could have been multiple writes waiting at the lock_share_entry
+        * gate we may not be the first to enter. Hence the state of the op_types
+        * in the share mode entries may be partly NO_OPLOCK and partly LEVEL_II
+        * oplock. It will do no harm to re-send break messages to those smbd's
+        * that are still waiting their turn to remove their LEVEL_II state, and
+        * also no harm to ignore existing NO_OPLOCK states. JRA.
+        */
+ 
+       if (share_entry->op_type == NO_OPLOCK)
+         continue;
+ 
+       /* Paranoia .... */
+       if (EXCLUSIVE_OPLOCK_TYPE(share_entry->op_type)) {
+         DEBUG(0,("write_file: PANIC. share mode entry %d is an exlusive oplock !\n", i ));
+         abort();
+       }
+ 
+       /*
+        * Check if this is a file we have open (including the
+        * file we've been called to do write_file on. If so
+        * then break it directly without releasing the lock.
+        */
+ 
+       if (pid == share_entry->pid) {
+         files_struct *new_fsp = file_find_dit(dev, inode, &share_entry->time);
+ 
+         /* Paranoia check... */
+         if(new_fsp == NULL) {
+           DEBUG(0,("write_file: PANIC. share mode entry %d is not a local file !\n", i ));
+           abort();
+         }
+         oplock_break_level2(new_fsp, True, token);
+ 
+       } else {
+ 
+         /*
+          * This is a remote file and so we send an asynchronous
+          * message.
+          */
+ 
+         request_oplock_break(share_entry, dev, inode);
+       }
+     }
+  
+     free((char *)share_list);
+     unlock_share_entry(fsp->conn, dev, inode, token);
+   }
+ 
+   /* Paranoia check... */
+   if (LEVEL_II_OPLOCK_TYPE(fsp->oplock_type)) {
+     DEBUG(0,("write_file: PANIC. File %s still has a level II oplock.\n", fsp->fsp_name));
+     abort();
+   }
+ 
    if ((pos != -1) && (seek_file(fsp,pos) == -1))
      return -1;
  
diff -btc samba-2.0.6/source/smbd/oplock.c original/samba-2.0.6/source/smbd/oplock.c
*** samba-2.0.6/source/smbd/oplock.c	Mon Apr 17 10:23:13 2000
--- original/samba-2.0.6/source/smbd/oplock.c	Wed Nov 10 18:36:10 1999
***************
*** 416,422 ****
      ret = False;
    }
  
!   if (fsp->sent_oplock_break == BREAK_TO_NONE_SENT) {
  
      /*
       * Deal with a reply when a break-to-none was sent.
--- 416,422 ----
      ret = False;
    }
  
!   if (fsp->sent_oplock_break == EXCLUSIVE_BREAK_SENT) {
  
      /*
       * Deal with a reply when a break-to-none was sent.
***************
*** 784,798 ****
    }
  
    /*
-    * We have already sent a break to l2, this is a break to none.
-    */
-   if (EXCLUSIVE_OPLOCK_TYPE(fsp->oplock_type) &&
-       (fsp->sent_oplock_break == BREAK_TO_LEVEL2_SENT)) {
-         fsp->sent_oplock_break = BREAK_TO_NONE_SENT;
-         return True;
-   }
- 
-   /*
     * Now we must update the shared memory structure to tell
     * everyone else we no longer have a level II oplock on 
     * this open file. If local_request is true then token is
--- 784,789 ----
***************
*** 859,866 ****
  
    /*
     * Deal with a level II oplock going break to none separately.
-    * Send a break for all the open file handles on the same file.
     */
    if (LEVEL_II_OPLOCK_TYPE(fsp->oplock_type))
          return oplock_break_level2(fsp, local_request, -1);
  
--- 850,857 ----
  
    /*
     * Deal with a level II oplock going break to none separately.
     */
+ 
    if (LEVEL_II_OPLOCK_TYPE(fsp->oplock_type))
      return oplock_break_level2(fsp, local_request, -1);
  
***************
*** 929,935 ****
    prepare_break_message( outbuf, fsp, using_levelII);
    /* Remember if we just sent a break to level II on this file. */
    fsp->sent_oplock_break = using_levelII?
!           BREAK_TO_LEVEL2_SENT:BREAK_TO_NONE_SENT;
  
    send_smb(Client, outbuf);
  
--- 920,926 ----
    prepare_break_message( outbuf, fsp, using_levelII);
    /* Remember if we just sent a break to level II on this file. */
    fsp->sent_oplock_break = using_levelII?
!           LEVEL_II_BREAK_SENT:EXCLUSIVE_BREAK_SENT;
  
    send_smb(Client, outbuf);
  
***************
*** 1304,1437 ****
  }
  
  /****************************************************************************
-  * Called from write_file and from reply_lockingX.
-  * If the file is levelII oplocked then we need to grab the shared memory
-  * lock and inform all other clients with a levelII oplock that they need
-  * to flush their read caches.
-  *
-  * We need to send another break if this write or record lock is in response to
-  * an earlier break-to-level2 message. In that case we send another 
-  * break-to-no-oplock. We do not wait for a response (because this is 
-  * basically a levelII oplock break and that waiting would cause us to 
-  * deadlock.
-  *
-  * Also, if we are the only ones holding a level2 oplock then we need
-  * not send any breaks.
- ****************************************************************************/
- void do_level2_oplock_break(files_struct *fsp)
- {
-   if (LEVEL_II_OPLOCK_TYPE(fsp->oplock_type) ||
-       (EXCLUSIVE_OPLOCK_TYPE(fsp->oplock_type) &&
-        (fsp->sent_oplock_break == BREAK_TO_LEVEL2_SENT))) {
- 
-     SMB_DEV_T dev = fsp->fd_ptr->dev;
-     SMB_INO_T inode = fsp->fd_ptr->inode;
-     share_mode_entry *share_list = NULL;
-     pid_t pid = getpid();
-     int token = -1;
-     int num_share_modes = 0;
-     int i;
- 
-     DEBUG(0,("write_file: We need to break an oplock on  %s\n", fsp->fsp_name));
- 
-     if (lock_share_entry(fsp->conn, dev, inode, &token) == False) {
-       DEBUG(0,("write_file: failed to lock share mode entry for file %s.\n",
-             fsp->fsp_name));
-     }
- 
-     num_share_modes = get_share_modes(fsp->conn, token, dev, inode,
-                                       &share_list);
- 
-     if (num_share_modes == 1) {
-         /*
-          * Don't break the level2 oplock if this client is the only one.
-          */
-         if (!LEVEL_II_OPLOCK_TYPE(fsp->oplock_type)) {
-                 share_mode_entry *share_entry = &share_list[0];
- 
-                 /*
-                  * Paranoia check.
-                  */
-                 if (share_entry->pid != pid ||
-                     !EXCLUSIVE_OPLOCK_TYPE(share_entry->op_type)) {
-                    DEBUG(0,("write_file: PANIC. Exclusive oplock expected \n"));
-                    abort();
-                 }
- 
-                 /*
-                  * This client is the only one that has an oplock on the file
-                  * and its an exclusive oplock. We send a break_to_none
-                  * message to the client, and we DONT wait here.
-                  */
-                 oplock_break_level2(fsp, True, token);
-         }
-     } else {
-         for(i = 0; i < num_share_modes; i++) {
-               share_mode_entry *share_entry = &share_list[i];
-               /*
-                * As there could have been multiple writes waiting at the
-                * lock_share_entry gate we may not be the first to enter. Hence
-                * the state of the state of the op_types in the share mode
-                * may be partly NO_OPLOCK and partly LEVEL_II oplock.
-                * It will do no harm to re-send break messages to those smbd's
-                * that are still waiting their turn to remove their LEVEL_II
-                * state, and also no harm to ignore existing NO_OPLOCK states.
-                * JRA.
-                */
- 
-               if (share_entry->op_type == NO_OPLOCK)
-                 continue;
- 
-               /* Paranoia .... */
-               if (EXCLUSIVE_OPLOCK_TYPE(share_entry->op_type)) {
-                 DEBUG(0,("write_file: PANIC. share mode entry %d is an \
- exlusive oplock !\n", i ));
-                 abort();
-               }
- 
-               /*
-                * Check if this is a file we have open (including the
-                * file we've been called to do write_file on. If so
-                * then break it directly without releasing the lock.
-                */
- 
-               if (pid == share_entry->pid) {
-                 files_struct *new_fsp = file_find_dit(dev, inode,
-                                                       &share_entry->time);
- 
-                 /* Paranoia check... */
-                 if(new_fsp == NULL) {
-                   DEBUG(0,("write_file: PANIC. share mode entry %d is not a \
- local file !\n", i ));
-                   abort();
-                 }
- 
-                 oplock_break_level2(new_fsp, True, token);
-               } else {
-                 /*
-                  * This is a remote file and so we send an asynchronous
-                  * message.
-                  */
- 
-                 request_oplock_break(share_entry, dev, inode);
-               }
-         }
-         /* Paranoia check... */
-         if (LEVEL_II_OPLOCK_TYPE(fsp->oplock_type)) {
-             DEBUG(0,("write_file: PANIC. File %s still has a level II oplock.\n"
- , fsp->fsp_name));
-             abort();
-         }
-     }
- 
-     free((char *)share_list);
-     unlock_share_entry(fsp->conn, dev, inode, token);
-   }
-   return;
- }
- 
- 
- /****************************************************************************
    Attempt to break an oplock on a file (if oplocked).
    Returns True if the file was closed as a result of
    the oplock break, False otherwise.
--- 1295,1300 ----
diff -btc samba-2.0.6/source/smbd/reply.c original/samba-2.0.6/source/smbd/reply.c
*** samba-2.0.6/source/smbd/reply.c	Sat Apr 15 15:37:04 2000
--- original/samba-2.0.6/source/smbd/reply.c	Wed Nov 10 18:36:11 1999
***************
*** 4214,4224 ****
      }
    }
  
-   /*
-    * oplocks should be broken on LOCKING_ANDX requests as well.
-    */
-    do_level2_oplock_break(fsp);
- 
    /* Data now points at the beginning of the list
       of smb_unlkrng structs */
    for(i = 0; i < (int)num_ulocks; i++) {
--- 4214,4219 ----




More information about the samba-technical mailing list