| Message ID | 20220721135006.3345302-3-nborisov@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Remove duplicate code in btrfs_prune_dentries/find_next_inode | expand |
On Thu, Jul 21, 2022 at 04:50:05PM +0300, Nikolay Borisov wrote: > Now that we have a standalone function which encapsulates the logic of > searching the root's ino rb tree use that. It results in massive > simplification of the code. > > Signed-off-by: Nikolay Borisov <nborisov@suse.com> > --- > fs/btrfs/inode.c | 47 +++++++++++++++++------------------------------ > 1 file changed, 17 insertions(+), 30 deletions(-) > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index c11169ba28b2..06724925a3d3 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -4635,44 +4635,27 @@ struct rb_node *btrfs_find_inode(struct btrfs_root *root, const u64 objectid) > static void btrfs_prune_dentries(struct btrfs_root *root) > { > struct btrfs_fs_info *fs_info = root->fs_info; > - struct rb_node *node; > - struct rb_node *prev; > - struct btrfs_inode *entry; > - struct inode *inode; > + struct rb_node *node = NULL; > u64 objectid = 0; > > if (!BTRFS_FS_ERROR(fs_info)) > WARN_ON(btrfs_root_refs(&root->root_item) != 0); > > spin_lock(&root->inode_lock); > -again: > - node = root->inode_tree.rb_node; > - prev = NULL; > - while (node) { > - prev = node; > - entry = rb_entry(node, struct btrfs_inode, rb_node); > + do { > + struct btrfs_inode *entry; > + struct inode *inode; > > - if (objectid < btrfs_ino(entry)) > - node = node->rb_left; > - else if (objectid > btrfs_ino(entry)) > - node = node->rb_right; > - else > - break; > - } > - if (!node) { > - while (prev) { > - entry = rb_entry(prev, struct btrfs_inode, rb_node); > - if (objectid <= btrfs_ino(entry)) { > - node = prev; > + if (!node) { > + node = btrfs_find_inode(root, objectid); > + if (!node) > break; > - } > - prev = rb_next(prev); > } > - } > - while (node) { > + > entry = rb_entry(node, struct btrfs_inode, rb_node); > objectid = btrfs_ino(entry) + 1; > inode = igrab(&entry->vfs_inode); > + > if (inode) { > spin_unlock(&root->inode_lock); > if (atomic_read(&inode->i_count) > 1) > @@ -4684,14 +4667,18 @@ static void btrfs_prune_dentries(struct btrfs_root *root) > iput(inode); > cond_resched(); > spin_lock(&root->inode_lock); > - goto again; > + node = NULL; This sets node to NULL and continues, which sends it down to while (node), which makes it effectively a break and it's not equivalent to the original behaviour that jumps back to again: or "do {" , or I'm missing something. > + continue; > } > > - if (cond_resched_lock(&root->inode_lock)) > - goto again; > + if (cond_resched_lock(&root->inode_lock)) { > + node = NULL; > + continue; > + } > > node = rb_next(node); > - } > + } while(node); > + > spin_unlock(&root->inode_lock); > } > > -- > 2.25.1
On 4.08.22 г. 18:41 ч., David Sterba wrote: > On Thu, Jul 21, 2022 at 04:50:05PM +0300, Nikolay Borisov wrote: >> Now that we have a standalone function which encapsulates the logic of >> searching the root's ino rb tree use that. It results in massive >> simplification of the code. >> >> Signed-off-by: Nikolay Borisov <nborisov@suse.com> >> --- >> fs/btrfs/inode.c | 47 +++++++++++++++++------------------------------ >> 1 file changed, 17 insertions(+), 30 deletions(-) >> >> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c >> index c11169ba28b2..06724925a3d3 100644 >> --- a/fs/btrfs/inode.c >> +++ b/fs/btrfs/inode.c >> @@ -4635,44 +4635,27 @@ struct rb_node *btrfs_find_inode(struct btrfs_root *root, const u64 objectid) >> static void btrfs_prune_dentries(struct btrfs_root *root) >> { >> struct btrfs_fs_info *fs_info = root->fs_info; >> - struct rb_node *node; >> - struct rb_node *prev; >> - struct btrfs_inode *entry; >> - struct inode *inode; >> + struct rb_node *node = NULL; >> u64 objectid = 0; >> >> if (!BTRFS_FS_ERROR(fs_info)) >> WARN_ON(btrfs_root_refs(&root->root_item) != 0); >> >> spin_lock(&root->inode_lock); >> -again: >> - node = root->inode_tree.rb_node; >> - prev = NULL; >> - while (node) { >> - prev = node; >> - entry = rb_entry(node, struct btrfs_inode, rb_node); >> + do { >> + struct btrfs_inode *entry; >> + struct inode *inode; >> >> - if (objectid < btrfs_ino(entry)) >> - node = node->rb_left; >> - else if (objectid > btrfs_ino(entry)) >> - node = node->rb_right; >> - else >> - break; >> - } >> - if (!node) { >> - while (prev) { >> - entry = rb_entry(prev, struct btrfs_inode, rb_node); >> - if (objectid <= btrfs_ino(entry)) { >> - node = prev; >> + if (!node) { >> + node = btrfs_find_inode(root, objectid); >> + if (!node) >> break; >> - } >> - prev = rb_next(prev); >> } >> - } >> - while (node) { >> + >> entry = rb_entry(node, struct btrfs_inode, rb_node); >> objectid = btrfs_ino(entry) + 1; >> inode = igrab(&entry->vfs_inode); >> + >> if (inode) { >> spin_unlock(&root->inode_lock); >> if (atomic_read(&inode->i_count) > 1) >> @@ -4684,14 +4667,18 @@ static void btrfs_prune_dentries(struct btrfs_root *root) >> iput(inode); >> cond_resched(); >> spin_lock(&root->inode_lock); >> - goto again; >> + node = NULL; > > This sets node to NULL and continues, which sends it down to while > (node), which makes it effectively a break and it's not equivalent to > the original behaviour that jumps back to again: or "do {" , or I'm > missing something. You are right, this is buggy, I'll rework it. > >> + continue; >> } >> >> - if (cond_resched_lock(&root->inode_lock)) >> - goto again; >> + if (cond_resched_lock(&root->inode_lock)) { >> + node = NULL; >> + continue; >> + } >> >> node = rb_next(node); >> - } >> + } while(node); >> + >> spin_unlock(&root->inode_lock); >> } >> >> -- >> 2.25.1
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index c11169ba28b2..06724925a3d3 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -4635,44 +4635,27 @@ struct rb_node *btrfs_find_inode(struct btrfs_root *root, const u64 objectid) static void btrfs_prune_dentries(struct btrfs_root *root) { struct btrfs_fs_info *fs_info = root->fs_info; - struct rb_node *node; - struct rb_node *prev; - struct btrfs_inode *entry; - struct inode *inode; + struct rb_node *node = NULL; u64 objectid = 0; if (!BTRFS_FS_ERROR(fs_info)) WARN_ON(btrfs_root_refs(&root->root_item) != 0); spin_lock(&root->inode_lock); -again: - node = root->inode_tree.rb_node; - prev = NULL; - while (node) { - prev = node; - entry = rb_entry(node, struct btrfs_inode, rb_node); + do { + struct btrfs_inode *entry; + struct inode *inode; - if (objectid < btrfs_ino(entry)) - node = node->rb_left; - else if (objectid > btrfs_ino(entry)) - node = node->rb_right; - else - break; - } - if (!node) { - while (prev) { - entry = rb_entry(prev, struct btrfs_inode, rb_node); - if (objectid <= btrfs_ino(entry)) { - node = prev; + if (!node) { + node = btrfs_find_inode(root, objectid); + if (!node) break; - } - prev = rb_next(prev); } - } - while (node) { + entry = rb_entry(node, struct btrfs_inode, rb_node); objectid = btrfs_ino(entry) + 1; inode = igrab(&entry->vfs_inode); + if (inode) { spin_unlock(&root->inode_lock); if (atomic_read(&inode->i_count) > 1) @@ -4684,14 +4667,18 @@ static void btrfs_prune_dentries(struct btrfs_root *root) iput(inode); cond_resched(); spin_lock(&root->inode_lock); - goto again; + node = NULL; + continue; } - if (cond_resched_lock(&root->inode_lock)) - goto again; + if (cond_resched_lock(&root->inode_lock)) { + node = NULL; + continue; + } node = rb_next(node); - } + } while(node); + spin_unlock(&root->inode_lock); }
Now that we have a standalone function which encapsulates the logic of searching the root's ino rb tree use that. It results in massive simplification of the code. Signed-off-by: Nikolay Borisov <nborisov@suse.com> --- fs/btrfs/inode.c | 47 +++++++++++++++++------------------------------ 1 file changed, 17 insertions(+), 30 deletions(-)