hammer2 - More indirect block work & test script work

* Correct strategy read and bmap offset calculations to allow for arbitrary
  differences between logical and physical block sizes.  This fixes data
  read-back issues past the first 16K of data.

* Correct a panic related to the use of an unlocked chain in the lookup
  code when indirect blocks have to be recursed through.

* Add vop_getpages/vop_putpages -> stdgetpages/stdputpages.

* Minor reordering for correctness.

* Adjust test scripts to be a bit smarter, adjust debugging kprintf()s

diff --git a/sys/vfs/hammer2/dotest b/sys/vfs/hammer2/dotest
index cdbd70a..803de0c 100755 (executable)
--- a/sys/vfs/hammer2/dotest
+++ b/sys/vfs/hammer2/dotest
@@ -2,5 +2,10 @@
 #
 
 # ./mkvntest
-# kldload /usr/obj/usr/src/sys/vfs/hammer2/hammer2.ko
-mount_hammer2 /dev/vn0@ROOTx /mnt
+umount /mnt >& /dev/null
+kldunload hammer2.ko >& /dev/null
+kldstat | fgrep hammer2.ko >& /dev/null
+if ( $status > 0 ) then
+    kldload /usr/obj/usr/src/sys/vfs/hammer2/hammer2.ko
+endif
+mount_hammer2 /dev/vn0@ROOT /mnt

diff --git a/sys/vfs/hammer2/hammer2_chain.c b/sys/vfs/hammer2/hammer2_chain.c
index 431a1bf..1f3334b 100644 (file)
--- a/sys/vfs/hammer2/hammer2_chain.c
+++ b/sys/vfs/hammer2/hammer2_chain.c
@@ -520,7 +520,7 @@ hammer2_chain_get(hammer2_mount_t *hmp, hammer2_chain_t *parent,
        if (SPLAY_INSERT(hammer2_chain_splay, &parent->shead, chain))
                panic("hammer2_chain_link: collision");
        KKASSERT(parent->refs > 1);
-       atomic_add_int(&parent->refs, 1);
+       atomic_add_int(&parent->refs, 1);       /* for splay entry */
 
        /*
         * Additional linkage for inodes.  Reuse the parent pointer to
@@ -543,7 +543,6 @@ hammer2_chain_get(hammer2_mount_t *hmp, hammer2_chain_t *parent,
        if ((flags & HAMMER2_LOOKUP_NOLOCK) == 0)
                hammer2_chain_lock(hmp, chain);
        lockmgr(&chain->lk, LK_RELEASE);
-       /* still locked */
 
        return (chain);
 }
@@ -626,6 +625,8 @@ again:
                count = HAMMER2_SET_COUNT;
                break;
        case HAMMER2_BREF_TYPE_INDIRECT:
+               if (parent->data == NULL)
+                       panic("parent->data is NULL");
                base = &parent->data->npdata.blockref[0];
                count = HAMMER2_IND_COUNT;
                break;
@@ -649,15 +650,11 @@ again:
                bref = (tmp) ? &tmp->bref : &base[i];
                if (bref->type == 0)
                        continue;
-               kprintf("lookup(%016jx,%d) %d: %016jx/%d\n",
-                       parent->bref.data_off, i,
-                       bref->type,bref->key, bref->keybits);
                scan_beg = bref->key;
                scan_end = scan_beg + ((hammer2_key_t)1 << bref->keybits) - 1;
                if (key_beg <= scan_end && key_end >= scan_beg)
                        break;
        }
-       kprintf("lookup scan %d\n", i);
        if (i == count) {
                if (key_beg == key_end)
                        return (NULL);
@@ -675,11 +672,15 @@ again:
 
        /*
         * If the chain element is an indirect block it becomes the new
-        * parent and we loop on it.
+        * parent and we loop on it.  We must fixup the chain we loop on
+        * if the caller passed flags to us that aren't sufficient for our
+        * needs.
         */
        if (chain->bref.type == HAMMER2_BREF_TYPE_INDIRECT) {
                hammer2_chain_put(hmp, parent);
                *parentp = parent = chain;
+               if (flags & HAMMER2_LOOKUP_NOLOCK)
+                       hammer2_chain_lock(hmp, chain);
                goto again;
        }
 
@@ -742,10 +743,9 @@ again:
                nparent = parent->parent;
                hammer2_chain_ref(hmp, nparent);        /* ref new parent */
                hammer2_chain_unlock(hmp, parent);
-               parent = nparent;
-               hammer2_chain_lock(hmp, parent);        /* lock new parent */
-               hammer2_chain_drop(hmp, *parentp);      /* drop old parent */
-               *parentp = parent;                      /* new parent */
+               hammer2_chain_lock(hmp, nparent);       /* lock new parent */
+               hammer2_chain_drop(hmp, parent);        /* drop old parent */
+               *parentp = parent = nparent;
        }
 
 again2:
@@ -788,9 +788,11 @@ again2:
                        ++i;
                        continue;
                }
+#if 0
                kprintf("nextxx(%016jx,%d) %d: %016jx/%d\n",
                        parent->bref.data_off, i,
                        bref->type,bref->key, bref->keybits);
+#endif
                scan_beg = bref->key;
                scan_end = scan_beg + ((hammer2_key_t)1 << bref->keybits) - 1;
                if (key_beg <= scan_end && key_end >= scan_beg)
@@ -1059,8 +1061,6 @@ hammer2_chain_create_indirect(hammer2_mount_t *hmp, hammer2_chain_t *parent,
        int count;
        int i;
 
-       kprintf("create_indirect1\n");
-
        /*
         * Mark the parent modified so our base[] pointer remains valid
         * while we move entries.
@@ -1146,8 +1146,6 @@ hammer2_chain_create_indirect(hammer2_mount_t *hmp, hammer2_chain_t *parent,
                        ++hicount;
                else
                        ++locount;
-               kprintf("consolidate %016jx keybits %d lo %d hi %d\n",
-                       bref->key, keybits, locount, hicount);
        }
 
        /*
@@ -1203,7 +1201,6 @@ hammer2_chain_create_indirect(hammer2_mount_t *hmp, hammer2_chain_t *parent,
                                        ichain->index = i;
                                continue;
                        }
-                       kprintf("pre-move index %d\n", i);
                        bref = &chain->bref;
                }
 
@@ -1221,7 +1218,6 @@ hammer2_chain_create_indirect(hammer2_mount_t *hmp, hammer2_chain_t *parent,
                 * This element is being moved, its slot is available
                 * for our indirect block.
                 */
-               kprintf("move index %d\n", i);
                if (ichain->index < 0)
                        ichain->index = i;
                bzero(&base[i], sizeof(base[i]));
@@ -1233,8 +1229,14 @@ hammer2_chain_create_indirect(hammer2_mount_t *hmp, hammer2_chain_t *parent,
                 *
                 * Flagging the new chain entry MOVED will cause a flush
                 * to synchronize its block into the new indirect block.
-                * We must still set SUBMODIFIED but we do that after the
-                * loop.
+                * The chain is unlocked after being moved but needs to
+                * retain a reference for the MOVED state
+                *
+                * We must still set SUBMODIFIED in the parent but we do
+                * that after the loop.
+                *
+                * XXX we really need a lock here but we don't need the
+                *     data.  NODATA feature needed.
                 */
                chain = hammer2_chain_get(hmp, parent, i,
                                          HAMMER2_LOOKUP_NOLOCK);
@@ -1242,10 +1244,13 @@ hammer2_chain_create_indirect(hammer2_mount_t *hmp, hammer2_chain_t *parent,
                if (SPLAY_INSERT(hammer2_chain_splay, &ichain->shead, chain))
                        panic("hammer2_chain_create_indirect: collision");
                chain->parent = ichain;
-               atomic_set_int(&chain->flags, HAMMER2_CHAIN_MOVED);
                atomic_add_int(&parent->refs, -1);
                atomic_add_int(&ichain->refs, 1);
-               hammer2_chain_drop(hmp, chain);
+               if (chain->flags & HAMMER2_CHAIN_MOVED) {
+                       hammer2_chain_drop(hmp, chain);
+               } else {
+                       atomic_set_int(&chain->flags, HAMMER2_CHAIN_MOVED);
+               }
                KKASSERT(parent->refs > 0);
                chain = NULL;
        }
@@ -1397,8 +1402,11 @@ hammer2_chain_flush(hammer2_mount_t *hmp, hammer2_chain_t *chain,
                                } else {
                                        KKASSERT(scan->index < count);
                                        base[scan->index] = bref;
-                                       atomic_clear_int(&scan->flags,
+                                       if (scan->flags & HAMMER2_CHAIN_MOVED) {
+                                               atomic_clear_int(&scan->flags,
                                                         HAMMER2_CHAIN_MOVED);
+                                               hammer2_chain_drop(hmp, scan);
+                                       }
                                }
                                hammer2_chain_put(hmp, scan);
                        }

diff --git a/sys/vfs/hammer2/hammer2_vnops.c b/sys/vfs/hammer2/hammer2_vnops.c
index d578c7f..ed2076f 100644 (file)
--- a/sys/vfs/hammer2/hammer2_vnops.c
+++ b/sys/vfs/hammer2/hammer2_vnops.c
@@ -721,7 +721,8 @@ hammer2_vop_bmap(struct vop_bmap_args *ap)
        hammer2_inode_t *ip;
        hammer2_chain_t *parent;
        hammer2_chain_t *chain;
-       hammer2_key_t off_hi;
+       hammer2_key_t loff;
+       hammer2_off_t poff;
 
        /*
         * Only supported on regular files
@@ -737,15 +738,18 @@ hammer2_vop_bmap(struct vop_bmap_args *ap)
 
        ip = VTOI(vp);
        hmp = ip->hmp;
-       off_hi = ap->a_loffset & HAMMER2_OFF_MASK_HI;
-       KKASSERT((ap->a_loffset & HAMMER2_LBUFMASK64) == 0);
+
+       loff = ap->a_loffset;
+       KKASSERT((loff & HAMMER2_LBUFMASK64) == 0);
 
        parent = &ip->chain;
        hammer2_chain_ref(hmp, parent);
        hammer2_chain_lock(hmp, parent);
-       chain = hammer2_chain_lookup(hmp, &parent, off_hi, off_hi, 0);
+       chain = hammer2_chain_lookup(hmp, &parent, loff, loff, 0);
        if (chain) {
-               *ap->a_doffsetp = (chain->bref.data_off & ~HAMMER2_LBUFMASK64);
+               poff = loff - chain->bref.key +
+                      (chain->bref.data_off & HAMMER2_OFF_MASK);
+               *ap->a_doffsetp = poff;
                hammer2_chain_put(hmp, chain);
        } else {
                *ap->a_doffsetp = ZFOFFSET;     /* zero-fill hole */
@@ -854,7 +858,8 @@ hammer2_strategy_read(struct vop_strategy_args *ap)
        hammer2_inode_t *ip;
        hammer2_chain_t *parent;
        hammer2_chain_t *chain;
-       hammer2_key_t off_hi;
+       hammer2_key_t loff;
+       hammer2_off_t poff;
 
        bio = ap->a_bio;
        bp = bio->bio_buf;
@@ -863,8 +868,8 @@ hammer2_strategy_read(struct vop_strategy_args *ap)
        nbio = push_bio(bio);
 
        if (nbio->bio_offset == NOOFFSET) {
-               off_hi = bio->bio_offset & HAMMER2_OFF_MASK_HI;
-               KKASSERT((bio->bio_offset & HAMMER2_LBUFMASK64) == 0);
+               loff = bio->bio_offset;
+               KKASSERT((loff & HAMMER2_LBUFMASK64) == 0);
 
                parent = &ip->chain;
                hammer2_chain_ref(hmp, parent);
@@ -875,16 +880,14 @@ hammer2_strategy_read(struct vop_strategy_args *ap)
                 * chain element's content.  We just need the block device
                 * offset.
                 */
-               kprintf("lookup data logical %016jx\n", off_hi);
-               chain = hammer2_chain_lookup(hmp, &parent, off_hi, off_hi,
+               chain = hammer2_chain_lookup(hmp, &parent, loff, loff,
                                             HAMMER2_LOOKUP_NOLOCK);
                if (chain) {
-                       kprintf("lookup success\n");
-                       nbio->bio_offset = (chain->bref.data_off &
-                                           ~HAMMER2_LBUFMASK64);
+                       poff = loff - chain->bref.key +
+                              (chain->bref.data_off & HAMMER2_OFF_MASK);
+                       nbio->bio_offset = poff;
                        hammer2_chain_drop(hmp, chain);
                } else {
-                       kprintf("lookup zero-fill\n");
                        nbio->bio_offset = ZFOFFSET;
                }
                hammer2_chain_put(hmp, parent);
@@ -895,7 +898,6 @@ hammer2_strategy_read(struct vop_strategy_args *ap)
                vfs_bio_clrbuf(bp);
                biodone(nbio);
        } else {
-               kprintf("data read %016jx\n", nbio->bio_offset);
                vn_strategy(hmp->devvp, nbio);
        }
        return (0);
@@ -993,6 +995,8 @@ struct vop_ops hammer2_vnode_vops = {
        .vop_ncreate    = hammer2_vop_ncreate,
        .vop_getattr    = hammer2_vop_getattr,
        .vop_readdir    = hammer2_vop_readdir,
+       .vop_getpages   = vop_stdgetpages,
+       .vop_putpages   = vop_stdputpages,
        .vop_read       = hammer2_vop_read,
        .vop_write      = hammer2_vop_write,
        .vop_open       = hammer2_vop_open,