hammer2 - serialized flush work part 4
authorMatthew Dillon <dillon@apollo.backplane.com>
Fri, 1 Feb 2013 17:19:41 +0000 (09:19 -0800)
committerMatthew Dillon <dillon@apollo.backplane.com>
Fri, 1 Feb 2013 17:19:41 +0000 (09:19 -0800)
* Hardlink helper functions now take and return hammer2_chain pointers
  instead of messing with hammer2_inode's, which simplifies matters
  greatly.

* Major cleanup of the hardlink handling code.

* Get hardlink creation/deletion mostly working as expected again.
  There is still an issue with hammer_inode aliasing (see TODO),
  and the rename-directory-to-some-parent hardlink issue has not been
  addressed yet.

sys/vfs/hammer2/TODO
sys/vfs/hammer2/hammer2.h
sys/vfs/hammer2/hammer2_chain.c
sys/vfs/hammer2/hammer2_inode.c
sys/vfs/hammer2/hammer2_vnops.c

index ec48567..cf97587 100644 (file)
@@ -1,4 +1,13 @@
 
+* on fresh mount with multiple hardlinks present separate lookups will
+  result in separate vnodes pointing to separate inodes pointing to a
+  common chain (the hardlink target).
+
+  When the hardlink target consolidates upward only one vp/ip will be
+  adjusted.  We need code to fixup the other chains (probably put in
+  inode_lock_*()) which will be pointing to an older deleted hardlink
+  target.
+
 * Filesystem must ensure that modify_tid is not too large relative to
   the iterator in the volume header, on load, or flush sequencing will
   not work properly.  We should be able to just override it, but we
index 99de8ea..d1bc94f 100644 (file)
@@ -247,6 +247,7 @@ typedef struct hammer2_inode hammer2_inode_t;
 
 #define HAMMER2_INODE_MODIFIED         0x0001
 #define HAMMER2_INODE_DIRTYEMBED       0x0002
+#define HAMMER2_INODE_RENAME_INPROG    0x0004
 
 /*
  * XXX
@@ -408,10 +409,9 @@ int hammer2_inode_create(hammer2_inode_t *dip,
                        const uint8_t *name, size_t name_len,
                        hammer2_inode_t **nipp, hammer2_chain_t **nchainp);
 
-int hammer2_inode_duplicate(hammer2_inode_t *dip, hammer2_inode_t *oip,
-                       hammer2_inode_t **nipp, hammer2_chain_t **nchainp,
-                       const uint8_t *name, size_t name_len);
-int hammer2_inode_connect(hammer2_inode_t *dip, hammer2_inode_t *oip,
+int hammer2_inode_duplicate(hammer2_inode_t *dip,
+                       hammer2_chain_t *ochain, hammer2_chain_t **nchainp);
+int hammer2_inode_connect(hammer2_inode_t *dip, hammer2_chain_t **chainp,
                        const uint8_t *name, size_t name_len);
 hammer2_inode_t *hammer2_inode_common_parent(hammer2_mount_t *hmp,
                        hammer2_inode_t *fdip, hammer2_inode_t *tdip);
@@ -419,7 +419,8 @@ hammer2_inode_t *hammer2_inode_common_parent(hammer2_mount_t *hmp,
 int hammer2_unlink_file(hammer2_inode_t *dip,
                        const uint8_t *name, size_t name_len,
                        int isdir, hammer2_chain_t *retain_chain);
-int hammer2_hardlink_consolidate(hammer2_inode_t **ipp, hammer2_inode_t *tdip);
+int hammer2_hardlink_consolidate(hammer2_inode_t *ip, hammer2_chain_t **chainp,
+                       hammer2_inode_t *tdip, int linkcnt);
 int hammer2_hardlink_deconsolidate(hammer2_inode_t *dip,
                        hammer2_chain_t **chainp, hammer2_chain_t **ochainp);
 int hammer2_hardlink_find(hammer2_inode_t *dip,
index eedb990..490278a 100644 (file)
@@ -2246,7 +2246,7 @@ hammer2_chain_create_indirect(hammer2_mount_t *hmp, hammer2_chain_t *parent,
 
        /*
         * If we hit a chain that is undergoing flushing we're screwed and
-        * we have to duno the whole mess.  Since ichain has not been linked
+        * we have to undo the whole mess.  Since ichain has not been linked
         * in yet, the moved chains are not reachable and will not have been
         * disposed of.
         *
@@ -2348,11 +2348,12 @@ hammer2_chain_create_indirect(hammer2_mount_t *hmp, hammer2_chain_t *parent,
  * referenced.  (*parentp) will be modified in a manner similar to a lookup
  * or iteration when indirect blocks are also deleted as a side effect.
  *
+ * Must be called with an exclusively locked parent and chain.  parent and
+ * chain are both left locked on return.
+ *
  * XXX This currently does not adhere to the MOVED flag protocol in that
  *     the removal is immediately indicated in the parent's blockref[]
  *     array.
- *
- * Must be called with an exclusively locked parent and chain.
  */
 void
 hammer2_chain_delete(hammer2_mount_t *hmp, hammer2_chain_t *parent,
index 07186df..d682aad 100644 (file)
@@ -68,7 +68,6 @@ hammer2_inode_drop(hammer2_inode_t *ip)
                cpu_ccfence();
                if (refs == 1) {
                        if (atomic_cmpset_int(&ip->refs, 1, 0)) {
-                               kprintf("hammer2_inode_drop: 1->0 %p\n", ip);
                                KKASSERT(ip->topo_cst.count == 0);
 
                                hmp = ip->hmp;
@@ -410,7 +409,6 @@ retry:
         * NOTE: *_get() integrates chain's lock into the inode lock.
         */
        nip = hammer2_inode_get(dip->hmp, dip->pmp, dip, chain);
-       kprintf("nip %p chain %p\n", nip, nip->chain);
        *nipp = nip;
        *nchainp = chain;
        nipdata = &chain->data->ipdata;
@@ -476,58 +474,44 @@ retry:
 }
 
 /*
- * Duplicate the specified existing inode in the specified target directory.
- * If name is NULL the inode is duplicated as a hidden directory entry.
+ * Create a duplicate of the inode (chain) in the specified target directory
+ * (dip), return the duplicated chain in *nchainp (locked).  chain is locked
+ * on call and remains locked on return.
  *
- * Returns the new inode.  The old inode is left alone.
+ * If name is NULL the inode is duplicated as a hidden directory entry.
  *
  * XXX name needs to be NULL for now.
  */
 int
-hammer2_inode_duplicate(hammer2_inode_t *dip, hammer2_inode_t *oip,
-                       hammer2_inode_t **nipp, hammer2_chain_t **nchainp,
-                       const uint8_t *name, size_t name_len)
+hammer2_inode_duplicate(hammer2_inode_t *dip,
+                       hammer2_chain_t *ochain, hammer2_chain_t **nchainp)
 {
        hammer2_inode_data_t *nipdata;
        hammer2_mount_t *hmp;
-       hammer2_inode_t *nip;
        hammer2_chain_t *parent;
        hammer2_chain_t *chain;
        hammer2_key_t lhc;
-       int error;
+       int error = 0;
 
        hmp = dip->hmp;
-       if (name) {
-               lhc = hammer2_dirhash(name, name_len);
-       } else {
-               parent = hammer2_inode_lock_ex(oip);
-               lhc = parent->data->ipdata.inum;
-               hammer2_inode_unlock_ex(oip, parent);
-               KKASSERT((lhc & HAMMER2_DIRHASH_VISIBLE) == 0);
-       }
+       lhc = ochain->data->ipdata.inum;
+       *nchainp = NULL;
+       KKASSERT((lhc & HAMMER2_DIRHASH_VISIBLE) == 0);
 
        /*
         * Locate the inode or indirect block to create the new
-        * entry in.  At the same time check for key collisions
-        * and iterate until we don't get one.
+        * entry in.
+        *
+        * There should be no key collisions with invisible inode keys.
         */
-       nip = NULL;
 retry:
-       parent = hammer2_inode_lock_ex(dip);
-
-       error = 0;
-       while (error == 0) {
-               chain = hammer2_chain_lookup(hmp, &parent, lhc, lhc, 0);
-               if (chain == NULL)
-                       break;
-               /* XXX bcmp name if not NULL */
-               if ((lhc & HAMMER2_DIRHASH_LOMASK) == HAMMER2_DIRHASH_LOMASK)
-                       error = ENOSPC;
-               if ((lhc & HAMMER2_DIRHASH_VISIBLE) == 0) /* shouldn't happen */
-                       error = ENOSPC;
+       parent = dip->chain;
+       hammer2_chain_lock(hmp, parent, HAMMER2_RESOLVE_ALWAYS);
+       chain = hammer2_chain_lookup(hmp, &parent, lhc, lhc, 0);
+       if (chain) {
                hammer2_chain_unlock(hmp, chain);
                chain = NULL;
-               ++lhc;
+               error = ENOSPC;
        }
 
        /*
@@ -546,7 +530,7 @@ retry:
         */
        if (error == EAGAIN)
                hammer2_chain_ref(hmp, parent);
-       hammer2_inode_unlock_ex(dip, parent);
+       hammer2_chain_unlock(hmp, parent);
 
        /*
         * Handle the error case
@@ -575,98 +559,75 @@ retry:
         *     Well, it would work I guess, but you might catch some files
         *     mid-operation.
         *
-        * We cannot leave oip with any in-memory chains because (for a
-        * hardlink), oip will become a OBJTYPE_HARDLINK which is just a
+        * We cannot leave ochain with any in-memory chains because (for a
+        * hardlink), ochain will become a OBJTYPE_HARDLINK which is just a
         * pointer to the real hardlink's inum and can't have any sub-chains.
         * XXX might be 0-ref chains left.
         */
-       parent = hammer2_inode_lock_ex(oip);
-       hammer2_chain_flush(hmp, parent, 0);
-       /*KKASSERT(RB_EMPTY(&oip->chain.rbhead));*/
+       hammer2_chain_flush(hmp, ochain, 0);
+       /*KKASSERT(RB_EMPTY(&ochain.rbhead));*/
 
-       /*
-        * nip is a duplicate of oip.  Meta-data will be synchronized to
-        * media when nip is flushed.
-        *
-        * NOTE: *_get() integrates chain's lock into the inode lock.
-        */
-       nip = hammer2_inode_get(dip->hmp, dip->pmp, dip, chain);
        hammer2_chain_modify(hmp, chain, 0);
        nipdata = &chain->data->ipdata;
-       *nipdata = parent->data->ipdata;
-       hammer2_inode_unlock_ex(oip, parent);
+       *nipdata = ochain->data->ipdata;
+
+       /*
+        * Directory entries are inodes but this is a hidden hardlink
+        * target.  The name isn't used but to ease debugging give it
+        * a name after its inode number.
+        */
+       ksnprintf(nipdata->filename, sizeof(nipdata->filename),
+                 "0x%016jx", (intmax_t)nipdata->inum);
+       nipdata->name_len = strlen(nipdata->filename);
+       nipdata->name_key = lhc;
 
-       if (name) {
-               /*
-                * Directory entries are inodes so if the name has changed
-                * we have to update the inode.
-                */
-               KKASSERT(name_len < HAMMER2_INODE_MAXNAME);
-               bcopy(name, nipdata->filename, name_len);
-               nipdata->name_key = lhc;
-               nipdata->name_len = name_len;
-       } else {
-               /*
-                * Directory entries are inodes but this is a hidden hardlink
-                * target.  The name isn't used but to ease debugging give it
-                * a name after its inode number.
-                */
-               ksnprintf(nipdata->filename, sizeof(nipdata->filename),
-                         "0x%016jx", (intmax_t)nipdata->inum);
-               nipdata->name_len = strlen(nipdata->filename);
-               nipdata->name_key = lhc;
-       }
-       *nipp = nip;
        *nchainp = chain;
 
        return (0);
 }
 
-
 /*
- * Connect inode (oip) to the specified directory using the specified name.
- * (oip) must be locked.
+ * Connect *chainp to the media topology represented by (dip, name, len).
+ * A directory entry is created which points to *chainp.  *chainp is then
+ * unlocked and set to NULL.
  *
- * If (oip) is not currently connected we simply connect it up.
+ * If *chainp is not currently connected we simply connect it up.
  *
- * If (oip) is already connected we create a OBJTYPE_HARDLINK entry which
- * points to (oip)'s inode number.  (oip) is expected to be the terminus of
+ * If *chainp is already connected we create a OBJTYPE_HARDLINK entry which
+ * points to chain's inode number.  *chainp is expected to be the terminus of
  * the hardlink sitting as a hidden file in a common parent directory
  * in this situation.
+ *
+ * The caller always wants to reference the hardlink terminus, not the
+ * hardlink pointer that we might be creating, so we do NOT replace
+ * *chainp here, we simply unlock and NULL it out.
  */
 int
-hammer2_inode_connect(hammer2_inode_t *dip, hammer2_inode_t *oip,
+hammer2_inode_connect(hammer2_inode_t *dip, hammer2_chain_t **chainp,
                      const uint8_t *name, size_t name_len)
 {
-       hammer2_inode_data_t *nipdata;
+       hammer2_inode_data_t *ipdata;
        hammer2_mount_t *hmp;
-       hammer2_chain_t *chain;
+       hammer2_chain_t *nchain;
        hammer2_chain_t *parent;
        hammer2_chain_t *ochain;
-       hammer2_inode_t *nip;
        hammer2_key_t lhc;
        int error;
        int hlink;
 
+       hmp = dip->hmp;
+
+       ochain = *chainp;
+       *chainp = NULL;
+
        /*
-        * (oip) is the terminus of the hardlink sitting in the common
-        * parent directory.  This means that if oip->pip != dip then
-        * the already locked oip is ABOVE dip.
-        *
-        * But if the common parent directory IS dip, then we would have
-        * a lock-order reversal and must rearrange the lock ordering.
-        * For now the caller deals with this for us by locking dip in
-        * that case (and our lock here winds up just being recursive)
+        * Since ochain is either disconnected from the topology or represents
+        * a hardlink terminus which is always a parent of or equal to dip,
+        * we should be able to safely lock dip->chain for our setup.
         */
-       hmp = dip->hmp;
 retry:
-       if (oip->pip == dip) {
-               parent = hammer2_inode_lock_ex(dip);
-               ochain = hammer2_inode_lock_ex(oip);
-       } else {
-               ochain = hammer2_inode_lock_ex(oip);
-               parent = hammer2_inode_lock_ex(dip);
-       }
+       parent = dip->chain;
+       hammer2_chain_lock(hmp, parent, HAMMER2_RESOLVE_ALWAYS);
 
        lhc = hammer2_dirhash(name, name_len);
        hlink = (ochain->parent != NULL);
@@ -684,13 +645,13 @@ retry:
         */
        error = 0;
        while (error == 0) {
-               chain = hammer2_chain_lookup(hmp, &parent, lhc, lhc, 0);
-               if (chain == NULL)
+               nchain = hammer2_chain_lookup(hmp, &parent, lhc, lhc, 0);
+               if (nchain == NULL)
                        break;
                if ((lhc & HAMMER2_DIRHASH_LOMASK) == HAMMER2_DIRHASH_LOMASK)
                        error = ENOSPC;
-               hammer2_chain_unlock(hmp, chain);
-               chain = NULL;
+               hammer2_chain_unlock(hmp, nchain);
+               nchain = NULL;
                ++lhc;
        }
 
@@ -701,21 +662,25 @@ retry:
         */
        if (error == 0) {
                if (hlink) {
-                       chain = hammer2_chain_create(hmp, parent,
+                       nchain = hammer2_chain_create(hmp, parent,
                                                     NULL, lhc, 0,
                                                     HAMMER2_BREF_TYPE_INODE,
                                                     HAMMER2_INODE_BYTES,
                                                     &error);
                } else {
-                       chain = hammer2_chain_create(hmp, parent,
-                                                    oip->chain, lhc, 0,
+                       /*
+                        * NOTE: reconnects oip->chain to the media
+                        *       topology and returns its argument
+                        *       (oip->chain).
+                        *
+                        * No additional locks or refs are obtained on
+                        * the returned chain so don't double-unlock!
+                        */
+                       nchain = hammer2_chain_create(hmp, parent,
+                                                    ochain, lhc, 0,
                                                     HAMMER2_BREF_TYPE_INODE,
                                                     HAMMER2_INODE_BYTES,
                                                     &error);
-                       if (chain) {
-                               KKASSERT(chain == oip->chain);
-                               KKASSERT(ochain == oip->chain);
-                       }
                }
        }
 
@@ -725,22 +690,21 @@ retry:
         */
        if (error == EAGAIN)
                hammer2_chain_ref(hmp, parent);
-       hammer2_inode_unlock_ex(dip, parent);
+       hammer2_chain_unlock(hmp, parent);
 
        /*
-        * oip/ochain still active.
+        * ochain still active.
         *
         * Handle the error case
         */
        if (error) {
-               KKASSERT(chain == NULL);
+               KKASSERT(nchain == NULL);
                if (error == EAGAIN) {
                        hammer2_chain_wait(hmp, parent);
                        hammer2_chain_drop(hmp, parent);
-                       hammer2_inode_unlock_ex(oip, ochain);
                        goto retry;
                }
-               hammer2_inode_unlock_ex(oip, ochain);
+               hammer2_chain_unlock(hmp, ochain);
                return (error);
        }
 
@@ -759,35 +723,35 @@ retry:
                 *
                 * NOTE: *_get() integrates chain's lock into the inode lock.
                 */
-               nip = hammer2_inode_get(dip->hmp, dip->pmp, dip, chain);
-               hammer2_chain_modify(hmp, chain, 0);
+               hammer2_chain_modify(hmp, nchain, 0);
                KKASSERT(name_len < HAMMER2_INODE_MAXNAME);
-               nipdata = &nip->chain->data->ipdata;
-               bcopy(name, nipdata->filename, name_len);
-               nipdata->name_key = lhc;
-               nipdata->name_len = name_len;
-               nipdata->target_type = ochain->data->ipdata.type;
-               nipdata->type = HAMMER2_OBJTYPE_HARDLINK;
-               nipdata->inum = ochain->data->ipdata.inum;
-               nipdata->nlinks = 1;
+               ipdata = &nchain->data->ipdata;
+               bcopy(name, ipdata->filename, name_len);
+               ipdata->name_key = lhc;
+               ipdata->name_len = name_len;
+               ipdata->target_type = ochain->data->ipdata.type;
+               ipdata->type = HAMMER2_OBJTYPE_HARDLINK;
+               ipdata->inum = ochain->data->ipdata.inum;
+               ipdata->nlinks = 1;
                kprintf("created hardlink %*.*s\n",
                        (int)name_len, (int)name_len, name);
+               hammer2_chain_unlock(hmp, nchain);
        } else if (hlink && hammer2_hardlink_enable < 0) {
                /*
                 * Create a snapshot (hardlink fake mode for debugging).
                 *
-                * NOTE: *_get() integrates chain's lock into the inode lock.
+                * NOTE: *_get() integrates nchain's lock into the inode lock.
                 */
-               nip = hammer2_inode_get(dip->hmp, dip->pmp, dip, chain);
-               hammer2_chain_modify(hmp, chain, 0);
+               hammer2_chain_modify(hmp, nchain, 0);
                KKASSERT(name_len < HAMMER2_INODE_MAXNAME);
-               nipdata = &nip->chain->data->ipdata;
-               *nipdata = ochain->data->ipdata;
-               bcopy(name, nipdata->filename, name_len);
-               nipdata->name_key = lhc;
-               nipdata->name_len = name_len;
+               ipdata = &nchain->data->ipdata;
+               *ipdata = ochain->data->ipdata;
+               bcopy(name, ipdata->filename, name_len);
+               ipdata->name_key = lhc;
+               ipdata->name_len = name_len;
                kprintf("created fake hardlink %*.*s\n",
                        (int)name_len, (int)name_len, name);
+               hammer2_chain_unlock(hmp, nchain);
        } else {
                /*
                 * Normally disconnected inode (e.g. during a rename) that
@@ -797,19 +761,19 @@ retry:
                 * We are using oip as chain, already locked by caller,
                 * do not unlock it.
                 */
-               hammer2_chain_modify(hmp, chain, 0);
-               nipdata = &ochain->data->ipdata;
+               hammer2_chain_modify(hmp, ochain, 0);
+               ipdata = &ochain->data->ipdata;
 
-               if (nipdata->name_len != name_len ||
-                   bcmp(nipdata->filename, name, name_len) != 0) {
+               if (ipdata->name_len != name_len ||
+                   bcmp(ipdata->filename, name, name_len) != 0) {
                        KKASSERT(name_len < HAMMER2_INODE_MAXNAME);
-                       bcopy(name, nipdata->filename, name_len);
-                       nipdata->name_key = lhc;
-                       nipdata->name_len = name_len;
+                       bcopy(name, ipdata->filename, name_len);
+                       ipdata->name_key = lhc;
+                       ipdata->name_len = name_len;
                }
-               nipdata->nlinks = 1;
+               ipdata->nlinks = 1;
        }
-       hammer2_inode_unlock_ex(oip, ochain);
+       hammer2_chain_unlock(hmp, ochain);
        return (0);
 }
 
@@ -871,6 +835,7 @@ hammer2_unlink_file(hammer2_inode_t *dip,
 
        /*
         * Not found or wrong type (isdir < 0 disables the type check).
+        * If a hardlink pointer, type checks use the hardlink target.
         */
        if (chain == NULL) {
                error = ENOENT;
@@ -970,6 +935,7 @@ hammer2_unlink_file(hammer2_inode_t *dip,
                }
                hammer2_chain_delete(hmp, parent, ochain,
                                     (ochain == retain_chain));
+               hammer2_chain_unlock(hmp, ochain);
                hammer2_chain_unlock(hmp, parent);
                hammer2_chain_drop(hmp, parent);
                parent = NULL;
@@ -1045,152 +1011,161 @@ hammer2_inode_calc_alloc(hammer2_key_t filesize)
 }
 
 /*
- * Consolidate for hard link creation.  This moves the specified terminal
- * hardlink inode to a directory common to its current directory and tdip
- * if necessary, replacing *ipp with the new inode chain element and
- * modifying the original inode chain element to OBJTYPE_HARDLINK.
- *
- * If the original inode chain element was a prior incarnation of a hidden
- * inode it can simply be deleted instead of converted.
+ * Given an unlocked ip consolidate for hardlink creation, adding (nlinks)
+ * to the file's link count and potentially relocating the file to a
+ * directory common to ip->pip and tdip.
  *
- * (*ipp) must be referenced on entry and the new (*ipp) will be referenced
- * on return (with the original dropped).  (*ipp) must not be locked.
- *
- * The link count is bumped if requested.
+ * If the file has to be relocated ip->chain will also be adjusted.
  */
 int
-hammer2_hardlink_consolidate(hammer2_inode_t **ipp, hammer2_inode_t *tdip)
+hammer2_hardlink_consolidate(hammer2_inode_t *ip, hammer2_chain_t **chainp,
+                            hammer2_inode_t *tdip, int nlinks)
 {
-       hammer2_inode_data_t *oipdata;
+       hammer2_inode_data_t *ipdata;
        hammer2_mount_t *hmp;
-       hammer2_inode_t *oip;
-       hammer2_inode_t *nip;
        hammer2_inode_t *fdip;
        hammer2_inode_t *cdip;
-       hammer2_chain_t *ochain;
+       hammer2_chain_t *chain;
        hammer2_chain_t *nchain;
        hammer2_chain_t *parent;
        int error;
 
        hmp = tdip->hmp;
-       oip = *ipp;
-       nip = NULL;
+       *chainp = NULL;
+       chain = hammer2_inode_lock_ex(ip);
 
-       if (hammer2_hardlink_enable < 0)
+       if (nlinks == 0 &&                      /* no hardlink needed */
+           (chain->data->ipdata.name_key & HAMMER2_DIRHASH_VISIBLE)) {
+               hammer2_inode_unlock_ex(ip, NULL);
+               *chainp = chain;
+               return (0);
+       }
+       if (hammer2_hardlink_enable < 0) {      /* fake hardlinks */
+               hammer2_inode_unlock_ex(ip, NULL);
+               *chainp = chain;
                return (0);
-       if (hammer2_hardlink_enable == 0)
+       }
+       if (hammer2_hardlink_enable == 0) {     /* disallow hardlinks */
+               hammer2_inode_unlock_ex(ip, chain);
                return (ENOTSUP);
+       }
 
        /*
         * cdip will be returned with a ref, but not locked.
         */
-       fdip = oip->pip;
+       fdip = ip->pip;
        cdip = hammer2_inode_common_parent(hmp, fdip, tdip);
 
        /*
-        * Nothing to do (except bump the link count) if the hardlink has
-        * already been consolidated in the correct place.
+        * If no change in the hardlink's target directory is required and
+        * this is already a hardlink target, all we need to do is adjust
+        * the link count.
         */
-       if (cdip == fdip) {
-               ochain = hammer2_inode_lock_ex(oip);
-               if ((ochain->data->ipdata.name_key &
-                    HAMMER2_DIRHASH_VISIBLE) == 0) {
-                       hammer2_chain_modify(hmp, ochain, 0);
-                       ++ochain->data->ipdata.nlinks;
-                       hammer2_inode_unlock_ex(oip, ochain);
-                       hammer2_inode_drop(cdip);
-                       return(0);
+       if (cdip == fdip &&
+           (chain->data->ipdata.name_key & HAMMER2_DIRHASH_VISIBLE) == 0) {
+               if (nlinks) {
+                       hammer2_chain_modify(hmp, chain, 0);
+                       chain->data->ipdata.nlinks += nlinks;
                }
-               hammer2_inode_unlock_ex(oip, ochain);
+               *chainp = chain;
+               error = 0;
+               goto done;
        }
 
        /*
-        * Create a hidden inode directory entry in the parent, copying
-        * (*oip)'s state.  Then replace oip with OBJTYPE_HARDLINK.
+        * We either have to move an existing hardlink target or we have
+        * to create a fresh hardlink target.
         *
-        * The duplication function will either flush or move any chains
-        * under oip to the new hardlink target inode, retiring all chains
-        * related to oip before returning.  XXX vp->ip races.
+        * Hardlink targets are hidden inodes in a parent directory common
+        * to all directory entries referencing the hardlink.
         */
-       error = hammer2_inode_duplicate(cdip, oip, &nip, &nchain, NULL, 0);
+       error = hammer2_inode_duplicate(cdip, chain, &nchain);
        if (error == 0) {
                /*
                 * Bump nlinks on duplicated hidden inode.
                 */
-               hammer2_inode_ref(nip);                 /* ref new *ipp */
                hammer2_chain_modify(hmp, nchain, 0);
-               ++nchain->data->ipdata.nlinks;
-               hammer2_inode_unlock_ex(nip, nchain);
-               ochain = hammer2_inode_lock_ex(oip);
-               hammer2_inode_drop(oip);                /* unref old *ipp */
+               nchain->data->ipdata.nlinks += nlinks;
 
-               if (ochain->data->ipdata.name_key & HAMMER2_DIRHASH_VISIBLE) {
-                       /*
-                        * Replace the old inode with an OBJTYPE_HARDLINK
-                        * pointer.
-                        */
-                       hammer2_chain_modify(hmp, ochain, 0);
-                       oipdata = &ochain->data->ipdata;
-                       oipdata->target_type = oipdata->type;
-                       oipdata->type = HAMMER2_OBJTYPE_HARDLINK;
-                       oipdata->uflags = 0;
-                       oipdata->rmajor = 0;
-                       oipdata->rminor = 0;
-                       oipdata->ctime = 0;
-                       oipdata->mtime = 0;
-                       oipdata->atime = 0;
-                       oipdata->btime = 0;
-                       bzero(&oipdata->uid, sizeof(oipdata->uid));
-                       bzero(&oipdata->gid, sizeof(oipdata->gid));
-                       oipdata->op_flags = HAMMER2_OPFLAG_DIRECTDATA;
-                       oipdata->cap_flags = 0;
-                       oipdata->mode = 0;
-                       oipdata->size = 0;
-                       oipdata->nlinks = 1;
-                       oipdata->iparent = 0;   /* XXX */
-                       oipdata->pfs_type = 0;
-                       oipdata->pfs_inum = 0;
-                       bzero(&oipdata->pfs_clid, sizeof(oipdata->pfs_clid));
-                       bzero(&oipdata->pfs_fsid, sizeof(oipdata->pfs_fsid));
-                       oipdata->data_quota = 0;
-                       oipdata->data_count = 0;
-                       oipdata->inode_quota = 0;
-                       oipdata->inode_count = 0;
-                       oipdata->attr_tid = 0;
-                       oipdata->dirent_tid = 0;
-                       bzero(&oipdata->u, sizeof(oipdata->u));
+               /*
+                * If the old chain is not a hardlink target then replace
+                * it with a OBJTYPE_HARDLINK pointer.
+                *
+                * If the old chain IS a hardlink target then delete it.
+                */
+               if (chain->data->ipdata.name_key & HAMMER2_DIRHASH_VISIBLE) {
+                       hammer2_chain_modify(hmp, chain, 0);
+                       ipdata = &chain->data->ipdata;
+                       ipdata->target_type = ipdata->type;
+                       ipdata->type = HAMMER2_OBJTYPE_HARDLINK;
+                       ipdata->uflags = 0;
+                       ipdata->rmajor = 0;
+                       ipdata->rminor = 0;
+                       ipdata->ctime = 0;
+                       ipdata->mtime = 0;
+                       ipdata->atime = 0;
+                       ipdata->btime = 0;
+                       bzero(&ipdata->uid, sizeof(ipdata->uid));
+                       bzero(&ipdata->gid, sizeof(ipdata->gid));
+                       ipdata->op_flags = HAMMER2_OPFLAG_DIRECTDATA;
+                       ipdata->cap_flags = 0;
+                       ipdata->mode = 0;
+                       ipdata->size = 0;
+                       ipdata->nlinks = 1;
+                       ipdata->iparent = 0;    /* XXX */
+                       ipdata->pfs_type = 0;
+                       ipdata->pfs_inum = 0;
+                       bzero(&ipdata->pfs_clid, sizeof(ipdata->pfs_clid));
+                       bzero(&ipdata->pfs_fsid, sizeof(ipdata->pfs_fsid));
+                       ipdata->data_quota = 0;
+                       ipdata->data_count = 0;
+                       ipdata->inode_quota = 0;
+                       ipdata->inode_count = 0;
+                       ipdata->attr_tid = 0;
+                       ipdata->dirent_tid = 0;
+                       bzero(&ipdata->u, sizeof(ipdata->u));
                        /* XXX transaction ids */
-
-                       hammer2_inode_unlock_ex(oip, ochain);
                } else {
-                       /*
-                        * The old inode was a hardlink target, which we
-                        * have now moved.  We must delete it so the new
-                        * hardlink target at a higher directory level
-                        * becomes the only hardlink target for this inode.
-                        */
                        kprintf("DELETE INVISIBLE\n");
                        for (;;) {
-                               parent = ochain->parent;
+                               parent = chain->parent;
                                hammer2_chain_ref(hmp, parent);
-                               hammer2_inode_unlock_ex(oip, ochain);
+                               hammer2_chain_ref(hmp, chain);
+                               hammer2_chain_unlock(hmp, chain);
                                hammer2_chain_lock(hmp, parent,
                                                   HAMMER2_RESOLVE_ALWAYS);
-                               ochain = hammer2_inode_lock_ex(oip);
-                               if (oip->chain->parent == parent)
+                               hammer2_chain_lock(hmp, chain,
+                                                  HAMMER2_RESOLVE_ALWAYS);
+                               hammer2_chain_drop(hmp, chain);
+                               if (chain->parent == parent)
                                        break;
                                hammer2_chain_unlock(hmp, parent);
                                hammer2_chain_drop(hmp, parent);
                        }
-                       hammer2_chain_delete(hmp, parent, ochain, 0);
-                       hammer2_inode_put(oip, ochain); /* unconditional */
+                       hammer2_chain_delete(hmp, parent, chain, 0);
                        hammer2_chain_unlock(hmp, parent);
                        hammer2_chain_drop(hmp, parent);
                }
-               *ipp = nip;
+
+               /*
+                * Replace ip->chain with nchain (ip is still locked).
+                */
+               hammer2_chain_ref(hmp, nchain);                 /* ip->chain */
+               if (ip->chain)
+                       hammer2_chain_drop(hmp, ip->chain);     /* ip->chain */
+               ip->chain = nchain;
+
+               hammer2_chain_unlock(hmp, chain);
+               *chainp = nchain;
        } else {
-               KKASSERT(nip == NULL);
+               hammer2_chain_unlock(hmp, chain);
        }
+
+       /*
+        * Cleanup, chain/nchain already dealt with.
+        */
+done:
+       hammer2_inode_unlock_ex(ip, NULL);
        hammer2_inode_drop(cdip);
 
        return (error);
@@ -1311,6 +1286,10 @@ hammer2_inode_common_parent(hammer2_mount_t *hmp,
                hammer2_inode_ref(tdip);
                return(tdip);
        }
+
+       /*
+        * XXX not MPSAFE
+        */
        for (scan1 = fdip; scan1->pmp == fdip->pmp; scan1 = scan1->pip) {
                scan2 = tdip;
                while (scan2->pmp == tdip->pmp) {
index 6ff831b..2153544 100644 (file)
@@ -146,6 +146,8 @@ hammer2_vop_reclaim(struct vop_reclaim_args *ap)
                                              HAMMER2_CHAIN_SUBMODIFIED);
        }
        hammer2_chain_flush(hmp, chain, 0);
+       kprintf("vop_reclaim vp %p ip %p refs %d\n",
+               vp, ip, ip->refs);
        if (ip->refs > 2)                       /* (our lock + vp ref) */
                hammer2_inode_unlock_ex(ip, chain); /* unlock */
        else
@@ -1715,10 +1717,8 @@ hammer2_vop_nlink(struct vop_nlink_args *ap)
 {
        hammer2_inode_t *dip;   /* target directory to create link in */
        hammer2_inode_t *ip;    /* inode we are hardlinking to */
-       hammer2_inode_t *oip;
        hammer2_mount_t *hmp;
        hammer2_chain_t *chain;
-       hammer2_chain_t *ochain;
        struct namecache *ncp;
        const uint8_t *name;
        size_t name_len;
@@ -1729,59 +1729,32 @@ hammer2_vop_nlink(struct vop_nlink_args *ap)
        if (hmp->ronly)
                return (EROFS);
 
-       /*
-        * (ip) is the inode we are linking to.
-        */
-       ip = oip = VTOI(ap->a_vp);
-       hammer2_inode_ref(ip);
-
        ncp = ap->a_nch->ncp;
        name = ncp->nc_name;
        name_len = ncp->nc_nlen;
 
        /*
-        * Create a consolidated real file for the hardlink, adjust (ip),
-        * and move the nlinks lock if necessary.  Tell the function to
-        * bump the hardlink count on the consolidated file.
+        * ip represents the file being hardlinked.  The file could be a
+        * normal file or a hardlink target if it has already been hardlinked.
+        * If ip is a hardlinked target then ip->pip represents the location
+        * of the hardlinked target, NOT the location of the hardlink pointer.
+        *
+        * Bump nlinks and potentially also create or move the hardlink
+        * target in the parent directory common to (ip) and (dip).  The
+        * consolidation code can modify ip->chain and ip->pip.  The
+        * returned chain is locked.
         */
-       error = hammer2_hardlink_consolidate(&ip, dip);
+       ip = VTOI(ap->a_vp);
+       hammer2_inode_ref(ip);
+       error = hammer2_hardlink_consolidate(ip, &chain, dip, 1);
        if (error)
                goto done;
 
        /*
-        * If the consolidation changed ip to a HARDLINK pointer we have
-        * to adjust the vnode to point to the actual ip.
-        *
-        * XXX this can race against concurrent vnode ops.
-        */
-       if (oip != ip) {
-               hammer2_inode_ref(ip);                  /* vp ref+ */
-               chain = hammer2_inode_lock_ex(ip);
-               ochain = hammer2_inode_lock_ex(oip);
-               if (ip->vp) {
-                       KKASSERT(ip->vp == ap->a_vp);
-                       hammer2_inode_drop(ip);         /* vp already ref'd */
-               } else {
-                       ip->vp = ap->a_vp;
-                       ap->a_vp->v_data = ip;
-               }
-               if (oip->vp) {
-                       KKASSERT(oip->vp == ap->a_vp);
-                       oip->vp = NULL;
-                       hammer2_inode_drop(oip);        /* vp ref- */
-               }
-               hammer2_inode_unlock_ex(oip, ochain);
-               hammer2_inode_unlock_ex(ip, chain);
-       }
-
-       /*
-        * The act of connecting the existing (ip) will properly bump the
-        * nlinks count.  However, vp will incorrectly point at the old
-        * inode which has now been turned into a OBJTYPE_HARDLINK pointer.
-        *
-        * We must reconnect the vp.
+        * Create a directory entry connected to the specified chain.
+        * This function unlocks and NULL's chain on return.
         */
-       error = hammer2_inode_connect(dip, ip, name, name_len);
+       error = hammer2_inode_connect(dip, &chain, name, name_len);
        if (error == 0) {
                cache_setunresolved(ap->a_nch);
                cache_setvp(ap->a_nch, ap->a_vp);
@@ -2024,10 +1997,12 @@ hammer2_vop_nrename(struct vop_nrename_args *ap)
         * ip represents the actual file and not the hardlink marker.
         */
        ip = VTOI(fncp->nc_vp);
+       chain = NULL;
 
        /*
-        * Keep a tight grip on the inode as removing it should disconnect
-        * it and we don't want to destroy it.
+        * Keep a tight grip on the inode so the temporary unlinking from
+        * the source location prior to linking to the target location
+        * does not cause the chain to be destroyed.
         *
         * NOTE: To avoid deadlocks we cannot lock (ip) while we are
         *       unlinking elements from their directories.  Locking
@@ -2044,33 +2019,25 @@ hammer2_vop_nrename(struct vop_nrename_args *ap)
        cache_setunresolved(ap->a_tnch);
 
        /*
-        * Disconnect (fdip, fname) from the source directory.  This will
-        * disconnect (ip) if it represents a direct file.  If (ip) represents
-        * a hardlink the HARDLINK pointer object will be removed but the
-        * hardlink will stay intact.
+        * When renaming a hardlinked file we may have to re-consolidate
+        * the location of the hardlink target.  Since the element is simply
+        * being moved, nlinks is not modified in this case.
         *
-        * If (ip) is already hardlinked we have to resolve to a consolidated
-        * file but we do not bump the nlinks count.  (ip) must hold the nlinks
-        * lock & ref for the operation.  If the consolidated file has been
-        * relocated (ip) will be adjusted and the related nlinks lock moved
-        * along with it.
+        * If ip represents a regular file the consolidation code essentially
+        * does nothing other than return the locked chain.
         *
-        * If (ip) does not have multiple links we can just copy the physical
-        * contents of the inode.
+        * The returned chain will be locked.
         */
-       chain = hammer2_inode_lock_sh(ip);
-       hammer2_chain_ref(hmp, chain);          /* for unlink file */
-       if (chain->data->ipdata.nlinks > 1) {
-               hammer2_inode_unlock_sh(ip, chain);
-               error = hammer2_hardlink_consolidate(&ip, tdip);
-               if (error)
-                       goto done;
-       } else {
-               hammer2_inode_unlock_sh(ip, chain);
-       }
-       /* chain ref still intact */
+       error = hammer2_hardlink_consolidate(ip, &chain, tdip, 0);
+       if (error)
+               goto done;
 
        /*
+        * Disconnect (fdip, fname) from the source directory.  This will
+        * disconnect (ip) if it represents a direct file.  If (ip) represents
+        * a hardlink the HARDLINK pointer object will be removed but the
+        * hardlink will stay intact.
+        *
         * NOTE! Because we are retaining (ip) the unlink can fail with
         *       an EAGAIN.
         */
@@ -2081,7 +2048,6 @@ hammer2_vop_nrename(struct vop_nrename_args *ap)
                kprintf("hammer2_vop_nrename: unlink race %s\n", fname);
                tsleep(fdip, 0, "h2renr", 1);
        }
-       hammer2_chain_drop(hmp, chain); /* drop temporary ref */
        if (error)
                goto done;
 
@@ -2090,13 +2056,15 @@ hammer2_vop_nrename(struct vop_nrename_args *ap)
         *
         * WARNING: chain locks can lock buffer cache buffers, to avoid
         *          deadlocks we want to unlock before issuing a cache_*()
-        *          op (that might have to lock a vnode).
+        *          op (that might have to lock a vnode).  The *_connect()
+        *          function does this for us.
         */
-       error = hammer2_inode_connect(tdip, ip, tname, tname_len);
-       if (error == 0) {
+       error = hammer2_inode_connect(tdip, &chain, tname, tname_len);
+       if (error == 0)
                cache_rename(ap->a_fnch, ap->a_tnch);
-       }
 done:
+       if (chain)
+               hammer2_chain_unlock(hmp, chain);
        hammer2_inode_drop(ip);
 
        return (error);