[PATCH 0/3] btrfs-progs: a couple fixes

[PATCH 0/3] btrfs-progs: a couple fixes

[Josef Bacik]

Hello,

These are a set of fixes that I needed for the ctree sync to work properly, and
one fix that was just broken in general.  They apply cleanly to devel and should
be straightforward enough.  Thanks,

Josef

Josef Bacik (3):
  btrfs-progs: take a ref in the root locking code
  btrfs-progs: clear root dirty when we update the root
  btrfs-progs: fix improper error handling in btrfs filesystem usage

 cmds/filesystem-usage.c     | 8 +++-----
 kernel-shared/locking.c     | 3 +++
 kernel-shared/transaction.c | 2 ++
 3 files changed, 8 insertions(+), 5 deletions(-)

-- 
2.41.0

[PATCH 1/3] btrfs-progs: take a ref in the root locking code

[Josef Bacik]

This code in the kernel not only returns the locked root, but also takes
a reference on the node.  This is important for when we sync ctree.c
into btrfs-progs, it expects that references are held on the root node
after calling these helpers.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 kernel-shared/locking.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel-shared/locking.c b/kernel-shared/locking.c
index a41ce06b..bd24571e 100644
--- a/kernel-shared/locking.c
+++ b/kernel-shared/locking.c
@@ -5,15 +5,18 @@
 
 struct extent_buffer *btrfs_read_lock_root_node(struct btrfs_root *root)
 {
+	root->node->refs++;
 	return root->node;
 }
 
 struct extent_buffer *btrfs_try_read_lock_root_node(struct btrfs_root *root)
 {
+	root->node->refs++;
 	return root->node;
 }
 
 struct extent_buffer *btrfs_lock_root_node(struct btrfs_root *root)
 {
+	root->node->refs++;
 	return root->node;
 }
-- 
2.41.0

[PATCH 2/3] btrfs-progs: clear root dirty when we update the root

[Josef Bacik]

We don't currently use the bit to track whether or not the root is
dirty, but when we sync ctree.c it uses this bit to determine if we
should add the root to the dirty list.  Clear this bit when we update
the root so that the dirty tracking works properly when we sync ctree.c.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 kernel-shared/transaction.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel-shared/transaction.c b/kernel-shared/transaction.c
index d30be5b5..49b435f6 100644
--- a/kernel-shared/transaction.c
+++ b/kernel-shared/transaction.c
@@ -14,6 +14,7 @@
  * Boston, MA 021110-1307, USA.
  */
 
+#include "kernel-lib/bitops.h"
 #include "kerncompat.h"
 #include "kernel-shared/disk-io.h"
 #include "kernel-shared/transaction.h"
@@ -119,6 +120,7 @@ int commit_tree_roots(struct btrfs_trans_handle *trans,
 		next = fs_info->dirty_cowonly_roots.next;
 		list_del_init(next);
 		root = list_entry(next, struct btrfs_root, dirty_list);
+		clear_bit(BTRFS_ROOT_DIRTY, &root->state);
 		ret = update_cowonly_root(trans, root);
 		free_extent_buffer(root->commit_root);
 		root->commit_root = NULL;
-- 
2.41.0

Re: [PATCH 2/3] btrfs-progs: clear root dirty when we update the root

[David Sterba]

On Wed, Aug 23, 2023 at 10:27:49AM -0400, Josef Bacik wrote:
> We don't currently use the bit to track whether or not the root is
> dirty, but when we sync ctree.c it uses this bit to determine if we
> should add the root to the dirty list.  Clear this bit when we update
> the root so that the dirty tracking works properly when we sync ctree.c.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>  kernel-shared/transaction.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/kernel-shared/transaction.c b/kernel-shared/transaction.c
> index d30be5b5..49b435f6 100644
> --- a/kernel-shared/transaction.c
> +++ b/kernel-shared/transaction.c
> @@ -14,6 +14,7 @@
>   * Boston, MA 021110-1307, USA.
>   */
>  
> +#include "kernel-lib/bitops.h"
>  #include "kerncompat.h"
>  #include "kernel-shared/disk-io.h"
>  #include "kernel-shared/transaction.h"

The includes in all files have been reorganized so that they're grouped,
the kerncompat.h is first, then system includes and libraries, then
kernel-lib, kernel-shared, common/, cmds/, the rest.

[PATCH 3/3] btrfs-progs: fix improper error handling in btrfs filesystem usage

[Josef Bacik]

I was seeing test-cli/016 failures because it claimed we were getting
EPERM from the TREE_SEARCH ioctl to get the chunk info out of the file
system.  This turned out to be because errno was already set going into
this function, the ioctl itself wasn't actually failing.  Fix this by
checking for a return value from the ioctl first, and then returning
-EPERM if appropriate.  This fixed the failures in my setup.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 cmds/filesystem-usage.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/cmds/filesystem-usage.c b/cmds/filesystem-usage.c
index 73b0ca35..f38051a2 100644
--- a/cmds/filesystem-usage.c
+++ b/cmds/filesystem-usage.c
@@ -145,7 +145,7 @@ static int load_chunk_info(int fd, struct chunk_info **chunkinfo_ret,
 	struct btrfs_ioctl_search_key *sk = &args.key;
 	struct btrfs_ioctl_search_header *sh;
 	unsigned long off = 0;
-	int i, e;
+	int i;
 
 	memset(&args, 0, sizeof(args));
 
@@ -168,11 +168,9 @@ static int load_chunk_info(int fd, struct chunk_info **chunkinfo_ret,
 
 	while (1) {
 		ret = ioctl(fd, BTRFS_IOC_TREE_SEARCH, &args);
-		e = errno;
-		if (e == EPERM)
-			return -e;
-
 		if (ret < 0) {
+			if (errno == EPERM)
+				return -errno;
 			error("cannot look up chunk tree info: %m");
 			return 1;
 		}
-- 
2.41.0

Re: [PATCH 0/3] btrfs-progs: a couple fixes

[David Sterba]

On Wed, Aug 23, 2023 at 10:27:47AM -0400, Josef Bacik wrote:
> Hello,
> 
> These are a set of fixes that I needed for the ctree sync to work properly, and
> one fix that was just broken in general.  They apply cleanly to devel and should
> be straightforward enough.  Thanks,
> 
> Josef
> 
> Josef Bacik (3):
>   btrfs-progs: take a ref in the root locking code
>   btrfs-progs: clear root dirty when we update the root
>   btrfs-progs: fix improper error handling in btrfs filesystem usage

Added to devel, thanks.