scoutfs: add inodes_since ioctl

Add the ioctl that let's us find out about inodes that have changed
since a given sequence number.

A sequence number is added to the btree items so that we can track the
tree update that it last changed in.  We update this as we modify
items and maintain it across item copying for splits and merges.

The big change is using the parent item ref and item sequence numbers
to guide iteration over items in the tree.  The easier change is to have
the current iteration skip over items whose sequence number is too old.

The more subtle change has to do with how iteration is terminated.  The
current termination could stop when it doesn't find an item because that
could only happen at the final leaf.  When we're ignoring items with old
seqs this can happen at the end of any leaf.  So we change iteration to
keep advancing through leaf blocks until it crosses the last key value.

We add an argument to btree walking which communicates the next key that
can be used to continue iterating from the next leaf block.  This works
for the normal walk case as well as the seq walking case where walking
terminates prematurely in an interior node full of parent items with old
seqs.

Now that we're more robustly advancing iteration with btree walk calls
and the next key we can get rid fo the 'next_leaf' hack which was trying
to do the same thing inside the btree walk code.  It wasn't right for
the seq walking case and was pretty fiddly.

The next_key increment could wrap the maximal key at the right spine of
the tree so we have _inc saturate instead of wrap.

And finally, we want these inode scans to not have to skip over all the
other items associated with each inode as it walks looking for inodes
with the given sequence number.  We change the item sort order to first
sort by type instead of by inode.  We've wanted this more generally to
isolate item types that have different access patterns.

Signed-off-by: Zach Brown <zab@versity.com>

kmod/src/btree.c

@@ -20,6 +20,7 @@
 #include "key.h"
 #include "treap.h"
 #include "btree.h"
+#include "trace.h"
 
 /*
  * scoutfs stores file system metadata in btrees whose items have fixed
@@ -48,6 +49,11 @@
  * updated if we insert an item with a key greater than everything in
  * the tree.
  *
+ * btree blocks, block references, and items all have sequence numbers
+ * that are set to the current dirty btree sequence number when they're
+ * modified.  This lets us efficiently search a range of keys for items
+ * that are newer than a given sequence number.
+ *
  * Operations are performed in one pass down the tree.  This lets us
  * cascade locks from the root down to the leaves and avoids having to
  * maintain a record of the path down the tree.  Splits and merges are
@@ -171,6 +177,7 @@ static struct scoutfs_btree_item *create_item(struct scoutfs_btree_block *bt,
 	le16_add_cpu(&bt->nr_items, 1);
 
 	item->key = *key;
+	item->seq = bt->hdr.seq;
 	item->val_len = cpu_to_le16(val_len);
 
 	scoutfs_treap_insert(&bt->treap, cmp_tnode_items, &item->tnode);
@@ -222,6 +229,7 @@ static void move_items(struct scoutfs_btree_block *dst,
 
 		to = create_item(dst, &from->key, val_len);
 		memcpy(to->val, from->val, val_len);
+		to->seq = from->seq;
 
 		del = from;
 		if (move_right)
@@ -527,51 +535,10 @@ enum {
 	WALK_INSERT = 1,
 	WALK_DELETE,
 	WALK_NEXT,
-	WALK_PREV,
+	WALK_NEXT_SEQ,
 	WALK_DIRTY,
 };
 
-/*
- * Usually we descend to a leaf that contains the key.  But if we're
- * searching for a next or previous item then we might hit a leaf block
- * that contains the key but no items in the direction of the search.
- * We might need to ascend and continue the search in the next block.
- *
- * The caller has just descended to a leaf and is asking us to discover
- * this case.  We set the parent item and return true to tell the caller
- * to read the new parent item's referenced block.
- *
- * XXX I don't think this can see bts with 0 items?  would need to verify?
- */
-static bool next_leaf(struct scoutfs_btree_block *parent,
-		     struct scoutfs_btree_item **par_item,
-		     struct scoutfs_btree_block *bt, int op, int level,
-		     struct scoutfs_key *key)
-{
-	struct scoutfs_btree_item *nei;
-
-	if (level > 0 || !parent || le16_to_cpu(parent->nr_items) < 2)
-		return false;
-
-	if (op == WALK_NEXT) {
-		nei = bt_next(parent, *par_item);
-		if (nei && (scoutfs_key_cmp(key, greatest_key(bt)) > 0)) {
-			*par_item = nei;
-			return true;
-		}
-	}
-
-	if (op == WALK_PREV) {
-		nei = bt_prev(parent, *par_item);
-		if (nei && (scoutfs_key_cmp(key, least_key(bt)) < 0)) {
-			*par_item = nei;
-			return true;
-		}
-	}
-
-	return false;
-}
-
 /*
  * As we descend we lock parent blocks (or the root), then lock the child,
  * then unlock the parent.
@@ -608,16 +575,72 @@ static void unlock_block(struct scoutfs_sb_info *sbi, struct scoutfs_block *bl,
 		up_read(rwsem);
 }
 
+static u64 item_block_ref_seq(struct scoutfs_btree_item *item)
+{
+	struct scoutfs_block_ref *ref = (void *)item->val;
+
+	return le64_to_cpu(ref->seq);
+}
+
+/*
+ * Return true if we should skip this item while iterating by sequence
+ * number.  If it's a parent then we test the block ref's seq, if it's a
+ * leaf item then we check the item's seq.
+ */
+static int item_skip_seq(struct scoutfs_btree_item *item,
+			 int level, u64 seq, int op)
+{
+	return op == WALK_NEXT_SEQ && item &&
+	       ((level > 0 && item_block_ref_seq(item) < seq) ||
+	        (level == 0 && le64_to_cpu(item->seq) < seq));
+}
+
+/*
+ * Return the next item, possibly skipping those with sequence numbers
+ * less than the desired sequence number.
+ */
+static struct scoutfs_btree_item *
+item_next_seq(struct scoutfs_btree_block *bt, struct scoutfs_btree_item *item,
+	      int level, u64 seq, int op)
+{
+	do {
+		item = bt_next(bt, item);
+	} while (item_skip_seq(item, level, seq, op));
+
+	return item;
+}
+
+/*
+ * Return the first item after the given key, possibly skipping those
+ * with sequence numbers less than the desired sequence number.
+ */
+static struct scoutfs_btree_item *
+item_after_seq(struct scoutfs_btree_block *bt, struct scoutfs_key *key,
+	       int level, u64 seq, int op)
+{
+	struct scoutfs_btree_item *item;
+
+	item = bt_after(bt, key);
+	if (item_skip_seq(item, level, seq, op))
+		item = item_next_seq(bt, item, level, seq, op);
+
+	return item;
+}
 
 /*
  * Return the leaf block that should contain the given key.  The caller
  * is responsible for searching the leaf block and performing their
- * operation.  The block is returned locked for either reading or writing
- * depending on the operation.
+ * operation.  The block is returned locked for either reading or
+ * writing depending on the operation.
+ *
+ * As we descend through parent items we set next_key to the first key
+ * in the next sibling's block.  This is used by iteration to advance to
+ * the next block when they're done with the block this returns.
  */
 static struct scoutfs_block *btree_walk(struct super_block *sb,
 					struct scoutfs_key *key,
-					unsigned int val_len, int op)
+					struct scoutfs_key *next_key,
+					unsigned int val_len, u64 seq, int op)
 {
 	struct scoutfs_sb_info *sbi = SCOUTFS_SB(sb);
 	struct scoutfs_btree_block *parent = NULL;
@@ -630,6 +653,13 @@ static struct scoutfs_block *btree_walk(struct super_block *sb,
 	const bool dirty = op == WALK_INSERT || op == WALK_DELETE ||
 			   op == WALK_DIRTY;
 
+	scoutfs_trace(sb, "key "CKF" level %llu seq %llu op %llu",
+		      CKA(key), val_len, seq, op);
+
+	/* no sibling blocks if we don't have parent blocks */
+	if (next_key)
+		scoutfs_set_max_key(next_key);
+
 	lock_block(sbi, par_bl, dirty);
 
 	/* XXX one for now */
@@ -649,6 +679,13 @@ static struct scoutfs_block *btree_walk(struct super_block *sb,
 		return bl;
 	}
 
+
+	/* skip the whole tree if the root ref's seq is old */
+	if (op == WALK_NEXT_SEQ && le64_to_cpu(ref->seq) < seq) {
+		unlock_block(sbi, par_bl, dirty);
+		return ERR_PTR(-ENOENT);
+	}
+
 	while (level--) {
 		/* XXX hmm, need to think about retry */
 		if (dirty) {
@@ -659,12 +696,14 @@ static struct scoutfs_block *btree_walk(struct super_block *sb,
 		if (IS_ERR(bl))
 			break;
 
-		/* see if a search needs to move to the next parent ref */
-		if (next_leaf(parent, &item, bl->data, op, level, key)) {
-			ref = (void *)item->val;
-			level++;
-			scoutfs_put_block(bl);
-			continue;
+		/*
+		 * Update the next key an iterator should read from.
+		 * Keep in mind that iteration is read only so the
+		 * parent item won't be changed splitting or merging.
+		 */
+		if (parent && next_key) {
+			*next_key = item->key;
+			scoutfs_inc_key(next_key);
 		}
 
 		if (op == WALK_INSERT)
@@ -686,12 +725,20 @@ static struct scoutfs_block *btree_walk(struct super_block *sb,
 		par_bl = bl;
 		parent = par_bl->data;
 
-		/* there should always be a parent item */
-		item = bt_after(parent, key);
+		/*
+		 * Find the parent item that references the next child
+		 * block to search.  If we're skipping items with old
+		 * seqs then we might not have any child items to
+		 * search.
+		 */
+		item = item_after_seq(parent, key, level, seq, op);
 		if (!item) {
 			/* current block dropped as parent below */
-			bl = ERR_PTR(-EIO);
-			break;
+			if (op == WALK_NEXT_SEQ) {
+				bl = ERR_PTR(-ENOENT);
+			} else {
+				bl = ERR_PTR(-EIO);
+			} break;
 		}
 
 		/* XXX verify sane length */
@@ -711,6 +758,7 @@ static void set_cursor(struct scoutfs_btree_cursor *curs,
 	curs->bl = bl;
 	curs->item = item;
 	curs->key = &item->key;
+	curs->seq = le64_to_cpu(item->seq);
 	curs->val = item->val;
 	curs->val_len = le16_to_cpu(item->val_len);
 	curs->write = !!write;
@@ -729,7 +777,7 @@ int scoutfs_btree_lookup(struct super_block *sb, struct scoutfs_key *key,
 
 	BUG_ON(curs->bl);
 
-	bl = btree_walk(sb, key, 0, 0);
+	bl = btree_walk(sb, key, NULL, 0, 0, 0);
 	if (IS_ERR(bl))
 		return PTR_ERR(bl);
 
@@ -765,7 +813,7 @@ int scoutfs_btree_insert(struct super_block *sb, struct scoutfs_key *key,
 
 	BUG_ON(curs->bl);
 
-	bl = btree_walk(sb, key, val_len, WALK_INSERT);
+	bl = btree_walk(sb, key, NULL, val_len, 0, WALK_INSERT);
 	if (IS_ERR(bl))
 		return PTR_ERR(bl);
 	bt = bl->data;
@@ -797,7 +845,7 @@ int scoutfs_btree_delete(struct super_block *sb, struct scoutfs_key *key)
 	struct scoutfs_block *bl;
 	int ret;
 
-	bl = btree_walk(sb, key, 0, WALK_DELETE);
+	bl = btree_walk(sb, key, NULL, 0, 0, WALK_DELETE);
 	if (IS_ERR(bl))
 		return PTR_ERR(bl);
 	bt = bl->data;
@@ -826,57 +874,84 @@ int scoutfs_btree_delete(struct super_block *sb, struct scoutfs_key *key)
 }
 
 /*
- * The caller initializes the cursor and first and last keys and then
- * gets the cursor set to each item within those keys.
+ * Iterate over items in the tree starting with first and ending with
+ * last.  We point the cursor at each item and return to the caller.
+ * The caller continues the search with the cursor.
  *
- * The btree walk takes care of advancing past interior leaves that
- * don't contain items past the key.  Our job is to find the next item
- * after the key.  If that next item's key is past the caller's last key
- * then the iteration is done.
+ * The caller can limit results to items with a sequence number greater
+ * than or equal to their sequence number.
  *
- * returns 0 if no next, > 0 when curs contains next, < 0 on error
+ * When there isn't an item in the cursor then we walk the btree to the
+ * leaf that should contain the key and look for items from there.  When
+ * we exhaust leaves we search the tree again from the next key that was
+ * increased past the leaf's parent's item.
+ *
+ * Returns > 0 when the cursor has an item, 0 when done, and -errno on error.
  */
-int scoutfs_btree_next(struct super_block *sb, struct scoutfs_key *first,
-		       struct scoutfs_key *last,
-		       struct scoutfs_btree_cursor *curs)
+static int btree_next(struct super_block *sb, struct scoutfs_key *first,
+		      struct scoutfs_key *last, u64 seq, int op,
+		      struct scoutfs_btree_cursor *curs)
 {
 	struct scoutfs_btree_block *bt;
 	struct scoutfs_block *bl;
 	struct scoutfs_key key = *first;
+	struct scoutfs_key next_key;
 	int ret;
 
-	trace_printk("first "CKF" last "CKF" %d curs "CKF"\n",
-		CKA(first), CKA(last),
-		!!curs->bl, CKA(curs->bl ? curs->key : &key));
+	scoutfs_trace(sb, "first "CKF" last "CKF" seq %llu op %llu curs "CKF,
+		      CKA(first), CKA(last), seq, op,
+		      CKA(curs->bl ? curs->key : first));
+
+	if (scoutfs_key_cmp(first, last) > 0)
+		return 0;
 
 	/* find the next item after the cursor, releasing if we're done */
 	if (curs->bl) {
 		key = curs->item->key;
 		scoutfs_inc_key(&key);
 
-		curs->item = bt_next(curs->bl->data, curs->item);
-		trace_printk("next %p\n", curs->item);
+		curs->item = item_next_seq(curs->bl->data, curs->item,
+					   0, seq, op);
 		if (curs->item)
 			set_cursor(curs, curs->bl, curs->item, curs->write);
 		else
 			scoutfs_btree_release(curs);
 	}
 
-	/* walk the tree to find the key, can be first or later */
-	if (!curs->bl) {
-		bl = btree_walk(sb, &key, 0, WALK_NEXT);
-		if (IS_ERR(bl))
+	/* find the leaf that contains the next item after the key */
+	while (!curs->bl && scoutfs_key_cmp(&key, last) <= 0) {
+
+		bl = btree_walk(sb, &key, &next_key, 0, seq, op);
+
+		/* next seq walks can terminate in parents with old seqs */
+		if (op == WALK_NEXT_SEQ && bl == ERR_PTR(-ENOENT)) {
+			key = next_key;
+			continue;
+		}
+
+		if (IS_ERR(bl)) {
+			if (bl == ERR_PTR(-ENOENT))
+				break;
 			return PTR_ERR(bl);
+		}
 		bt = bl->data;
 
-		curs->item = bt_after(bl->data, &key);
-		trace_printk("after %p\n", curs->item);
+		/* keep trying leaves until next_key passes last */
+		curs->item = item_after_seq(bl->data, &key, 0, seq, op);
+		if (!curs->item) {
+			key = next_key;
+			up_read(&bl->rwsem);
+			scoutfs_put_block(bl);
+			continue;
+		}
+
 		if (curs->item) {
 			set_cursor(curs, bl, curs->item, false);
 		} else {
 			up_read(&bl->rwsem);
 			scoutfs_put_block(bl);
 		}
+		break;
 	}
 
 	/* only return the next item if it's within last */
@@ -887,10 +962,23 @@ int scoutfs_btree_next(struct super_block *sb, struct scoutfs_key *first,
 		ret = 0;
 	}
 
-	trace_printk("ret %d\n", ret);
 	return ret;
 }
 
+int scoutfs_btree_next(struct super_block *sb, struct scoutfs_key *first,
+		       struct scoutfs_key *last,
+		       struct scoutfs_btree_cursor *curs)
+{
+	return btree_next(sb, first, last, 0, WALK_NEXT, curs);
+}
+
+int scoutfs_btree_since(struct super_block *sb, struct scoutfs_key *first,
+		        struct scoutfs_key *last, u64 seq,
+		        struct scoutfs_btree_cursor *curs)
+{
+	return btree_next(sb, first, last, seq, WALK_NEXT_SEQ, curs);
+}
+
 /*
  * Ensure that the blocks that lead to the item with the given key are
  * dirty.  caller can hold a transaction to pin the dirty blocks and
@@ -904,7 +992,7 @@ int scoutfs_btree_dirty(struct super_block *sb, struct scoutfs_key *key)
 	struct scoutfs_block *bl;
 	int ret;
 
-	bl = btree_walk(sb, key, 0, WALK_DIRTY);
+	bl = btree_walk(sb, key, NULL, 0, 0, WALK_DIRTY);
 	if (IS_ERR(bl))
 		return PTR_ERR(bl);
 
@@ -929,16 +1017,19 @@ void scoutfs_btree_update(struct super_block *sb, struct scoutfs_key *key,
 			  struct scoutfs_btree_cursor *curs)
 {
 	struct scoutfs_btree_item *item;
+	struct scoutfs_btree_block *bt;
 	struct scoutfs_block *bl;
 
 	BUG_ON(curs->bl);
 
-	bl = btree_walk(sb, key, 0, WALK_DIRTY);
+	bl = btree_walk(sb, key, NULL, 0, 0, WALK_DIRTY);
 	BUG_ON(IS_ERR(bl));
 
 	item = bt_lookup(bl->data, key);
 	BUG_ON(!item);
 
+	bt = bl->data;
+	item->seq = bt->hdr.seq;
 	set_cursor(curs, bl, item, true);
 }
 

kmod/src/btree.h

@@ -8,6 +8,7 @@ struct scoutfs_btree_cursor {
 
 	/* for callers */
 	struct scoutfs_key *key;
+	u64 seq;
 	void *val;
 	u16 val_len;
 	u16 write:1;
@@ -30,6 +31,9 @@ void scoutfs_btree_update(struct super_block *sb, struct scoutfs_key *key,
                           struct scoutfs_btree_cursor *curs);
 int scoutfs_btree_hole(struct super_block *sb, struct scoutfs_key *first,
 		       struct scoutfs_key *last, struct scoutfs_key *hole);
+int scoutfs_btree_since(struct super_block *sb, struct scoutfs_key *first,
+		        struct scoutfs_key *last, u64 seq,
+		        struct scoutfs_btree_cursor *curs);
 
 void scoutfs_btree_release(struct scoutfs_btree_cursor *curs);
 

kmod/src/filerw.c

@@ -21,6 +21,7 @@
 #include "trans.h"
 #include "scoutfs_trace.h"
 #include "btree.h"
+#include "ioctl.h"
 
 /*
  * File data is stored in items just like everything else.  This is very
@@ -246,4 +247,5 @@ const struct file_operations scoutfs_file_fops = {
 	.write		= do_sync_write,
 	.aio_read	= generic_file_aio_read,
 	.aio_write	= generic_file_aio_write,
+	.unlocked_ioctl	= scoutfs_ioctl,
 };

kmod/src/format.h

@@ -91,9 +91,14 @@ struct scoutfs_btree_block {
 	__le16 nr_items;
 } __packed;
 
+/*
+ * The item sequence number is set to the dirty block's sequence number
+ * when the item is modified.  It is not changed by splits or merges.
+ */
 struct scoutfs_btree_item {
 	struct scoutfs_key key;
 	struct scoutfs_treap_node tnode;
+	__le64 seq;
 	__le16 val_len;
 	char val[0];
 } __packed;

kmod/src/ioctl.c

@@ -16,6 +16,9 @@
 #include <linux/compiler.h>
 #include <linux/uio.h>
 
+#include "format.h"
+#include "btree.h"
+#include "key.h"
 #include "ioctl.h"
 #include "trace.h"
 
@@ -44,3 +47,85 @@ int scoutfs_copy_ibuf(struct iovec *iov, unsigned long arg)
 
 	return 0;
 }
+
+/*
+ * Find all the inodes in the given inode range that have changed since
+ * the given tree update sequence number.
+ *
+ * The inodes are returned in inode order, not sequence order.
+ */
+static long scoutfs_ioc_inodes_since(struct file *file, unsigned long arg)
+{
+	struct super_block *sb = file_inode(file)->i_sb;
+	struct scoutfs_ioctl_inodes_since __user *uargs = (void __user *)arg;
+	struct scoutfs_ioctl_inodes_since args;
+	struct scoutfs_ioctl_ino_seq __user *uiseq;
+	struct scoutfs_ioctl_ino_seq iseq;
+	struct scoutfs_key first;
+	struct scoutfs_key last;
+	DECLARE_SCOUTFS_BTREE_CURSOR(curs);
+	long bytes;
+	int ret;
+
+	if (copy_from_user(&args, uargs, sizeof(args)))
+		return -EFAULT;
+
+	uiseq = (void __user *)(unsigned long)args.results.ptr;
+	if (args.results.len < sizeof(iseq))
+		return -EINVAL;
+
+	scoutfs_set_key(&first, args.first_ino, SCOUTFS_INODE_KEY, 0);
+	scoutfs_set_key(&last, args.last_ino, SCOUTFS_INODE_KEY, 0);
+
+	bytes = 0;
+	while ((ret = scoutfs_btree_since(sb, &first, &last,
+					  args.seq, &curs)) > 0) {
+
+		iseq.ino = scoutfs_key_inode(curs.key);
+		iseq.seq = curs.seq;
+
+		/*
+		 * We can't copy to userspace with our locks held
+		 * because faults could try to use tree blocks that we
+		 * have locked.  If a non-faulting copy fails we release
+		 * the cursor and try a blocking copy and pick up where
+		 * we left off.
+		 */
+		pagefault_disable();
+		ret = __copy_to_user_inatomic(uiseq, &iseq, sizeof(iseq));
+		pagefault_enable();
+		if (ret) {
+			first = *curs.key;
+			scoutfs_inc_key(&first);
+			scoutfs_btree_release(&curs);
+			if (copy_to_user(uiseq, &iseq, sizeof(iseq))) {
+				ret = -EFAULT;
+				break;
+			}
+		}
+
+		uiseq++;
+		bytes += sizeof(iseq);
+		if (bytes + sizeof(iseq) > args.results.len) {
+			ret = 0;
+			break;
+		}
+	}
+
+	scoutfs_btree_release(&curs);
+
+	if (bytes)
+		ret = bytes;
+
+	return ret;
+}
+
+long scoutfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+	switch (cmd) {
+	case SCOUTFS_IOC_INODES_SINCE:
+		return scoutfs_ioc_inodes_since(file, arg);
+	}
+
+	return -ENOTTY;
+}

kmod/src/ioctl.h

@@ -1,6 +1,9 @@
 #ifndef _SCOUTFS_IOCTL_H_
 #define _SCOUTFS_IOCTL_H_
 
+int scoutfs_copy_ibuf(struct iovec *iov, unsigned long arg);
+long scoutfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
+
 /* XXX I have no idea how these are chosen. */
 #define SCOUTFS_IOCTL_MAGIC 's'
 
@@ -27,6 +30,23 @@ struct scoutfs_trace_record {
 #define SCOUTFS_IOC_GET_TRACE_RECORDS _IOW(SCOUTFS_IOCTL_MAGIC, 2, \
 					   struct scoutfs_ioctl_buf)
 
-int scoutfs_copy_ibuf(struct iovec *iov, unsigned long arg);
+struct scoutfs_ioctl_ino_seq {
+	__u64 ino;
+	__u64 seq;
+} __packed;
+
+struct scoutfs_ioctl_inodes_since {
+	__u64 first_ino;
+	__u64 last_ino;
+	__u64 seq;
+	struct scoutfs_ioctl_buf results;
+} __packed;
+
+/*
+ * Adds entries to the user's buffer for each inode whose sequence
+ * number is greater than or equal to the given seq.
+ */
+#define SCOUTFS_IOC_INODES_SINCE _IOW(SCOUTFS_IOCTL_MAGIC, 3, \
+				      struct scoutfs_ioctl_inodes_since)
 
 #endif

kmod/src/key.h

@@ -4,7 +4,7 @@
 #include <linux/types.h>
 #include "format.h"
 
-#define CKF "%llu.%u.%llu"
+#define CKF "%llu.%llu.%llu"
 #define CKA(key) \
 	le64_to_cpu((key)->inode), (key)->type, le64_to_cpu((key)->offset)
 
@@ -24,10 +24,17 @@ static inline int le64_cmp(__le64 a, __le64 b)
 	       le64_to_cpu(a) > le64_to_cpu(b) ? 1 : 0;
 }
 
+/*
+ * Items are sorted by type and then by inode to reflect the relative
+ * frequency of use.  Inodes and xattrs are hot, then dirents, then file
+ * data extents.  We want each use class to be hot and dense, we don't
+ * want a scan of the inodes to have to skip over each inode's extent
+ * items.
+ */
 static inline int scoutfs_key_cmp(struct scoutfs_key *a, struct scoutfs_key *b)
 {
-	return le64_cmp(a->inode, b->inode) ?:
-	       ((short)a->type - (short)b->type) ?: 
+	return ((short)a->type - (short)b->type) ?:
+	       le64_cmp(a->inode, b->inode) ?:
 	       le64_cmp(a->offset, b->offset);
 }
 
@@ -67,8 +74,19 @@ static inline void scoutfs_set_max_key(struct scoutfs_key *key)
 	scoutfs_set_key(key, ~0ULL, ~0, ~0ULL);
 }
 
+/*
+ * This saturates at (~0,~0,~0) instead of wrapping.  This will never be
+ * an issue for real item keys but parent item keys along the right
+ * spine of the tree have maximal key values that could wrap if
+ * incremented.
+ */
 static inline void scoutfs_inc_key(struct scoutfs_key *key)
 {
+	if (key->inode == cpu_to_le64(~0ULL) &&
+	    key->type == (u8)~0 &&
+	    key->offset == cpu_to_le64(~0ULL))
+		return;
+
 	le64_add_cpu(&key->offset, 1);
 	if (!key->offset) {
 		if (++key->type == 0)