mirror of
https://github.com/versity/scoutfs.git
synced 2026-09-20 23:14:23 +00:00
scoutfs: remove btree cursor
The btree cursor was built to address two problems. First it accelerates iteration by avoiding full descents down the tree by holding on to leaf blocks. Second it lets callers reference item value contents directly to avoid copies. But it also has serious complexity costs. It pushes refcounting and locking out to the caller. There have already been a few bugs where callers did things while holding the cursor without realizing that they're holding a btree lock and can't perform certain btree operations or even copies to user space. Future changes to the allocator to use the btree motivates cleaning up the tree locking which is complicated by the cursor being a stand alone lock reference. Instead of continuing to layer complexity onto this construct let's remove it. The iteration acceleration will be addressed the same way we're going to accelerate the other btree operations: with per-cpu cached leaf block references. Unlike the cursor this doesn't push interface changes out to callers who want repeated btree calls to perform well. We'll leave the value copying for now. If it becomes an issue we can add variants that call a function to operate on the value. Let's hope we don't have to go there. This change replaces the cursor with a vector to memory that the value should be copied to and from. The vector has a fixed number of elements and is wrapped in a struct for easy declaration and initialization. This change to the interface looks noisy but each caller's change is pretty mechanical. They tend to involve: - replace the cursor with the value struct and initialization - allocate some memory to copy the value in to - reading functions return the number of value bytes copied - verify copied bytes makes sense for item being read - getting rid of confusing ((ret = _next())) looping - _next now returns -ENOENT instead of 0 for no next item - _next iterators now need to increase the key themselves - make sure to free allocated mem Sometimes the order of operations changes significantly. Now that we can't modify in place we need to read, modify, write. This looks like changing a modification of the item through the cursor to a lookup/update pattern. The symlink item iterators didn't need to use next because they walk a contiguous set of keys. They're changed to use simple insert or lookup. Signed-off-by: Zach Brown <zab@versity.com>
This commit is contained in:
+34
-22
@@ -125,21 +125,25 @@ static void load_inode(struct inode *inode, struct scoutfs_inode *cinode)
|
||||
|
||||
static int scoutfs_read_locked_inode(struct inode *inode)
|
||||
{
|
||||
DECLARE_SCOUTFS_BTREE_CURSOR(curs);
|
||||
struct super_block *sb = inode->i_sb;
|
||||
struct scoutfs_btree_root *meta = SCOUTFS_META(sb);
|
||||
struct scoutfs_btree_val val;
|
||||
struct scoutfs_inode sinode;
|
||||
struct scoutfs_key key;
|
||||
int ret;
|
||||
|
||||
scoutfs_set_key(&key, scoutfs_ino(inode), SCOUTFS_INODE_KEY, 0);
|
||||
scoutfs_btree_init_val(&val, &sinode, sizeof(sinode));
|
||||
|
||||
ret = scoutfs_btree_lookup(sb, meta, &key, &curs);
|
||||
if (!ret) {
|
||||
load_inode(inode, curs.val);
|
||||
scoutfs_btree_release(&curs);
|
||||
ret = scoutfs_btree_lookup(sb, meta, &key, &val);
|
||||
if (ret == sizeof(sinode)) {
|
||||
load_inode(inode, &sinode);
|
||||
ret = 0;
|
||||
} else if (ret >= 0) {
|
||||
ret = -EIO;
|
||||
}
|
||||
|
||||
return 0;
|
||||
return ret;
|
||||
}
|
||||
|
||||
static int scoutfs_iget_test(struct inode *inode, void *arg)
|
||||
@@ -252,19 +256,20 @@ int scoutfs_dirty_inode_item(struct inode *inode)
|
||||
*/
|
||||
void scoutfs_update_inode_item(struct inode *inode)
|
||||
{
|
||||
DECLARE_SCOUTFS_BTREE_CURSOR(curs);
|
||||
struct super_block *sb = inode->i_sb;
|
||||
struct scoutfs_btree_root *meta = SCOUTFS_META(sb);
|
||||
struct scoutfs_btree_val val;
|
||||
struct scoutfs_inode sinode;
|
||||
struct scoutfs_key key;
|
||||
int err;
|
||||
|
||||
scoutfs_set_key(&key, scoutfs_ino(inode), SCOUTFS_INODE_KEY, 0);
|
||||
scoutfs_btree_init_val(&val, &sinode, sizeof(sinode));
|
||||
store_inode(&sinode, inode);
|
||||
|
||||
err = scoutfs_btree_update(sb, meta, &key, &curs);
|
||||
err = scoutfs_btree_update(sb, meta, &key, &val);
|
||||
BUG_ON(err);
|
||||
|
||||
store_inode(curs.val, inode);
|
||||
scoutfs_btree_release(&curs);
|
||||
trace_scoutfs_update_inode(inode);
|
||||
}
|
||||
|
||||
@@ -313,8 +318,9 @@ struct inode *scoutfs_new_inode(struct super_block *sb, struct inode *dir,
|
||||
umode_t mode, dev_t rdev)
|
||||
{
|
||||
struct scoutfs_btree_root *meta = SCOUTFS_META(sb);
|
||||
DECLARE_SCOUTFS_BTREE_CURSOR(curs);
|
||||
struct scoutfs_inode_info *ci;
|
||||
struct scoutfs_btree_val val;
|
||||
struct scoutfs_inode sinode;
|
||||
struct scoutfs_key key;
|
||||
struct inode *inode;
|
||||
u64 ino;
|
||||
@@ -341,15 +347,15 @@ struct inode *scoutfs_new_inode(struct super_block *sb, struct inode *dir,
|
||||
set_inode_ops(inode);
|
||||
|
||||
scoutfs_set_key(&key, scoutfs_ino(inode), SCOUTFS_INODE_KEY, 0);
|
||||
scoutfs_btree_init_val(&val, &sinode, sizeof(sinode));
|
||||
store_inode(&sinode, inode);
|
||||
|
||||
ret = scoutfs_btree_insert(inode->i_sb, meta, &key,
|
||||
sizeof(struct scoutfs_inode), &curs);
|
||||
ret = scoutfs_btree_insert(inode->i_sb, meta, &key, &val);
|
||||
if (ret) {
|
||||
iput(inode);
|
||||
return ERR_PTR(ret);
|
||||
}
|
||||
|
||||
scoutfs_btree_release(&curs);
|
||||
return inode;
|
||||
}
|
||||
|
||||
@@ -359,22 +365,28 @@ struct inode *scoutfs_new_inode(struct super_block *sb, struct inode *dir,
|
||||
static void drop_inode_items(struct super_block *sb, u64 ino)
|
||||
{
|
||||
struct scoutfs_btree_root *meta = SCOUTFS_META(sb);
|
||||
DECLARE_SCOUTFS_BTREE_CURSOR(curs);
|
||||
struct scoutfs_inode *sinode;
|
||||
struct scoutfs_btree_val val;
|
||||
struct scoutfs_inode sinode;
|
||||
struct scoutfs_key key;
|
||||
bool release = false;
|
||||
umode_t mode;
|
||||
int ret;
|
||||
|
||||
/* sample the inode mode */
|
||||
/* sample the inode mode, XXX don't need to copy whole thing here */
|
||||
scoutfs_set_key(&key, ino, SCOUTFS_INODE_KEY, 0);
|
||||
ret = scoutfs_btree_lookup(sb, meta, &key, &curs);
|
||||
if (ret)
|
||||
scoutfs_btree_init_val(&val, &sinode, sizeof(sinode));
|
||||
|
||||
ret = scoutfs_btree_lookup(sb, meta, &key, &val);
|
||||
if (ret < 0)
|
||||
goto out;
|
||||
|
||||
sinode = curs.val;
|
||||
mode = le32_to_cpu(sinode->mode);
|
||||
scoutfs_btree_release(&curs);
|
||||
/* XXX corruption */
|
||||
if (ret != sizeof(sinode)) {
|
||||
ret = -EIO;
|
||||
goto out;
|
||||
}
|
||||
|
||||
mode = le32_to_cpu(sinode.mode);
|
||||
|
||||
ret = scoutfs_hold_trans(sb);
|
||||
if (ret)
|
||||
|
||||
Reference in New Issue
Block a user