Changeset 895

Timestamp:

Mar 18, 2016, 7:53:30 PM (9 years ago)

Author:

Silvan Scherrer

Message:

tdb: Use DosSetFileLocks directly for db locks.

This is to overcome some fcntl() API incompleteness in kLIBC
(like inability to upgrade locks or join adjacent lock regions
into one). It made the torture test run a bit better but there
are still two major problems both coming from DosSetFileLocks
impl (which fcntl() is currently based upon too): a) inability to
detect deadlocks and b) missing atomic unlock/lock support if
unlock/lock regions don't match.

With the current implementation, tdborture works fine for 1 or 2
worker processes but hangs when there are 3 or more. [Before that,
it would only work with 1 process and would likely to corrupt
the database and terminate if there were 2 or more processes].

Author: Dmitriy Kuminov (@dmik).

Location:

trunk/server/lib/tdb/common

Files:

• io.c (modified) (2 diffs)
• lock.c (modified) (14 diffs)
• open.c (modified) (6 diffs)
• tdb_private.h (modified) (2 diffs)

trunk/server/lib/tdb/common/io.c ¶

@@ -91 +91 @@
                 return -1;
 
-#ifdef __OS2__
-        // YD we must upgrade read locks to write locks (exclusive), as otherwise
-        // the owner (us) is not allowed to write to the file (different from unix)
-        // if a wider previous lock is in effect, we cannot write lock our segment
-        // (e.g. a lock_upgrade locks all the file), so we hope the previous lock 
-        // is a write lock: do not wait for lock.
-        // so this is what we try here:
-        // 1. add a write lock and see it it works
-        // 2. if the write lock wasn't set, we try to unlock the segment
-        //    first and add the write lock afterwards
-        // 3. we remove the write lock further down 
-        // 4. we add the removed lock from step #2 again 
-        int upgradeLockRC = 0;
-        int unlockRC = -1;
-        upgradeLockRC = tdb_brlock(tdb, F_WRLCK, off, len, TDB_LOCK_NOWAIT);
-        if (upgradeLockRC != 0) {
-                unlockRC = tdb_brunlock(tdb, F_RDLCK, off, 1);
-                upgradeLockRC = tdb_brlock(tdb, F_WRLCK, off, len, TDB_LOCK_NOWAIT);
-        }
-        // no need to log a successful upgrade
-        if (upgradeLockRC != 0)
-        TDB_LOG((tdb, TDB_DEBUG_TRACE,"upgrading lock at %d len=%d "
-                "before writing %s (rc=%d).\n", off, len,
-                 upgradeLockRC ? "failed":"was successful", upgradeLockRC));
-#endif
         if (tdb->map_ptr) {
                 memcpy(off + (char *)tdb->map_ptr, buf, len);
@@ -141 +116 @@
                                  "write %d bytes at %d in two attempts\n",
                                  len, off));
-#ifdef __OS2__ // remove our lock, if upgrade succeded
-                        if (upgradeLockRC == 0)
-                                tdb_brunlock( tdb, F_WRLCK, off, len);
-                        if (unlockRC == 0)
-                                tdb_brlock( tdb, F_RDLCK, off, 1, TDB_LOCK_WAIT);
-#endif
-                        return -1;
-                }
-        }
-#ifdef __OS2__ // remove our lock, if upgrade succeded
-        if (upgradeLockRC == 0)
-                tdb_brunlock( tdb, F_WRLCK, off, len);
-        if (unlockRC == 0)
-                tdb_brlock( tdb, F_RDLCK, off, 1, TDB_LOCK_WAIT);
-#endif
+                        return -1;
+                }
+        }
         return 0;
 }

trunk/server/lib/tdb/common/lock.c ¶

@@ -33 +33 @@
 }
 
+#ifdef __OS2__
+enum os2_fl {
+    OS2_FL_LOCK = 0x1,
+    OS2_FL_RW = 0x2,
+    OS2_FL_WAIT = 0x4,
+    OS2_FL_UPGRADE = 0x8,
+};
+
+static int os2_set_file_locks(struct tdb_context *tdb, enum os2_fl flags,
+                              off_t start, off_t offset, off_t len)
+{
+    FILELOCKL locks[2] = {0};
+    ULONG fl;
+    BOOL dowait;
+    APIRET arc;
+
+    if ((!(flags & OS2_FL_UPGRADE) && (start > offset || start < 0)) ||
+        offset < 0 || offset + len < 0) {
+        errno = EINVAL;
+        return -1;
+    }
+
+    fl = (flags & OS2_FL_RW) ? 0 : 1; /* excluive when rw, shared otherwise */
+
+    if (flags & OS2_FL_UPGRADE) {
+        locks[0].lOffset = offset;
+        locks[0].lRange = len == 0 ? OFF_MAX : len;
+        locks[1] = locks[0];
+        fl |= 2; /* atomic */
+    } else {
+        if (start != offset) {
+            locks[0].lOffset = start;
+            locks[0].lRange = offset - start;
+            fl |= 2; /* atomic */
+        }
+        locks[1].lOffset = start;
+        locks[1].lRange = len == 0 ? OFF_MAX : (locks[0].lRange + len);
+    }
+
+    dowait = (flags & (OS2_FL_LOCK | OS2_FL_UPGRADE)) && (flags & OS2_FL_WAIT);
+
+    TDB_LOG((tdb, TDB_DEBUG_TRACE, "os2_file_locks: fd=%d lock_fd=%d flags=%x start=%lld offset=%lld len=%lld pid=%d\n",
+             tdb->fd, tdb->lock_fd, flags, start, offset, len, getpid()));
+
+    arc = DosSetFileLocksL(tdb->lock_fd,
+                           &locks[(flags & OS2_FL_LOCK) ? 0 : 1],
+                           &locks[(flags & OS2_FL_LOCK) ? 1 : 0],
+                           dowait ? SEM_INDEFINITE_WAIT : SEM_IMMEDIATE_RETURN,
+                           fl);
+
+    TDB_LOG((tdb, TDB_DEBUG_TRACE, "os2_file_locks: arc=%d pid=%d\n", arc, getpid()));
+
+    if (arc) {
+        switch (arc) {
+            case ERROR_LOCK_VIOLATION:
+                errno = EACCES;
+                break;
+            case ERROR_INTERRUPT:
+                errno = EINTR;
+                break;
+            case ERROR_TIMEOUT:
+                errno = EDEADLK;
+                break;
+            default:
+                TDB_LOG((tdb, TDB_DEBUG_ERROR, "os2_file_locks failed, lock_fd=%d flags=%x start=%d off=%d len=%d (arc=%d)\n",
+                         tdb->lock_fd, flags, start, offset, len, arc));
+        }
+        return -1;
+    }
+
+    return 0;
+}
+#else /* __OS2__ */
 static int fcntl_lock(struct tdb_context *tdb,
                       int rw, off_t off, off_t len, bool waitflag)
@@ -44 +117 @@
         fl.l_pid = 0;
 
-#ifdef __OS2__
-        int rc = 0;
-        int lockFile = 0;
-
-        if (off == ACTIVE_LOCK || off == OPEN_LOCK || off == TRANSACTION_LOCK)
-                lockFile = tdb->hActiveLock;
-        else
-                lockFile = tdb->fd;
-
-        int cmd = 0;
-        if (waitflag)
-                cmd = F_SETLKW;
-        else
-                cmd = F_SETLK;
-
-        rc = fcntl(lockFile, cmd, &fl);
-        // if the first lock doesn't work and it's a complete lock,
-        // we split it in 2 parts. first hash size*4 and then the rest
-        if (rc != 0 && off == FREELIST_TOP && len == 0) { 
-                fl.l_len = tdb->header.hash_size * 4;
-                rc = fcntl(lockFile, cmd, &fl);
-                if (rc == 0) {
-                        fl.l_start = off + tdb->header.hash_size * 4;
-                        fl.l_len = 0;
-                        rc = fcntl(lockFile, cmd, &fl);
-                }
-        }
-
-        TDB_LOG((tdb, TDB_DEBUG_TRACE,"fcntl_lock: (fd=%d) offset=%lld rw_type=%d len=%lld waitflag=%d (rc=%d) pid=%d\n",
-                lockFile, off, rw, len, waitflag, rc, getpid()));
-
-        return rc;
-#else
         if (waitflag)
                 return fcntl(tdb->fd, F_SETLKW, &fl);
         else
                 return fcntl(tdb->fd, F_SETLK, &fl);
-#endif
 }
 
@@ -151 +190 @@
         fl.l_pid = 0;
 
-#ifdef __OS2__
-        int rc = 0;
-        int lockFile = 0;
-        if (off == ACTIVE_LOCK || off == OPEN_LOCK || off == TRANSACTION_LOCK)
-                lockFile = tdb->hActiveLock;
-        else
-                lockFile = tdb->fd;
-
-        rc = fcntl(lockFile, F_SETLKW, &fl);
-        // if the first unlock doesn't work and it's a complete unlock,
-        // we split it in 2 parts. first hash size*4 and then the rest
-        // as it was locked that way as well. and it seems fcntl() doesn't care
-        if (rc != 0 && off == FREELIST_TOP && len == 0) { 
-                fl.l_len = tdb->header.hash_size * 4;
-                rc = fcntl(lockFile, F_SETLKW, &fl);
-                if (rc == 0) {
-                        fl.l_start = off + tdb->header.hash_size * 4;
-                        fl.l_len = 0;
-                        rc = fcntl(lockFile, F_SETLKW, &fl);
-                }
-        }
-
-        TDB_LOG((tdb, TDB_DEBUG_TRACE,"fcntl_unlock: (fd=%d) offset=%lld rw_type=%d len=%lld (rc=%d) pid=%d\n",
-                 lockFile, off, rw, len, rc, getpid()));
-
-        return rc;
-#else
         return fcntl(tdb->fd, F_SETLKW, &fl);
-#endif
-}
+}
+#endif /* __OS2__ */
 
 /* list -1 is the alloc list, otherwise a hash chain. */
@@ -196 +208 @@
    note that a len of zero means lock to end of file
 */
+#ifdef __OS2__
+static int tdb_brlock_ex(struct tdb_context *tdb,
+                         int rw_type, tdb_off_t start,
+                         tdb_off_t offset, size_t len,
+                         enum tdb_lock_flags flags)
+#else
 int tdb_brlock(struct tdb_context *tdb,
                int rw_type, tdb_off_t offset, size_t len,
                enum tdb_lock_flags flags)
+#endif
 {
         int ret;
@@ -215 +234 @@
         }
 
+#ifdef __OS2__
+        int os2_flags = OS2_FL_LOCK;
+        if (rw_type == F_WRLCK) {
+            os2_flags |= OS2_FL_RW;
+        }
+        if (flags & TDB_LOCK_WAIT) {
+            os2_flags |= OS2_FL_WAIT;
+        }
+        if (flags & TDB_LOCK_UPGRADE) {
+            os2_flags |= OS2_FL_UPGRADE;
+        }
+#endif
+
         do {
+#ifdef __OS2__
+                ret = os2_set_file_locks(tdb, os2_flags, start, offset, len);
+#else
                 ret = fcntl_lock(tdb, rw_type, offset, len,
                                  flags & TDB_LOCK_WAIT);
+#endif
                 /* Check for a sigalarm break. */
                 if (ret == -1 && errno == EINTR &&
@@ -232 +268 @@
                  * locks. */
                 if (!(flags & TDB_LOCK_PROBE) && errno != EAGAIN) {
-                        TDB_LOG((tdb, TDB_DEBUG_TRACE,"tdb_brlock failed (fd=%d) at offset %d rw_type=%d flags=%d len=%d\n",
-                                 tdb->fd, offset, rw_type, flags, (int)len));
+                        TDB_LOG((tdb, TDB_DEBUG_TRACE,"tdb_brlock failed (fd=%d) at offset %d rw_type=%d flags=%d len=%d (errno=%d)\n",
+                                 tdb->fd, offset, rw_type, flags, (int)len, errno));
                 }
                 return -1;
@@ -240 +276 @@
 }
 
+#ifdef __OS2__
+int tdb_brlock(struct tdb_context *tdb,
+               int rw_type, tdb_off_t offset, size_t len,
+               enum tdb_lock_flags flags)
+{
+    return tdb_brlock_ex(tdb, rw_type, offset, offset, len, flags);
+}
+#endif
+
+#ifdef __OS2__
+static int tdb_brunlock_ex(struct tdb_context *tdb,
+                           int rw_type, tdb_off_t start,
+                           tdb_off_t offset, size_t len)
+#else
 int tdb_brunlock(struct tdb_context *tdb,
                  int rw_type, tdb_off_t offset, size_t len)
+#endif
 {
         int ret;
@@ -250 +301 @@
 
         do {
+#ifdef __OS2__
+                ret = os2_set_file_locks(tdb, rw_type == F_WRLCK ? OS2_FL_RW : 0,
+                                         start, offset, len);
+#else
                 ret = fcntl_unlock(tdb, rw_type, offset, len);
+#endif
         } while (ret == -1 && errno == EINTR);
 
         if (ret == -1) {
-                TDB_LOG((tdb, TDB_DEBUG_TRACE,"tdb_brunlock failed (fd=%d) at offset %d rw_type=%d len=%d\n",
-                         tdb->fd, offset, rw_type, (int)len));
+                TDB_LOG((tdb, TDB_DEBUG_TRACE,"tdb_brunlock failed (fd=%d) at offset %d rw_type=%d len=%d (errno=%d)\n",
+                         tdb->fd, offset, rw_type, (int)len, errno));
         }
         return ret;
 }
+
+#ifdef __OS2__
+int tdb_brunlock(struct tdb_context *tdb,
+                 int rw_type, tdb_off_t offset, size_t len)
+{
+        return tdb_brunlock_ex(tdb, rw_type, offset, offset, len);
+}
+#endif
 
 /*
@@ -285 +349 @@
         while (count--) {
                 struct timeval tv;
-#ifdef __OS2__
-                // we need to remove locks, as upgrade doesn't work
-                tdb_brunlock(tdb, F_RDLCK, FREELIST_TOP, 0);
-#endif
                 if (tdb_brlock(tdb, F_WRLCK, FREELIST_TOP, 0,
-                               TDB_LOCK_WAIT|TDB_LOCK_PROBE) == 0) {
+                               TDB_LOCK_WAIT|TDB_LOCK_PROBE|TDB_LOCK_UPGRADE) == 0) {
                         tdb->allrecord_lock.ltype = F_WRLCK;
                         tdb->allrecord_lock.off = 0;
@@ -580 +640 @@
 }
 
+#ifdef __OS2__
+#define TDB_ADJLOCK_START_DECL size_t start,
+#define TDB_ADJLOCK_START start,
+#define TDB_ADJLOCK(tdb, rw_type, start, offset, len, flags) \
+        tdb_brlock_ex(tdb, rw_type, start, offset, len, flags)
+#define TDB_ADJUNLOCK(tdb, rw_type, start, offset, len) \
+        tdb_brunlock_ex(tdb, rw_type, start, offset, len)
+#define TDB_CHAINLOCK_GRADUAL(tdb, ltype, flags, start, offset, len) \
+        tdb_chainlock_gradual(tdb, ltype, flags, start, offset, len)
+#else
+#define TDB_ADJLOCK_START_DECL
+#define TDB_ADJLOCK_START_REF
+#define TDB_ADJLOCK(tdb, rw_type, start, offset, len, flags) \
+        tdb_brlock(tdb, rw_type, offset, len, flags)
+#define TDB_ADJUNLOCK(tdb, rw_type, start, offset, len) \
+        tdb_brunlock(tdb, rw_type, offset, len)
+#define TDB_CHAINLOCK_GRADUAL(tdb, ltype, flags, start, offset, len) \
+        tdb_chainlock_gradual(tdb, ltype, flags, offset, len)
+#endif
+
 /* We only need to lock individual bytes, but Linux merges consecutive locks
  * so we lock in contiguous ranges. */
 static int tdb_chainlock_gradual(struct tdb_context *tdb,
                                  int ltype, enum tdb_lock_flags flags,
-                                 size_t off, size_t len)
+                                 TDB_ADJLOCK_START_DECL size_t off, size_t len)
 {
         int ret;
@@ -591 +671 @@
         if (len <= 4) {
                 /* Single record.  Just do blocking lock. */
-                return tdb_brlock(tdb, ltype, off, len, flags);
+                return TDB_ADJLOCK(tdb, ltype, start, off, len, flags);
         }
 
         /* First we try non-blocking. */
-        ret = tdb_brlock(tdb, ltype, off, len, nb_flags);
+        ret = TDB_ADJLOCK(tdb, ltype, start, off, len, nb_flags);
         if (ret == 0) {
                 return 0;
@@ -601 +681 @@
 
         /* Try locking first half, then second. */
-        ret = tdb_chainlock_gradual(tdb, ltype, flags, off, len / 2);
+        ret = TDB_CHAINLOCK_GRADUAL(tdb, ltype, flags, start, off, len / 2);
         if (ret == -1)
                 return -1;
 
-        ret = tdb_chainlock_gradual(tdb, ltype, flags,
+        ret = TDB_CHAINLOCK_GRADUAL(tdb, ltype, flags, start,
                                     off + len / 2, len - len / 2);
         if (ret == -1) {
-                tdb_brunlock(tdb, ltype, off, len / 2);
+                TDB_ADJUNLOCK(tdb, ltype, start, off, len / 2);
                 return -1;
         }
@@ -634 +714 @@
          * It is (1) which cause the starvation problem, so we're only
          * gradual for that. */
-        if (tdb_chainlock_gradual(tdb, ltype, flags, FREELIST_TOP,
+        if (TDB_CHAINLOCK_GRADUAL(tdb, ltype, flags, FREELIST_TOP, FREELIST_TOP,
                                   tdb->header.hash_size * 4) == -1) {
                 return -1;
@@ -640 +720 @@
 
         /* Grab individual record locks. */
-        if (tdb_brlock(tdb, ltype, lock_offset(tdb->header.hash_size), 0,
-                       flags) == -1) {
+        if (TDB_ADJLOCK(tdb, ltype, FREELIST_TOP, lock_offset(tdb->header.hash_size), 0,
+                        flags) == -1) {
                 tdb_brunlock(tdb, ltype, FREELIST_TOP,
                              tdb->header.hash_size * 4);

trunk/server/lib/tdb/common/open.c ¶

@@ -162 +162 @@
         return check_header_hash(tdb, false, m1, m2);
 }
+
+#ifdef __OS2__
+static int os2_create_lockfile(struct tdb_context *tdb, const char *name, const char *origin)
+{
+        /* name could be null, so handle it */
+        if (name == NULL)
+                return 0;
+
+        char lock_name[_MAX_PATH + 5];
+        snprintf(lock_name, sizeof(lock_name), "%s.lock", name);
+        tdb->lock_fd = open(lock_name, tdb->open_flags | O_CREAT | O_TRUNC, 0777);
+        if (tdb->lock_fd == -1) {
+                TDB_LOG((tdb, TDB_DEBUG_ERROR, "os2_create_lockfile: cannot create lock file %s, errno=%d\n",
+                         lock_name, errno));
+                return -1;
+        }
+
+        return 0;
+}
+#endif
 
 _PUBLIC_ struct tdb_context *tdb_open_ex(const char *name, int hash_size, int tdb_flags,
@@ -304 +324 @@
 
 #ifdef __OS2__
-        if (os2_crtActiveLock(tdb, name, "tdb_open_ex") != 0)
+        if (os2_create_lockfile(tdb, name, "tdb_open_ex") != 0)
                 goto fail;
 #endif
@@ -476 +496 @@
 
 #ifdef __OS2__
-        close(tdb->hActiveLock);
-        tdb->hActiveLock = -1;
+        close(tdb->lock_fd);
+        tdb->lock_fd = -1;
 #endif
         if (!tdb)
@@ -548 +568 @@
 
 #ifdef __OS2__
-        close(tdb->hActiveLock);
-        tdb->hActiveLock = -1;
+        close(tdb->lock_fd);
+        tdb->lock_fd = -1;
 #endif
 
@@ -620 +640 @@
 
 #ifdef __OS2__
-        close(tdb->hActiveLock);
-        tdb->hActiveLock = -1;
-
-        if (os2_crtActiveLock(tdb, tdb->name, "tdb_reopen") != 0)
+        close(tdb->lock_fd);
+        tdb->lock_fd = -1;
+
+        if (os2_create_lockfile(tdb, tdb->name, "tdb_reopen") != 0)
                 goto fail;
 #endif
@@ -679 +699 @@
         return 0;
 }
-#ifdef __OS2__
-int os2_crtActiveLock(struct tdb_context *tdb, const char *name, const char *origin)
-{
-        // name could be null, so handle it
-        if (name == NULL)
-                return 0;
-
-        struct stat st;
-        bool emptytdb = false;
-        char activeLockName[_MAX_PATH];
-        char *emptyString = "used for active lock\n\0";
-        sprintf(activeLockName, "%s_AL", name);
-
-        if ((stat(activeLockName, &st) == -1) || (st.st_size < strlen(emptyString)))
-                emptytdb = true;
-
-        tdb->hActiveLock = open(activeLockName, tdb->open_flags | O_CREAT, 0777);
-        if (tdb->hActiveLock == -1) {
-                TDB_LOG((tdb, TDB_DEBUG_ERROR, "os2_crtActiveLock: cannot create activeLock %s called from %s %s\n",
-                        activeLockName, origin, strerror(errno)));
-                errno = EINVAL;
-                return -1;
-        }
-
-        // we try to truncate the db when called from tdb_open_ex
-        // but if it's not working it's ok as well
-        if (emptytdb)
-                tdb_write_all(tdb->hActiveLock, emptyString, strlen(emptyString));
-
-        return 0;
-} 
-#endif

trunk/server/lib/tdb/common/tdb_private.h ¶

@@ -180 +180 @@
         /* If set, don't actually lock at all. */
         TDB_LOCK_MARK_ONLY = 4,
+        /* If set, upgrade the existiong lock (used only on OS/2). */
+        TDB_LOCK_UPGRADE = 8,
 };
 
@@ -220 +222 @@
         volatile sig_atomic_t *interrupt_sig_ptr;
 #ifdef __OS2__
-        int hActiveLock;
+        int lock_fd;
 #endif
 };