[PATCH v14.7 09/14] btrfs: dedupe: Implement btrfs_dedupe_calc_hash interface

[Lu Fengqi]

From: Wang Xiaoguang 

Unlike in-memory or on-disk dedupe method, only SHA256 hash method is
supported yet, so implement btrfs_dedupe_calc_hash() interface using
SHA256.

Signed-off-by: Qu Wenruo 
Signed-off-by: Wang Xiaoguang 
Reviewed-by: Josef Bacik 
Signed-off-by: Lu Fengqi 
---
v14.7: replace SHASH_DESC_ON_STACK with kmalloc to remove VLA.

 fs/btrfs/dedupe.c | 50 +++
 1 file changed, 50 insertions(+)

diff --git a/fs/btrfs/dedupe.c b/fs/btrfs/dedupe.c
index 550080523bbb..67ba0362d8c3 100644
--- a/fs/btrfs/dedupe.c
+++ b/fs/btrfs/dedupe.c
@@ -632,3 +632,53 @@ int btrfs_dedupe_search(struct btrfs_fs_info *fs_info,
}
return ret;
 }
+
+int btrfs_dedupe_calc_hash(struct btrfs_fs_info *fs_info,
+  struct inode *inode, u64 start,
+  struct btrfs_dedupe_hash *hash)
+{
+   int i;
+   int ret;
+   struct page *p;
+   struct shash_desc *shash;
+   struct btrfs_dedupe_info *dedupe_info = fs_info->dedupe_info;
+   struct crypto_shash *tfm = dedupe_info->dedupe_driver;
+   u64 dedupe_bs;
+   u64 sectorsize = fs_info->sectorsize;
+
+   shash = kmalloc(sizeof(*shash) + crypto_shash_descsize(tfm), GFP_NOFS);
+   if (!shash)
+   return -ENOMEM;
+
+   if (!fs_info->dedupe_enabled || !hash)
+   return 0;
+
+   if (WARN_ON(dedupe_info == NULL))
+   return -EINVAL;
+
+   WARN_ON(!IS_ALIGNED(start, sectorsize));
+
+   dedupe_bs = dedupe_info->blocksize;
+
+   shash->tfm = tfm;
+   shash->flags = 0;
+   ret = crypto_shash_init(shash);
+   if (ret)
+   return ret;
+   for (i = 0; sectorsize * i < dedupe_bs; i++) {
+   char *d;
+
+   p = find_get_page(inode->i_mapping,
+ (start >> PAGE_SHIFT) + i);
+   if (WARN_ON(!p))
+   return -ENOENT;
+   d = kmap(p);
+   ret = crypto_shash_update(shash, d, sectorsize);
+   kunmap(p);
+   put_page(p);
+   if (ret)
+   return ret;
+   }
+   ret = crypto_shash_final(shash, hash->hash);
+   return ret;
+}
-- 
2.17.0



--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v14.7 04/14] btrfs: dedupe: Introduce function to initialize dedupe info

[Lu Fengqi]

From: Wang Xiaoguang 

Add generic function to initialize dedupe info.

Signed-off-by: Qu Wenruo 
Signed-off-by: Wang Xiaoguang 
Reviewed-by: Josef Bacik 
Signed-off-by: Lu Fengqi 
---
v14.7: fixed the following errors by switching to div_u64.
├── arm-allmodconfig
│   └── ERROR:__aeabi_uldivmod-fs-btrfs-btrfs.ko-undefined
└── i386-allmodconfig
    └── ERROR:__udivdi3-fs-btrfs-btrfs.ko-undefined

 fs/btrfs/Makefile  |   2 +-
 fs/btrfs/dedupe.c  | 174 +
 fs/btrfs/dedupe.h  |  13 ++-
 include/uapi/linux/btrfs.h |   4 +-
 4 files changed, 189 insertions(+), 4 deletions(-)
 create mode 100644 fs/btrfs/dedupe.c

diff --git a/fs/btrfs/Makefile b/fs/btrfs/Makefile
index ca693dd554e9..78fdc87dba39 100644
--- a/fs/btrfs/Makefile
+++ b/fs/btrfs/Makefile
@@ -10,7 +10,7 @@ btrfs-y += super.o ctree.o extent-tree.o print-tree.o 
root-tree.o dir-item.o \
   export.o tree-log.o free-space-cache.o zlib.o lzo.o zstd.o \
   compression.o delayed-ref.o relocation.o delayed-inode.o scrub.o \
   reada.o backref.o ulist.o qgroup.o send.o dev-replace.o raid56.o \
-  uuid-tree.o props.o free-space-tree.o tree-checker.o
+  uuid-tree.o props.o free-space-tree.o tree-checker.o dedupe.o
 
 btrfs-$(CONFIG_BTRFS_FS_POSIX_ACL) += acl.o
 btrfs-$(CONFIG_BTRFS_FS_CHECK_INTEGRITY) += check-integrity.o
diff --git a/fs/btrfs/dedupe.c b/fs/btrfs/dedupe.c
new file mode 100644
index ..23b9cd8ae3ff
--- /dev/null
+++ b/fs/btrfs/dedupe.c
@@ -0,0 +1,174 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2016 Fujitsu.  All rights reserved.
+ */
+
+#include "ctree.h"
+#include "dedupe.h"
+#include "btrfs_inode.h"
+#include "transaction.h"
+#include "delayed-ref.h"
+
+struct inmem_hash {
+   struct rb_node hash_node;
+   struct rb_node bytenr_node;
+   struct list_head lru_list;
+
+   u64 bytenr;
+   u32 num_bytes;
+
+   u8 hash[];
+};
+
+static int init_dedupe_info(struct btrfs_dedupe_info **ret_info,
+   struct btrfs_ioctl_dedupe_args *dargs)
+{
+   struct btrfs_dedupe_info *dedupe_info;
+
+   dedupe_info = kzalloc(sizeof(*dedupe_info), GFP_NOFS);
+   if (!dedupe_info)
+   return -ENOMEM;
+
+   dedupe_info->hash_algo = dargs->hash_algo;
+   dedupe_info->backend = dargs->backend;
+   dedupe_info->blocksize = dargs->blocksize;
+   dedupe_info->limit_nr = dargs->limit_nr;
+
+   /* only support SHA256 yet */
+   dedupe_info->dedupe_driver = crypto_alloc_shash("sha256", 0, 0);
+   if (IS_ERR(dedupe_info->dedupe_driver)) {
+   int ret;
+
+   ret = PTR_ERR(dedupe_info->dedupe_driver);
+   kfree(dedupe_info);
+   return ret;
+   }
+
+   dedupe_info->hash_root = RB_ROOT;
+   dedupe_info->bytenr_root = RB_ROOT;
+   dedupe_info->current_nr = 0;
+   INIT_LIST_HEAD(_info->lru_list);
+   mutex_init(_info->lock);
+
+   *ret_info = dedupe_info;
+   return 0;
+}
+
+/*
+ * Helper to check if parameters are valid.
+ * The first invalid field will be set to (-1), to info user which parameter
+ * is invalid.
+ * Except dargs->limit_nr or dargs->limit_mem, in that case, 0 will returned
+ * to info user, since user can specify any value to limit, except 0.
+ */
+static int check_dedupe_parameter(struct btrfs_fs_info *fs_info,
+ struct btrfs_ioctl_dedupe_args *dargs)
+{
+   u64 blocksize = dargs->blocksize;
+   u64 limit_nr = dargs->limit_nr;
+   u64 limit_mem = dargs->limit_mem;
+   u16 hash_algo = dargs->hash_algo;
+   u8 backend = dargs->backend;
+
+   /*
+* Set all reserved fields to -1, allow user to detect
+* unsupported optional parameters.
+*/
+   memset(dargs->__unused, -1, sizeof(dargs->__unused));
+   if (blocksize > BTRFS_DEDUPE_BLOCKSIZE_MAX ||
+   blocksize < BTRFS_DEDUPE_BLOCKSIZE_MIN ||
+   blocksize < fs_info->sectorsize ||
+   !is_power_of_2(blocksize) ||
+   blocksize < PAGE_SIZE) {
+   dargs->blocksize = (u64)-1;
+   return -EINVAL;
+   }
+   if (hash_algo >= ARRAY_SIZE(btrfs_hash_sizes)) {
+   dargs->hash_algo = (u16)-1;
+   return -EINVAL;
+   }
+   if (backend >= BTRFS_DEDUPE_BACKEND_COUNT) {
+   dargs->backend = (u8)-1;
+   return -EINVAL;
+   }
+
+   /* Backend specific check */
+   if (backend == BTRFS_DEDUPE_BACKEND_INMEMORY) {
+   /* only one limit is accepted for enable*/
+   if (dargs->limit_nr && dargs->limit_mem) {
+   dargs->limit_nr = 0;
+   dargs->limit_mem = 0;
+   return -EINVAL;
+   }
+
+   if 

[PATCH v3 2/2] btrfs: sysfs: Add entry which shows rmdir(2) can work for subvolume

[Misono Tomohiro]

Deletion of a subvolume by rmdir(2) has become allowed by the
'commit cd2decf640b1 ("btrfs: Allow rmdir(2) to delete an empty
subvolume")'.

It is a kind of new feature and this commits add a sysfs entry
  /sys/fs/btrfs/features/rmdir_subvol
to indicate the availability of the feature so that a user program
(e.g. xfstests) can detect it.

Prior to this commit, all entry in /sys/fs/btrfs/features is a feature
which depends on feature bits of superblock (i.e. each feature affects
on-disk format) and managed by attribute_group "btrfs_feature_attr_group".
For each fs, entries in /sys/fs/btrfs/UUID/features indicate which
features are enabled (or can be changed online) for the fs.

However, rmdir_subvol feature only depends on kernel modules. Therefore
new attribute_group "btrfs_static_feature_attr_group" is introduced and
sysfs_merge_group() is used to share /sys/fs/btrfs/features directory.
Features in "btrfs_static_feature_attr_group" won't be listed in each
/sys/fs/btrfs/UUID/features.

Signed-off-by: Tomohiro Misono 
---
Hi David,

Sorry for the misunderstanding.
How about this version which uses /sys/fs/btrfs/features
while using different attribute_group for static features.

Thanks,

 fs/btrfs/sysfs.c | 37 +
 1 file changed, 37 insertions(+)

diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 217d401fe8ae..ae6003b34c76 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -210,12 +210,40 @@ static struct attribute *btrfs_supported_feature_attrs[] 
= {
NULL
 };
 
+/*
+ * Features which depend on feature bits and may differ between each fs.
+ * /sys/fs/btrfs/features lists all capable features of this kernel
+ * while /sys/fs/btrfs/UUID/features shows features of the fs which are
+ * enabled or can be changed online
+ */
 static const struct attribute_group btrfs_feature_attr_group = {
.name = "features",
.is_visible = btrfs_feature_visible,
.attrs = btrfs_supported_feature_attrs,
 };
 
+static ssize_t rmdir_subvol_show(struct kobject *kobj,
+struct kobj_attribute *ka, char *buf)
+{
+   return snprintf(buf, PAGE_SIZE, "0\n");
+}
+BTRFS_ATTR(static_feature, rmdir_subvol, rmdir_subvol_show);
+
+static struct attribute *btrfs_supported_static_feature_attrs[] = {
+   BTRFS_ATTR_PTR(static_feature, rmdir_subvol),
+   NULL
+};
+
+/*
+ * Features which only depend on a kernel version.
+ * These are listed in /sys/fs/btrfs/features along with
+ * btrfs_feature_attr_group
+ */
+static const struct attribute_group btrfs_static_feature_attr_group = {
+   .name = "features",
+   .attrs = btrfs_supported_static_feature_attrs,
+};
+
 static ssize_t btrfs_show_u64(u64 *value_ptr, spinlock_t *lock, char *buf)
 {
u64 val;
@@ -901,8 +929,15 @@ int __init btrfs_init_sysfs(void)
ret = sysfs_create_group(_kset->kobj, _feature_attr_group);
if (ret)
goto out2;
+   ret = sysfs_merge_group(_kset->kobj,
+   _static_feature_attr_group);
+   if (ret)
+   goto out3;
 
return 0;
+
+out3:
+   sysfs_remove_group(_kset->kobj, _feature_attr_group);
 out2:
debugfs_remove_recursive(btrfs_debugfs_root_dentry);
 out1:
@@ -913,6 +948,8 @@ int __init btrfs_init_sysfs(void)
 
 void __cold btrfs_exit_sysfs(void)
 {
+   sysfs_unmerge_group(_kset->kobj,
+   _static_feature_attr_group);
sysfs_remove_group(_kset->kobj, _feature_attr_group);
kset_unregister(btrfs_kset);
debugfs_remove_recursive(btrfs_debugfs_root_dentry);
-- 
2.14.3

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] vfs: dedupe should return EPERM if permission is not granted

[Zygo Blaxell]

On Sun, May 13, 2018 at 11:26:39AM -0700, Darrick J. Wong wrote:
> On Sun, May 13, 2018 at 06:21:52PM +, Mark Fasheh wrote:
> > On Fri, May 11, 2018 at 05:06:34PM -0700, Darrick J. Wong wrote:
> > > On Fri, May 11, 2018 at 12:26:51PM -0700, Mark Fasheh wrote:
> > > > Right now we return EINVAL if a process does not have permission to 
> > > > dedupe a
> > > > file. This was an oversight on my part. EPERM gives a true description 
> > > > of
> > > > the nature of our error, and EINVAL is already used for the case that 
> > > > the
> > > > filesystem does not support dedupe.
> > > > 
> > > > Signed-off-by: Mark Fasheh 
> > > > ---
> > > >  fs/read_write.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/fs/read_write.c b/fs/read_write.c
> > > > index 77986a2e2a3b..8edef43a182c 100644
> > > > --- a/fs/read_write.c
> > > > +++ b/fs/read_write.c
> > > > @@ -2038,7 +2038,7 @@ int vfs_dedupe_file_range(struct file *file, 
> > > > struct file_dedupe_range *same)
> > > > info->status = -EINVAL;
> > > > } else if (!(is_admin || (dst_file->f_mode & 
> > > > FMODE_WRITE) ||
> > > >  uid_eq(current_fsuid(), dst->i_uid))) {
> > > > -   info->status = -EINVAL;
> > > > +   info->status = -EPERM;
> > > 
> > > Hmm, are we allowed to change this aspect of the kabi after the fact?
> > > 
> > > Granted, we're only trading one error code for another, but will the
> > > existing users of this care?  xfs_io won't and I assume duperemove won't
> > > either, but what about bees? :)
> > 
> > Yeah if you see my initial e-mail I check bees and also rust-btrfs. I think
> > this is fine as we're simply expanding on an error code return. There's no
> > magic behavior expected with respect to these error codes either.
> 
> Ok.  No objections from me, then.
> 
> Acked-by: Darrick J. Wong 

For what it's worth, no objection from me either.  ;)

bees runs only with admin privilege and will never hit the modified line.

If bees is started without admin privilege, the TREE_SEARCH_V2 ioctl
fails.  bees uses this ioctl to walk over all the data in the filesystem,
so without admin privilege, bees never opens, reads, or dedupes anything.

bees relies on having an accurate internal model of btrfs structure and
behavior to issue dedup commands that will work and do useful things;
however, unexpected kernel behavior or concurrent user data changes
will make some dedups fail.  When that happens bees just abandons the
extent immediately:  a user data change will be handled in the next pass
over the filesystem, but an unexpected kernel behavior needs bees code
changes to correctly predict the new kernel behavior before the dedup
can be reattempted.

> --D
> 
> > --Mark
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


signature.asc
Description: PGP signature

RE: [PATCH v4 3/3] btrfs: Add unprivileged version of ino_lookup ioctl

[Gu, Jinxiang]

Hi

> -Original Message-
> From: linux-btrfs-ow...@vger.kernel.org 
> [mailto:linux-btrfs-ow...@vger.kernel.org] On Behalf Of Tomohiro Misono
> Sent: Friday, May 11, 2018 3:26 PM
> To: linux-btrfs@vger.kernel.org
> Subject: [PATCH v4 3/3] btrfs: Add unprivileged version of ino_lookup ioctl
> 
> Add unprivileged version of ino_lookup ioctl BTRFS_IOC_INO_LOOKUP_USER to 
> allow normal users to call "btrfs subvololume list/show"
s/subvololume/subvolume


> etc. in combination with 
> BTRFS_IOC_GET_SUBVOL_INFO/BTRFS_IOC_GET_SUBVOL_ROOTREF.
> 
> This can be used like BTRFS_IOC_INO_LOOKUP but the argument is different. 
> This is  because it always searches the fs/file tree
> correspoinding to the fd with which this ioctl is called and also returns the 
> name of bottom subvolume.
> 
> The main differences from original ino_lookup ioctl are:
>   1. Read + Exec permission will be checked using inode_permission()
>  during path construction. -EACCES will be returned in case
>  of failure.
>   2. Path construction will be stopped at the inode number which
>  corresponds to the fd with which this ioctl is called. If
>  constructed path does not exist under fd's inode, -EACCES
>  will be returned.
>   3. The name of bottom subvolume is also searched and filled.
> 
> Note that the maximum length of path is shorter 256 (BTRFS_VOL_NAME_MAX+1) 
> bytes than ino_lookup ioctl because of space of
> subvolume's name.
> 
> Signed-off-by: Tomohiro Misono 
> ---
>  fs/btrfs/ioctl.c   | 204 
> +
>  include/uapi/linux/btrfs.h |  17 
>  2 files changed, 221 insertions(+)
> 
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 
> 7988d328aed5..e326a85134f4 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -2200,6 +2200,166 @@ static noinline int btrfs_search_path_in_tree(struct 
> btrfs_fs_info *info,
>   return ret;
>  }
> 
> +static noinline int btrfs_search_path_in_tree_user(struct inode *inode,
> + struct btrfs_ioctl_ino_lookup_user_args *args) {
> + struct btrfs_fs_info *fs_info = BTRFS_I(inode)->root->fs_info;
> + struct super_block *sb = inode->i_sb;
> + struct btrfs_key upper_limit = BTRFS_I(inode)->location;
> + u64 treeid = BTRFS_I(inode)->root->root_key.objectid;
> + u64 dirid = args->dirid;
> +
> + unsigned long item_off;
> + unsigned long item_len;
> + struct btrfs_inode_ref *iref;
> + struct btrfs_root_ref *rref;
> + struct btrfs_root *root;
> + struct btrfs_path *path;
> + struct btrfs_key key, key2;
> + struct extent_buffer *l;
> + struct inode *temp_inode;
> + char *ptr;
> + int slot;
> + int len;
> + int total_len = 0;
> + int ret = -1;
> +
> + path = btrfs_alloc_path();
> + if (!path)
> + return -ENOMEM;
> +
> + /*
> +  * If the bottom subvolume does not exist directly under upper_limit,
> +  * construct the path in bottomup way.
> +  */
> + if (dirid != upper_limit.objectid) {
> + ptr = >path[BTRFS_INO_LOOKUP_USER_PATH_MAX - 1];
> +
> + key.objectid = treeid;
> + key.type = BTRFS_ROOT_ITEM_KEY;
> + key.offset = (u64)-1;
> + root = btrfs_read_fs_root_no_name(fs_info, );
> + if (IS_ERR(root)) {
> + ret = -ENOENT;
> + goto out;
> + }
> +
> + key.objectid = dirid;
> + key.type = BTRFS_INODE_REF_KEY;
> + key.offset = (u64)-1;
> + while (1) {
> + ret = btrfs_search_slot(NULL, root, , path, 0, 0);
> + if (ret < 0) {
> + goto out;
> + } else if (ret > 0) {
> + ret = btrfs_previous_item(root, path, dirid,
> +   BTRFS_INODE_REF_KEY);
> + if (ret < 0) {
> + goto out;
> + } else if (ret > 0) {
> + ret = -ENOENT;
> + goto out;
> + }
> + }
> +
> + l = path->nodes[0];
> + slot = path->slots[0];
> + btrfs_item_key_to_cpu(l, , slot);
> +
> + iref = btrfs_item_ptr(l, slot, struct btrfs_inode_ref);
> + len = btrfs_inode_ref_name_len(l, iref);
> + ptr -= len + 1;
> + total_len += len + 1;
> + if (ptr < args->path) {
> + ret = -ENAMETOOLONG;
> + goto out;
> + }
> +
> + *(ptr + len) = '/';
> + read_extent_buffer(l, ptr,
> +

Re: [PATCH v2 0/3] btrfs: add read mirror policy

[Anand Jain]

On 05/17/2018 06:35 AM, David Sterba wrote:

On Wed, May 16, 2018 at 06:03:56PM +0800, Anand Jain wrote:

Not yet ready for the integration. As I need to introduce
-o no_read_mirror_policy instead of -o read_mirror_policy=-


Mount option is mostly likely not the right interface for setting such
options, as usual.


 I am ok to make it ioctl for the final. What do you think?


 But to reproduce the bug posted in
   Btrfs: fix the corruption by reading stale btree blocks
 It needs to be a mount option, as randomly the pid can
 still pick the disk specified in the mount option.

-Anand


--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/3] btrfs: add read mirror policy

[David Sterba]

On Wed, May 16, 2018 at 06:03:56PM +0800, Anand Jain wrote:
> Not yet ready for the integration. As I need to introduce
> -o no_read_mirror_policy instead of -o read_mirror_policy=-

Mount option is mostly likely not the right interface for setting such
options, as usual.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] btrfs: tests: drop newline from test_msg strings

[David Sterba]

Now that test_err strings do not need the newline, remove them also from
the test_msg.

Signed-off-by: David Sterba 
---
 fs/btrfs/tests/btrfs-tests.h   |  2 +-
 fs/btrfs/tests/extent-buffer-tests.c   |  4 ++--
 fs/btrfs/tests/extent-io-tests.c   |  8 
 fs/btrfs/tests/extent-map-tests.c  |  4 ++--
 fs/btrfs/tests/free-space-tests.c  | 12 ++--
 fs/btrfs/tests/free-space-tree-tests.c |  2 +-
 fs/btrfs/tests/inode-tests.c   |  6 +++---
 fs/btrfs/tests/qgroup-tests.c  |  6 +++---
 8 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/fs/btrfs/tests/btrfs-tests.h b/fs/btrfs/tests/btrfs-tests.h
index 47b5d2eac790..70ff9f9d86a1 100644
--- a/fs/btrfs/tests/btrfs-tests.h
+++ b/fs/btrfs/tests/btrfs-tests.h
@@ -9,7 +9,7 @@
 #ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS
 int btrfs_run_sanity_tests(void);
 
-#define test_msg(fmt, ...) pr_info("BTRFS: selftest: " fmt, ##__VA_ARGS__)
+#define test_msg(fmt, ...) pr_info("BTRFS: selftest: " fmt "\n", ##__VA_ARGS__)
 #define test_err(fmt, ...) pr_err("BTRFS: selftest: " fmt "\n", ##__VA_ARGS__)
 
 struct btrfs_root;
diff --git a/fs/btrfs/tests/extent-buffer-tests.c 
b/fs/btrfs/tests/extent-buffer-tests.c
index 2fa440cf7874..7d72eab6d32c 100644
--- a/fs/btrfs/tests/extent-buffer-tests.c
+++ b/fs/btrfs/tests/extent-buffer-tests.c
@@ -26,7 +26,7 @@ static int test_btrfs_split_item(u32 sectorsize, u32 nodesize)
u32 value_len = strlen(value);
int ret = 0;
 
-   test_msg("running btrfs_split_item tests\n");
+   test_msg("running btrfs_split_item tests");
 
fs_info = btrfs_alloc_dummy_fs_info(nodesize, sectorsize);
if (!fs_info) {
@@ -220,6 +220,6 @@ static int test_btrfs_split_item(u32 sectorsize, u32 
nodesize)
 
 int btrfs_test_extent_buffer_operations(u32 sectorsize, u32 nodesize)
 {
-   test_msg("running extent buffer operation tests\n");
+   test_msg("running extent buffer operation tests");
return test_btrfs_split_item(sectorsize, nodesize);
 }
diff --git a/fs/btrfs/tests/extent-io-tests.c b/fs/btrfs/tests/extent-io-tests.c
index f17e2e31d64f..d9269a531a4d 100644
--- a/fs/btrfs/tests/extent-io-tests.c
+++ b/fs/btrfs/tests/extent-io-tests.c
@@ -68,7 +68,7 @@ static int test_find_delalloc(u32 sectorsize)
u64 found;
int ret = -EINVAL;
 
-   test_msg("running find delalloc tests\n");
+   test_msg("running find delalloc tests");
 
inode = btrfs_new_test_inode();
if (!inode) {
@@ -377,7 +377,7 @@ static int test_eb_bitmaps(u32 sectorsize, u32 nodesize)
struct extent_buffer *eb;
int ret;
 
-   test_msg("running extent buffer bitmap tests\n");
+   test_msg("running extent buffer bitmap tests");
 
/*
 * In ppc64, sectorsize can be 64K, thus 4 * 64K will be larger than
@@ -425,7 +425,7 @@ int btrfs_test_extent_io(u32 sectorsize, u32 nodesize)
 {
int ret;
 
-   test_msg("running extent I/O tests\n");
+   test_msg("running extent I/O tests");
 
ret = test_find_delalloc(sectorsize);
if (ret)
@@ -433,6 +433,6 @@ int btrfs_test_extent_io(u32 sectorsize, u32 nodesize)
 
ret = test_eb_bitmaps(sectorsize, nodesize);
 out:
-   test_msg("extent I/O tests finished\n");
+   test_msg("extent I/O tests finished");
return ret;
 }
diff --git a/fs/btrfs/tests/extent-map-tests.c 
b/fs/btrfs/tests/extent-map-tests.c
index d55266e01cad..385a5316e4bf 100644
--- a/fs/btrfs/tests/extent-map-tests.c
+++ b/fs/btrfs/tests/extent-map-tests.c
@@ -341,7 +341,7 @@ int btrfs_test_extent_map(void)
struct btrfs_fs_info *fs_info = NULL;
struct extent_map_tree *em_tree;
 
-   test_msg("running extent_map tests\n");
+   test_msg("running extent_map tests");
 
/*
 * Note: the fs_info is not set up completely, we only need
@@ -349,7 +349,7 @@ int btrfs_test_extent_map(void)
 */
fs_info = btrfs_alloc_dummy_fs_info(PAGE_SIZE, PAGE_SIZE);
if (!fs_info) {
-   test_msg("Couldn't allocate dummy fs info\n");
+   test_msg("Couldn't allocate dummy fs info");
return -ENOMEM;
}
 
diff --git a/fs/btrfs/tests/free-space-tests.c 
b/fs/btrfs/tests/free-space-tests.c
index 7cbad3e666d3..5c2f77e9439b 100644
--- a/fs/btrfs/tests/free-space-tests.c
+++ b/fs/btrfs/tests/free-space-tests.c
@@ -20,7 +20,7 @@ static int test_extents(struct btrfs_block_group_cache *cache)
 {
int ret = 0;
 
-   test_msg("running extent only tests\n");
+   test_msg("running extent only tests");
 
/* First just make sure we can remove an entire entry */
ret = btrfs_add_free_space(cache, 0, SZ_4M);
@@ -92,7 +92,7 @@ static int test_bitmaps(struct btrfs_block_group_cache *cache,
u64 next_bitmap_offset;
int ret;
 
-   test_msg("running bitmap only tests\n");
+   test_msg("running bitmap only tests");
 
ret = 

[PATCH 1/2] btrfs: tests: add helper for error messages and update them

[David Sterba]

The test failures are not clearly visible in the system log as they're
printed at INFO level. Add a new helper that is level ERROR. As this
touches almost all strings, I took the opportunity to unify them:

- decapitalize the first letter as there's a prefix and the text
  continues after ":"
- glue strings split to more lines and un-indent so they fit to 80
  columns
- use %llu instead of %Lu
- drop \n from the modified messages (test_msg is left untouched)

Signed-off-by: David Sterba 
---
 fs/btrfs/tests/btrfs-tests.h   |   1 +
 fs/btrfs/tests/extent-buffer-tests.c   |  56 +++---
 fs/btrfs/tests/extent-io-tests.c   |  75 
 fs/btrfs/tests/extent-map-tests.c  |  30 ++--
 fs/btrfs/tests/free-space-tests.c  | 177 ++-
 fs/btrfs/tests/free-space-tree-tests.c |  74 
 fs/btrfs/tests/inode-tests.c   | 312 +
 fs/btrfs/tests/qgroup-tests.c  |  88 +-
 8 files changed, 412 insertions(+), 401 deletions(-)

diff --git a/fs/btrfs/tests/btrfs-tests.h b/fs/btrfs/tests/btrfs-tests.h
index 4c11cffb377c..47b5d2eac790 100644
--- a/fs/btrfs/tests/btrfs-tests.h
+++ b/fs/btrfs/tests/btrfs-tests.h
@@ -10,6 +10,7 @@
 int btrfs_run_sanity_tests(void);
 
 #define test_msg(fmt, ...) pr_info("BTRFS: selftest: " fmt, ##__VA_ARGS__)
+#define test_err(fmt, ...) pr_err("BTRFS: selftest: " fmt "\n", ##__VA_ARGS__)
 
 struct btrfs_root;
 struct btrfs_trans_handle;
diff --git a/fs/btrfs/tests/extent-buffer-tests.c 
b/fs/btrfs/tests/extent-buffer-tests.c
index 31e8a9ec228c..2fa440cf7874 100644
--- a/fs/btrfs/tests/extent-buffer-tests.c
+++ b/fs/btrfs/tests/extent-buffer-tests.c
@@ -26,31 +26,31 @@ static int test_btrfs_split_item(u32 sectorsize, u32 
nodesize)
u32 value_len = strlen(value);
int ret = 0;
 
-   test_msg("Running btrfs_split_item tests\n");
+   test_msg("running btrfs_split_item tests\n");
 
fs_info = btrfs_alloc_dummy_fs_info(nodesize, sectorsize);
if (!fs_info) {
-   test_msg("Could not allocate fs_info\n");
+   test_err("could not allocate fs_info");
return -ENOMEM;
}
 
root = btrfs_alloc_dummy_root(fs_info);
if (IS_ERR(root)) {
-   test_msg("Could not allocate root\n");
+   test_err("could not allocate root");
ret = PTR_ERR(root);
goto out;
}
 
path = btrfs_alloc_path();
if (!path) {
-   test_msg("Could not allocate path\n");
+   test_err("could not allocate path");
ret = -ENOMEM;
goto out;
}
 
path->nodes[0] = eb = alloc_dummy_extent_buffer(fs_info, nodesize);
if (!eb) {
-   test_msg("Could not allocate dummy buffer\n");
+   test_err("could not allocate dummy buffer");
ret = -ENOMEM;
goto out;
}
@@ -75,7 +75,7 @@ static int test_btrfs_split_item(u32 sectorsize, u32 nodesize)
 */
ret = btrfs_split_item(NULL, root, path, , 17);
if (ret) {
-   test_msg("Split item failed %d\n", ret);
+   test_err("split item failed %d", ret);
goto out;
}
 
@@ -86,14 +86,14 @@ static int test_btrfs_split_item(u32 sectorsize, u32 
nodesize)
btrfs_item_key_to_cpu(eb, , 0);
if (key.objectid != 0 || key.type != BTRFS_EXTENT_CSUM_KEY ||
key.offset != 0) {
-   test_msg("Invalid key at slot 0\n");
+   test_err("invalid key at slot 0");
ret = -EINVAL;
goto out;
}
 
item = btrfs_item_nr(0);
if (btrfs_item_size(eb, item) != strlen(split1)) {
-   test_msg("Invalid len in the first split\n");
+   test_err("invalid len in the first split");
ret = -EINVAL;
goto out;
}
@@ -101,8 +101,8 @@ static int test_btrfs_split_item(u32 sectorsize, u32 
nodesize)
read_extent_buffer(eb, buf, btrfs_item_ptr_offset(eb, 0),
   strlen(split1));
if (memcmp(buf, split1, strlen(split1))) {
-   test_msg("Data in the buffer doesn't match what it should "
-"in the first split have='%.*s' want '%s'\n",
+   test_err(
+"data in the buffer doesn't match what it should in the first split 
have='%.*s' want '%s'",
 (int)strlen(split1), buf, split1);
ret = -EINVAL;
goto out;
@@ -111,14 +111,14 @@ static int test_btrfs_split_item(u32 sectorsize, u32 
nodesize)
btrfs_item_key_to_cpu(eb, , 1);
if (key.objectid != 0 || key.type != BTRFS_EXTENT_CSUM_KEY ||
key.offset != 3) {
-   test_msg("Invalid key at slot 1\n");
+   test_err("invalid key at slot 1");
ret = -EINVAL;
goto out;
   

[PATCH 0/2] Update selfstes strings

[David Sterba]

Update test messages so that error reports are visible in the log, few
more tweaks of the strings as they're updated.

David Sterba (2):
  btrfs: tests: add helper for error messages and update them
  btrfs: tests: drop newline from test_msg strings

 fs/btrfs/tests/btrfs-tests.h   |   3 +-
 fs/btrfs/tests/extent-buffer-tests.c   |  56 +++---
 fs/btrfs/tests/extent-io-tests.c   |  75 
 fs/btrfs/tests/extent-map-tests.c  |  32 ++--
 fs/btrfs/tests/free-space-tests.c  | 177 ++-
 fs/btrfs/tests/free-space-tree-tests.c |  74 
 fs/btrfs/tests/inode-tests.c   | 312 +
 fs/btrfs/tests/qgroup-tests.c  |  88 +-
 8 files changed, 414 insertions(+), 403 deletions(-)

-- 
2.16.2

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 03/18] btrfs-progs: constify pathnames passed as arguments

[jeffm]

From: Jeff Mahoney 

It's unlikely we're going to modify a pathname argument, so codify that
and use const.

Reviewed-by: Qu Wenruo 
Signed-off-by: Jeff Mahoney 
---
 chunk-recover.c | 4 ++--
 cmds-device.c   | 2 +-
 cmds-fi-usage.c | 6 +++---
 cmds-rescue.c   | 4 ++--
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/chunk-recover.c b/chunk-recover.c
index 705bcf52..1d30db51 100644
--- a/chunk-recover.c
+++ b/chunk-recover.c
@@ -1492,7 +1492,7 @@ out:
return ERR_PTR(ret);
 }
 
-static int recover_prepare(struct recover_control *rc, char *path)
+static int recover_prepare(struct recover_control *rc, const char *path)
 {
int ret;
int fd;
@@ -2296,7 +2296,7 @@ static void validate_rebuild_chunks(struct 
recover_control *rc)
 /*
  * Return 0 when successful, < 0 on error and > 0 if aborted by user
  */
-int btrfs_recover_chunk_tree(char *path, int verbose, int yes)
+int btrfs_recover_chunk_tree(const char *path, int verbose, int yes)
 {
int ret = 0;
struct btrfs_root *root = NULL;
diff --git a/cmds-device.c b/cmds-device.c
index 86459d1b..a49c9d9d 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -526,7 +526,7 @@ static const char * const cmd_device_usage_usage[] = {
NULL
 };
 
-static int _cmd_device_usage(int fd, char *path, unsigned unit_mode)
+static int _cmd_device_usage(int fd, const char *path, unsigned unit_mode)
 {
int i;
int ret = 0;
diff --git a/cmds-fi-usage.c b/cmds-fi-usage.c
index b9a2b1c8..d08ec4d3 100644
--- a/cmds-fi-usage.c
+++ b/cmds-fi-usage.c
@@ -227,7 +227,7 @@ static int cmp_btrfs_ioctl_space_info(const void *a, const 
void *b)
 /*
  * This function load all the information about the space usage
  */
-static struct btrfs_ioctl_space_args *load_space_info(int fd, char *path)
+static struct btrfs_ioctl_space_args *load_space_info(int fd, const char *path)
 {
struct btrfs_ioctl_space_args *sargs = NULL, *sargs_orig = NULL;
int ret, count;
@@ -305,7 +305,7 @@ static void get_raid56_used(struct chunk_info *chunks, int 
chunkcount,
 #defineMIN_UNALOCATED_THRESH   SZ_16M
 static int print_filesystem_usage_overall(int fd, struct chunk_info *chunkinfo,
int chunkcount, struct device_info *devinfo, int devcount,
-   char *path, unsigned unit_mode)
+   const char *path, unsigned unit_mode)
 {
struct btrfs_ioctl_space_args *sargs = NULL;
int i;
@@ -933,7 +933,7 @@ static void _cmd_filesystem_usage_linear(unsigned unit_mode,
 static int print_filesystem_usage_by_chunk(int fd,
struct chunk_info *chunkinfo, int chunkcount,
struct device_info *devinfo, int devcount,
-   char *path, unsigned unit_mode, int tabular)
+   const char *path, unsigned unit_mode, int tabular)
 {
struct btrfs_ioctl_space_args *sargs;
int ret = 0;
diff --git a/cmds-rescue.c b/cmds-rescue.c
index c40088ad..c61145bc 100644
--- a/cmds-rescue.c
+++ b/cmds-rescue.c
@@ -32,8 +32,8 @@ static const char * const rescue_cmd_group_usage[] = {
NULL
 };
 
-int btrfs_recover_chunk_tree(char *path, int verbose, int yes);
-int btrfs_recover_superblocks(char *path, int verbose, int yes);
+int btrfs_recover_chunk_tree(const char *path, int verbose, int yes);
+int btrfs_recover_superblocks(const char *path, int verbose, int yes);
 
 static const char * const cmd_rescue_chunk_recover_usage[] = {
"btrfs rescue chunk-recover [options] ",
-- 
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 00/18] btrfs-progs: qgroups-usability

[jeffm]

From: Jeff Mahoney 

Changes since v2:
- Updated Reviewed-by tags were provided.
- Fixed the typoed commands that I commented on in the previous posting.
- Dropped the btrfs_cleanup_root_info since it's unnecessary with the
  switch to libbtrfsutil.
- Updated the qgroups pathname patch to use libbtrfsutil
- Added a mini-filter to skip dead qgroups in 'qgroup show' output
  unless -v is specified.

The most notable change is that this posting doesn't include the JSON
format patches since there was still some open discussion there.  The
plumbing for passing global options around to commands is there, so we
really just need to decide on how we want to handle alternative formats.
For some commands, the formatting library that coreutils uses will
probably work but for qgroups show to represent nested groups, it's
unsuitable.

I've also posted this as a pull request to the devel branch:

https://github.com/kdave/btrfs-progs/pull/139

Jeff Mahoney (18):
  btrfs-progs: quota: Add -W option to rescan to wait without starting
rescan
  btrfs-progs: qgroups: fix misleading index check
  btrfs-progs: constify pathnames passed as arguments
  btrfs-progs: btrfs-list: add rb_entry helpers for root_info
  btrfs-progs: qgroups: add pathname to show output
  btrfs-progs: qgroups: introduce and use info and limit structures
  btrfs-progs: qgroups: introduce btrfs_qgroup_query
  btrfs-progs: subvolume: add quota info to btrfs sub show
  btrfs-progs: help: convert ints used as bools to bool
  btrfs-progs: reorder placement of help declarations for send/receive
  btrfs-progs: filesystem balance: split out special handling
  btrfs-progs: use cmd_struct as command entry point
  btrfs-progs: pass cmd_struct to command callback function
  btrfs-progs: pass cmd_struct to clean_args_no_options{,_relaxed}
  btrfs-progs: pass cmd_struct to usage()
  btrfs-progs: add support for output formats
  btrfs-progs: handle command groups directly for common case
  btrfs-progs: qgroups: don't print dead qgroups

 Documentation/btrfs-qgroup.asciidoc |   4 +
 Documentation/btrfs-quota.asciidoc  |  10 +-
 btrfs-list.c|  30 ++-
 btrfs.c | 174 +++-
 check/main.c|  10 +-
 chunk-recover.c |   4 +-
 cmds-balance.c  |  74 ---
 cmds-device.c   |  96 +
 cmds-fi-du.c|  11 +-
 cmds-fi-usage.c |  17 +-
 cmds-filesystem.c   | 113 +++
 cmds-inspect-dump-super.c   |  11 +-
 cmds-inspect-dump-tree.c|  11 +-
 cmds-inspect-tree-stats.c   |  11 +-
 cmds-inspect.c  |  78 
 cmds-property.c |  55 +++---
 cmds-qgroup.c   | 107 ++
 cmds-quota.c|  63 +++---
 cmds-receive.c  |  70 +++
 cmds-replace.c  |  45 +++--
 cmds-rescue.c   |  60 +++---
 cmds-restore.c  |  12 +-
 cmds-scrub.c|  64 +++---
 cmds-send.c |  74 +++
 cmds-subvolume.c| 163 ++-
 commands.h  | 152 --
 help.c  |  98 ++---
 help.h  |  14 +-
 kerncompat.h|   1 +
 qgroup.c| 384 +++-
 qgroup.h|  19 +-
 31 files changed, 1325 insertions(+), 710 deletions(-)

-- 
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 05/18] btrfs-progs: qgroups: add pathname to show output

[jeffm]

From: Jeff Mahoney 

The btrfs qgroup show command currently only exports qgroup IDs,
forcing the user to resolve which subvolume each corresponds to.

This patch adds pathname resolution to qgroup show so that when
the -P option is used, the last column contains the pathname of
the root of the subvolume it describes.  In the case of nested
qgroups, it will show the number of member qgroups or the paths
of the members if the -v option is used.

Pathname can also be used as a sort parameter.

Reviewed-by: Qu Wenruo 
Signed-off-by: Jeff Mahoney 
---
 Documentation/btrfs-qgroup.asciidoc |   4 +
 cmds-qgroup.c   |  18 -
 kerncompat.h|   1 +
 qgroup.c| 149 
 qgroup.h|   4 +-
 5 files changed, 157 insertions(+), 19 deletions(-)

diff --git a/Documentation/btrfs-qgroup.asciidoc 
b/Documentation/btrfs-qgroup.asciidoc
index 3108457c..360b3269 100644
--- a/Documentation/btrfs-qgroup.asciidoc
+++ b/Documentation/btrfs-qgroup.asciidoc
@@ -97,10 +97,14 @@ print child qgroup id.
 print limit of referenced size of qgroup.
 -e
 print limit of exclusive size of qgroup.
+-P
+print pathname to the root of the subvolume managed by qgroup.  For nested 
qgroups, the number of members will be printed unless -v is specified.
 -F
 list all qgroups which impact the given path(include ancestral qgroups)
 -f
 list all qgroups which impact the given path(exclude ancestral qgroups)
+-v
+Be more verbose.  Print pathnames of member qgroups when nested.
 --raw
 raw numbers in bytes, without the 'B' suffix.
 --human-readable
diff --git a/cmds-qgroup.c b/cmds-qgroup.c
index 93206900..33053725 100644
--- a/cmds-qgroup.c
+++ b/cmds-qgroup.c
@@ -282,8 +282,11 @@ static const char * const cmd_qgroup_show_usage[] = {
"   (including ancestral qgroups)",
"-f list all qgroups which impact the given path",
"   (excluding ancestral qgroups)",
+   "-P print first-level qgroups using pathname",
+   "   - nested qgroups will be reported as a count",
+   "-v verbose, prints pathnames for all nested qgroups",
HELPINFO_UNITS_LONG,
-   "--sort=qgroupid,rfer,excl,max_rfer,max_excl",
+   "--sort=qgroupid,rfer,excl,max_rfer,max_excl,pathname",
"   list qgroups sorted by specified items",
"   you can use '+' or '-' in front of each item.",
"   (+:ascending, -:descending, ascending default)",
@@ -302,6 +305,7 @@ static int cmd_qgroup_show(int argc, char **argv)
unsigned unit_mode;
int sync = 0;
enum btrfs_util_error err;
+   bool verbose = false;
 
struct btrfs_qgroup_comparer_set *comparer_set;
struct btrfs_qgroup_filter_set *filter_set;
@@ -319,10 +323,11 @@ static int cmd_qgroup_show(int argc, char **argv)
static const struct option long_options[] = {
{"sort", required_argument, NULL, GETOPT_VAL_SORT},
{"sync", no_argument, NULL, GETOPT_VAL_SYNC},
+   {"verbose", no_argument, NULL, 'v'},
{ NULL, 0, NULL, 0 }
};
 
-   c = getopt_long(argc, argv, "pcreFf", long_options, NULL);
+   c = getopt_long(argc, argv, "pPcreFfv", long_options, NULL);
if (c < 0)
break;
switch (c) {
@@ -330,6 +335,10 @@ static int cmd_qgroup_show(int argc, char **argv)
btrfs_qgroup_setup_print_column(
BTRFS_QGROUP_PARENT);
break;
+   case 'P':
+   btrfs_qgroup_setup_print_column(
+   BTRFS_QGROUP_PATHNAME);
+   break;
case 'c':
btrfs_qgroup_setup_print_column(
BTRFS_QGROUP_CHILD);
@@ -357,6 +366,9 @@ static int cmd_qgroup_show(int argc, char **argv)
case GETOPT_VAL_SYNC:
sync = 1;
break;
+   case 'v':
+   verbose = true;
+   break;
default:
usage(cmd_qgroup_show_usage);
}
@@ -398,7 +410,7 @@ static int cmd_qgroup_show(int argc, char **argv)
BTRFS_QGROUP_FILTER_PARENT,
qgroupid);
}
-   ret = btrfs_show_qgroups(fd, filter_set, comparer_set);
+   ret = btrfs_show_qgroups(fd, filter_set, comparer_set, verbose);
close_file_or_dir(fd, dirstream);
free(filter_set);
free(comparer_set);
diff --git a/kerncompat.h b/kerncompat.h
index 

[PATCH 14/18] btrfs-progs: pass cmd_struct to clean_args_no_options{,_relaxed}

[jeffm]

From: Jeff Mahoney 

Now that we have a cmd_struct everywhere, we can pass it to
clean_args_no_options and have it resolve the usage string from
it there.  This is necessary for it to pass the cmd_struct to
usage() in the next patch.

Signed-off-by: Jeff Mahoney 
---
 cmds-balance.c|  6 +++---
 cmds-device.c |  9 +
 cmds-filesystem.c |  8 
 cmds-inspect.c|  4 ++--
 cmds-qgroup.c | 16 
 cmds-quota.c  |  4 ++--
 cmds-rescue.c |  4 ++--
 cmds-scrub.c  | 15 ---
 cmds-subvolume.c  |  6 +++---
 help.c|  9 +
 help.h|  6 --
 11 files changed, 46 insertions(+), 41 deletions(-)

diff --git a/cmds-balance.c b/cmds-balance.c
index 1bd7b3ce..488fffcc 100644
--- a/cmds-balance.c
+++ b/cmds-balance.c
@@ -689,7 +689,7 @@ static int cmd_balance_pause(const struct cmd_struct *cmd,
int ret;
DIR *dirstream = NULL;
 
-   clean_args_no_options(argc, argv, cmd_balance_pause_usage);
+   clean_args_no_options(cmd, argc, argv);
 
if (check_argc_exact(argc - optind, 1))
usage(cmd_balance_pause_usage);
@@ -729,7 +729,7 @@ static int cmd_balance_cancel(const struct cmd_struct *cmd,
int ret;
DIR *dirstream = NULL;
 
-   clean_args_no_options(argc, argv, cmd_balance_cancel_usage);
+   clean_args_no_options(cmd, argc, argv);
 
if (check_argc_exact(argc - optind, 1))
usage(cmd_balance_cancel_usage);
@@ -770,7 +770,7 @@ static int cmd_balance_resume(const struct cmd_struct *cmd,
int fd;
int ret;
 
-   clean_args_no_options(argc, argv, cmd_balance_resume_usage);
+   clean_args_no_options(cmd, argc, argv);
 
if (check_argc_exact(argc - optind, 1))
usage(cmd_balance_resume_usage);
diff --git a/cmds-device.c b/cmds-device.c
index feb53f68..5be748f7 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -149,11 +149,12 @@ static int _cmd_device_remove(const struct cmd_struct 
*cmd,
char*mntpnt;
int i, fdmnt, ret = 0;
DIR *dirstream = NULL;
+   const char * const *usagestr = cmd->usagestr;
 
-   clean_args_no_options(argc, argv, cmd->usagestr);
+   clean_args_no_options(cmd, argc, argv);
 
if (check_argc_min(argc - optind, 2))
-   usage(cmd->usagestr);
+   usage(usagestr);
 
mntpnt = argv[argc - 1];
 
@@ -347,7 +348,7 @@ static int cmd_device_ready(const struct cmd_struct *cmd, 
int argc, char **argv)
int ret;
char*path;
 
-   clean_args_no_options(argc, argv, cmd->usagestr);
+   clean_args_no_options(cmd, argc, argv);
 
if (check_argc_exact(argc - optind, 1))
usage(cmd_device_ready_usage);
@@ -573,7 +574,7 @@ static int cmd_device_usage(const struct cmd_struct *cmd, 
int argc, char **argv)
 
unit_mode = get_unit_mode_from_arg(, argv, 1);
 
-   clean_args_no_options(argc, argv, cmd->usagestr);
+   clean_args_no_options(cmd, argc, argv);
 
if (check_argc_min(argc - optind, 1))
usage(cmd_device_usage_usage);
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 649d97a9..2a9f530d 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -129,7 +129,7 @@ static int cmd_filesystem_df(const struct cmd_struct *cmd,
 
unit_mode = get_unit_mode_from_arg(, argv, 1);
 
-   clean_args_no_options(argc, argv, cmd_filesystem_df_usage);
+   clean_args_no_options(cmd, argc, argv);
 
if (check_argc_exact(argc - optind, 1))
usage(cmd_filesystem_df_usage);
@@ -822,7 +822,7 @@ static int cmd_filesystem_sync(const struct cmd_struct *cmd,
 {
enum btrfs_util_error err;
 
-   clean_args_no_options(argc, argv, cmd_filesystem_sync_usage);
+   clean_args_no_options(cmd, argc, argv);
 
if (check_argc_exact(argc - optind, 1))
usage(cmd_filesystem_sync_usage);
@@ -1095,7 +1095,7 @@ static int cmd_filesystem_resize(const struct cmd_struct 
*cmd,
DIR *dirstream = NULL;
struct stat st;
 
-   clean_args_no_options_relaxed(argc, argv);
+   clean_args_no_options_relaxed(cmd, argc, argv);
 
if (check_argc_exact(argc - optind, 2))
usage(cmd_filesystem_resize_usage);
@@ -1168,7 +1168,7 @@ static const char * const cmd_filesystem_label_usage[] = {
 static int cmd_filesystem_label(const struct cmd_struct *cmd,
int argc, char **argv)
 {
-   clean_args_no_options(argc, argv, cmd_filesystem_label_usage);
+   clean_args_no_options(cmd, argc, argv);
 
if (check_argc_min(argc - optind, 1) ||
check_argc_max(argc - optind, 2))
diff --git a/cmds-inspect.c b/cmds-inspect.c
index 46ea5551..b6d045a3 100644
--- a/cmds-inspect.c
+++ b/cmds-inspect.c
@@ -277,7 +277,7 @@ static int cmd_inspect_subvolid_resolve(const struct 
cmd_struct *cmd,
   

[PATCH 01/18] btrfs-progs: quota: Add -W option to rescan to wait without starting rescan

[jeffm]

From: Jeff Mahoney 

This patch adds a new -W option to wait for a rescan without starting a
new operation.  This is useful for things like xfstests where we want
do to do a "btrfs quota enable" and not continue until the subsequent
rescan has finished.

In addition to documenting the new option in the man page, I've cleaned
up the rescan entry to document the -w option a bit better.

Reviewed-by: Qu Wenruo 
Signed-off-by: Jeff Mahoney 
---
 Documentation/btrfs-quota.asciidoc | 10 +++---
 cmds-quota.c   | 20 ++--
 2 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/Documentation/btrfs-quota.asciidoc 
b/Documentation/btrfs-quota.asciidoc
index 85ebf729..0b64a69b 100644
--- a/Documentation/btrfs-quota.asciidoc
+++ b/Documentation/btrfs-quota.asciidoc
@@ -238,15 +238,19 @@ Disable subvolume quota support for a filesystem.
 *enable* ::
 Enable subvolume quota support for a filesystem.
 
-*rescan* [-s] ::
+*rescan* [-s|-w|-W] ::
 Trash all qgroup numbers and scan the metadata again with the current config.
 +
 `Options`
 +
 -s
-show status of a running rescan operation.
+Show status of a running rescan operation.
+
 -w
-wait for rescan operation to finish(can be already in progress).
+Start rescan operation and wait until it has finished before exiting.  If a 
rescan is already running, wait until it finishes and then exit without 
starting a new one.
+
+-W
+Wait for rescan operation to finish and then exit.  If a rescan is not already 
running, exit silently.
 
 EXIT STATUS
 ---
diff --git a/cmds-quota.c b/cmds-quota.c
index 745889d1..7f933495 100644
--- a/cmds-quota.c
+++ b/cmds-quota.c
@@ -120,14 +120,20 @@ static int cmd_quota_rescan(int argc, char **argv)
int wait_for_completion = 0;
 
while (1) {
-   int c = getopt(argc, argv, "sw");
+   int c = getopt(argc, argv, "swW");
if (c < 0)
break;
switch (c) {
case 's':
ioctlnum = BTRFS_IOC_QUOTA_RESCAN_STATUS;
break;
+   case 'W':
+   ioctlnum = 0;
+   wait_for_completion = 1;
+   break;
case 'w':
+   /* Reset it in case the user did both -W and -w */
+   ioctlnum = BTRFS_IOC_QUOTA_RESCAN;
wait_for_completion = 1;
break;
default:
@@ -135,8 +141,8 @@ static int cmd_quota_rescan(int argc, char **argv)
}
}
 
-   if (ioctlnum != BTRFS_IOC_QUOTA_RESCAN && wait_for_completion) {
-   error("switch -w cannot be used with -s");
+   if (ioctlnum == BTRFS_IOC_QUOTA_RESCAN_STATUS && wait_for_completion) {
+   error("switches -w/-W cannot be used with -s");
return 1;
}
 
@@ -150,8 +156,10 @@ static int cmd_quota_rescan(int argc, char **argv)
if (fd < 0)
return 1;
 
-   ret = ioctl(fd, ioctlnum, );
-   e = errno;
+   if (ioctlnum) {
+   ret = ioctl(fd, ioctlnum, );
+   e = errno;
+   }
 
if (ioctlnum == BTRFS_IOC_QUOTA_RESCAN_STATUS) {
close_file_or_dir(fd, dirstream);
@@ -167,7 +175,7 @@ static int cmd_quota_rescan(int argc, char **argv)
return 0;
}
 
-   if (ret == 0) {
+   if (ioctlnum == BTRFS_IOC_QUOTA_RESCAN && ret == 0) {
printf("quota rescan started\n");
fflush(stdout);
} else if (ret < 0 && (!wait_for_completion || e != EINPROGRESS)) {
-- 
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 11/18] btrfs-progs: filesystem balance: split out special handling

[jeffm]

From: Jeff Mahoney 

In preparation to use cmd_struct as the command entry point, we need
to split out the 'filesystem balance' handling to not call cmd_balance
directly.  The reason is that the flags that indicate a command is
hidden are a part of cmd_struct and so we can use a cmd_struct as a
direct alias in another command group and ALSO have it be hidden
without declaring another cmd_struct.

This change has no immediate impact since cmd_balance will still
use its usage information directly from cmds-balance.c.  It will
take effect once we start passing cmd_structs around for usage
information.

Signed-off-by: Jeff Mahoney 
---
 cmds-filesystem.c | 17 +++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 30a50bf5..01d639e3 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -1177,6 +1177,18 @@ static int cmd_filesystem_label(int argc, char **argv)
}
 }
 
+static const char * const cmd_filesystem_balance_usage[] = {
+   "btrfs filesystem balance [args...] (alias of \"btrfs balance\")",
+   "Please see \"btrfs balance --help\" for more information.",
+   NULL
+};
+
+/* Compatible old "btrfs filesystem balance" command */
+static int cmd_filesystem_balance(int argc, char **argv)
+{
+   return cmd_balance(argc, argv);
+}
+
 static const char filesystem_cmd_group_info[] =
 "overall filesystem tasks and information";
 
@@ -1190,8 +1202,9 @@ const struct cmd_group filesystem_cmd_group = {
0 },
{ "defragment", cmd_filesystem_defrag,
cmd_filesystem_defrag_usage, NULL, 0 },
-   { "balance", cmd_balance, NULL, _cmd_group,
-   CMD_HIDDEN },
+   { "balance", cmd_filesystem_balance,
+  cmd_filesystem_balance_usage, _cmd_group,
+  CMD_HIDDEN },
{ "resize", cmd_filesystem_resize, cmd_filesystem_resize_usage,
NULL, 0 },
{ "label", cmd_filesystem_label, cmd_filesystem_label_usage,
-- 
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 18/18] btrfs-progs: qgroups: don't print dead qgroups

[jeffm]

From: Jeff Mahoney 

When qgroup items get left behind, we still print them in
'btrfs qgroup show' even though there is nothing to show.  Since we
now look up the pathname and that means we look up the subvolume,
we can filter out first-level qgroups that correspond to roots
that have been removed.  Specifying -v will still show them.

Signed-off-by: Jeff Mahoney 
---
 qgroup.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/qgroup.c b/qgroup.c
index 647bc2f3..08e78887 100644
--- a/qgroup.c
+++ b/qgroup.c
@@ -313,6 +313,13 @@ static void print_qgroup_column(struct btrfs_qgroup 
*qgroup,
}
 }
 
+static bool qgroup_target_exists(const struct btrfs_qgroup *qgroup)
+{
+   if (btrfs_qgroup_level(qgroup->qgroupid) > 0)
+   return true;
+   return qgroup->pathname != NULL;
+}
+
 static void print_single_qgroup_table(struct btrfs_qgroup *qgroup, bool 
verbose)
 {
int i;
@@ -1369,7 +1376,8 @@ static void print_all_qgroups(struct qgroup_lookup 
*qgroup_lookup, bool verbose)
n = rb_first(_lookup->root);
while (n) {
entry = rb_entry(n, struct btrfs_qgroup, sort_node);
-   print_single_qgroup_table(entry, verbose);
+   if (qgroup_target_exists(entry) || verbose)
+   print_single_qgroup_table(entry, verbose);
n = rb_next(n);
}
 }
-- 
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 15/18] btrfs-progs: pass cmd_struct to usage()

[jeffm]

From: Jeff Mahoney 

Now that every call site has a cmd_struct, we can just pass the cmd_struct
to usage to print the usager information.  This allows us to interpret
the format flags we'll add later in this series to inform the user of
which output formats any given command supports.

Signed-off-by: Jeff Mahoney 
---
 check/main.c  |  4 ++--
 cmds-balance.c| 14 +++---
 cmds-device.c | 19 +--
 cmds-fi-du.c  |  4 ++--
 cmds-fi-usage.c   |  4 ++--
 cmds-filesystem.c | 18 +-
 cmds-inspect-dump-super.c |  4 ++--
 cmds-inspect-dump-tree.c  |  4 ++--
 cmds-inspect-tree-stats.c |  4 ++--
 cmds-inspect.c| 16 
 cmds-property.c   | 22 +-
 cmds-qgroup.c | 18 +-
 cmds-quota.c  |  8 
 cmds-receive.c|  4 ++--
 cmds-replace.c| 12 ++--
 cmds-rescue.c | 12 ++--
 cmds-restore.c|  8 
 cmds-scrub.c  | 25 ++---
 cmds-send.c   |  2 +-
 cmds-subvolume.c  | 30 +++---
 help.c|  6 +++---
 help.h|  2 +-
 22 files changed, 115 insertions(+), 125 deletions(-)

diff --git a/check/main.c b/check/main.c
index 0375eec3..49ccdf2f 100644
--- a/check/main.c
+++ b/check/main.c
@@ -9483,7 +9483,7 @@ static int cmd_check(const struct cmd_struct *cmd, int 
argc, char **argv)
break;
case '?':
case 'h':
-   usage(cmd_check_usage);
+   usage(cmd);
case GETOPT_VAL_REPAIR:
printf("enabling repair mode\n");
repair = 1;
@@ -9534,7 +9534,7 @@ static int cmd_check(const struct cmd_struct *cmd, int 
argc, char **argv)
}
 
if (check_argc_exact(argc - optind, 1))
-   usage(cmd_check_usage);
+   usage(cmd);
 
if (ctx.progress_enabled) {
ctx.tp = TASK_NOTHING;
diff --git a/cmds-balance.c b/cmds-balance.c
index 488fffcc..c639459f 100644
--- a/cmds-balance.c
+++ b/cmds-balance.c
@@ -585,12 +585,12 @@ static int cmd_balance_start(const struct cmd_struct *cmd,
background = 1;
break;
default:
-   usage(cmd_balance_start_usage);
+   usage(cmd);
}
}
 
if (check_argc_exact(argc - optind, 1))
-   usage(cmd_balance_start_usage);
+   usage(cmd);
 
/*
 * allow -s only under --force, otherwise do with system chunks
@@ -692,7 +692,7 @@ static int cmd_balance_pause(const struct cmd_struct *cmd,
clean_args_no_options(cmd, argc, argv);
 
if (check_argc_exact(argc - optind, 1))
-   usage(cmd_balance_pause_usage);
+   usage(cmd);
 
path = argv[optind];
 
@@ -732,7 +732,7 @@ static int cmd_balance_cancel(const struct cmd_struct *cmd,
clean_args_no_options(cmd, argc, argv);
 
if (check_argc_exact(argc - optind, 1))
-   usage(cmd_balance_cancel_usage);
+   usage(cmd);
 
path = argv[optind];
 
@@ -773,7 +773,7 @@ static int cmd_balance_resume(const struct cmd_struct *cmd,
clean_args_no_options(cmd, argc, argv);
 
if (check_argc_exact(argc - optind, 1))
-   usage(cmd_balance_resume_usage);
+   usage(cmd);
 
path = argv[optind];
 
@@ -856,12 +856,12 @@ static int cmd_balance_status(const struct cmd_struct 
*cmd,
verbose = 1;
break;
default:
-   usage(cmd_balance_status_usage);
+   usage(cmd);
}
}
 
if (check_argc_exact(argc - optind, 1))
-   usage(cmd_balance_status_usage);
+   usage(cmd);
 
path = argv[optind];
 
diff --git a/cmds-device.c b/cmds-device.c
index 5be748f7..6c74ca8e 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -77,12 +77,12 @@ static int cmd_device_add(const struct cmd_struct *cmd,
force = 1;
break;
default:
-   usage(cmd_device_add_usage);
+   usage(cmd);
}
}
 
if (check_argc_min(argc - optind, 2))
-   usage(cmd_device_add_usage);
+   usage(cmd);
 
last_dev = argc - 1;
mntpnt = argv[last_dev];
@@ -149,12 +149,11 @@ static int _cmd_device_remove(const struct cmd_struct 
*cmd,
char*mntpnt;
int i, fdmnt, ret = 0;
DIR *dirstream = NULL;
-   const char * const *usagestr = 

[PATCH 13/18] btrfs-progs: pass cmd_struct to command callback function

[jeffm]

From: Jeff Mahoney 

This patch passes the cmd_struct to the command callback function.  This
has several purposes: It allows the command callback to identify which
command was used to call it.  It also gives us direct access to the
usage associated with that command.

Signed-off-by: Jeff Mahoney 
---
 btrfs.c   |  8 
 check/main.c  |  2 +-
 cmds-balance.c| 19 ---
 cmds-device.c | 35 +++
 cmds-fi-du.c  |  3 ++-
 cmds-fi-usage.c   |  3 ++-
 cmds-filesystem.c | 24 
 cmds-inspect-dump-super.c |  3 ++-
 cmds-inspect-dump-tree.c  |  3 ++-
 cmds-inspect-tree-stats.c |  3 ++-
 cmds-inspect.c| 17 +++--
 cmds-property.c   | 12 
 cmds-qgroup.c | 18 +++---
 cmds-quota.c  |  9 +
 cmds-receive.c|  2 +-
 cmds-replace.c| 11 +++
 cmds-rescue.c | 14 +-
 cmds-restore.c|  2 +-
 cmds-scrub.c  | 23 +++
 cmds-send.c   |  2 +-
 cmds-subvolume.c  | 27 +--
 commands.h|  4 ++--
 22 files changed, 146 insertions(+), 98 deletions(-)

diff --git a/btrfs.c b/btrfs.c
index 1e68b0c0..49128182 100644
--- a/btrfs.c
+++ b/btrfs.c
@@ -148,7 +148,7 @@ static const char * const cmd_help_usage[] = {
NULL
 };
 
-static int cmd_help(int argc, char **argv)
+static int cmd_help(const struct cmd_struct *unused, int argc, char **argv)
 {
help_command_group(_cmd_group, argc, argv);
return 0;
@@ -162,7 +162,7 @@ static const char * const cmd_version_usage[] = {
NULL
 };
 
-static int cmd_version(int argc, char **argv)
+static int cmd_version(const struct cmd_struct *unused, int argc, char **argv)
 {
printf("%s\n", PACKAGE_STRING);
return 0;
@@ -231,13 +231,13 @@ void handle_special_globals(int shift, int argc, char 
**argv)
if (has_full)
usage_command_group(_cmd_group, true, false);
else
-   cmd_help(argc, argv);
+   cmd_execute(_struct_help, argc, argv);
exit(0);
}
 
for (i = 0; i < shift; i++)
if (strcmp(argv[i], "--version") == 0) {
-   cmd_version(argc, argv);
+   cmd_execute(_struct_version, argc, argv);
exit(0);
}
 }
diff --git a/check/main.c b/check/main.c
index a1b685e7..0375eec3 100644
--- a/check/main.c
+++ b/check/main.c
@@ -9392,7 +9392,7 @@ static const char * const cmd_check_usage[] = {
NULL
 };
 
-static int cmd_check(int argc, char **argv)
+static int cmd_check(const struct cmd_struct *cmd, int argc, char **argv)
 {
struct cache_tree root_cache;
struct btrfs_root *root;
diff --git a/cmds-balance.c b/cmds-balance.c
index 7a60be61..1bd7b3ce 100644
--- a/cmds-balance.c
+++ b/cmds-balance.c
@@ -515,7 +515,8 @@ static const char * const cmd_balance_start_usage[] = {
NULL
 };
 
-static int cmd_balance_start(int argc, char **argv)
+static int cmd_balance_start(const struct cmd_struct *cmd,
+int argc, char **argv)
 {
struct btrfs_ioctl_balance_args args;
struct btrfs_balance_args *ptrs[] = { , ,
@@ -680,7 +681,8 @@ static const char * const cmd_balance_pause_usage[] = {
NULL
 };
 
-static int cmd_balance_pause(int argc, char **argv)
+static int cmd_balance_pause(const struct cmd_struct *cmd,
+int argc, char **argv)
 {
const char *path;
int fd;
@@ -719,7 +721,8 @@ static const char * const cmd_balance_cancel_usage[] = {
NULL
 };
 
-static int cmd_balance_cancel(int argc, char **argv)
+static int cmd_balance_cancel(const struct cmd_struct *cmd,
+ int argc, char **argv)
 {
const char *path;
int fd;
@@ -758,7 +761,8 @@ static const char * const cmd_balance_resume_usage[] = {
NULL
 };
 
-static int cmd_balance_resume(int argc, char **argv)
+static int cmd_balance_resume(const struct cmd_struct *cmd,
+ int argc, char **argv)
 {
struct btrfs_ioctl_balance_args args;
const char *path;
@@ -826,7 +830,8 @@ static const char * const cmd_balance_status_usage[] = {
  *   1 : Successful to know status of a pending balance
  *   0 : When there is no pending balance or completed
  */
-static int cmd_balance_status(int argc, char **argv)
+static int cmd_balance_status(const struct cmd_struct *cmd,
+ int argc, char **argv)
 {
struct btrfs_ioctl_balance_args args;
const char *path;
@@ -904,7 +909,7 @@ out:
 }
 static DEFINE_SIMPLE_COMMAND(balance_status, "status");
 
-static int 

[PATCH 04/18] btrfs-progs: btrfs-list: add rb_entry helpers for root_info

[jeffm]

From: Jeff Mahoney 

We use rb_entry all over the place for the root_info pointers.  Add
a helper to make the code more readable.

Signed-off-by: Jeff Mahoney 
---
 btrfs-list.c | 30 --
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/btrfs-list.c b/btrfs-list.c
index e01c5899..90c98be1 100644
--- a/btrfs-list.c
+++ b/btrfs-list.c
@@ -44,6 +44,16 @@ struct root_lookup {
struct rb_root root;
 };
 
+static inline struct root_info *to_root_info(struct rb_node *node)
+{
+   return rb_entry(node, struct root_info, rb_node);
+}
+
+static inline struct root_info *to_root_info_sorted(struct rb_node *node)
+{
+   return rb_entry(node, struct root_info, sort_node);
+}
+
 static struct {
char*name;
char*column_name;
@@ -309,7 +319,7 @@ static int sort_tree_insert(struct root_lookup *sort_tree,
 
while (*p) {
parent = *p;
-   curr = rb_entry(parent, struct root_info, sort_node);
+   curr = to_root_info_sorted(parent);
 
ret = sort_comp(ins, curr, comp_set);
if (ret < 0)
@@ -340,7 +350,7 @@ static int root_tree_insert(struct root_lookup *root_tree,
 
while(*p) {
parent = *p;
-   curr = rb_entry(parent, struct root_info, rb_node);
+   curr = to_root_info(parent);
 
ret = comp_entry_with_rootid(ins, curr, 0);
if (ret < 0)
@@ -371,7 +381,7 @@ static struct root_info *root_tree_search(struct 
root_lookup *root_tree,
tmp.root_id = root_id;
 
while(n) {
-   entry = rb_entry(n, struct root_info, rb_node);
+   entry = to_root_info(n);
 
ret = comp_entry_with_rootid(, entry, 0);
if (ret < 0)
@@ -528,7 +538,7 @@ static void free_root_info(struct rb_node *node)
 {
struct root_info *ri;
 
-   ri = rb_entry(node, struct root_info, rb_node);
+   ri = to_root_info(node);
free(ri->name);
free(ri->path);
free(ri->full_path);
@@ -1268,7 +1278,7 @@ static void filter_and_sort_subvol(struct root_lookup 
*all_subvols,
 
n = rb_last(_subvols->root);
while (n) {
-   entry = rb_entry(n, struct root_info, rb_node);
+   entry = to_root_info(n);
 
ret = resolve_root(all_subvols, entry, top_id);
if (ret == -ENOENT) {
@@ -1300,7 +1310,7 @@ static int list_subvol_fill_paths(int fd, struct 
root_lookup *root_lookup)
while (n) {
struct root_info *entry;
int ret;
-   entry = rb_entry(n, struct root_info, rb_node);
+   entry = to_root_info(n);
ret = lookup_ino_path(fd, entry);
if (ret && ret != -ENOENT)
return ret;
@@ -1467,7 +1477,7 @@ static void print_all_subvol_info(struct root_lookup 
*sorted_tree,
 
n = rb_first(_tree->root);
while (n) {
-   entry = rb_entry(n, struct root_info, sort_node);
+   entry = to_root_info_sorted(n);
 
/* The toplevel subvolume is not listed by default */
if (entry->root_id == BTRFS_FS_TREE_OBJECTID)
@@ -1558,7 +1568,7 @@ int btrfs_get_toplevel_subvol(int fd, struct root_info 
*the_ri)
return ret;
 
rbn = rb_first();
-   ri = rb_entry(rbn, struct root_info, rb_node);
+   ri = to_root_info(rbn);
 
if (ri->root_id != BTRFS_FS_TREE_OBJECTID)
return -ENOENT;
@@ -1590,7 +1600,7 @@ int btrfs_get_subvol(int fd, struct root_info *the_ri)
 
rbn = rb_first();
while(rbn) {
-   ri = rb_entry(rbn, struct root_info, rb_node);
+   ri = to_root_info(rbn);
rr = resolve_root(, ri, root_id);
if (rr == -ENOENT) {
ret = -ENOENT;
@@ -1814,7 +1824,7 @@ char *btrfs_list_path_for_root(int fd, u64 root)
while (n) {
struct root_info *entry;
 
-   entry = rb_entry(n, struct root_info, rb_node);
+   entry = to_root_info(n);
ret = resolve_root(_lookup, entry, top_id);
if (ret == -ENOENT && entry->root_id == root) {
ret_path = NULL;
-- 
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 08/18] btrfs-progs: subvolume: add quota info to btrfs sub show

[jeffm]

From: Jeff Mahoney 

This patch reports on the first-level qgroup, if any, associated with
a particular subvolume.  It displays the usage and limit, subject
to the usual unit parameters.

Signed-off-by: Jeff Mahoney 
---
 cmds-subvolume.c | 51 ++-
 1 file changed, 50 insertions(+), 1 deletion(-)

diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index 45363a5a..d3c8e37c 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -910,6 +910,7 @@ static const char * const cmd_subvol_show_usage[] = {
"Show more information about the subvolume",
"-r|--rootid   rootid of the subvolume",
"-u|--uuid uuid of the subvolume",
+   HELPINFO_UNITS_SHORT_LONG,
"",
"If no option is specified,  will be shown, otherwise",
"the rootid or uuid are resolved relative to the  path.",
@@ -932,6 +933,13 @@ static int cmd_subvol_show(int argc, char **argv)
struct btrfs_util_subvolume_info subvol;
char *subvol_path = NULL;
enum btrfs_util_error err;
+   struct btrfs_qgroup_stats stats;
+   unsigned int unit_mode;
+   const char *referenced_size;
+   const char *referenced_limit_size = "-";
+   unsigned int field_width = 0;
+
+   unit_mode = get_unit_mode_from_arg(, argv, 1);
 
while (1) {
int c;
@@ -1101,7 +1109,48 @@ static int cmd_subvol_show(int argc, char **argv)
}
btrfs_util_destroy_subvolume_iterator(iter);
 
-   ret = 0;
+   ret = btrfs_qgroup_query(fd, subvol.id, );
+   if (ret && ret != -ENOTTY && ret != -ENODATA) {
+   fprintf(stderr,
+   "\nERROR: BTRFS_IOC_QUOTA_QUERY failed: %s\n",
+   strerror(-ret));
+   goto out;
+   }
+
+   printf("\tQuota Usage:\t\t");
+   fflush(stdout);
+   if (ret) {
+   if (ret == -ENOTTY)
+   printf("quotas not enabled\n");
+   else
+   printf("quotas not available\n");
+   goto out;
+   }
+
+   referenced_size = pretty_size_mode(stats.info.referenced, unit_mode);
+   if (stats.limit.max_referenced)
+   referenced_limit_size = pretty_size_mode(
+   stats.limit.max_referenced,
+   unit_mode);
+   field_width = max(strlen(referenced_size),
+ strlen(referenced_limit_size));
+
+   printf("%-*s referenced, %s exclusive\n ", field_width,
+  referenced_size,
+  pretty_size_mode(stats.info.exclusive, unit_mode));
+
+   printf("\tQuota Limits:\t\t");
+   if (stats.limit.max_referenced || stats.limit.max_exclusive) {
+   const char *excl = "-";
+
+   if (stats.limit.max_exclusive)
+   excl = pretty_size_mode(stats.limit.max_exclusive,
+   unit_mode);
+   printf("%-*s referenced, %s exclusive\n", field_width,
+  referenced_limit_size, excl);
+   } else
+   printf("None\n");
+
 out:
free(subvol_path);
close_file_or_dir(fd, dirstream1);
-- 
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 10/18] btrfs-progs: reorder placement of help declarations for send/receive

[jeffm]

From: Jeff Mahoney 

The usage definitions for send and receive follow the command
definitions, which use them.  This works because we declare them
in commands.h.  When we move to using cmd_struct as the entry point,
these declarations will be removed, breaking the commands.  Since
that would be an otherwise unrelated change, this patch reorders
them separately.

Signed-off-by: Jeff Mahoney 
---
 cmds-receive.c | 62 ++--
 cmds-send.c| 69 +-
 2 files changed, 66 insertions(+), 65 deletions(-)

diff --git a/cmds-receive.c b/cmds-receive.c
index 68123a31..b3709f36 100644
--- a/cmds-receive.c
+++ b/cmds-receive.c
@@ -1248,6 +1248,37 @@ out:
return ret;
 }
 
+const char * const cmd_receive_usage[] = {
+   "btrfs receive [options] \n"
+   "btrfs receive --dump [options]",
+   "Receive subvolumes from a stream",
+   "Receives one or more subvolumes that were previously",
+   "sent with btrfs send. The received subvolumes are stored",
+   "into MOUNT.",
+   "The receive will fail in case the receiving subvolume",
+   "already exists. It will also fail in case a previously",
+   "received subvolume has been changed after it was received.",
+   "After receiving a subvolume, it is immediately set to",
+   "read-only.",
+   "",
+   "-v   increase verbosity about performed actions",
+   "-f FILE  read the stream from FILE instead of stdin",
+   "-e   terminate after receiving an  marker in the 
stream.",
+   " Without this option the receiver side terminates only 
in case",
+   " of an error on end of file.",
+   "-C|--chroot  confine the process to  using chroot",
+   "-E|--max-errors NERR",
+   " terminate as soon as NERR errors occur while",
+   " stream processing commands from the stream.",
+   " Default value is 1. A value of 0 means no limit.",
+   "-m ROOTMOUNT the root mount point of the destination filesystem.",
+   " If /proc is not accessible, use this to tell us 
where",
+   " this file system is mounted.",
+   "--dump   dump stream metadata, one line per operation,",
+   " does not require the MOUNT parameter",
+   NULL
+};
+
 int cmd_receive(int argc, char **argv)
 {
char *tomnt = NULL;
@@ -1357,34 +1388,3 @@ out:
 
return !!ret;
 }
-
-const char * const cmd_receive_usage[] = {
-   "btrfs receive [options] \n"
-   "btrfs receive --dump [options]",
-   "Receive subvolumes from a stream",
-   "Receives one or more subvolumes that were previously",
-   "sent with btrfs send. The received subvolumes are stored",
-   "into MOUNT.",
-   "The receive will fail in case the receiving subvolume",
-   "already exists. It will also fail in case a previously",
-   "received subvolume has been changed after it was received.",
-   "After receiving a subvolume, it is immediately set to",
-   "read-only.",
-   "",
-   "-v   increase verbosity about performed actions",
-   "-f FILE  read the stream from FILE instead of stdin",
-   "-e   terminate after receiving an  marker in the 
stream.",
-   " Without this option the receiver side terminates only 
in case",
-   " of an error on end of file.",
-   "-C|--chroot  confine the process to  using chroot",
-   "-E|--max-errors NERR",
-   " terminate as soon as NERR errors occur while",
-   " stream processing commands from the stream.",
-   " Default value is 1. A value of 0 means no limit.",
-   "-m ROOTMOUNT the root mount point of the destination filesystem.",
-   " If /proc is not accessible, use this to tell us 
where",
-   " this file system is mounted.",
-   "--dump   dump stream metadata, one line per operation,",
-   " does not require the MOUNT parameter",
-   NULL
-};
diff --git a/cmds-send.c b/cmds-send.c
index c5ecdaa1..8365e9c9 100644
--- a/cmds-send.c
+++ b/cmds-send.c
@@ -489,6 +489,41 @@ static void free_send_info(struct btrfs_send *sctx)
subvol_uuid_search_finit(>sus);
 }
 
+
+const char * const cmd_send_usage[] = {
+   "btrfs send [-ve] [-p ] [-c ] [-f ] 
 [...]",
+   "Send the subvolume(s) to stdout.",
+   "Sends the subvolume(s) specified by  to stdout.",
+   " should be read-only here.",
+   "By default, this will send the whole subvolume. To do an incremental",
+   "send, use '-p '. If you want to allow btrfs to clone from",
+   "any additional local snapshots, use '-c ' (multiple times",
+   

[PATCH 12/18] btrfs-progs: use cmd_struct as command entry point

[jeffm]

From: Jeff Mahoney 

Rather than having global command usage and callbacks used to create
cmd_structs in the command array, establish the cmd_struct structures
separately and use those.  The next commit in the series passes the
cmd_struct to the command callbacks such that we can access flags
and determine which of several potential command we were called as.

This establishes several macros to more easily define the commands
within each command's source.

Signed-off-by: Jeff Mahoney 
---
 btrfs.c   |  48 ++
 check/main.c  |   5 +-
 cmds-balance.c|  27 ++
 cmds-device.c |  31 ++-
 cmds-fi-du.c  |   5 +-
 cmds-fi-usage.c   |   5 +-
 cmds-filesystem.c |  52 ++-
 cmds-inspect-dump-super.c |   5 +-
 cmds-inspect-dump-tree.c  |   5 +-
 cmds-inspect-tree-stats.c |   5 +-
 cmds-inspect.c|  36 +++--
 cmds-property.c   |  19 +++
 cmds-qgroup.c |  31 +--
 cmds-quota.c  |  17 ---
 cmds-receive.c|   5 +-
 cmds-replace.c|  19 +++
 cmds-rescue.c |  22 
 cmds-restore.c|   5 +-
 cmds-scrub.c  |  20 +---
 cmds-send.c   |   6 +--
 cmds-subvolume.c  |  38 --
 commands.h| 127 +++---
 help.c|  21 +---
 23 files changed, 317 insertions(+), 237 deletions(-)

diff --git a/btrfs.c b/btrfs.c
index fec1a135..1e68b0c0 100644
--- a/btrfs.c
+++ b/btrfs.c
@@ -42,10 +42,11 @@ static inline const char *skip_prefix(const char *str, 
const char *prefix)
 static int parse_one_token(const char *arg, const struct cmd_group *grp,
   const struct cmd_struct **cmd_ret)
 {
-   const struct cmd_struct *cmd = grp->commands;
const struct cmd_struct *abbrev_cmd = NULL, *ambiguous_cmd = NULL;
+   int i = 0;
 
-   for (; cmd->token; cmd++) {
+   for (i = 0; grp->commands[i]; i++) {
+   const struct cmd_struct *cmd = grp->commands[i];
const char *rest;
 
rest = skip_prefix(arg, cmd->token);
@@ -134,7 +135,7 @@ int handle_command_group(const struct cmd_group *grp, int 
argc,
handle_help_options_next_level(cmd, argc, argv);
 
fixup_argv0(argv, cmd->token);
-   return cmd->fn(argc, argv);
+   return cmd_execute(cmd, argc, argv);
 }
 
 static const struct cmd_group btrfs_cmd_group;
@@ -153,6 +154,8 @@ static int cmd_help(int argc, char **argv)
return 0;
 }
 
+static DEFINE_SIMPLE_COMMAND(help, "help");
+
 static const char * const cmd_version_usage[] = {
"btrfs version",
"Display btrfs-progs version",
@@ -164,6 +167,7 @@ static int cmd_version(int argc, char **argv)
printf("%s\n", PACKAGE_STRING);
return 0;
 }
+static DEFINE_SIMPLE_COMMAND(version, "version");
 
 /*
  * Parse global options, between binary name and first non-option argument
@@ -240,24 +244,24 @@ void handle_special_globals(int shift, int argc, char 
**argv)
 
 static const struct cmd_group btrfs_cmd_group = {
btrfs_cmd_group_usage, btrfs_cmd_group_info, {
-   { "subvolume", cmd_subvolume, NULL, _cmd_group, 0 },
-   { "filesystem", cmd_filesystem, NULL, _cmd_group, 0 
},
-   { "balance", cmd_balance, NULL, _cmd_group, 0 },
-   { "device", cmd_device, NULL, _cmd_group, 0 },
-   { "scrub", cmd_scrub, NULL, _cmd_group, 0 },
-   { "check", cmd_check, cmd_check_usage, NULL, 0 },
-   { "rescue", cmd_rescue, NULL, _cmd_group, 0 },
-   { "restore", cmd_restore, cmd_restore_usage, NULL, 0 },
-   { "inspect-internal", cmd_inspect, NULL, _cmd_group, 0 
},
-   { "property", cmd_property, NULL, _cmd_group, 0 },
-   { "send", cmd_send, cmd_send_usage, NULL, 0 },
-   { "receive", cmd_receive, cmd_receive_usage, NULL, 0 },
-   { "quota", cmd_quota, NULL, _cmd_group, 0 },
-   { "qgroup", cmd_qgroup, NULL, _cmd_group, 0 },
-   { "replace", cmd_replace, NULL, _cmd_group, 0 },
-   { "help", cmd_help, cmd_help_usage, NULL, 0 },
-   { "version", cmd_version, cmd_version_usage, NULL, 0 },
-   NULL_CMD_STRUCT
+   _struct_subvolume,
+   _struct_filesystem,
+   _struct_balance,
+   _struct_device,
+   _struct_scrub,
+   _struct_check,
+   _struct_rescue,
+   _struct_restore,
+   _struct_inspect,
+   _struct_property,
+   _struct_send,
+   _struct_receive,
+   _struct_quota,
+   _struct_qgroup,
+   _struct_replace,
+   _struct_help,
+   

[PATCH 17/18] btrfs-progs: handle command groups directly for common case

[jeffm]

From: Jeff Mahoney 

Most command groups just pass their own command group to
handle_command_group.  We can remove the explicit definitions
of command group callbacks by passing the cmd_struct to
handle_command_group and allowing it to resolve the group from it.

Signed-off-by: Jeff Mahoney 
---
 btrfs.c   | 14 +++---
 cmds-balance.c|  6 +++---
 cmds-device.c |  5 -
 cmds-filesystem.c |  6 --
 cmds-inspect.c|  5 -
 cmds-property.c   |  6 --
 cmds-qgroup.c |  5 -
 cmds-quota.c  |  5 -
 cmds-replace.c|  5 -
 cmds-rescue.c |  5 -
 cmds-scrub.c  |  6 --
 cmds-subvolume.c  |  5 -
 commands.h|  7 ---
 13 files changed, 14 insertions(+), 66 deletions(-)

diff --git a/btrfs.c b/btrfs.c
index 32b8e090..427e14c8 100644
--- a/btrfs.c
+++ b/btrfs.c
@@ -140,26 +140,26 @@ static void handle_help_options_next_level(const struct 
cmd_struct *cmd,
}
 }
 
-int handle_command_group(const struct cmd_group *grp,
+int handle_command_group(const struct cmd_struct *cmd,
 const struct cmd_context *cmdcxt,
 int argc, char **argv)
 
 {
-   const struct cmd_struct *cmd;
+   const struct cmd_struct *subcmd;
 
argc--;
argv++;
if (argc < 1) {
-   usage_command_group(grp, false, false);
+   usage_command_group(cmd->next, false, false);
exit(1);
}
 
-   cmd = parse_command_token(argv[0], grp);
+   subcmd = parse_command_token(argv[0], cmd->next);
 
-   handle_help_options_next_level(cmd, cmdcxt, argc, argv);
+   handle_help_options_next_level(subcmd, cmdcxt, argc, argv);
 
-   fixup_argv0(argv, cmd->token);
-   return cmd_execute(cmd, cmdcxt, argc, argv);
+   fixup_argv0(argv, subcmd->token);
+   return cmd_execute(subcmd, cmdcxt, argc, argv);
 }
 
 static const struct cmd_group btrfs_cmd_group;
diff --git a/cmds-balance.c b/cmds-balance.c
index c17b9ee3..e414ca27 100644
--- a/cmds-balance.c
+++ b/cmds-balance.c
@@ -943,7 +943,7 @@ static const struct cmd_group balance_cmd_group = {
}
 };
 
-static int cmd_balance(const struct cmd_struct *unused,
+static int cmd_balance(const struct cmd_struct *cmd,
   const struct cmd_context *cmdcxt, int argc, char **argv)
 {
if (argc == 2 && strcmp("start", argv[1]) != 0) {
@@ -956,7 +956,7 @@ static int cmd_balance(const struct cmd_struct *unused,
return do_balance(argv[1], , 0);
}
 
-   return handle_command_group(_cmd_group, cmdcxt, argc, argv);
+   return handle_command_group(cmd, cmdcxt, argc, argv);
 }
 
-DEFINE_GROUP_COMMAND(balance, "balance");
+DEFINE_COMMAND(balance, "balance", cmd_balance, NULL, _cmd_group, 0, 
0);
diff --git a/cmds-device.c b/cmds-device.c
index ac9e82b1..f8c0ff20 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -630,9 +630,4 @@ static const struct cmd_group device_cmd_group = {
}
 };
 
-static int cmd_device(const struct cmd_struct *unused,
- const struct cmd_context *cmdcxt, int argc, char **argv)
-{
-   return handle_command_group(_cmd_group, cmdcxt, argc, argv);
-}
 DEFINE_GROUP_COMMAND_TOKEN(device);
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 6701b16f..24852ec6 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -1235,10 +1235,4 @@ static const struct cmd_group filesystem_cmd_group = {
}
 };
 
-static int cmd_filesystem(const struct cmd_struct *unused,
- const struct cmd_context *cmdcxt,
- int argc, char **argv)
-{
-   return handle_command_group(_cmd_group, cmdcxt, argc, argv);
-}
 DEFINE_GROUP_COMMAND_TOKEN(filesystem);
diff --git a/cmds-inspect.c b/cmds-inspect.c
index eecfd7f9..22c5a5d6 100644
--- a/cmds-inspect.c
+++ b/cmds-inspect.c
@@ -658,9 +658,4 @@ static const struct cmd_group inspect_cmd_group = {
}
 };
 
-static int cmd_inspect(const struct cmd_struct *unused,
-  const struct cmd_context *cmdcxt, int argc, char **argv)
-{
-   return handle_command_group(_cmd_group, cmdcxt, argc, argv);
-}
 DEFINE_GROUP_COMMAND(inspect, "inspect-internal");
diff --git a/cmds-property.c b/cmds-property.c
index 498fa456..58f6c48a 100644
--- a/cmds-property.c
+++ b/cmds-property.c
@@ -422,10 +422,4 @@ static const struct cmd_group property_cmd_group = {
}
 };
 
-static int cmd_property(const struct cmd_struct *unused,
-   const struct cmd_context *cmdcxt,
-   int argc, char **argv)
-{
-   return handle_command_group(_cmd_group, cmdcxt, argc, argv);
-}
 DEFINE_GROUP_COMMAND_TOKEN(property);
diff --git a/cmds-qgroup.c b/cmds-qgroup.c
index 9325b568..ce9aa1c6 100644
--- a/cmds-qgroup.c
+++ b/cmds-qgroup.c
@@ -541,9 +541,4 @@ static const struct cmd_group qgroup_cmd_group = {
}
 };
 
-static int cmd_qgroup(const 

[PATCH 09/18] btrfs-progs: help: convert ints used as bools to bool

[jeffm]

From: Jeff Mahoney 

We use an int for 'full', 'all', and 'err' when we really mean a boolean.

Reviewed-by: Qu Wenruo 
Signed-off-by: Jeff Mahoney 
---
 btrfs.c | 14 +++---
 help.c  | 25 +
 help.h  |  4 ++--
 3 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/btrfs.c b/btrfs.c
index 2d39f2ce..fec1a135 100644
--- a/btrfs.c
+++ b/btrfs.c
@@ -109,7 +109,7 @@ static void handle_help_options_next_level(const struct 
cmd_struct *cmd,
argv++;
help_command_group(cmd->next, argc, argv);
} else {
-   usage_command(cmd, 1, 0);
+   usage_command(cmd, true, false);
}
 
exit(0);
@@ -125,7 +125,7 @@ int handle_command_group(const struct cmd_group *grp, int 
argc,
argc--;
argv++;
if (argc < 1) {
-   usage_command_group(grp, 0, 0);
+   usage_command_group(grp, false, false);
exit(1);
}
 
@@ -212,20 +212,20 @@ static int handle_global_options(int argc, char **argv)
 
 void handle_special_globals(int shift, int argc, char **argv)
 {
-   int has_help = 0;
-   int has_full = 0;
+   bool has_help = false;
+   bool has_full = false;
int i;
 
for (i = 0; i < shift; i++) {
if (strcmp(argv[i], "--help") == 0)
-   has_help = 1;
+   has_help = true;
else if (strcmp(argv[i], "--full") == 0)
-   has_full = 1;
+   has_full = true;
}
 
if (has_help) {
if (has_full)
-   usage_command_group(_cmd_group, 1, 0);
+   usage_command_group(_cmd_group, true, false);
else
cmd_help(argc, argv);
exit(0);
diff --git a/help.c b/help.c
index f1dd3946..99fd325b 100644
--- a/help.c
+++ b/help.c
@@ -196,8 +196,8 @@ static int do_usage_one_command(const char * const 
*usagestr,
 }
 
 static int usage_command_internal(const char * const *usagestr,
- const char *token, int full, int lst,
- int alias, FILE *outf)
+ const char *token, bool full, bool lst,
+ bool alias, FILE *outf)
 {
unsigned int flags = 0;
int ret;
@@ -223,17 +223,17 @@ static int usage_command_internal(const char * const 
*usagestr,
 }
 
 static void usage_command_usagestr(const char * const *usagestr,
-  const char *token, int full, int err)
+  const char *token, bool full, bool err)
 {
FILE *outf = err ? stderr : stdout;
int ret;
 
-   ret = usage_command_internal(usagestr, token, full, 0, 0, outf);
+   ret = usage_command_internal(usagestr, token, full, false, false, outf);
if (!ret)
fputc('\n', outf);
 }
 
-void usage_command(const struct cmd_struct *cmd, int full, int err)
+void usage_command(const struct cmd_struct *cmd, bool full, bool err)
 {
usage_command_usagestr(cmd->usagestr, cmd->token, full, err);
 }
@@ -241,11 +241,11 @@ void usage_command(const struct cmd_struct *cmd, int 
full, int err)
 __attribute__((noreturn))
 void usage(const char * const *usagestr)
 {
-   usage_command_usagestr(usagestr, NULL, 1, 1);
+   usage_command_usagestr(usagestr, NULL, true, true);
exit(1);
 }
 
-static void usage_command_group_internal(const struct cmd_group *grp, int full,
+static void usage_command_group_internal(const struct cmd_group *grp, bool 
full,
 FILE *outf)
 {
const struct cmd_struct *cmd = grp->commands;
@@ -265,7 +265,8 @@ static void usage_command_group_internal(const struct 
cmd_group *grp, int full,
}
 
usage_command_internal(cmd->usagestr, cmd->token, full,
-  1, cmd->flags & CMD_ALIAS, outf);
+  true, cmd->flags & CMD_ALIAS,
+  outf);
if (cmd->flags & CMD_ALIAS)
putchar('\n');
continue;
@@ -327,7 +328,7 @@ void usage_command_group_short(const struct cmd_group *grp)
fprintf(stderr, "All command groups have their manual page named 
'btrfs-'.\n");
 }
 
-void usage_command_group(const struct cmd_group *grp, int full, int err)
+void usage_command_group(const struct cmd_group *grp, bool full, bool err)
 {
const char * const *usagestr = grp->usagestr;
FILE *outf = err ? stderr : stdout;
@@ -350,7 +351,7 @@ __attribute__((noreturn))
 void help_unknown_token(const char *arg, const struct cmd_group *grp)
 {

[PATCH 07/18] btrfs-progs: qgroups: introduce btrfs_qgroup_query

[jeffm]

From: Jeff Mahoney 

The only mechanism we have in the progs for searching qgroups is to load
all of them and filter the results.  This works for qgroup show but
to add quota information to 'btrfs subvoluem show' it's pretty wasteful.

This patch splits out setting up the search and performing the search so
we can search for a single qgroupid more easily.  Since TREE_SEARCH
will give results that don't strictly match the search terms, we add
a filter to match only the results we care about.

Reviewed-by: Qu Wenruo 
Signed-off-by: Jeff Mahoney 
---
 qgroup.c | 143 ---
 qgroup.h |   7 
 2 files changed, 116 insertions(+), 34 deletions(-)

diff --git a/qgroup.c b/qgroup.c
index 247f1bfe..647bc2f3 100644
--- a/qgroup.c
+++ b/qgroup.c
@@ -1162,11 +1162,30 @@ static inline void print_status_flag_warning(u64 flags)
warning("qgroup data inconsistent, rescan recommended");
 }
 
-static int __qgroups_search(int fd, struct qgroup_lookup *qgroup_lookup)
+static bool key_in_range(const struct btrfs_key *key,
+const struct btrfs_ioctl_search_key *sk)
+{
+   if (key->objectid < sk->min_objectid ||
+   key->objectid > sk->max_objectid)
+   return false;
+
+   if (key->type < sk->min_type ||
+   key->type > sk->max_type)
+   return false;
+
+   if (key->offset < sk->min_offset ||
+   key->offset > sk->max_offset)
+   return false;
+
+   return true;
+}
+
+static int __qgroups_search(int fd, struct btrfs_ioctl_search_args *args,
+   struct qgroup_lookup *qgroup_lookup)
 {
int ret;
-   struct btrfs_ioctl_search_args args;
-   struct btrfs_ioctl_search_key *sk = 
+   struct btrfs_ioctl_search_key *sk = >key;
+   struct btrfs_ioctl_search_key filter_key = args->key;
struct btrfs_ioctl_search_header *sh;
unsigned long off = 0;
unsigned int i;
@@ -1177,30 +1196,15 @@ static int __qgroups_search(int fd, struct 
qgroup_lookup *qgroup_lookup)
u64 qgroupid;
u64 child, parent;
 
-   memset(, 0, sizeof(args));
-
-   sk->tree_id = BTRFS_QUOTA_TREE_OBJECTID;
-   sk->max_type = BTRFS_QGROUP_RELATION_KEY;
-   sk->min_type = BTRFS_QGROUP_STATUS_KEY;
-   sk->max_objectid = (u64)-1;
-   sk->max_offset = (u64)-1;
-   sk->max_transid = (u64)-1;
-   sk->nr_items = 4096;
-
qgroup_lookup_init(qgroup_lookup);
 
while (1) {
-   ret = ioctl(fd, BTRFS_IOC_TREE_SEARCH, );
+   ret = ioctl(fd, BTRFS_IOC_TREE_SEARCH, args);
if (ret < 0) {
-   if (errno == ENOENT) {
-   error("can't list qgroups: quotas not enabled");
+   if (errno == ENOENT)
ret = -ENOTTY;
-   } else {
-   error("can't list qgroups: %s",
-  strerror(errno));
+   else
ret = -errno;
-   }
-
break;
}
 
@@ -1214,37 +1218,46 @@ static int __qgroups_search(int fd, struct 
qgroup_lookup *qgroup_lookup)
 * read the root_ref item it contains
 */
for (i = 0; i < sk->nr_items; i++) {
-   sh = (struct btrfs_ioctl_search_header *)(args.buf +
+   struct btrfs_key key;
+
+   sh = (struct btrfs_ioctl_search_header *)(args->buf +
  off);
off += sizeof(*sh);
 
-   switch (btrfs_search_header_type(sh)) {
+   key.objectid = btrfs_search_header_objectid(sh);
+   key.type = btrfs_search_header_type(sh);
+   key.offset = btrfs_search_header_offset(sh);
+
+   if (!key_in_range(, _key))
+   goto next;
+
+   switch (key.type) {
case BTRFS_QGROUP_STATUS_KEY:
si = (struct btrfs_qgroup_status_item *)
-(args.buf + off);
+(args->buf + off);
flags = btrfs_stack_qgroup_status_flags(si);
 
print_status_flag_warning(flags);
break;
case BTRFS_QGROUP_INFO_KEY:
-   qgroupid = btrfs_search_header_offset(sh);
+   qgroupid = key.offset;
info = (struct btrfs_qgroup_info_item *)
-  (args.buf + off);
+  

[PATCH 06/18] btrfs-progs: qgroups: introduce and use info and limit structures

[jeffm]

From: Jeff Mahoney 

We use structures to pass the info and limit from the kernel as items
but store the individual values separately in btrfs_qgroup.  We already
have a btrfs_qgroup_limit structure that's used for setting the limit.

This patch introduces a btrfs_qgroup_info structure and uses that and
btrfs_qgroup_limit in btrfs_qgroup.

Reviewed-by: Qu Wenruo 
Signed-off-by: Jeff Mahoney 
---
 qgroup.c | 82 ++--
 qgroup.h |  8 +++
 2 files changed, 52 insertions(+), 38 deletions(-)

diff --git a/qgroup.c b/qgroup.c
index d8baca33..247f1bfe 100644
--- a/qgroup.c
+++ b/qgroup.c
@@ -47,20 +47,12 @@ struct btrfs_qgroup {
/*
 * info_item
 */
-   u64 generation;
-   u64 rfer;   /*referenced*/
-   u64 rfer_cmpr;  /*referenced compressed*/
-   u64 excl;   /*exclusive*/
-   u64 excl_cmpr;  /*exclusive compressed*/
+   struct btrfs_qgroup_info info;
 
/*
 *limit_item
 */
-   u64 flags;  /*which limits are set*/
-   u64 max_rfer;
-   u64 max_excl;
-   u64 rsv_rfer;
-   u64 rsv_excl;
+   struct btrfs_qgroup_limit limit;
 
/*qgroups this group is member of*/
struct list_head qgroups;
@@ -262,6 +254,11 @@ void print_pathname_column(struct btrfs_qgroup *qgroup, 
bool verbose)
fputs("", stdout);
 }
 
+static int print_u64(u64 value, int unit_mode, int max_len)
+{
+   return printf("%*s", max_len, pretty_size_mode(value, unit_mode));
+}
+
 static void print_qgroup_column(struct btrfs_qgroup *qgroup,
enum btrfs_qgroup_column_enum column,
bool verbose)
@@ -281,24 +278,26 @@ static void print_qgroup_column(struct btrfs_qgroup 
*qgroup,
print_qgroup_column_add_blank(BTRFS_QGROUP_QGROUPID, len);
break;
case BTRFS_QGROUP_RFER:
-   len = printf("%*s", max_len, pretty_size_mode(qgroup->rfer, 
unit_mode));
+   len = print_u64(qgroup->info.referenced, unit_mode, max_len);
break;
case BTRFS_QGROUP_EXCL:
-   len = printf("%*s", max_len, pretty_size_mode(qgroup->excl, 
unit_mode));
+   len = print_u64(qgroup->info.exclusive, unit_mode, max_len);
break;
case BTRFS_QGROUP_PARENT:
len = print_parent_column(qgroup);
print_qgroup_column_add_blank(BTRFS_QGROUP_PARENT, len);
break;
case BTRFS_QGROUP_MAX_RFER:
-   if (qgroup->flags & BTRFS_QGROUP_LIMIT_MAX_RFER)
-   len = printf("%*s", max_len, 
pretty_size_mode(qgroup->max_rfer, unit_mode));
+   if (qgroup->limit.flags & BTRFS_QGROUP_LIMIT_MAX_RFER)
+   len = print_u64(qgroup->limit.max_referenced,
+   unit_mode, max_len);
else
len = printf("%*s", max_len, "none");
break;
case BTRFS_QGROUP_MAX_EXCL:
-   if (qgroup->flags & BTRFS_QGROUP_LIMIT_MAX_EXCL)
-   len = printf("%*s", max_len, 
pretty_size_mode(qgroup->max_excl, unit_mode));
+   if (qgroup->limit.flags & BTRFS_QGROUP_LIMIT_MAX_EXCL)
+   len = print_u64(qgroup->limit.max_exclusive,
+   unit_mode, max_len);
else
len = printf("%*s", max_len, "none");
break;
@@ -441,9 +440,9 @@ static int comp_entry_with_rfer(struct btrfs_qgroup *entry1,
 {
int ret;
 
-   if (entry1->rfer > entry2->rfer)
+   if (entry1->info.referenced > entry2->info.referenced)
ret = 1;
-   else if (entry1->rfer < entry2->rfer)
+   else if (entry1->info.referenced < entry2->info.referenced)
ret = -1;
else
ret = 0;
@@ -457,9 +456,9 @@ static int comp_entry_with_excl(struct btrfs_qgroup *entry1,
 {
int ret;
 
-   if (entry1->excl > entry2->excl)
+   if (entry1->info.exclusive > entry2->info.exclusive)
ret = 1;
-   else if (entry1->excl < entry2->excl)
+   else if (entry1->info.exclusive < entry2->info.exclusive)
ret = -1;
else
ret = 0;
@@ -473,9 +472,9 @@ static int comp_entry_with_max_rfer(struct btrfs_qgroup 
*entry1,
 {
int ret;
 
-   if (entry1->max_rfer > entry2->max_rfer)
+   if (entry1->limit.max_referenced > entry2->limit.max_referenced)
ret = 1;
-   else if (entry1->max_rfer < entry2->max_rfer)
+   else if (entry1->limit.max_referenced < entry2->limit.max_referenced)
ret = -1;
else
ret = 0;
@@ -489,9 +488,9 @@ static int comp_entry_with_max_excl(struct btrfs_qgroup 
*entry1,
 {
int ret;
 
-   if 

[PATCH 16/18] btrfs-progs: add support for output formats

[jeffm]

From: Jeff Mahoney 

This adds a global --format option to request extended output formats
from each command.  Most of it is plumbing a new cmd_context structure
that's established at the beginning of argument parsing into the command
callbacks.  That structure currently only contains the output mode enum.

We currently only support text mode.  Command help reports what
output formats are available for each command.  Global help reports
what valid formats are.

If an invalid format is requested, an error is reported and we global usage
is dumped that lists the valid formats.

Each command sets a bitmask that describes which formats it is capable
of outputting.  If a globally valid format is requested of a command
that doesn't support it, an error is reported and command usage dumped.

Commands don't need to specify that they support text output.  All
commands are required to output text.

Signed-off-by: Jeff Mahoney 
---
 btrfs.c   | 110 ++
 check/main.c  |   3 +-
 cmds-balance.c|  16 +--
 cmds-device.c |  31 +
 cmds-fi-du.c  |   1 +
 cmds-fi-usage.c   |   1 +
 cmds-filesystem.c |  14 --
 cmds-inspect-dump-super.c |   1 +
 cmds-inspect-dump-tree.c  |   1 +
 cmds-inspect-tree-stats.c |   1 +
 cmds-inspect.c|  10 -
 cmds-property.c   |   6 ++-
 cmds-qgroup.c |  17 +--
 cmds-quota.c  |  14 --
 cmds-receive.c|   3 +-
 cmds-replace.c|   8 +++-
 cmds-rescue.c |   9 +++-
 cmds-restore.c|   3 +-
 cmds-scrub.c  |  21 ++---
 cmds-send.c   |   3 +-
 cmds-subvolume.c  |  24 +++---
 commands.h|  32 +++---
 help.c|  51 -
 help.h|   2 +
 24 files changed, 299 insertions(+), 83 deletions(-)

diff --git a/btrfs.c b/btrfs.c
index 49128182..32b8e090 100644
--- a/btrfs.c
+++ b/btrfs.c
@@ -26,7 +26,7 @@
 #include "help.h"
 
 static const char * const btrfs_cmd_group_usage[] = {
-   "btrfs [--help] [--version]  [...]  []",
+   "btrfs [--help] [--version] [--format ]  [...] 
 []",
NULL
 };
 
@@ -98,13 +98,36 @@ parse_command_token(const char *arg, const struct cmd_group 
*grp)
return cmd;
 }
 
+static bool cmd_provides_output_format(const struct cmd_struct *cmd,
+  const struct cmd_context *cmdcxt)
+{
+   if (!cmdcxt->output_mode)
+   return true;
+
+   return (1 << cmdcxt->output_mode) & cmd->cmd_format_flags;
+}
+
 static void handle_help_options_next_level(const struct cmd_struct *cmd,
-   int argc, char **argv)
+  const struct cmd_context *cmdcxt,
+  int argc, char **argv)
 {
+   int err = 0;
+
if (argc < 2)
return;
 
-   if (!strcmp(argv[1], "--help")) {
+   /* Check if the command can provide the requested output format */
+   if (!cmd->next && !cmd_provides_output_format(cmd, cmdcxt)) {
+   ASSERT(cmdcxt->output_mode >= 0);
+   ASSERT(cmdcxt->output_mode < CMD_OUTPUT_MAX);
+   fprintf(stderr,
+   "error: %s output is unsupported for this command.\n\n",
+   cmd_outputs[cmdcxt->output_mode]);
+
+   err = 1;
+   }
+
+   if (!strcmp(argv[1], "--help") || err) {
if (cmd->next) {
argc--;
argv++;
@@ -113,12 +136,13 @@ static void handle_help_options_next_level(const struct 
cmd_struct *cmd,
usage_command(cmd, true, false);
}
 
-   exit(0);
+   exit(err);
}
 }
 
-int handle_command_group(const struct cmd_group *grp, int argc,
-char **argv)
+int handle_command_group(const struct cmd_group *grp,
+const struct cmd_context *cmdcxt,
+int argc, char **argv)
 
 {
const struct cmd_struct *cmd;
@@ -132,10 +156,10 @@ int handle_command_group(const struct cmd_group *grp, int 
argc,
 
cmd = parse_command_token(argv[0], grp);
 
-   handle_help_options_next_level(cmd, argc, argv);
+   handle_help_options_next_level(cmd, cmdcxt, argc, argv);
 
fixup_argv0(argv, cmd->token);
-   return cmd_execute(cmd, argc, argv);
+   return cmd_execute(cmd, cmdcxt, argc, argv);
 }
 
 static const struct cmd_group btrfs_cmd_group;
@@ -148,7 +172,8 @@ static const char * const cmd_help_usage[] = {
NULL
 };
 
-static int cmd_help(const struct cmd_struct *unused, int argc, char **argv)
+static int cmd_help(const struct cmd_struct *unused,
+   const struct cmd_context *cmdcxt, int argc, char **argv)

[PATCH 02/18] btrfs-progs: qgroups: fix misleading index check

[jeffm]

From: Jeff Mahoney 

In print_single_qgroup_table we check the loop index against
BTRFS_QGROUP_CHILD, but what we really mean is "last column."  Since
we have an enum value to indicate the last value, use that instead
of assuming that BTRFS_QGROUP_CHILD is always last.

Reviewed-by: Qu Wenruo 
Reviewed-by: Nikolay Borisov 
Signed-off-by: Jeff Mahoney 
---
 qgroup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qgroup.c b/qgroup.c
index 267cd7f1..3269feb2 100644
--- a/qgroup.c
+++ b/qgroup.c
@@ -267,7 +267,7 @@ static void print_single_qgroup_table(struct btrfs_qgroup 
*qgroup)
continue;
print_qgroup_column(qgroup, i);
 
-   if (i != BTRFS_QGROUP_CHILD)
+   if (i != BTRFS_QGROUP_ALL - 1)
printf(" ");
}
printf("\n");
-- 
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/5] generic: enable swapfile tests on Btrfs

[Omar Sandoval]

From: Omar Sandoval 

Commit 8c96cfbfe530 ("generic/35[67]: disable swapfile tests on Btrfs")
disabled the swapfile tests on Btrfs because it did not support
swapfiles at the time. Now that we're adding support, we want these
tests to run, but they don't. _require_scratch_swapfile always fails for
Btrfs because swapfiles on Btrfs must be set to nocow. After fixing
that, generic/356 and generic/357 fail for the same reason. After fixing
_that_, both tests still fail because we don't allow reflinking a
non-checksummed extent (which nocow implies) to a checksummed extent.
Add a helper for formatting a swap file which does the chattr, and
chattr the second file, which gets these tests running on kernels
supporting Btrfs swapfiles.

Signed-off-by: Omar Sandoval 
---
 common/rc | 15 ---
 tests/generic/356 |  7 ---
 tests/generic/357 |  6 +++---
 3 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/common/rc b/common/rc
index ffe53236..814b8b5c 100644
--- a/common/rc
+++ b/common/rc
@@ -,6 +,17 @@ _require_odirect()
rm -f $testfile 2>&1 > /dev/null
 }
 
+_format_swapfile() {
+   local fname="$1"
+   local sz="$2"
+
+   touch "$fname"
+   chmod 0600 "$fname"
+   $CHATTR_PROG +C "$fname" > /dev/null 2>&1
+   _pwrite_byte 0x61 0 "$sz" "$fname" >> $seqres.full
+   mkswap -U 27376b42-ff65-42ca-919f-6c9b62292a5c "$fname" >> $seqres.full
+}
+
 # Check that the filesystem supports swapfiles
 _require_scratch_swapfile()
 {
@@ -2231,10 +2242,8 @@ _require_scratch_swapfile()
_scratch_mount
 
# Minimum size for mkswap is 10 pages
-   local size=$(($(get_page_size) * 10))
+   _format_swapfile "$SCRATCH_MNT/swap" $(($(get_page_size) * 10))
 
-   _pwrite_byte 0x61 0 "$size" "$SCRATCH_MNT/swap" >/dev/null 2>&1
-   mkswap "$SCRATCH_MNT/swap" >/dev/null 2>&1
if ! swapon "$SCRATCH_MNT/swap" >/dev/null 2>&1; then
_scratch_unmount
_notrun "swapfiles are not supported"
diff --git a/tests/generic/356 b/tests/generic/356
index 51eeb652..b4a38f84 100755
--- a/tests/generic/356
+++ b/tests/generic/356
@@ -59,11 +59,12 @@ blocks=160
 blksz=65536
 
 echo "Initialize file"
-echo >> $seqres.full
-_pwrite_byte 0x61 0 $((blocks * blksz)) $testdir/file1 >> $seqres.full
-mkswap -U 27376b42-ff65-42ca-919f-6c9b62292a5c $testdir/file1 >> $seqres.full
+_format_swapfile "$testdir/file1" $((blocks * blksz))
 swapon $testdir/file1
 
+touch "$testdir/file2"
+$CHATTR_PROG +C "$testdir/file2" >/dev/null 2>&1
+
 echo "Try to reflink"
 _cp_reflink $testdir/file1 $testdir/file2 2>&1 | _filter_scratch
 
diff --git a/tests/generic/357 b/tests/generic/357
index 0dd0c10f..9a83a283 100755
--- a/tests/generic/357
+++ b/tests/generic/357
@@ -59,9 +59,9 @@ blocks=160
 blksz=65536
 
 echo "Initialize file"
-echo >> $seqres.full
-_pwrite_byte 0x61 0 $((blocks * blksz)) $testdir/file1 >> $seqres.full
-mkswap -U 27376b42-ff65-42ca-919f-6c9b62292a5c $testdir/file1 >> $seqres.full
+_format_swapfile "$testdir/file1" $((blocks * blksz))
+touch "$testdir/file2"
+$CHATTR_PROG +C "$testdir/file2" >/dev/null 2>&1
 _cp_reflink $testdir/file1 $testdir/file2 2>&1 | _filter_scratch
 
 echo "Try to swapon"
-- 
2.17.0

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/5] xfstests: create swap group

[Omar Sandoval]

From: Omar Sandoval 

I'm going to add a bunch of tests for swap files, so create a group for
them and add the existing tests.

Signed-off-by: Omar Sandoval 
---
 tests/generic/group | 4 ++--
 tests/xfs/group | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/generic/group b/tests/generic/group
index dc637c96..cded10f8 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -358,8 +358,8 @@
 353 auto quick clone
 354 auto
 355 auto quick
-356 auto quick clone
-357 auto quick clone
+356 auto quick clone swap
+357 auto quick clone swap
 358 auto quick clone
 359 auto quick clone
 360 auto quick metadata
diff --git a/tests/xfs/group b/tests/xfs/group
index c2e6677e..dff8b99f 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -416,7 +416,7 @@
 416 dangerous_fuzzers dangerous_scrub dangerous_repair
 417 dangerous_fuzzers dangerous_scrub dangerous_online_repair
 418 dangerous_fuzzers dangerous_scrub dangerous_repair
-419 auto quick
+419 auto quick swap
 420 auto quick clone dedupe
 421 auto quick clone dedupe
 422 dangerous_scrub dangerous_online_repair
-- 
2.17.0

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 0/5] xfstests: generic swap file tests

[Omar Sandoval]

From: Omar Sandoval 

Changes since v1:

- Add patch 1 to create a group for swap
- Add a helper for formatting a swap file instead of open coding
  everywhere
- Use $CHATTR_PROG instead of chattr in a few places that I forgot

Thanks!

Omar Sandoval (5):
  xfstests: create swap group
  generic: enable swapfile tests on Btrfs
  generic: add test for dedupe on an active swapfile
  generic: add test for truncate/fpunch of an active swapfile
  generic: test invalid swap file activation

 .gitignore|  2 ++
 common/rc | 15 ++--
 src/Makefile  |  2 +-
 src/mkswap.c  | 83 +++
 src/swapon.c  | 24 +
 tests/generic/356 |  7 ++--
 tests/generic/357 |  6 ++--
 tests/generic/488 | 76 +++
 tests/generic/488.out |  7 
 tests/generic/489 | 73 +
 tests/generic/489.out |  8 +
 tests/generic/490 | 77 +++
 tests/generic/490.out |  5 +++
 tests/generic/group   |  7 ++--
 tests/xfs/group   |  2 +-
 15 files changed, 381 insertions(+), 13 deletions(-)
 create mode 100644 src/mkswap.c
 create mode 100644 src/swapon.c
 create mode 100755 tests/generic/488
 create mode 100644 tests/generic/488.out
 create mode 100755 tests/generic/489
 create mode 100644 tests/generic/489.out
 create mode 100755 tests/generic/490
 create mode 100644 tests/generic/490.out

-- 
2.17.0

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 4/5] generic: add test for truncate/fpunch of an active swapfile

[Omar Sandoval]

From: Omar Sandoval 

These should not be allowed.

Signed-off-by: Omar Sandoval 
---
 tests/generic/489 | 73 +++
 tests/generic/489.out |  8 +
 tests/generic/group   |  1 +
 3 files changed, 82 insertions(+)
 create mode 100755 tests/generic/489
 create mode 100644 tests/generic/489.out

diff --git a/tests/generic/489 b/tests/generic/489
new file mode 100755
index ..dd40a81d
--- /dev/null
+++ b/tests/generic/489
@@ -0,0 +1,73 @@
+#! /bin/bash
+# FS QA Test 489
+#
+# Test truncation/hole punching of an active swapfile.
+#
+#---
+# Copyright (c) 2018 Facebook.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#---
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1   # failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+   cd /
+   rm -f $tmp.*
+}
+
+. ./common/rc
+. ./common/filter
+
+rm -f $seqres.full
+
+_supported_fs generic
+_supported_os Linux
+_require_scratch_swapfile
+_require_xfs_io_command "fpunch"
+
+echo "Format and mount"
+_scratch_mkfs > $seqres.full 2>&1
+_scratch_mount >> $seqres.full 2>&1
+
+testdir="$SCRATCH_MNT/test-$seq"
+mkdir "$testdir"
+
+blocks=160
+blksz=65536
+
+echo "Initialize file"
+_format_swapfile "$testdir/file1" $((blocks * blksz))
+swapon "$testdir/file1"
+
+echo "Try to truncate"
+$XFS_IO_PROG -c "truncate $blksz" "$testdir/file1"
+
+echo "Try to punch hole"
+$XFS_IO_PROG -c "fpunch $blksz $((2 * blksz))" "$testdir/file1"
+
+echo "Tear it down"
+swapoff "$testdir/file1"
+
+status=0
+exit
diff --git a/tests/generic/489.out b/tests/generic/489.out
new file mode 100644
index ..e2ebc530
--- /dev/null
+++ b/tests/generic/489.out
@@ -0,0 +1,8 @@
+QA output created by 489
+Format and mount
+Initialize file
+Try to truncate
+ftruncate: Text file busy
+Try to punch hole
+fallocate: Text file busy
+Tear it down
diff --git a/tests/generic/group b/tests/generic/group
index 1f504ce1..a5252aeb 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -491,3 +491,4 @@
 486 auto quick attr
 487 auto quick
 488 auto quick swap
+489 auto quick swap
-- 
2.17.0

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 5/5] generic: test invalid swap file activation

[Omar Sandoval]

From: Omar Sandoval 

Swap files cannot have holes, and they must at least two pages.
swapon(8) and mkswap(8) have stricter restrictions, so add versions of
those commands without any restrictions.

Signed-off-by: Omar Sandoval 
---
 .gitignore|  2 ++
 src/Makefile  |  2 +-
 src/mkswap.c  | 83 +++
 src/swapon.c  | 24 +
 tests/generic/490 | 77 +++
 tests/generic/490.out |  5 +++
 tests/generic/group   |  1 +
 7 files changed, 193 insertions(+), 1 deletion(-)
 create mode 100644 src/mkswap.c
 create mode 100644 src/swapon.c
 create mode 100755 tests/generic/490
 create mode 100644 tests/generic/490.out

diff --git a/.gitignore b/.gitignore
index 53029e24..efc73a7c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -92,6 +92,7 @@
 /src/lstat64
 /src/makeextents
 /src/metaperf
+/src/mkswap
 /src/mmapcat
 /src/multi_open_unlink
 /src/nametest
@@ -111,6 +112,7 @@
 /src/seek_sanity_test
 /src/stale_handle
 /src/stat_test
+/src/swapon
 /src/t_access_root
 /src/t_dir_offset
 /src/t_dir_offset2
diff --git a/src/Makefile b/src/Makefile
index c42d3bb1..01fe99ef 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -26,7 +26,7 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize 
preallo_rw_pattern_reader \
renameat2 t_getcwd e4compact test-nextquota punch-alternating \
attr-list-by-handle-cursor-test listxattr dio-interleaved t_dir_type \
dio-invalidate-cache stat_test t_encrypted_d_revalidate \
-   attr_replace_test
+   attr_replace_test swapon mkswap
 
 SUBDIRS = log-writes perf
 
diff --git a/src/mkswap.c b/src/mkswap.c
new file mode 100644
index ..d0bce2bd
--- /dev/null
+++ b/src/mkswap.c
@@ -0,0 +1,83 @@
+/* mkswap(8) without any sanity checks */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct swap_header {
+   charbootbits[1024];
+   uint32_tversion;
+   uint32_tlast_page;
+   uint32_tnr_badpages;
+   unsigned char   sws_uuid[16];
+   unsigned char   sws_volume[16];
+   uint32_tpadding[117];
+   uint32_tbadpages[1];
+};
+
+int main(int argc, char **argv)
+{
+   struct swap_header *hdr;
+   FILE *file;
+   struct stat st;
+   long page_size;
+   int ret;
+
+   if (argc != 2) {
+   fprintf(stderr, "usage: %s PATH\n", argv[0]);
+   return EXIT_FAILURE;
+   }
+
+   page_size = sysconf(_SC_PAGESIZE);
+   if (page_size == -1) {
+   perror("sysconf");
+   return EXIT_FAILURE;
+   }
+
+   hdr = calloc(1, page_size);
+   if (!hdr) {
+   perror("calloc");
+   return EXIT_FAILURE;
+   }
+
+   file = fopen(argv[1], "r+");
+   if (!file) {
+   perror("fopen");
+   free(hdr);
+   return EXIT_FAILURE;
+   }
+
+   ret = fstat(fileno(file), );
+   if (ret) {
+   perror("fstat");
+   free(hdr);
+   fclose(file);
+   return EXIT_FAILURE;
+   }
+
+   hdr->version = 1;
+   hdr->last_page = st.st_size / page_size - 1;
+   memset(>sws_uuid, 0x99, sizeof(hdr->sws_uuid));
+   memcpy((char *)hdr + page_size - 10, "SWAPSPACE2", 10);
+
+   if (fwrite(hdr, page_size, 1, file) != 1) {
+   perror("fwrite");
+   free(hdr);
+   fclose(file);
+   return EXIT_FAILURE;
+   }
+
+   if (fclose(file) == EOF) {
+   perror("fwrite");
+   free(hdr);
+   return EXIT_FAILURE;
+   }
+
+   free(hdr);
+
+   return EXIT_SUCCESS;
+}
diff --git a/src/swapon.c b/src/swapon.c
new file mode 100644
index ..0cb7108a
--- /dev/null
+++ b/src/swapon.c
@@ -0,0 +1,24 @@
+/* swapon(8) without any sanity checks; simply calls swapon(2) directly. */
+
+#include 
+#include 
+#include 
+#include 
+
+int main(int argc, char **argv)
+{
+   int ret;
+
+   if (argc != 2) {
+   fprintf(stderr, "usage: %s PATH\n", argv[0]);
+   return EXIT_FAILURE;
+   }
+
+   ret = swapon(argv[1], 0);
+   if (ret) {
+   perror("swapon");
+   return EXIT_FAILURE;
+   }
+
+   return EXIT_SUCCESS;
+}
diff --git a/tests/generic/490 b/tests/generic/490
new file mode 100755
index ..6ba2ecb3
--- /dev/null
+++ b/tests/generic/490
@@ -0,0 +1,77 @@
+#! /bin/bash
+# FS QA Test 490
+#
+# Test invalid swap files.
+#
+#---
+# Copyright (c) 2018 Facebook.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope 

[PATCH v2 3/5] generic: add test for dedupe on an active swapfile

[Omar Sandoval]

From: Omar Sandoval 

Similar to generic/356 that makes sure we can't reflink an active
swapfile.

Signed-off-by: Omar Sandoval 
---
 tests/generic/488 | 76 +++
 tests/generic/488.out |  7 
 tests/generic/group   |  1 +
 3 files changed, 84 insertions(+)
 create mode 100755 tests/generic/488
 create mode 100644 tests/generic/488.out

diff --git a/tests/generic/488 b/tests/generic/488
new file mode 100755
index ..39fc887c
--- /dev/null
+++ b/tests/generic/488
@@ -0,0 +1,76 @@
+#! /bin/bash
+# FS QA Test 488
+#
+# Check that we can't dedupe a swapfile.
+#
+#---
+# Copyright (c) 2018 Facebook.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#---
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1   # failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+   cd /
+   rm -f $tmp.*
+}
+
+. ./common/rc
+. ./common/filter
+. ./common/reflink
+
+rm -f $seqres.full
+
+_supported_fs generic
+_supported_os Linux
+_require_scratch_swapfile
+_require_scratch_dedupe
+
+echo "Format and mount"
+_scratch_mkfs > $seqres.full 2>&1
+_scratch_mount >> $seqres.full 2>&1
+
+testdir="$SCRATCH_MNT/test-$seq"
+mkdir "$testdir"
+
+blocks=160
+blksz=65536
+
+echo "Initialize file"
+_format_swapfile "$testdir/file1" $((blocks * blksz))
+swapon "$testdir/file1"
+
+touch "$testdir/file2"
+$CHATTR_PROG +C "$testdir/file2" >/dev/null 2>&1
+
+echo "Try to dedupe"
+cp "$testdir/file1" "$testdir/file2"
+_dedupe_range "$testdir/file1" 0 "$testdir/file2" 0 $((blocks * blksz))
+_dedupe_range "$testdir/file2" 0 "$testdir/file1" 0 $((blocks * blksz))
+
+echo "Tear it down"
+swapoff "$testdir/file1"
+
+status=0
+exit
diff --git a/tests/generic/488.out b/tests/generic/488.out
new file mode 100644
index ..e5a195d1
--- /dev/null
+++ b/tests/generic/488.out
@@ -0,0 +1,7 @@
+QA output created by 488
+Format and mount
+Initialize file
+Try to dedupe
+XFS_IOC_FILE_EXTENT_SAME: Text file busy
+XFS_IOC_FILE_EXTENT_SAME: Text file busy
+Tear it down
diff --git a/tests/generic/group b/tests/generic/group
index cded10f8..1f504ce1 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -490,3 +490,4 @@
 485 auto quick insert
 486 auto quick attr
 487 auto quick
+488 auto quick swap
-- 
2.17.0

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 5/6] btrfs: add send_stream_version attribute to sysfs

[Omar Sandoval]

On Tue, May 08, 2018 at 10:06:50PM -0400, Howard McLauchlan wrote:
> From: Filipe David Borba Manana 
> 
> So that applications can find out what's the highest send stream
> version supported/implemented by the running kernel:
> 
> $ cat /sys/fs/btrfs/send/stream_version
> 2
> 
> [Howard: rebased on 4.17-rc4]
> Signed-off-by: Howard McLauchlan 
> Signed-off-by: Filipe David Borba Manana 
> Reviewed-by: David Sterba 
> ---
>  fs/btrfs/send.h  |  1 +
>  fs/btrfs/sysfs.c | 29 +
>  2 files changed, 30 insertions(+)
> 
> diff --git a/fs/btrfs/send.h b/fs/btrfs/send.h
> index a5830d216ac1..2f5e25e03def 100644
> --- a/fs/btrfs/send.h
> +++ b/fs/btrfs/send.h
> @@ -12,6 +12,7 @@
>  #define BTRFS_SEND_STREAM_MAGIC "btrfs-stream"
>  #define BTRFS_SEND_STREAM_VERSION_1 1
>  #define BTRFS_SEND_STREAM_VERSION_2 2
> +#define BTRFS_SEND_STREAM_VERSION_LATEST BTRFS_SEND_STREAM_VERSION_2
>  
>  #define BTRFS_SEND_BUF_SIZE SZ_64K
>  #define BTRFS_SEND_READ_SIZE (48 * SZ_1K)
> diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
> index 4848a4318fb5..3c82cba91ff6 100644
> --- a/fs/btrfs/sysfs.c
> +++ b/fs/btrfs/sysfs.c
> @@ -18,6 +18,7 @@
>  #include "transaction.h"
>  #include "sysfs.h"
>  #include "volumes.h"
> +#include "send.h"
>  
>  static inline struct btrfs_fs_info *to_fs_info(struct kobject *kobj);
>  static inline struct btrfs_fs_devices *to_fs_devs(struct kobject *kobj);
> @@ -884,6 +885,28 @@ static int btrfs_init_debugfs(void)
>   return 0;
>  }
>  
> +static ssize_t send_stream_version_show(struct kobject *kobj,
> + struct kobj_attribute *a,
> + char *buf)
> +{
> + return snprintf(buf, PAGE_SIZE, "%d\n",
> + BTRFS_SEND_STREAM_VERSION_LATEST);
> +}
> +
> +BTRFS_ATTR(, stream_version, send_stream_version_show);
> +
> +static struct attribute *btrfs_send_attrs[] = {
> + BTRFS_ATTR_PTR(, stream_version),
> + NULL
> +};
> +
> +static const struct attribute_group btrfs_send_attr_group = {
> + .name = "send",
> + .attrs = btrfs_send_attrs,
> +};
> +
> +
> +

I know this is Filipe's code, but there are two extra newlines there ;)
Otherwise, assuming you tested this and it works as advertised,

Reviewed-by: Omar Sandoval 

>  int __init btrfs_init_sysfs(void)
>  {
>   int ret;
> @@ -900,8 +923,13 @@ int __init btrfs_init_sysfs(void)
>   ret = sysfs_create_group(_kset->kobj, _feature_attr_group);
>   if (ret)
>   goto out2;
> + ret = sysfs_create_group(_kset->kobj, _send_attr_group);
> + if (ret)
> + goto out3;
>  
>   return 0;
> +out3:
> + sysfs_remove_group(_kset->kobj, _feature_attr_group);
>  out2:
>   debugfs_remove_recursive(btrfs_debugfs_root_dentry);
>  out1:
> @@ -913,6 +941,7 @@ int __init btrfs_init_sysfs(void)
>  void __cold btrfs_exit_sysfs(void)
>  {
>   sysfs_remove_group(_kset->kobj, _feature_attr_group);
> + sysfs_remove_group(_kset->kobj, _send_attr_group);
>   kset_unregister(btrfs_kset);
>   debugfs_remove_recursive(btrfs_debugfs_root_dentry);
>  }
> -- 
> 2.17.0
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 6/6] btrfs: add chattr support for send/receive

[Omar Sandoval]

On Tue, May 08, 2018 at 10:06:51PM -0400, Howard McLauchlan wrote:
> From: Howard McLauchlan 
> 
> Presently btrfs send/receive does not propagate inode attribute flags;
> all chattr operations are effectively discarded upon transmission.
> 
> This patch adds kernel support for inode attribute flags. Userspace
> support can be found under the commit:
> 
> btrfs-progs: add chattr support for send/receive
> 
> An associated xfstest can be found at:
> 
> btrfs: add verify chattr support for send/receive test
> 
> These changes are only enabled for send stream version 2
> 
> Signed-off-by: Howard McLauchlan 
> ---
>  fs/btrfs/ctree.h |   2 +
>  fs/btrfs/ioctl.c |   2 +-
>  fs/btrfs/send.c  | 181 ---
>  fs/btrfs/send.h  |   4 +-
>  4 files changed, 159 insertions(+), 30 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 2771cc56a622..0a2359144b18 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -1461,6 +1461,8 @@ struct btrfs_map_token {
>   unsigned long offset;
>  };
>  
> +unsigned int btrfs_flags_to_ioctl(unsigned int flags);
> +
>  #define BTRFS_BYTES_TO_BLKS(fs_info, bytes) \
>   ((bytes) >> (fs_info)->sb->s_blocksize_bits)
>  
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 632e26d6f7ce..36ce1e589f9e 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -106,7 +106,7 @@ static unsigned int btrfs_mask_flags(umode_t mode, 
> unsigned int flags)
>  /*
>   * Export inode flags to the format expected by the FS_IOC_GETFLAGS ioctl.
>   */
> -static unsigned int btrfs_flags_to_ioctl(unsigned int flags)
> +unsigned int btrfs_flags_to_ioctl(unsigned int flags)
>  {
>   unsigned int iflags = 0;
>  
> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index c8ea1ccaa3d8..fa7db1474a7f 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -108,6 +108,13 @@ struct send_ctx {
>   u64 cur_inode_last_extent;
>   u64 cur_inode_next_write_offset;
>  
> + /*
> +  * state for chattr purposes
> +  */
> + u64 cur_inode_flip_flags;
> + u64 cur_inode_receive_flags;
> + int receive_flags_valid;

I'd use bool here (and change the places that set it to use true or
false).

> +
>   u64 send_progress;
>   u64 total_data_size;
>  
> @@ -815,7 +822,7 @@ static int send_rmdir(struct send_ctx *sctx, struct 
> fs_path *path)
>   */
>  static int __get_inode_info(struct btrfs_root *root, struct btrfs_path *path,
> u64 ino, u64 *size, u64 *gen, u64 *mode, u64 *uid,
> -   u64 *gid, u64 *rdev)
> +   u64 *gid, u64 *rdev, u64 *flags)
>  {
>   int ret;
>   struct btrfs_inode_item *ii;
> @@ -845,6 +852,8 @@ static int __get_inode_info(struct btrfs_root *root, 
> struct btrfs_path *path,
>   *gid = btrfs_inode_gid(path->nodes[0], ii);
>   if (rdev)
>   *rdev = btrfs_inode_rdev(path->nodes[0], ii);
> + if (flags)
> + *flags = btrfs_inode_flags(path->nodes[0], ii);
>  
>   return ret;
>  }
> @@ -852,7 +861,7 @@ static int __get_inode_info(struct btrfs_root *root, 
> struct btrfs_path *path,
>  static int get_inode_info(struct btrfs_root *root,
> u64 ino, u64 *size, u64 *gen,
> u64 *mode, u64 *uid, u64 *gid,
> -   u64 *rdev)
> +   u64 *rdev, u64 *flags)
>  {
>   struct btrfs_path *path;
>   int ret;
> @@ -861,7 +870,7 @@ static int get_inode_info(struct btrfs_root *root,
>   if (!path)
>   return -ENOMEM;
>   ret = __get_inode_info(root, path, ino, size, gen, mode, uid, gid,
> -rdev);
> +rdev, flags);
>   btrfs_free_path(path);
>   return ret;
>  }
> @@ -1250,7 +1259,7 @@ static int __iterate_backrefs(u64 ino, u64 offset, u64 
> root, void *ctx_)
>* accept clones from these extents.
>*/
>   ret = __get_inode_info(found->root, bctx->path, ino, _size, NULL, 
> NULL,
> -NULL, NULL, NULL);
> +NULL, NULL, NULL, NULL);
>   btrfs_release_path(bctx->path);
>   if (ret < 0)
>   return ret;
> @@ -1610,7 +1619,7 @@ static int get_cur_inode_state(struct send_ctx *sctx, 
> u64 ino, u64 gen)
>   u64 right_gen;
>  
>   ret = get_inode_info(sctx->send_root, ino, NULL, _gen, NULL, NULL,
> - NULL, NULL);
> + NULL, NULL, NULL);
>   if (ret < 0 && ret != -ENOENT)
>   goto out;
>   left_ret = ret;
> @@ -1619,7 +1628,7 @@ static int get_cur_inode_state(struct send_ctx *sctx, 
> u64 ino, u64 gen)
>   right_ret = -ENOENT;
>   } else {
>   ret = get_inode_info(sctx->parent_root, ino, NULL, _gen,
> - NULL, NULL, NULL, NULL);
> +   

Re: [RFC PATCH 1/6] btrfs: send, bump stream version

[Omar Sandoval]

On Tue, May 08, 2018 at 10:06:46PM -0400, Howard McLauchlan wrote:
> From: Filipe David Borba Manana 
> 
> This increases the send stream version from version 1 to version 2, adding
> new commands:
> 
> 1) total data size - used to tell the receiver how much file data the stream
>will add or update;
> 
> 2) fallocate - used to pre-allocate space for files and to punch holes in 
> files;
> 
> 3) inode set flags;
> 
> 4) set inode otime.
> 
> This is preparation work for subsequent changes that implement the new 
> features.
> 
> A version 2 stream is only produced if the send ioctl caller passes in one of 
> the
> new flags (BTRFS_SEND_FLAG_CALCULATE_DATA_SIZE | BTRFS_SEND_FLAG_STREAM_V2), 
> meaning
> old clients are unaffected.
> 
> [Howard: rebased on 4.17-rc4]
> Signed-off-by: Howard McLauchlan 
> Signed-off-by: Filipe David Borba Manana 
> ---
>  fs/btrfs/send.c|  7 ++-
>  fs/btrfs/send.h| 21 -
>  include/uapi/linux/btrfs.h | 21 -
>  3 files changed, 46 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index c0074d2d7d6d..eccd69387065 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -649,7 +649,10 @@ static int send_header(struct send_ctx *sctx)
>   struct btrfs_stream_header hdr;
>  
>   strcpy(hdr.magic, BTRFS_SEND_STREAM_MAGIC);
> - hdr.version = cpu_to_le32(BTRFS_SEND_STREAM_VERSION);
> + if (sctx->flags & BTRFS_SEND_FLAG_STREAM_V2)
> + hdr.version = cpu_to_le32(BTRFS_SEND_STREAM_VERSION_2);
> + else
> + hdr.version = cpu_to_le32(BTRFS_SEND_STREAM_VERSION_1);
>  
>   return write_buf(sctx->send_filp, , sizeof(hdr),
>   >send_off);
> @@ -6535,6 +6538,8 @@ long btrfs_ioctl_send(struct file *mnt_file, struct 
> btrfs_ioctl_send_args *arg)
>   INIT_LIST_HEAD(>name_cache_list);
>  
>   sctx->flags = arg->flags;
> + if (sctx->flags & BTRFS_SEND_FLAG_CALCULATE_DATA_SIZE)
> + sctx->flags |= BTRFS_SEND_FLAG_STREAM_V2;
>  
>   sctx->send_filp = fget(arg->send_fd);
>   if (!sctx->send_filp) {
> diff --git a/fs/btrfs/send.h b/fs/btrfs/send.h
> index ead397f7034f..a9b5489d690e 100644
> --- a/fs/btrfs/send.h
> +++ b/fs/btrfs/send.h
> @@ -10,7 +10,8 @@
>  #include "ctree.h"
>  
>  #define BTRFS_SEND_STREAM_MAGIC "btrfs-stream"
> -#define BTRFS_SEND_STREAM_VERSION 1
> +#define BTRFS_SEND_STREAM_VERSION_1 1
> +#define BTRFS_SEND_STREAM_VERSION_2 2
>  
>  #define BTRFS_SEND_BUF_SIZE SZ_64K
>  #define BTRFS_SEND_READ_SIZE (48 * SZ_1K)
> @@ -77,6 +78,15 @@ enum btrfs_send_cmd {
>  
>   BTRFS_SEND_C_END,
>   BTRFS_SEND_C_UPDATE_EXTENT,
> +
> + /*
> +  * The following commands were added in stream version 2.
> +  */
> + BTRFS_SEND_C_TOTAL_DATA_SIZE,
> + BTRFS_SEND_C_FALLOCATE,
> + BTRFS_SEND_C_INODE_SET_FLAGS,
> + BTRFS_SEND_C_UTIMES2, /* Same as UTIMES, but it includes OTIME too. */

I've had feature requests for the ability to send compressed data
without decompressing it on the send side or the receive side. Could you
add a placeholder BTRFS_SEND_C_WRITE_COMPRESSED here? This way when I
get around to implementing it I won't have to do a v3.

Other than that,

Reviewed-by: Omar Sandoval 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 14/14] mm: turn on vm_fault_t type checking

[Christoph Hellwig]

On Wed, May 16, 2018 at 08:08:29AM -0700, Darrick J. Wong wrote:
> Uh, we're changing function signatures /and/ redefinining vm_fault_t?
> All in the same 90K patch?
> 
> I /was/ expecting a series of "convert X and all callers/users"
> patches followed by a trivial one to switch the definition, not a giant
> pile of change.  FWIW I don't mind so much if you make a patch
> containing a change for some super-common primitive and a hojillion
> little diff hunks tree-wide, but only one logical change at a time for a
> big patch, please...
> 
> I quite prefer seeing the whole series from start to finish all packaged
> up in one series, but wow this was overwhelming. :/

Another vote to split the change of the typedef, ok I get the message..
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: vm_fault_t conversion, for real

[Christoph Hellwig]

On Wed, May 16, 2018 at 06:22:56AM -0700, Matthew Wilcox wrote:
> Perhaps you should try being less of an arsehole if you don't want to
> get yelled at?  I don't mind when you're an arsehole towards me, but I
> do mind when you're an arsehole towards newcomers.  How are we supposed
> to attract and retain new maintainers when you're so rude?

*plonk*  The only one I'm seeing being extremely rude here is you.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/5] Fix delalloc inodes leaking on btrfs unmount

[David Sterba]

On Tue, May 01, 2018 at 04:02:34PM +0200, David Sterba wrote:
> On Fri, Apr 27, 2018 at 12:21:49PM +0300, Nikolay Borisov wrote:
> > After investigating crashes on generic/176 it turned that the culprit in 
> > fact
> > is the random failure induced by generic/019. As it happens, if on unmount 
> > the 
> > filesystem is in BTRFS_FS_STATE_ERROR then btrfs_error_commit_super is 
> > called. 
> > This unveiled 2 bugs:
> >  1. btrfs_destroy_delalloc_inodes's implementation was completely bogus, 
> > since
> >  it only called btrfs_invalidate_inodes which only pruned dentries and 
> > didn't 
> >  do anything to free any inodes with pending delalloc bytes. Once this is 
> > fixed 
> >  with the use of invalide_inode_pages2 the second bug transpired. 
> >  2. The last call ot run_delayed_iputs is made before 
> > btrfs_cleanup_transaction
> >  is called. The latter in turn could queue up more delayed iputs resulting 
> > from 
> >  invalidates_inode_pages2. 
> > 
> > This series fixes the problem by first fixing btrfs_destroy_delalloc_inode 
> > to 
> > properly cleanup delalloc inodes and as a result cleans up the code a bit. 
> > 
> > I've given it a good bashing through xfstest (4 full xfstest cycles + 100 
> > iterations of generic/475 since it was hitting some early assertion 
> > failures,
> > which are fixed in the final version) so am pretty confident in the change. 
> 
> Thanks. I'll add it as topic branch to next, this needs some testing
> exposure. The plan is to push the core patches to some rc, possibly rc5.
> 
> Review of patch 3 is required.

The whole series is now in misc-next. I did not see another occurence of
the crash, so that was probably unrelated to this patchset but still
worth analyzing.

As there are going to be more patches in post-rc5, this patch is in the
pool.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/5] btrfs: Split btrfs_del_delalloc_inode into 2 functions

[David Sterba]

On Fri, May 11, 2018 at 11:39:56AM +0300, Nikolay Borisov wrote:
> On 11.05.2018 08:44, Anand Jain wrote:
> > On 04/27/2018 05:21 PM, Nikolay Borisov wrote:
> >> This is in preparation of fixing delalloc inodes leakage on transaction
> >> abort. Also export the new function.
> >>
> >> Signed-off-by: Nikolay Borisov 
> > 
> >  nit: I think we are reserving function prefix __ for some special
> >  functions. I am not sure if the function name should prefix with __
> >  here.
> 
> Generally __ prefix is used for some internal function. In this case the
> gist of the function (with no locking) is behind the __ prefixed
> function, whereas the non __ version adds the necessary locking. I think
> this is a fairly well-established pattern in the kernel.

Yes, this is one of few patterns where __ is ok.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] btrfs: sysfs: Add entry which shows rmdir(2) can work for subvolume

[David Sterba]

On Wed, May 16, 2018 at 05:09:27PM +0900, Tomohiro Misono wrote:
> Deletion of a subvolume by rmdir(2) has become allowed by the
> 'commit cd2decf640b1 ("btrfs: Allow rmdir(2) to delete an empty
> subvolume")'.
> 
> It is a kind of new feature and this commits add a sysfs entry
>   /sys/fs/btrfs/static_features/rmdir_subvol

> to indicate the availability of feature so that a user program
> (e.g. xfstests) can detect it.
> 
> Note that new sysfs directory "static_features" is created since a entry
> in /sys/fs/btrfs/features depends on feature bits of superblock (in other
> words, they may be different between each fs) and is not suitable to hold
> the features which only depend on kernel version. New attribute_group
> "btrfs_static_feature_attr_group" is created for this purpose.

No, we don't want to add another directory for that, please use
'/sys/fs/btrfs/features'. Listing in this directory reflects
capabilities of the kernel module, the filesystem specific features are
in the /sys/fs/btrfs/UUID/features directory.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] btrfs: Skip some btrfs_cross_ref_exist() check in nocow path

[David Sterba]

On Wed, May 16, 2018 at 04:28:54PM +0800, Ethan Lien wrote:
> In nocow path, we check if the extent is snapshotted in
> btrfs_cross_ref_exist(). We can do the similar check earlier and avoid
> unnecessary search into extent tree.
> 
> Signed-off-by: Ethan Lien 
> ---
>  fs/btrfs/inode.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index d241285a0d2a..96927f2ccd4b 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -1373,6 +1373,9 @@ static noinline int run_delalloc_nocow(struct inode 
> *inode,
>   btrfs_file_extent_encryption(leaf, fi) ||
>   btrfs_file_extent_other_encoding(leaf, fi))
>   goto out_check;
> + if (btrfs_file_extent_generation(leaf, fi) <=
> + btrfs_root_last_snapshot(>root_item))
> + goto out_check;
>   if (extent_type == BTRFS_FILE_EXTENT_REG && !force)
>   goto out_check;
>   if (btrfs_extent_readonly(fs_info, disk_bytenr))
> @@ -7368,6 +7371,10 @@ noinline int can_nocow_extent(struct inode *inode, u64 
> offset, u64 *len,
>   btrfs_file_extent_other_encoding(leaf, fi))
>   goto out;
>  
> + if (btrfs_file_extent_generation(leaf, fi) <=
> + btrfs_root_last_snapshot(>root_item))
> + goto out;

Please add some brief comment why we're taking a shortcut here, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 14/14] mm: turn on vm_fault_t type checking

[Darrick J. Wong]

On Wed, May 16, 2018 at 07:43:48AM +0200, Christoph Hellwig wrote:
> Switch vm_fault_t to point to an unsigned int with __bіtwise annotations.
> This both catches any old ->fault or ->page_mkwrite instance with plain
> compiler type checking, as well as finding more intricate problems with
> sparse.
> 
> Signed-off-by: Christoph Hellwig 
> ---



For the iomap and xfs parts,
Reviewed-by: Darrick J. Wong 

That said...

> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 54f1e05ecf3e..da2b77a19911 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -22,7 +22,8 @@
>  #endif
>  #define AT_VECTOR_SIZE (2*(AT_VECTOR_SIZE_ARCH + AT_VECTOR_SIZE_BASE + 1))
>  
> -typedef int vm_fault_t;
> +typedef unsigned __bitwise vm_fault_t;
> +
>  
>  struct address_space;
>  struct mem_cgroup;
> @@ -619,7 +620,7 @@ struct vm_special_mapping {
>* If non-NULL, then this is called to resolve page faults
>* on the special mapping.  If used, .pages is not checked.
>*/
> - int (*fault)(const struct vm_special_mapping *sm,
> + vm_fault_t (*fault)(const struct vm_special_mapping *sm,

Uh, we're changing function signatures /and/ redefinining vm_fault_t?
All in the same 90K patch?

I /was/ expecting a series of "convert X and all callers/users"
patches followed by a trivial one to switch the definition, not a giant
pile of change.  FWIW I don't mind so much if you make a patch
containing a change for some super-common primitive and a hojillion
little diff hunks tree-wide, but only one logical change at a time for a
big patch, please...

I quite prefer seeing the whole series from start to finish all packaged
up in one series, but wow this was overwhelming. :/

--D


--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/3] btrfs: balance: add args info during start and resume

[Adam Borowski]

On Wed, May 16, 2018 at 10:57:57AM +0300, Nikolay Borisov wrote:
> On 16.05.2018 05:51, Anand Jain wrote:
> > Balance args info is an important information to be reviewed for the
> > system audit. So this patch adds it to the kernel log.
> > 
> > Example:
> > 
> > -> btrfs bal start -dprofiles='raid1|single',convert=raid5 
> > -mprofiles='raid1|single',convert=raid5 /btrfs
> > 
> >  kernel: BTRFS info (device sdb): balance: start data profiles=raid1|single 
> > convert=raid5 metadata profiles=raid1|single convert=raid5 system 
> > profiles=raid1|single convert=raid5
> > 
> > -> btrfs bal start -dprofiles=raid5,convert=single 
> > -mprofiles='raid1|single',convert=raid5 --background /btrfs
> > 
> >  kernel: BTRFS info (device sdb): balance: start data profiles=raid5 
> > convert=single metadata profiles=raid1|single convert=raid5 system 
> > profiles=raid1|single convert=raid5
> > 
> > Signed-off-by: Anand Jain 
> 
> Why can't this code be part of progs, the bctl which you are parsing is
> constructed from the arguments passed from users space? I think you are
> adding way too much string parsing code to the kernel and this is never
> a good sign, since it's very easy to trip.

progs are not the only way to start a balance, they're merely the most
widespread one.  For example, Hans van Kranenburg has some smarter scripts
among his tools -- currently only of "example" quality, but quite useful
already.  "balance_least_used" works in greedy order (least used first) with
nice verbose output.  It's not unlikely he or someone else improves this
further.  Thus, I really think the logging should be kernel side.

On the other hand, the string producing (not parsing) code is so repetitive
that it indeed could use some refactoring.


Meow!
-- 
⢀⣴⠾⠻⢶⣦⠀ 
⣾⠁⢰⠒⠀⣿⡁ 
⢿⡄⠘⠷⠚⠋⠀ Certified airhead; got the CT scan to prove that!
⠈⠳⣄ 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3] test online label ioctl

[Eric Sandeen]

On 5/15/18 7:51 PM, Dave Chinner wrote:

On Tue, May 15, 2018 at 10:22:37AM -0500, Eric Sandeen wrote:

This tests the online label ioctl that btrfs has, which has been
recently proposed for XFS.

To run, it requires an updated xfs_io with the label command and a
filesystem that supports it

A slight change here to _require_xfs_io_command as well, so that tests
which simply fail with "Inappropriate ioctl" can be caught in the
common case.

Signed-off-by: Eric Sandeen
---

(urgh send as proper new thread, sorry)

This passes on btrfs, _notruns on xfs/ext4 of yore, and passes
on xfs w/ my online label patchset (as long as xfs_io has the new
capability)

V2: Add a max label length helper
 Set the proper btrfs max label length o_O oops
 Filter trailing whitespace from blkid output

V3: lowercase local vars, simplify max label len function

Looks good now, but I wondered about one thing the test doesn't
cover: can you clear the label by setting it to a null string?
i.e you check max length bounds, but don't check empty string
behaviour...


hohum, yes.  I'll fix that, which will also require a change to xfs_io
to be able to set a null string.

Will send a V3 in a bit.

-Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] btrfs: fix BUG trying to resume balance without resume flag

[David Sterba]

On Mon, Apr 30, 2018 at 05:48:45PM +0800, Anand Jain wrote:
> We set the BTRFS_BALANCE_RESUME flag in the btrfs_recover_balance(),
> which is not called during the remount. So when resuming from the
> paused balance we hit BUG.
> 
>  kernel: kernel BUG at fs/btrfs/volumes.c:3890!
>  ::
>  kernel:  balance_kthread+0x51/0x60 [btrfs]
>  kernel:  kthread+0x111/0x130
>  ::
>  kernel: RIP: btrfs_balance+0x12e1/0x1570 [btrfs] RSP: ba7d0090bde8
> 
> Reproducer:
>   On a mounted BTRFS.
> 
>   btrfs balance start --full-balance /btrfs
>   btrfs balance pause /btrfs
>   mount -o remount,ro /dev/sdb /btrfs
>   mount -o remount,rw /dev/sdb /btrfs
> 
> To fix this set the BTRFS_BALANCE_RESUME flag in btrfs_resume_balance_async()
> instead of btrfs_recover_balance().
> 
> Signed-off-by: Anand Jain 
> ---
> v1->v2: btrfs_resume_balance_async() can be called only from remount or
> mount, we don't need to hold fs_info->balance_lock.

For mount it's ok, there's nothing that can run in parallel, but remount
ro->rw that resumes the balance can be run with the ioctl that checks
balance status in parallel, at any time. As the fs_info::balance_ctl is
set up, btrfs_ioctl_balance_progress will proceed and return the current
status.


> Strictly speaking we should rather keep the balance at the paused state
> unless it is resumed by the user again, that means neither mount nor
> remount-rw should resume the balance automatically, former case needs
> writing balance status to the disk. Which needs compatibility
> verification.  So for now just avoid BUG. 
> 
>  fs/btrfs/volumes.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 3e6983a169c4..64bcaf25908b 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -4115,6 +4115,8 @@ int btrfs_resume_balance_async(struct btrfs_fs_info 
> *fs_info)
>   return 0;
>   }
>  
> + fs_info->balance_ctl->b_flags |= BTRFS_BALANCE_RESUME;

Though there's no update operation on flags, I think it's still better
to add the locks that set the resume status. And a comment why it's
here.

> +
>   tsk = kthread_run(balance_kthread, fs_info, "btrfs-balance");
>   return PTR_ERR_OR_ZERO(tsk);
>  }
> @@ -4156,7 +4158,6 @@ int btrfs_recover_balance(struct btrfs_fs_info *fs_info)
>  
>   bctl->fs_info = fs_info;
>   bctl->flags = btrfs_balance_flags(leaf, item);
> - bctl->flags |= BTRFS_BALANCE_RESUME;

Also, it does not hurt to leave this here, as it logically matches what
the function does: we found the balance item, thus we set the status
accordingly.

Please update and resend, I'd like to push this to 4.17-rc as it's a
crash fix with a reproducer. Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 3/3] btrfs: balance: add kernel log for end or paused

[Austin S. Hemmelgarn]

On 2018-05-16 09:23, Anand Jain wrote:



On 05/16/2018 07:25 PM, Austin S. Hemmelgarn wrote:

On 2018-05-15 22:51, Anand Jain wrote:

Add a kernel log when the balance ends, either for cancel or completed
or if it is paused.
---
v1->v2: Moved from 2/3 to 3/3

  fs/btrfs/volumes.c | 7 +++
  1 file changed, 7 insertions(+)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index ce68c4f42f94..a4e243a29f5c 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -4053,6 +4053,13 @@ int btrfs_balance(struct btrfs_fs_info *fs_info,
  ret = __btrfs_balance(fs_info);
  mutex_lock(_info->balance_mutex);
+    if (ret == -ECANCELED && atomic_read(_info->balance_pause_req))
+    btrfs_info(fs_info, "balance: paused");
+    else if (ret == -ECANCELED && 
atomic_read(_info->balance_cancel_req))

+    btrfs_info(fs_info, "balance: canceled");
+    else
+    btrfs_info(fs_info, "balance: ended with status: %d", ret);
+
  clear_bit(BTRFS_FS_BALANCE_RUNNING, _info->flags);
  if (bargs) {

Is there some way that these messages could be extended to include 
info about which volume the balance in question was on.  Ideally, 
something that matches up with what's listed in the message from the 
previous patch.  There's nothi9ng that prevents you from running 
balances on separate BTRFS volumes in parallel, so this message won't 
necessarily be for the most recent balance start message.


  Hm. That's not true, btrfs_info(fs_info,,) adds the fsid.
  So its already drilled down to the lowest granular possible.


Ah, you're right, it does.  Sorry about the noise.

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Btrfs: fix the corruption by reading stale btree blocks

[David Sterba]

On Wed, May 16, 2018 at 11:00:14AM +0300, Nikolay Borisov wrote:
> 
> 
> On 15.05.2018 20:37, Liu Bo wrote:
> > If a btree block, aka. extent buffer, is not available in the extent
> > buffer cache, it'll be read out from the disk instead, i.e.
> > 
> > btrfs_search_slot()
> >   read_block_for_search()  # hold parent and its lock, go to read child
> > btrfs_release_path()
> > read_tree_block()  # read child
> > 
> > Unfortunately, the parent lock got released before reading child, so
> > commit 5bdd3536cbbe ("Btrfs: Fix block generation verification race") had
> > used 0 as parent transid to read the child block.  It forces
> > read_tree_block() not to check if parent transid is different with the
> > generation id of the child that it reads out from disk.
> > 
> > A simple PoC is included in btrfs/124,
> > 
> > 0. A two-disk raid1 btrfs,
> > 
> > 1. Right after mkfs.btrfs, block A is allocated to be device tree's root.
> > 
> > 2. Mount this filesystem and put it in use, after a while, device tree's
> >root got COW but block A hasn't been allocated/overwritten yet.
> > 
> > 3. Umount it and reload the btrfs module to remove both disks from the
> >global @fs_devices list.
> > 
> > 4. mount -odegraded dev1 and write some data, so now block A is allocated
> >to be a leaf in checksum tree.  Note that only dev1 has the latest
> >metadata of this filesystem.
> > 
> > 5. Umount it and mount it again normally (with both disks), since raid1
> >can pick up one disk by the writer task's pid, if btrfs_search_slot()
> >needs to read block A, dev2 which does NOT have the latest metadata
> >might be read for block A, then we got a stale block A.
> > 
> > 6. As parent transid is not checked, block A is marked as uptodate and
> >put into the extent buffer cache, so the future search won't bother
> >to read disk again, which means it'll make changes on this stale
> >one and make it dirty and flush it onto disk.
> > 
> > To avoid the problem, parent transid needs to be passed to
> > read_tree_block().
> > 
> > In order to get a valid parent transid, we need to hold the parent's
> > lock until finish reading child.
> > 
> > Signed-off-by: Liu Bo 
> 
> Doesn't this warrant a stable tag and
> Fixes: 5bdd3536cbbe ("Btrfs: Fix block generation verification race")

The patch will not apply to < 4.16 as it depends on the addition of
_key check from 581c1760415c4, but yes we need it for stable.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: vm_fault_t conversion, for real

[Matthew Wilcox]

On Wed, May 16, 2018 at 03:03:09PM +0200, Christoph Hellwig wrote:
> On Wed, May 16, 2018 at 04:23:47AM -0700, Matthew Wilcox wrote:
> > On Wed, May 16, 2018 at 07:43:34AM +0200, Christoph Hellwig wrote:
> > > this series tries to actually turn vm_fault_t into a type that can be
> > > typechecked and checks the fallout instead of sprinkling random
> > > annotations without context.
> > 
> > Yes, why should we have small tasks that newcomers can do when the mighty
> > Christoph Hellwig can swoop in and take over from them?  Seriously,
> > can't your talents find a better use than this?
> 
> I've spent less time on this than trying to argue to you and Souptick
> that these changes are only to get ignored and yelled at as an
> "asshole maintainer".  So yes, I could have done more productive things
> if you hadn't forced this escalation.

Perhaps you should try being less of an arsehole if you don't want to
get yelled at?  I don't mind when you're an arsehole towards me, but I
do mind when you're an arsehole towards newcomers.  How are we supposed
to attract and retain new maintainers when you're so rude?

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 3/3] btrfs: balance: add kernel log for end or paused

[Anand Jain]

On 05/16/2018 07:25 PM, Austin S. Hemmelgarn wrote:

On 2018-05-15 22:51, Anand Jain wrote:

Add a kernel log when the balance ends, either for cancel or completed
or if it is paused.
---
v1->v2: Moved from 2/3 to 3/3

  fs/btrfs/volumes.c | 7 +++
  1 file changed, 7 insertions(+)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index ce68c4f42f94..a4e243a29f5c 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -4053,6 +4053,13 @@ int btrfs_balance(struct btrfs_fs_info *fs_info,
  ret = __btrfs_balance(fs_info);
  mutex_lock(_info->balance_mutex);
+    if (ret == -ECANCELED && atomic_read(_info->balance_pause_req))
+    btrfs_info(fs_info, "balance: paused");
+    else if (ret == -ECANCELED && 
atomic_read(_info->balance_cancel_req))

+    btrfs_info(fs_info, "balance: canceled");
+    else
+    btrfs_info(fs_info, "balance: ended with status: %d", ret);
+
  clear_bit(BTRFS_FS_BALANCE_RUNNING, _info->flags);
  if (bargs) {

Is there some way that these messages could be extended to include info 
about which volume the balance in question was on.  Ideally, something 
that matches up with what's listed in the message from the previous 
patch.  There's nothi9ng that prevents you from running balances on 
separate BTRFS volumes in parallel, so this message won't necessarily be 
for the most recent balance start message.


 Hm. That's not true, btrfs_info(fs_info,,) adds the fsid.
 So its already drilled down to the lowest granular possible.

Thanks. Anand


--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/14] vgem: separate errno from VM_FAULT_* values

[Matthew Wilcox]

On Wed, May 16, 2018 at 03:01:59PM +0200, Christoph Hellwig wrote:
> On Wed, May 16, 2018 at 11:53:03AM +0200, Daniel Vetter wrote:
> > Reviewed-by: Daniel Vetter 
> > 
> > Want me to merge this through drm-misc or plan to pick it up yourself?
> 
> For now I just want a honest discussion if people really actually
> want the vm_fault_t change with the whole picture in place.

That discussion already happened on the -mm mailing list.  And again
at LSFMM.  Both times the answer was yes.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 14/14] mm: turn on vm_fault_t type checking

[Christoph Hellwig]

On Wed, May 16, 2018 at 04:28:13AM -0700, Matthew Wilcox wrote:
> On Wed, May 16, 2018 at 07:43:48AM +0200, Christoph Hellwig wrote:
> > Switch vm_fault_t to point to an unsigned int with __bіtwise annotations.
> > This both catches any old ->fault or ->page_mkwrite instance with plain
> > compiler type checking, as well as finding more intricate problems with
> > sparse.
> 
> Come on, Christoph; you know better than this.  This patch is completely
> unreviewable.  Split it into one patch per maintainer tree, and in any
> event, the patch to convert vm_fault_t to an unsigned int should be
> separated from all the trivial conversions.

The whole point is that tiny split patches for mechnical translations
are totally pointless.  Switching the typedef might be worth splitting
if people really insist.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: vm_fault_t conversion, for real

[Christoph Hellwig]

On Wed, May 16, 2018 at 04:23:47AM -0700, Matthew Wilcox wrote:
> On Wed, May 16, 2018 at 07:43:34AM +0200, Christoph Hellwig wrote:
> > this series tries to actually turn vm_fault_t into a type that can be
> > typechecked and checks the fallout instead of sprinkling random
> > annotations without context.
> 
> Yes, why should we have small tasks that newcomers can do when the mighty
> Christoph Hellwig can swoop in and take over from them?  Seriously,
> can't your talents find a better use than this?

I've spent less time on this than trying to argue to you and Souptick
that these changes are only to get ignored and yelled at as an
"asshole maintainer".  So yes, I could have done more productive things
if you hadn't forced this escalation.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/14] vgem: separate errno from VM_FAULT_* values

[Christoph Hellwig]

On Wed, May 16, 2018 at 11:53:03AM +0200, Daniel Vetter wrote:
> Reviewed-by: Daniel Vetter 
> 
> Want me to merge this through drm-misc or plan to pick it up yourself?

For now I just want a honest discussion if people really actually
want the vm_fault_t change with the whole picture in place.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 14/14] mm: turn on vm_fault_t type checking

[Matthew Wilcox]

On Wed, May 16, 2018 at 07:43:48AM +0200, Christoph Hellwig wrote:
> Switch vm_fault_t to point to an unsigned int with __bіtwise annotations.
> This both catches any old ->fault or ->page_mkwrite instance with plain
> compiler type checking, as well as finding more intricate problems with
> sparse.

Come on, Christoph; you know better than this.  This patch is completely
unreviewable.  Split it into one patch per maintainer tree, and in any
event, the patch to convert vm_fault_t to an unsigned int should be
separated from all the trivial conversions.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 3/3] btrfs: balance: add kernel log for end or paused

[Austin S. Hemmelgarn]

On 2018-05-15 22:51, Anand Jain wrote:

Add a kernel log when the balance ends, either for cancel or completed
or if it is paused.
---
v1->v2: Moved from 2/3 to 3/3

  fs/btrfs/volumes.c | 7 +++
  1 file changed, 7 insertions(+)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index ce68c4f42f94..a4e243a29f5c 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -4053,6 +4053,13 @@ int btrfs_balance(struct btrfs_fs_info *fs_info,
ret = __btrfs_balance(fs_info);
  
  	mutex_lock(_info->balance_mutex);

+   if (ret == -ECANCELED && atomic_read(_info->balance_pause_req))
+   btrfs_info(fs_info, "balance: paused");
+   else if (ret == -ECANCELED && atomic_read(_info->balance_cancel_req))
+   btrfs_info(fs_info, "balance: canceled");
+   else
+   btrfs_info(fs_info, "balance: ended with status: %d", ret);
+
clear_bit(BTRFS_FS_BALANCE_RUNNING, _info->flags);
  
  	if (bargs) {


Is there some way that these messages could be extended to include info 
about which volume the balance in question was on.  Ideally, something 
that matches up with what's listed in the message from the previous 
patch.  There's nothi9ng that prevents you from running balances on 
separate BTRFS volumes in parallel, so this message won't necessarily be 
for the most recent balance start message.

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: vm_fault_t conversion, for real

[Matthew Wilcox]

On Wed, May 16, 2018 at 07:43:34AM +0200, Christoph Hellwig wrote:
> this series tries to actually turn vm_fault_t into a type that can be
> typechecked and checks the fallout instead of sprinkling random
> annotations without context.

Yes, why should we have small tasks that newcomers can do when the mighty
Christoph Hellwig can swoop in and take over from them?  Seriously,
can't your talents find a better use than this?

> The first one fixes a real bug in orangefs, the second and third fix
> mismatched existing vm_fault_t annotations on the same function, the
> fourth removes an unused export that was in the chain.  The remainder
> until the last one do some not quite trivial conversions, and the last
> one does the trivial mass annotation and flips vm_fault_t to a __bitwise
> unsigned int - the unsigned means we also get plain compiler type
> checking for the new ->fault signature even without sparse.

Yes, that was (part of) the eventual goal.  Well done.  Would you like
a biscuit?

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/14] orangefs: don't return errno values from ->fault

[Matthew Wilcox]

On Wed, May 16, 2018 at 07:43:35AM +0200, Christoph Hellwig wrote:
> + rc = orangefs_inode_getattr(file->f_mapping->host, 0, 1, STATX_SIZE);
>   if (rc) {
>   gossip_err("%s: orangefs_inode_getattr failed, "
>   "rc:%d:.\n", __func__, rc);
> - return rc;
> + return VM_FAULT_SIGBUS;

Nope.  orangefs_inode_getattr can return -ENOMEM.

>   }
>   return filemap_fault(vmf);
>  }
> -- 
> 2.17.0
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 06/14] btrfs: separate errno from VM_FAULT_* values

[David Sterba]

On Wed, May 16, 2018 at 07:43:40AM +0200, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig 

Reviewed-by: David Sterba 

I can add it to the btrfs queue now, unless you need the patch for the
rest of the series.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Btrfs: fix the corruption by reading stale btree blocks

[Anand Jain]

On 05/16/2018 01:37 AM, Liu Bo wrote:

If a btree block, aka. extent buffer, is not available in the extent
buffer cache, it'll be read out from the disk instead, i.e.

btrfs_search_slot()
   read_block_for_search()  # hold parent and its lock, go to read child
 btrfs_release_path()
 read_tree_block()  # read child

Unfortunately, the parent lock got released before reading child, so
commit 5bdd3536cbbe ("Btrfs: Fix block generation verification race") had
used 0 as parent transid to read the child block.  It forces
read_tree_block() not to check if parent transid is different with the
generation id of the child that it reads out from disk.

A simple PoC is included in btrfs/124,

0. A two-disk raid1 btrfs,

1. Right after mkfs.btrfs, block A is allocated to be device tree's root.

2. Mount this filesystem and put it in use, after a while, device tree's
root got COW but block A hasn't been allocated/overwritten yet.

3. Umount it and reload the btrfs module to remove both disks from the
global @fs_devices list.

4. mount -odegraded dev1 and write some data, so now block A is allocated
to be a leaf in checksum tree.  Note that only dev1 has the latest
metadata of this filesystem.

5. Umount it and mount it again normally (with both disks), since raid1
can pick up one disk by the writer task's pid, if btrfs_search_slot()
needs to read block A, dev2 which does NOT have the latest metadata
might be read for block A, then we got a stale block A.

6. As parent transid is not checked, block A is marked as uptodate and
put into the extent buffer cache, so the future search won't bother
to read disk again, which means it'll make changes on this stale
one and make it dirty and flush it onto disk.

To avoid the problem, parent transid needs to be passed to
read_tree_block().

In order to get a valid parent transid, we need to hold the parent's
lock until finish reading child.


 Thanks for the patch. I was trying to fix this as well.  I have
 a test case for this btrfs/161 which I just posted to the mailing
 list. And I find it still fails.

Thanks, Anand


Signed-off-by: Liu Bo 
---
  fs/btrfs/ctree.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 3fd44835b386..b3f6f300e492 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -2436,10 +2436,8 @@ noinline void btrfs_unlock_up_safe(struct btrfs_path 
*path, int level)
if (p->reada != READA_NONE)
reada_for_search(fs_info, p, level, slot, key->objectid);
  
-	btrfs_release_path(p);

-
ret = -EAGAIN;
-   tmp = read_tree_block(fs_info, blocknr, 0, parent_level - 1,
+   tmp = read_tree_block(fs_info, blocknr, gen, parent_level - 1,
  _key);
if (!IS_ERR(tmp)) {
/*
@@ -2454,6 +2452,8 @@ noinline void btrfs_unlock_up_safe(struct btrfs_path 
*path, int level)
} else {
ret = PTR_ERR(tmp);
}
+
+   btrfs_release_path(p);
return ret;
  }
  


--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/3] btrfs: balance: add args info during start and resume

[Anand Jain]

On 05/16/2018 03:57 PM, Nikolay Borisov wrote:



On 16.05.2018 05:51, Anand Jain wrote:

Balance args info is an important information to be reviewed for the
system audit. So this patch adds it to the kernel log.

Example:

-> btrfs bal start -dprofiles='raid1|single',convert=raid5 
-mprofiles='raid1|single',convert=raid5 /btrfs

  kernel: BTRFS info (device sdb): balance: start data profiles=raid1|single 
convert=raid5 metadata profiles=raid1|single convert=raid5 system 
profiles=raid1|single convert=raid5

-> btrfs bal start -dprofiles=raid5,convert=single 
-mprofiles='raid1|single',convert=raid5 --background /btrfs

  kernel: BTRFS info (device sdb): balance: start data profiles=raid5 
convert=single metadata profiles=raid1|single convert=raid5 system 
profiles=raid1|single convert=raid5

Signed-off-by: Anand Jain 


Why can't this code be part of progs, the bctl which you are parsing is
constructed from the arguments passed from users space? I think you are
adding way too much string parsing code to the kernel and this is never
a good sign, since it's very easy to trip.

Theoretically progs is a wrong place for this.
Unfortunately our bctl which is in the kernel, matches with the user
land balance ioctl args so it may feel like progs do it, but its wrong.
If kernel is modifying the chunks there must be proper logs for it,
which this patch does. Next this log is only called at the time of the 
balance setup and resume. And string parsing is behind the flag checks, 
so they aren't that intensive at run time.


Thanks, Anand


---
v1->v2: Change log update.
 Move adding the logs for balance complete and end to a new patch

  fs/btrfs/volumes.c | 146 +++--
  1 file changed, 143 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 27da66c47ef2..ce68c4f42f94 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -126,6 +126,32 @@ const char *get_raid_name(enum btrfs_raid_types type)
return btrfs_raid_array[type].raid_name;
  }
  
+static void get_all_raid_names(u64 bg_flags, char *raid_types)

+{
+   int i;
+   bool found = false;
+
+   for (i = 0; i < BTRFS_NR_RAID_TYPES; i++) {
+   if (bg_flags & btrfs_raid_array[i].bg_flag) {
+   if (found) {
+   strcat(raid_types, "|");
+   strcat(raid_types, 
btrfs_raid_array[i].raid_name);
+   } else {
+   found = true;
+   sprintf(raid_types, "%s", 
btrfs_raid_array[i].raid_name);
+   }
+   }
+   }
+   if (bg_flags & BTRFS_AVAIL_ALLOC_BIT_SINGLE) {
+   if (found) {
+   strcat(raid_types, "|");
+   strcat(raid_types, "single");
+   } else {
+   sprintf(raid_types, "%s", "single");
+   }
+   }
+}
+
  static int init_first_rw_device(struct btrfs_trans_handle *trans,
struct btrfs_fs_info *fs_info);
  static int btrfs_relocate_sys_chunks(struct btrfs_fs_info *fs_info);
@@ -3766,6 +3792,121 @@ static inline int validate_convert_profile(struct 
btrfs_balance_args *bctl_arg,
 (bctl_arg->target & ~allowed)));
  }
  
+static void get_balance_args(struct btrfs_balance_args *bargs, char *args)

+{
+   char value[64];
+
+   if (bargs->flags & BTRFS_BALANCE_ARGS_PROFILES) {
+   strcat(args, "profiles=");
+   get_all_raid_names(bargs->profiles, value);
+   strcat(args, value);
+   strcat(args, " ");
+   }
+
+   if (bargs->flags & BTRFS_BALANCE_ARGS_USAGE) {
+   snprintf(value, 64, "usage=%llu ", bargs->usage);
+   strcat(args, value);
+   }
+
+   if (bargs->flags & BTRFS_BALANCE_ARGS_USAGE_RANGE) {
+   snprintf(value, 64, "usage_min=%u usage_max=%u ",
+   bargs->usage_min, bargs->usage_max);
+   strcat(args, value);
+   }
+
+   if (bargs->flags & BTRFS_BALANCE_ARGS_DEVID) {
+   snprintf(value, 64, "devid=%llu ", bargs->devid);
+   strcat(args, value);
+   }
+
+   if (bargs->flags & BTRFS_BALANCE_ARGS_DRANGE) {
+   snprintf(value, 64, "pstart=%llu pend=%llu ",
+bargs->pstart, bargs->pend);
+   strcat(args, value);
+   }
+
+   if (bargs->flags & BTRFS_BALANCE_ARGS_VRANGE) {
+   snprintf(value, 64, "vstart=%llu vend %llu ",
+bargs->vstart, bargs->vend);
+   strcat(args, value);
+   }
+
+   if (bargs->flags & BTRFS_BALANCE_ARGS_LIMIT) {
+   snprintf(value, 64, "limit=%llu ", bargs->limit);
+   strcat(args, value);
+   }
+
+   if (bargs->flags & BTRFS_BALANCE_ARGS_LIMIT_RANGE) {
+  

[PATCH] fstests: btrfs/161: test raid1 missing writes

[Anand Jain]

Test to make sure that a raid1 device with missed write reads
good data when reassembled.

Signed-off-by: Anand Jain 
---

This test case fails as of now.

I am sending this to btrfs ML only as it depends on the
read_mirror_policy kernel patches which is in the ML.

Please use it using the mount option config.

MOUNT_OPTIONS="-o max_inline=0,nodatacow"
 OR
MOUNT_OPTIONS="-o max_inline=0,nodatasum"

max_inline=0 will make sure the data is not read when metadata is
read and we have the control on its checksum and datacow.

You can't reproduce it without nodatacow OR nodatasum because cow
and datasum makes the data block on the disk invalid just by fluke.

 tests/btrfs/161 | 160 
 tests/btrfs/161.out |   7 +++
 tests/btrfs/group   |   1 +
 tests/generic/479   |   0
 4 files changed, 168 insertions(+)
 create mode 100755 tests/btrfs/161
 create mode 100644 tests/btrfs/161.out
 mode change 100644 => 100755 tests/generic/479

diff --git a/tests/btrfs/161 b/tests/btrfs/161
new file mode 100755
index ..6c5dd9da389b
--- /dev/null
+++ b/tests/btrfs/161
@@ -0,0 +1,160 @@
+#! /bin/bash
+# FS QA Test 161
+#
+# Test to make sure that RAID1 write hole reads good data.
+# To see the failure use the following mount options.
+#   -o max_inline=0,nodatasum
+#   or
+#   -o max_inline=0,nodatacow
+#  and needs ML kernel patch so that we can avoid the arbitary
+#  disk to be used to read the mirror (which is based on the PID).
+#
+#  btrfs: add mount option read_mirror_policy
+#  btrfs: add read_mirror_policy parameter devid
+#  btrfs: read_mirror_policy ability to reset
+#
+#-
+# Copyright (c) 2018 Oracle.  All Rights Reserved.
+# Author: Anand Jain 
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1   # failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+   cd /
+   rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/module
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+
+_supported_fs btrfs
+_supported_os Linux
+_require_scratch_dev_pool 2
+_require_loadable_fs_module "btrfs"
+
+_scratch_dev_pool_get 2
+
+MNT_OPT=$(echo $MOUNT_OPTIONS | cut -d" " -f2-)
+
+BS=20
+COUNT=1
+TF="$SCRATCH_MNT/testfile"
+
+CHECKPOINT1=""
+CHECKPOINT2=""
+CHECKPOINT3=""
+DEV_GOOD=""
+DEV_BAD=""
+DEVID_BAD=""
+
+#RAID1 devs good (devid 1) and bad (devid 2)
+DEV_GOOD=$(echo $SCRATCH_DEV_POOL | awk '{print $1}')
+DEV_BAD=$(echo $SCRATCH_DEV_POOL | awk '{print $2}')
+echo DEV_GOOD=$DEV_GOOD" "DEV_BAD=$DEV_BAD >> $seqres.full
+
+mount_bad()
+{
+   run_check _mount -o 
$MNT_OPT,read_mirror_policy=$DEVID_BAD,device=$DEV_GOOD $DEV_BAD $SCRATCH_MNT
+}
+
+mount_good_remount_bad()
+{
+   run_check _mount -o 
$MNT_OPT,read_mirror_policy=$DEVID_GOOD,device=$DEV_GOOD $DEV_BAD $SCRATCH_MNT
+   run_check _mount -o 
$MNT_OPT,remount,read_mirror_policy=-$DEVID_GOOD,read_mirror_policy=$DEVID_BAD,device=$DEV_GOOD
 $DEV_BAD $SCRATCH_MNT
+}
+
+check()
+{
+   echo -e "\\n-- check --" | tee -a $seqres.full
+
+   _reload_fs_module "btrfs"
+
+   echo -- check mount -- >> /dev/kmsg && dmesg -C
+
+if (false); then
+   # parent transid verify failed, data corruption
+   mount_bad
+else
+   # silent data corruption
+   mount_good_remount_bad
+fi
+
+   CHECKPOINT3=$(md5sum $TF | cut -d" " -f1)
+   mdres_status=$?
+   echo mdres_status=$mdres_status >> $seqres.full
+   echo CHECKPOINT3=$CHECKPOINT3 >> $seqres.full
+
+   [[ $mdres_status -ne 0 ]] &&
+   _fail "Read failed in non-degraded mount. What happened to the 
mirror."
+
+   [[ $CHECKPOINT2 != $CHECKPOINT3 ]] &&
+   _fail "Read junk data !!!"
+}
+
+write_degrade()
+{
+   echo -e "\\n-- degrade write --" | tee -a $seqres.full
+
+   _reload_fs_module "btrfs"
+   echo -- degrade mount -- >> /dev/kmsg && dmesg -C
+   run_check _mount -o $MNT_OPT,degraded $DEV_GOOD $SCRATCH_MNT
+
+   

[PATCH v2 3/3] btrfs: read_mirror_policy ability to reset

[Anand Jain]

Adds an ability to change the read_mirror_policy at remount.

Signed-off-by: Anand Jain 
---
 fs/btrfs/super.c | 19 ---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 7ddecf4178a6..e70592036b95 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -856,6 +856,12 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char 
*options,
intarg = 0;
if (match_int([0], ) == 0) {
struct btrfs_device *device;
+   int clear = 0;
+
+   if (intarg < 0) {
+   clear = 1;
+   intarg = -intarg;
+   }
 
device = btrfs_find_device(info, intarg,
   NULL, NULL);
@@ -866,10 +872,17 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char 
*options,
ret = -EINVAL;
goto out;
}
-   info->read_mirror_policy =
+
+   if (clear) {
+   info->read_mirror_policy = 0;
+   clear_bit(BTRFS_DEV_STATE_READ_MIRROR,
+   >dev_state);
+   } else {
+   info->read_mirror_policy =
BTRFS_READ_MIRROR_BY_DEV;
-   set_bit(BTRFS_DEV_STATE_READ_MIRROR,
-   >dev_state);
+   set_bit(BTRFS_DEV_STATE_READ_MIRROR,
+ >dev_state);
+   }
break;
}
 
-- 
2.7.0

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 0/3] btrfs: add read mirror policy

[Anand Jain]

Not yet ready for the integration. As I need to introduce
-o no_read_mirror_policy instead of -o read_mirror_policy=-
to reset the policy as in 3/3. But I am sending this early so that
we can use it for btrfs/161 in the ML, and this patch-set is stable
enough for the testing.

Anand Jain (3):
  btrfs: add mount option read_mirror_policy
  btrfs: add read_mirror_policy parameter devid
  btrfs: read_mirror_policy ability to reset

 fs/btrfs/ctree.h   |  2 ++
 fs/btrfs/super.c   | 44 
 fs/btrfs/volumes.c | 18 +-
 fs/btrfs/volumes.h |  7 +++
 4 files changed, 70 insertions(+), 1 deletion(-)

-- 
2.7.0

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/3] btrfs: add read_mirror_policy parameter devid

[Anand Jain]

Adds the mount option:
  mount -o read_mirror_policy=

To set the devid of the device which should be used for read. That
means all the normal reads will go to that particular device only.

This also helps testing and gives a better control for the test
scripts including mount context reads.

Signed-off-by: Anand Jain 
---
 fs/btrfs/super.c   | 21 +
 fs/btrfs/volumes.c | 10 ++
 fs/btrfs/volumes.h |  2 ++
 3 files changed, 33 insertions(+)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 21eff0ac1e4f..7ddecf4178a6 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -852,6 +852,27 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char 
*options,
BTRFS_READ_MIRROR_BY_PID;
break;
}
+
+   intarg = 0;
+   if (match_int([0], ) == 0) {
+   struct btrfs_device *device;
+
+   device = btrfs_find_device(info, intarg,
+  NULL, NULL);
+   if (!device) {
+   btrfs_err(info,
+ "read_mirror_policy: invalid devid 
%d",
+ intarg);
+   ret = -EINVAL;
+   goto out;
+   }
+   info->read_mirror_policy =
+   BTRFS_READ_MIRROR_BY_DEV;
+   set_bit(BTRFS_DEV_STATE_READ_MIRROR,
+   >dev_state);
+   break;
+   }
+
ret = -EINVAL;
goto out;
case Opt_err:
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 64dba5c4cf33..3a9c3804ff17 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -5220,6 +5220,16 @@ static int find_live_mirror(struct btrfs_fs_info 
*fs_info,
num_stripes = map->num_stripes;
 
switch(fs_info->read_mirror_policy) {
+   case BTRFS_READ_MIRROR_BY_DEV:
+   preferred_mirror = first;
+   if (test_bit(BTRFS_DEV_STATE_READ_MIRROR,
+>stripes[preferred_mirror].dev->dev_state))
+   break;
+   if (test_bit(BTRFS_DEV_STATE_READ_MIRROR,
+>stripes[++preferred_mirror].dev->dev_state))
+   break;
+   preferred_mirror = first;
+   break;
case BTRFS_READ_MIRROR_DEFAULT:
case BTRFS_READ_MIRROR_BY_PID:
default:
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 953df9925832..55b2eebbf183 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -37,6 +37,7 @@ struct btrfs_pending_bios {
 enum btrfs_read_mirror_type {
BTRFS_READ_MIRROR_DEFAULT,
BTRFS_READ_MIRROR_BY_PID,
+   BTRFS_READ_MIRROR_BY_DEV,
 };
 
 #define BTRFS_DEV_STATE_WRITEABLE  (0)
@@ -44,6 +45,7 @@ enum btrfs_read_mirror_type {
 #define BTRFS_DEV_STATE_MISSING(2)
 #define BTRFS_DEV_STATE_REPLACE_TGT(3)
 #define BTRFS_DEV_STATE_FLUSH_SENT (4)
+#define BTRFS_DEV_STATE_READ_MIRROR(5)
 
 struct btrfs_device {
struct list_head dev_list;
-- 
2.7.0

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/3] btrfs: add mount option read_mirror_policy

[Anand Jain]

In case of RAID1 and RAID10 devices are mirror-ed, a read IO can
pick any device for reading. This choice of picking a device for
reading should be configurable. In short not one policy would
satisfy all types of workload and configs.

So before we add more policies, this patch-set makes existing
$pid policy configurable from the mount option.

For example..
  mount -o read_mirror_policy=pid (which is also default)

Signed-off-by: Anand Jain 
---
 fs/btrfs/ctree.h   |  2 ++
 fs/btrfs/super.c   | 10 ++
 fs/btrfs/volumes.c |  8 +++-
 fs/btrfs/volumes.h |  5 +
 4 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index bfa96697209a..0d997939cdae 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -1121,6 +1121,8 @@ struct btrfs_fs_info {
spinlock_t ref_verify_lock;
struct rb_root block_tree;
 #endif
+   /* Policy to balance read across mirrored devices */
+   int read_mirror_policy;
 };
 
 static inline struct btrfs_fs_info *btrfs_sb(struct super_block *sb)
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index c67fafaa2fe7..21eff0ac1e4f 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -345,6 +345,7 @@ enum {
 #ifdef CONFIG_BTRFS_FS_REF_VERIFY
Opt_ref_verify,
 #endif
+   Opt_read_mirror_policy,
Opt_err,
 };
 
@@ -414,6 +415,7 @@ static const match_table_t tokens = {
 #ifdef CONFIG_BTRFS_FS_REF_VERIFY
{Opt_ref_verify, "ref_verify"},
 #endif
+   {Opt_read_mirror_policy, "read_mirror_policy=%s"},
{Opt_err, NULL},
 };
 
@@ -844,6 +846,14 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char 
*options,
btrfs_set_opt(info->mount_opt, REF_VERIFY);
break;
 #endif
+   case Opt_read_mirror_policy:
+   if (strcmp(args[0].from, "pid") == 0) {
+   info->read_mirror_policy =
+   BTRFS_READ_MIRROR_BY_PID;
+   break;
+   }
+   ret = -EINVAL;
+   goto out;
case Opt_err:
btrfs_info(info, "unrecognized mount option '%s'", p);
ret = -EINVAL;
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 81fb38884cac..64dba5c4cf33 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -5219,7 +5219,13 @@ static int find_live_mirror(struct btrfs_fs_info 
*fs_info,
else
num_stripes = map->num_stripes;
 
-   preferred_mirror = first + current->pid % num_stripes;
+   switch(fs_info->read_mirror_policy) {
+   case BTRFS_READ_MIRROR_DEFAULT:
+   case BTRFS_READ_MIRROR_BY_PID:
+   default:
+   preferred_mirror = first + current->pid % num_stripes;
+   break;
+   }
 
if (dev_replace_is_ongoing &&
fs_info->dev_replace.cont_reading_from_srcdev_mode ==
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 5139ec8daf4c..953df9925832 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -34,6 +34,11 @@ struct btrfs_pending_bios {
 #define btrfs_device_data_ordered_init(device) do { } while (0)
 #endif
 
+enum btrfs_read_mirror_type {
+   BTRFS_READ_MIRROR_DEFAULT,
+   BTRFS_READ_MIRROR_BY_PID,
+};
+
 #define BTRFS_DEV_STATE_WRITEABLE  (0)
 #define BTRFS_DEV_STATE_IN_FS_METADATA (1)
 #define BTRFS_DEV_STATE_MISSING(2)
-- 
2.7.0

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/14] vgem: separate errno from VM_FAULT_* values

[Daniel Vetter]

On Wed, May 16, 2018 at 07:43:44AM +0200, Christoph Hellwig wrote:
> And streamline the code in vgem_fault with early returns so that it is
> a little bit more readable.
> 
> Signed-off-by: Christoph Hellwig 
> ---
>  drivers/gpu/drm/vgem/vgem_drv.c | 51 +++--
>  1 file changed, 23 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
> index 2524ff116f00..a261e0aab83a 100644
> --- a/drivers/gpu/drm/vgem/vgem_drv.c
> +++ b/drivers/gpu/drm/vgem/vgem_drv.c
> @@ -61,12 +61,13 @@ static void vgem_gem_free_object(struct drm_gem_object 
> *obj)
>   kfree(vgem_obj);
>  }
>  
> -static int vgem_gem_fault(struct vm_fault *vmf)
> +static vm_fault_t vgem_gem_fault(struct vm_fault *vmf)
>  {
>   struct vm_area_struct *vma = vmf->vma;
>   struct drm_vgem_gem_object *obj = vma->vm_private_data;
>   /* We don't use vmf->pgoff since that has the fake offset */
>   unsigned long vaddr = vmf->address;
> + struct page *page;
>   int ret;
>   loff_t num_pages;
>   pgoff_t page_offset;
> @@ -85,35 +86,29 @@ static int vgem_gem_fault(struct vm_fault *vmf)
>   ret = 0;
>   }
>   mutex_unlock(>pages_lock);
> - if (ret) {
> - struct page *page;
> -
> - page = shmem_read_mapping_page(
> - file_inode(obj->base.filp)->i_mapping,
> - page_offset);
> - if (!IS_ERR(page)) {
> - vmf->page = page;
> - ret = 0;
> - } else switch (PTR_ERR(page)) {
> - case -ENOSPC:
> - case -ENOMEM:
> - ret = VM_FAULT_OOM;
> - break;
> - case -EBUSY:
> - ret = VM_FAULT_RETRY;
> - break;
> - case -EFAULT:
> - case -EINVAL:
> - ret = VM_FAULT_SIGBUS;
> - break;
> - default:
> - WARN_ON(PTR_ERR(page));
> - ret = VM_FAULT_SIGBUS;
> - break;
> - }
> + if (!ret)
> + return 0;
> +
> + page = shmem_read_mapping_page(file_inode(obj->base.filp)->i_mapping,
> + page_offset);
> + if (!IS_ERR(page)) {
> + vmf->page = page;
> + return 0;
> + }
>  
> + switch (PTR_ERR(page)) {
> + case -ENOSPC:
> + case -ENOMEM:
> + return VM_FAULT_OOM;
> + case -EBUSY:
> + return VM_FAULT_RETRY;
> + case -EFAULT:
> + case -EINVAL:
> + return VM_FAULT_SIGBUS;
> + default:
> + WARN_ON(PTR_ERR(page));
> + return VM_FAULT_SIGBUS;
>   }
> - return ret;

Reviewed-by: Daniel Vetter 

Want me to merge this through drm-misc or plan to pick it up yourself?
-Daniel

>  }
>  
>  static const struct vm_operations_struct vgem_gem_vm_ops = {
> -- 
> 2.17.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] btrfs: Skip some btrfs_cross_ref_exist() check in nocow path

[Nikolay Borisov]

On 16.05.2018 11:28, Ethan Lien wrote:
> In nocow path, we check if the extent is snapshotted in
> btrfs_cross_ref_exist(). We can do the similar check earlier and avoid
> unnecessary search into extent tree.
> 
> Signed-off-by: Ethan Lien 

If this is supposed to be a performance improvement do you have any
measurements that prove that this is indeed the case?



> ---
>  fs/btrfs/inode.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index d241285a0d2a..96927f2ccd4b 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -1373,6 +1373,9 @@ static noinline int run_delalloc_nocow(struct inode 
> *inode,
>   btrfs_file_extent_encryption(leaf, fi) ||
>   btrfs_file_extent_other_encoding(leaf, fi))
>   goto out_check;
> + if (btrfs_file_extent_generation(leaf, fi) <=
> + btrfs_root_last_snapshot(>root_item))
> + goto out_check;
>   if (extent_type == BTRFS_FILE_EXTENT_REG && !force)
>   goto out_check;
>   if (btrfs_extent_readonly(fs_info, disk_bytenr))
> @@ -7368,6 +7371,10 @@ noinline int can_nocow_extent(struct inode *inode, u64 
> offset, u64 *len,
>   btrfs_file_extent_other_encoding(leaf, fi))
>   goto out;
>  
> + if (btrfs_file_extent_generation(leaf, fi) <=
> + btrfs_root_last_snapshot(>root_item))
> + goto out;
> +
>   backref_offset = btrfs_file_extent_offset(leaf, fi);
>  
>   if (orig_start) {
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] btrfs: Skip some btrfs_cross_ref_exist() check in nocow path

[Ethan Lien]

In nocow path, we check if the extent is snapshotted in
btrfs_cross_ref_exist(). We can do the similar check earlier and avoid
unnecessary search into extent tree.

Signed-off-by: Ethan Lien 
---
 fs/btrfs/inode.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index d241285a0d2a..96927f2ccd4b 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -1373,6 +1373,9 @@ static noinline int run_delalloc_nocow(struct inode 
*inode,
btrfs_file_extent_encryption(leaf, fi) ||
btrfs_file_extent_other_encoding(leaf, fi))
goto out_check;
+   if (btrfs_file_extent_generation(leaf, fi) <=
+   btrfs_root_last_snapshot(>root_item))
+   goto out_check;
if (extent_type == BTRFS_FILE_EXTENT_REG && !force)
goto out_check;
if (btrfs_extent_readonly(fs_info, disk_bytenr))
@@ -7368,6 +7371,10 @@ noinline int can_nocow_extent(struct inode *inode, u64 
offset, u64 *len,
btrfs_file_extent_other_encoding(leaf, fi))
goto out;
 
+   if (btrfs_file_extent_generation(leaf, fi) <=
+   btrfs_root_last_snapshot(>root_item))
+   goto out;
+
backref_offset = btrfs_file_extent_offset(leaf, fi);
 
if (orig_start) {
-- 
2.17.0

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] btrfs-progs: remove BTRFS_LIST_LAYOUT_RAW

[Nikolay Borisov]

On 15.05.2018 12:17, Gu Jinxiang wrote:
> Since
> commit 9005b603d723 ("btrfs-progs: use libbtrfsutil for subvol show"),
> BTRFS_LIST_LAYOUT_RAW has no usage.
> So, remove it.
> 
> Signed-off-by: Gu Jinxiang 
Reviewed-by: Nikolay Borisov 

> ---
>  btrfs-list.c | 20 
>  btrfs-list.h |  3 +--
>  2 files changed, 1 insertion(+), 22 deletions(-)
> 
> diff --git a/btrfs-list.c b/btrfs-list.c
> index e01c5899..16be6b2f 100644
> --- a/btrfs-list.c
> +++ b/btrfs-list.c
> @@ -1374,23 +1374,6 @@ static void print_subvolume_column(struct root_info 
> *subv,
>   }
>  }
>  
> -static void print_one_subvol_info_raw(struct root_info *subv,
> - const char *raw_prefix)
> -{
> - int i;
> -
> - for (i = 0; i < BTRFS_LIST_ALL; i++) {
> - if (!btrfs_list_columns[i].need_print)
> - continue;
> -
> - if (raw_prefix)
> - printf("%s",raw_prefix);
> -
> - print_subvolume_column(subv, i);
> - }
> - printf("\n");
> -}
> -
>  static void print_one_subvol_info_table(struct root_info *subv)
>  {
>   int i;
> @@ -1480,9 +1463,6 @@ static void print_all_subvol_info(struct root_lookup 
> *sorted_tree,
>   case BTRFS_LIST_LAYOUT_TABLE:
>   print_one_subvol_info_table(entry);
>   break;
> - case BTRFS_LIST_LAYOUT_RAW:
> - print_one_subvol_info_raw(entry, raw_prefix);
> - break;
>   }
>  next:
>   n = rb_next(n);
> diff --git a/btrfs-list.h b/btrfs-list.h
> index 6e5fc778..299e3122 100644
> --- a/btrfs-list.h
> +++ b/btrfs-list.h
> @@ -33,8 +33,7 @@
>  
>  enum btrfs_list_layout {
>   BTRFS_LIST_LAYOUT_DEFAULT = 0,
> - BTRFS_LIST_LAYOUT_TABLE,
> - BTRFS_LIST_LAYOUT_RAW
> + BTRFS_LIST_LAYOUT_TABLE
>  };
>  
>  /*
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/2] btrfs: sysfs: Add entry which shows rmdir(2) can work for subvolume

[Tomohiro Misono]

Deletion of a subvolume by rmdir(2) has become allowed by the
'commit cd2decf640b1 ("btrfs: Allow rmdir(2) to delete an empty
subvolume")'.

It is a kind of new feature and this commits add a sysfs entry
  /sys/fs/btrfs/static_features/rmdir_subvol
to indicate the availability of feature so that a user program
(e.g. xfstests) can detect it.

Note that new sysfs directory "static_features" is created since a entry
in /sys/fs/btrfs/features depends on feature bits of superblock (in other
words, they may be different between each fs) and is not suitable to hold
the features which only depend on kernel version. New attribute_group
"btrfs_static_feature_attr_group" is created for this purpose.

Signed-off-by: Tomohiro Misono 
---
 fs/btrfs/sysfs.c | 29 +
 1 file changed, 29 insertions(+)

diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 217d401fe8ae..35b3ac567899 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -210,12 +210,32 @@ static struct attribute *btrfs_supported_feature_attrs[] 
= {
NULL
 };
 
+/* Features which depend on feature bits and may differ between each fs */
 static const struct attribute_group btrfs_feature_attr_group = {
.name = "features",
.is_visible = btrfs_feature_visible,
.attrs = btrfs_supported_feature_attrs,
 };
 
+static ssize_t rmdir_subvol_show(struct kobject *kobj,
+struct kobj_attribute *ka, char *buf)
+{
+   /* No meaning for the value */
+   return snprintf(buf, PAGE_SIZE, "0\n");
+}
+BTRFS_ATTR(static_feature, rmdir_subvol, rmdir_subvol_show);
+
+static struct attribute *btrfs_supported_static_feature_attrs[] = {
+   BTRFS_ATTR_PTR(static_feature, rmdir_subvol),
+   NULL
+};
+
+/* Features which only depend on kernel version */
+static const struct attribute_group btrfs_static_feature_attr_group = {
+   .name = "static_features",
+   .attrs = btrfs_supported_static_feature_attrs,
+};
+
 static ssize_t btrfs_show_u64(u64 *value_ptr, spinlock_t *lock, char *buf)
 {
u64 val;
@@ -901,8 +921,15 @@ int __init btrfs_init_sysfs(void)
ret = sysfs_create_group(_kset->kobj, _feature_attr_group);
if (ret)
goto out2;
+   ret = sysfs_create_group(_kset->kobj,
+_static_feature_attr_group);
+   if (ret)
+   goto out3;
 
return 0;
+
+out3:
+   sysfs_remove_group(_kset->kobj, _feature_attr_group);
 out2:
debugfs_remove_recursive(btrfs_debugfs_root_dentry);
 out1:
@@ -914,6 +941,8 @@ int __init btrfs_init_sysfs(void)
 void __cold btrfs_exit_sysfs(void)
 {
sysfs_remove_group(_kset->kobj, _feature_attr_group);
+   sysfs_remove_group(_kset->kobj,
+  _static_feature_attr_group);
kset_unregister(btrfs_kset);
debugfs_remove_recursive(btrfs_debugfs_root_dentry);
 }
-- 
2.14.3


--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 0/2] btrfs: sysfs: Add entry which shows rmdir(2) can work for subvolume

[Tomohiro Misono]

[based on current misc-next]

-
changelog
  v1 -> v2
- Explicitly set start of enum btrfs_feature_set as 0 in 1st patch
- Create new sysfs group which shows static feature (i.e. features
  which only depends on kernel version). See 2nd path.
-

This adds new sysfs entry
  /sys/fs/btrfs/static_features/rmdir_subvol
to indicate that the kernel can delete a subvolume by rmdir(2),
which is allowed by: https://www.spinics.net/lists/linux-btrfs/msg76938.html

The aim of adding a entry is to enable a user program (e.g. xfstest)
to detect a feature.

The first patch is a cleanup and the second one is a main part.

Tomohiro Misono (2):
  btrfs: sysfs: Use enum/define value intead of magic number
  btrfs: sysfs: Add entry which shows rmdir(2) can work for subvolume

 fs/btrfs/sysfs.c | 40 +++-
 fs/btrfs/sysfs.h |  4 ++--
 2 files changed, 37 insertions(+), 7 deletions(-)

-- 
2.14.3


--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/2] btrfs: sysfs: Use enum/define value intead of magic number

[Tomohiro Misono]

Signed-off-by: Tomohiro Misono 
---
 fs/btrfs/sysfs.c | 11 ++-
 fs/btrfs/sysfs.h |  4 ++--
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index fa6c8c88b250..217d401fe8ae 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -514,10 +514,11 @@ static inline struct btrfs_fs_info *to_fs_info(struct 
kobject *kobj)
 }
 
 #define NUM_FEATURE_BITS 64
-static char btrfs_unknown_feature_names[3][NUM_FEATURE_BITS][13];
-static struct btrfs_feature_attr btrfs_feature_attrs[3][NUM_FEATURE_BITS];
+#define BTRFS_FEATURE_NAME_MAX 13
+static char 
btrfs_unknown_feature_names[FEAT_MAX][NUM_FEATURE_BITS][BTRFS_FEATURE_NAME_MAX];
+static struct btrfs_feature_attr 
btrfs_feature_attrs[FEAT_MAX][NUM_FEATURE_BITS];
 
-static const u64 supported_feature_masks[3] = {
+static const u64 supported_feature_masks[FEAT_MAX] = {
[FEAT_COMPAT]= BTRFS_FEATURE_COMPAT_SUPP,
[FEAT_COMPAT_RO] = BTRFS_FEATURE_COMPAT_RO_SUPP,
[FEAT_INCOMPAT]  = BTRFS_FEATURE_INCOMPAT_SUPP,
@@ -609,7 +610,7 @@ void btrfs_sysfs_remove_mounted(struct btrfs_fs_info 
*fs_info)
btrfs_sysfs_rm_device_link(fs_info->fs_devices, NULL);
 }
 
-const char * const btrfs_feature_set_names[3] = {
+const char * const btrfs_feature_set_names[FEAT_MAX] = {
[FEAT_COMPAT]= "compat",
[FEAT_COMPAT_RO] = "compat_ro",
[FEAT_INCOMPAT]  = "incompat",
@@ -673,7 +674,7 @@ static void init_feature_attrs(void)
if (fa->kobj_attr.attr.name)
continue;
 
-   snprintf(name, 13, "%s:%u",
+   snprintf(name, BTRFS_FEATURE_NAME_MAX, "%s:%u",
 btrfs_feature_set_names[set], i);
 
fa->kobj_attr.attr.name = name;
diff --git a/fs/btrfs/sysfs.h b/fs/btrfs/sysfs.h
index b567560d9aa9..c6ee600aff89 100644
--- a/fs/btrfs/sysfs.h
+++ b/fs/btrfs/sysfs.h
@@ -9,7 +9,7 @@
 extern u64 btrfs_debugfs_test;
 
 enum btrfs_feature_set {
-   FEAT_COMPAT,
+   FEAT_COMPAT = 0,
FEAT_COMPAT_RO,
FEAT_INCOMPAT,
FEAT_MAX
@@ -77,7 +77,7 @@ attr_to_btrfs_feature_attr(struct attribute *attr)
 }
 
 char *btrfs_printable_features(enum btrfs_feature_set set, u64 flags);
-extern const char * const btrfs_feature_set_names[3];
+extern const char * const btrfs_feature_set_names[FEAT_MAX];
 extern struct kobj_type space_info_ktype;
 extern struct kobj_type btrfs_raid_ktype;
 int btrfs_sysfs_add_device_link(struct btrfs_fs_devices *fs_devices,
-- 
2.14.3


--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] btrfs: sysfs: Add entry which shows rmdir(2) can work for subvolume

[Misono Tomohiro]

On 2018/05/15 22:22, David Sterba wrote:
> On Tue, May 15, 2018 at 04:33:12PM +0900, Misono Tomohiro wrote:
>> Deletion of a subvolume by rmdir(2) has become allowed by the
>> 'commit cd2decf640b1 ("btrfs: Allow rmdir(2) to delete an empty
>> subvolume")'.
>>
>> It is a kind of new feature and this commits add new sysfs entry
>>   /sys/fs/btrfs/features/rmdir_subvol
>> to indicate the feature.
>>
>> Since the behavior is independent of feature bits of superblock,
>> new type FEAT_KERNEL is added to struct btrfs_feature_set.
>> Features of FEAT_KERNEL is supposed to be visible only in /sys/fs/features
>> and not in /sys/fs/UUID/features.
> 
> As the rmdir_subvol is a static feature, depending only on the kernel
> version, it's not needed to use the same infrastructure as the optional
> features. It also makes it unnecesarily complicated, to distinguish the
> on-disk and kernel-only features and it's not a per-filesystem feature.
> 
> It should be exported among btrfs_feature_attr_group and
> btrfs_supported_feature_attrs, possibly adding a new type of helpers if
> needed.

Understood. I will send v2 which adds new attr_group for static features.

> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Btrfs: fix the corruption by reading stale btree blocks

[Nikolay Borisov]

On 15.05.2018 20:37, Liu Bo wrote:
> If a btree block, aka. extent buffer, is not available in the extent
> buffer cache, it'll be read out from the disk instead, i.e.
> 
> btrfs_search_slot()
>   read_block_for_search()  # hold parent and its lock, go to read child
> btrfs_release_path()
> read_tree_block()  # read child
> 
> Unfortunately, the parent lock got released before reading child, so
> commit 5bdd3536cbbe ("Btrfs: Fix block generation verification race") had
> used 0 as parent transid to read the child block.  It forces
> read_tree_block() not to check if parent transid is different with the
> generation id of the child that it reads out from disk.
> 
> A simple PoC is included in btrfs/124,
> 
> 0. A two-disk raid1 btrfs,
> 
> 1. Right after mkfs.btrfs, block A is allocated to be device tree's root.
> 
> 2. Mount this filesystem and put it in use, after a while, device tree's
>root got COW but block A hasn't been allocated/overwritten yet.
> 
> 3. Umount it and reload the btrfs module to remove both disks from the
>global @fs_devices list.
> 
> 4. mount -odegraded dev1 and write some data, so now block A is allocated
>to be a leaf in checksum tree.  Note that only dev1 has the latest
>metadata of this filesystem.
> 
> 5. Umount it and mount it again normally (with both disks), since raid1
>can pick up one disk by the writer task's pid, if btrfs_search_slot()
>needs to read block A, dev2 which does NOT have the latest metadata
>might be read for block A, then we got a stale block A.
> 
> 6. As parent transid is not checked, block A is marked as uptodate and
>put into the extent buffer cache, so the future search won't bother
>to read disk again, which means it'll make changes on this stale
>one and make it dirty and flush it onto disk.
> 
> To avoid the problem, parent transid needs to be passed to
> read_tree_block().
> 
> In order to get a valid parent transid, we need to hold the parent's
> lock until finish reading child.
> 
> Signed-off-by: Liu Bo 

Doesn't this warrant a stable tag and
Fixes: 5bdd3536cbbe ("Btrfs: Fix block generation verification race")
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/3] btrfs: balance: add args info during start and resume

[Nikolay Borisov]

On 16.05.2018 05:51, Anand Jain wrote:
> Balance args info is an important information to be reviewed for the
> system audit. So this patch adds it to the kernel log.
> 
> Example:
> 
> -> btrfs bal start -dprofiles='raid1|single',convert=raid5 
> -mprofiles='raid1|single',convert=raid5 /btrfs
> 
>  kernel: BTRFS info (device sdb): balance: start data profiles=raid1|single 
> convert=raid5 metadata profiles=raid1|single convert=raid5 system 
> profiles=raid1|single convert=raid5
> 
> -> btrfs bal start -dprofiles=raid5,convert=single 
> -mprofiles='raid1|single',convert=raid5 --background /btrfs
> 
>  kernel: BTRFS info (device sdb): balance: start data profiles=raid5 
> convert=single metadata profiles=raid1|single convert=raid5 system 
> profiles=raid1|single convert=raid5
> 
> Signed-off-by: Anand Jain 

Why can't this code be part of progs, the bctl which you are parsing is
constructed from the arguments passed from users space? I think you are
adding way too much string parsing code to the kernel and this is never
a good sign, since it's very easy to trip.

> ---
> v1->v2: Change log update.
> Move adding the logs for balance complete and end to a new patch
> 
>  fs/btrfs/volumes.c | 146 
> +++--
>  1 file changed, 143 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 27da66c47ef2..ce68c4f42f94 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -126,6 +126,32 @@ const char *get_raid_name(enum btrfs_raid_types type)
>   return btrfs_raid_array[type].raid_name;
>  }
>  
> +static void get_all_raid_names(u64 bg_flags, char *raid_types)
> +{
> + int i;
> + bool found = false;
> +
> + for (i = 0; i < BTRFS_NR_RAID_TYPES; i++) {
> + if (bg_flags & btrfs_raid_array[i].bg_flag) {
> + if (found) {
> + strcat(raid_types, "|");
> + strcat(raid_types, 
> btrfs_raid_array[i].raid_name);
> + } else {
> + found = true;
> + sprintf(raid_types, "%s", 
> btrfs_raid_array[i].raid_name);
> + }
> + }
> + }
> + if (bg_flags & BTRFS_AVAIL_ALLOC_BIT_SINGLE) {
> + if (found) {
> + strcat(raid_types, "|");
> + strcat(raid_types, "single");
> + } else {
> + sprintf(raid_types, "%s", "single");
> + }
> + }
> +}
> +
>  static int init_first_rw_device(struct btrfs_trans_handle *trans,
>   struct btrfs_fs_info *fs_info);
>  static int btrfs_relocate_sys_chunks(struct btrfs_fs_info *fs_info);
> @@ -3766,6 +3792,121 @@ static inline int validate_convert_profile(struct 
> btrfs_balance_args *bctl_arg,
>(bctl_arg->target & ~allowed)));
>  }
>  
> +static void get_balance_args(struct btrfs_balance_args *bargs, char *args)
> +{
> + char value[64];
> +
> + if (bargs->flags & BTRFS_BALANCE_ARGS_PROFILES) {
> + strcat(args, "profiles=");
> + get_all_raid_names(bargs->profiles, value);
> + strcat(args, value);
> + strcat(args, " ");
> + }
> +
> + if (bargs->flags & BTRFS_BALANCE_ARGS_USAGE) {
> + snprintf(value, 64, "usage=%llu ", bargs->usage);
> + strcat(args, value);
> + }
> +
> + if (bargs->flags & BTRFS_BALANCE_ARGS_USAGE_RANGE) {
> + snprintf(value, 64, "usage_min=%u usage_max=%u ",
> + bargs->usage_min, bargs->usage_max);
> + strcat(args, value);
> + }
> +
> + if (bargs->flags & BTRFS_BALANCE_ARGS_DEVID) {
> + snprintf(value, 64, "devid=%llu ", bargs->devid);
> + strcat(args, value);
> + }
> +
> + if (bargs->flags & BTRFS_BALANCE_ARGS_DRANGE) {
> + snprintf(value, 64, "pstart=%llu pend=%llu ",
> +  bargs->pstart, bargs->pend);
> + strcat(args, value);
> + }
> +
> + if (bargs->flags & BTRFS_BALANCE_ARGS_VRANGE) {
> + snprintf(value, 64, "vstart=%llu vend %llu ",
> +  bargs->vstart, bargs->vend);
> + strcat(args, value);
> + }
> +
> + if (bargs->flags & BTRFS_BALANCE_ARGS_LIMIT) {
> + snprintf(value, 64, "limit=%llu ", bargs->limit);
> + strcat(args, value);
> + }
> +
> + if (bargs->flags & BTRFS_BALANCE_ARGS_LIMIT_RANGE) {
> + snprintf(value, 64, "limit_min=%u limit_max=%u ",
> +  bargs->limit_min, bargs->limit_max);
> + strcat(args, value);
> + }
> +
> + if (bargs->flags & BTRFS_BALANCE_ARGS_STRIPES_RANGE) {
> + snprintf(value, 64, "stripes_min=%u stripes_max=%u ",
> +  bargs->stripes_min, bargs->stripes_max);
> + strcat(args, 

Re: [PATCH v2 3/3] btrfs: balance: add kernel log for end or paused

[Nikolay Borisov]

On 16.05.2018 05:51, Anand Jain wrote:
> Add a kernel log when the balance ends, either for cancel or completed
> or if it is paused.

Reviewed-by: Nikolay Borisov 

> ---
> v1->v2: Moved from 2/3 to 3/3
> 
>  fs/btrfs/volumes.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index ce68c4f42f94..a4e243a29f5c 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -4053,6 +4053,13 @@ int btrfs_balance(struct btrfs_fs_info *fs_info,
>   ret = __btrfs_balance(fs_info);
>  
>   mutex_lock(_info->balance_mutex);
> + if (ret == -ECANCELED && atomic_read(_info->balance_pause_req))
> + btrfs_info(fs_info, "balance: paused");
> + else if (ret == -ECANCELED && atomic_read(_info->balance_cancel_req))
> + btrfs_info(fs_info, "balance: canceled");
> + else
> + btrfs_info(fs_info, "balance: ended with status: %d", ret);
> +
>   clear_bit(BTRFS_FS_BALANCE_RUNNING, _info->flags);
>  
>   if (bargs) {
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/3] btrfs: balance: prefix kernel logs

[Nikolay Borisov]

On 16.05.2018 05:51, Anand Jain wrote:
> Kernel logs are very important for the forensic investigations of the
> issues in general make it easy to use it. This patch adds 'balance:'
> prefix so that it can be easily searched.
> 
> Signed-off-by: Anand Jain 

Straightforward:

Reviewed-by: Nikolay Borisov 

> ---
> v1-v2: Change log update.
>  fs/btrfs/volumes.c | 34 --
>  1 file changed, 20 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 9773bc143650..27da66c47ef2 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -3801,7 +3801,7 @@ int btrfs_balance(struct btrfs_fs_info *fs_info,
>   !(bctl->flags & BTRFS_BALANCE_METADATA) ||
>   memcmp(>data, >meta, sizeof(bctl->data))) {
>   btrfs_err(fs_info,
> -   "with mixed groups data and metadata balance 
> options must be the same");
> +   "balance: mixed groups data and metadata 
> options must be the same");
>   ret = -EINVAL;
>   goto out;
>   }
> @@ -3823,23 +3823,26 @@ int btrfs_balance(struct btrfs_fs_info *fs_info,
>   allowed |= (BTRFS_BLOCK_GROUP_RAID10 |
>   BTRFS_BLOCK_GROUP_RAID6);
>   if (validate_convert_profile(>data, allowed)) {
> + int index = btrfs_bg_flags_to_raid_index(bctl->data.target);
>   btrfs_err(fs_info,
> -   "unable to start balance with target data profile 
> %llu",
> -   bctl->data.target);
> +   "balance: invalid convert data profile %s",
> +   get_raid_name(index));
>   ret = -EINVAL;
>   goto out;
>   }
>   if (validate_convert_profile(>meta, allowed)) {
> + int index = btrfs_bg_flags_to_raid_index(bctl->meta.target);
>   btrfs_err(fs_info,
> -   "unable to start balance with target metadata profile 
> %llu",
> -   bctl->meta.target);
> +   "balance: invalid convert metadata profile %s",
> +   get_raid_name(index));
>   ret = -EINVAL;
>   goto out;
>   }
>   if (validate_convert_profile(>sys, allowed)) {
> + int index = btrfs_bg_flags_to_raid_index(bctl->sys.target);
>   btrfs_err(fs_info,
> -   "unable to start balance with target system profile 
> %llu",
> -   bctl->sys.target);
> +   "balance: invalid convert system profile %s",
> +   get_raid_name(index));
>   ret = -EINVAL;
>   goto out;
>   }
> @@ -3860,10 +3863,10 @@ int btrfs_balance(struct btrfs_fs_info *fs_info,
>!(bctl->meta.target & allowed))) {
>   if (bctl->flags & BTRFS_BALANCE_FORCE) {
>   btrfs_info(fs_info,
> -"force reducing metadata integrity");
> +"balance: force reducing metadata 
> integrity");
>   } else {
>   btrfs_err(fs_info,
> -   "balance will reduce metadata 
> integrity, use force if you want this");
> +   "balance: reduces metadata integrity, 
> use force if you want this");
>   ret = -EINVAL;
>   goto out;
>   }
> @@ -3877,9 +3880,11 @@ int btrfs_balance(struct btrfs_fs_info *fs_info,
>   bctl->data.target : fs_info->avail_data_alloc_bits;
>   if (btrfs_get_num_tolerated_disk_barrier_failures(meta_target) <
>   btrfs_get_num_tolerated_disk_barrier_failures(data_target)) {
> + int meta_index = btrfs_bg_flags_to_raid_index(meta_target);
> + int data_index = btrfs_bg_flags_to_raid_index(data_target);
>   btrfs_warn(fs_info,
> -"metadata profile 0x%llx has lower redundancy than 
> data profile 0x%llx",
> -meta_target, data_target);
> +"balance: metadata profile %s has lower redundancy 
> than data profile %s",
> +get_raid_name(meta_index), 
> get_raid_name(data_index));
>   }
>  
>   ret = insert_balance_item(fs_info, bctl);
> @@ -3939,7 +3944,7 @@ static int balance_kthread(void *data)
>  
>   mutex_lock(_info->balance_mutex);
>   if (fs_info->balance_ctl) {
> - btrfs_info(fs_info, "continuing balance");
> + btrfs_info(fs_info, "balance: resuming");
>   ret = btrfs_balance(fs_info, fs_info->balance_ctl, NULL);
>   }
>   mutex_unlock(_info->balance_mutex);
> @@ 

Re: [PATCH 5/6] Btrfs: grab write lock directly if write_lock_level is the max level

[Nikolay Borisov]

On 15.05.2018 20:52, Liu Bo wrote:
> In case of (cow && (p->keep_locks || p->lowest_level)), write_lock_level
> is the max level, and we should grab write lock of root node from the very
> beginning.

THis needs to be expanded to explain what are the adverse effects (if
any) without this commit. And then explain how this commit actually
improve things.

> 
> Signed-off-by: Liu Bo 
> ---
>  fs/btrfs/ctree.c | 29 -
>  1 file changed, 16 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index cf34eca41d4e..f7c3f581f647 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -2633,20 +2633,23 @@ static struct extent_buffer 
> *btrfs_search_slot_get_root(struct btrfs_root *root,
>   goto out;
>   }
>  
> - /*
> -  * we don't know the level of the root node until we actually
> -  * have it read locked
> -  */
> - b = btrfs_read_lock_root_node(root);
> - level = btrfs_header_level(b);
> - if (level > write_lock_level)
> - goto out;
> + if (write_lock_level < BTRFS_MAX_LEVEL) {
> + /*
> +  * we don't know the level of the root node until we actually
> +  * have it read locked
> +  */
> + b = btrfs_read_lock_root_node(root);
> + level = btrfs_header_level(b);
> + if (level > write_lock_level)
> + goto out;
> +
> + /*
> +  * whoops, must trade for write lock
> +  */
> + btrfs_tree_read_unlock(b);
> + free_extent_buffer(b);
> + }

Here just seems you are adding 1 extra branch so more context please.

>  
> - /*
> -  * whoops, must trade for write lock
> -  */
> - btrfs_tree_read_unlock(b);
> - free_extent_buffer(b);
>   b = btrfs_lock_root_node(root);
>   root_lock = BTRFS_WRITE_LOCK;
>   /*
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/6] Btrfs: remove unused check of skip_locking

[Qu Wenruo]

On 2018年05月16日 15:03, Nikolay Borisov wrote:
> 
> 
> On 15.05.2018 20:52, Liu Bo wrote:
>> The check is superfluous since all of callers who set search_for_commit
>> also have skip_locking set.
> 
> This is false. For example btrfs_qgroup_rescan_worker sets
> search_commit_root = 1 but doesn't set skip locking. So either your
> assumption is wrong or the code in btrfs_qgroup_rescan_worker has missed
> that (which seems more likely).

Liu's assumption on all commit root searcher don't need lock looks good
to me, thus qgroup code could set skip_locking.

> 
> I think this change necessitates either:
> 
> a) ASSERT(p->skip_locking) inside the if (p->search_commit_root) branch
> 
> or
> 
> b) Unconditionally setting ->skip_locking if we have set
> search_commit_root and removing opencoded set of skip_locking alongside
> search_commit_root.
> 
> I think b) is the better option since it provides "fire and forget"
> semantics when you want to search the commit root, since it's only read
> only.

b) looks better to me.

Thanks,
Qu

> 
>>
>> Signed-off-by: Liu Bo 
>> ---
>>  fs/btrfs/ctree.c | 2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
>> index 399839df7a8f..cf34eca41d4e 100644
>> --- a/fs/btrfs/ctree.c
>> +++ b/fs/btrfs/ctree.c
>> @@ -2623,8 +2623,6 @@ static struct extent_buffer 
>> *btrfs_search_slot_get_root(struct btrfs_root *root,
>>  level = btrfs_header_level(b);
>>  if (p->need_commit_sem)
>>  up_read(_info->commit_root_sem);
>> -if (!p->skip_locking)
>> -btrfs_tree_read_lock(b);
>>  
>>  goto out;
>>  }
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/6] Btrfs: remove unused check of skip_locking

[Nikolay Borisov]

On 15.05.2018 20:52, Liu Bo wrote:
> The check is superfluous since all of callers who set search_for_commit
> also have skip_locking set.

This is false. For example btrfs_qgroup_rescan_worker sets
search_commit_root = 1 but doesn't set skip locking. So either your
assumption is wrong or the code in btrfs_qgroup_rescan_worker has missed
that (which seems more likely).

I think this change necessitates either:

a) ASSERT(p->skip_locking) inside the if (p->search_commit_root) branch

or

b) Unconditionally setting ->skip_locking if we have set
search_commit_root and removing opencoded set of skip_locking alongside
search_commit_root.

I think b) is the better option since it provides "fire and forget"
semantics when you want to search the commit root, since it's only read
only.

> 
> Signed-off-by: Liu Bo 
> ---
>  fs/btrfs/ctree.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index 399839df7a8f..cf34eca41d4e 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -2623,8 +2623,6 @@ static struct extent_buffer 
> *btrfs_search_slot_get_root(struct btrfs_root *root,
>   level = btrfs_header_level(b);
>   if (p->need_commit_sem)
>   up_read(_info->commit_root_sem);
> - if (!p->skip_locking)
> - btrfs_tree_read_lock(b);
>  
>   goto out;
>   }
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/6] Btrfs: move get root of btrfs_search_slot to a helper

[Nikolay Borisov]

On 15.05.2018 20:52, Liu Bo wrote:
> It's good to have a helper instead of having all get-root details
> open-coded.
> 
> The new helper locks (if necessary) and sets root node of the path.
> 
> There is no functional change in this commit.
> 
> Signed-off-by: Liu Bo 

Codewise it looks ok, you've inverted the conditional so as to give a
more linear flow of the code. I've checked all the various branches and
they seem semantically identical to the open coded version. One minor
nit is that I don't think this is the most suitable name for this
function but I don't really have a better suggestions which would be
more specific.

For the code (the changelog should be more explicit):

Reviewed-by: Nikolay Borisov 

> ---
>  fs/btrfs/ctree.c | 112 
> +--
>  1 file changed, 67 insertions(+), 45 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index a96d308c51b8..399839df7a8f 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -2598,6 +2598,72 @@ int btrfs_find_item(struct btrfs_root *fs_root, struct 
> btrfs_path *path,
>   return 0;
>  }
>  
> +static struct extent_buffer *btrfs_search_slot_get_root(struct btrfs_root 
> *root,
> + struct btrfs_path *p,
> + int write_lock_level)
> +{
> + struct btrfs_fs_info *fs_info = root->fs_info;
> + struct extent_buffer *b;
> + int root_lock;
> + int level = 0;
> +
> + /*
> +  * we try very hard to do read locks on the root
> +  */
> + root_lock = BTRFS_READ_LOCK;
> +
> + if (p->search_commit_root) {
> + /*
> +  * the commit roots are read only so we always do read locks
> +  */
> + if (p->need_commit_sem)
> + down_read(_info->commit_root_sem);
> + b = root->commit_root;
> + extent_buffer_get(b);
> + level = btrfs_header_level(b);
> + if (p->need_commit_sem)
> + up_read(_info->commit_root_sem);
> + if (!p->skip_locking)
> + btrfs_tree_read_lock(b);
> +
> + goto out;
> + }
> +
> + if (p->skip_locking) {
> + b = btrfs_root_node(root);
> + level = btrfs_header_level(b);
> + goto out;
> + }
> +
> + /*
> +  * we don't know the level of the root node until we actually
> +  * have it read locked
> +  */
> + b = btrfs_read_lock_root_node(root);
> + level = btrfs_header_level(b);
> + if (level > write_lock_level)
> + goto out;
> +
> + /*
> +  * whoops, must trade for write lock
> +  */
> + btrfs_tree_read_unlock(b);
> + free_extent_buffer(b);
> + b = btrfs_lock_root_node(root);
> + root_lock = BTRFS_WRITE_LOCK;
> + /*
> +  * the level might have changed, check again
> +  */
> + level = btrfs_header_level(b);
> +
> +out:
> + p->nodes[level] = b;
> + if (!p->skip_locking)
> + p->locks[level] = root_lock;
> + return b;
> +}
> +
> +
>  /*
>   * btrfs_search_slot - look for a key in a tree and perform necessary
>   * modifications to preserve tree invariants.
> @@ -2634,7 +2700,6 @@ int btrfs_search_slot(struct btrfs_trans_handle *trans, 
> struct btrfs_root *root,
>   int err;
>   int level;
>   int lowest_unlock = 1;
> - int root_lock;
>   /* everything at write_lock_level or lower must be write locked */
>   int write_lock_level = 0;
>   u8 lowest_level = 0;
> @@ -2672,50 +2737,7 @@ int btrfs_search_slot(struct btrfs_trans_handle 
> *trans, struct btrfs_root *root,
>  
>  again:
>   prev_cmp = -1;
> - /*
> -  * we try very hard to do read locks on the root
> -  */
> - root_lock = BTRFS_READ_LOCK;
> - level = 0;
> - if (p->search_commit_root) {
> - /*
> -  * the commit roots are read only
> -  * so we always do read locks
> -  */
> - if (p->need_commit_sem)
> - down_read(_info->commit_root_sem);
> - b = root->commit_root;
> - extent_buffer_get(b);
> - level = btrfs_header_level(b);
> - if (p->need_commit_sem)
> - up_read(_info->commit_root_sem);
> - if (!p->skip_locking)
> - btrfs_tree_read_lock(b);
> - } else {
> - if (p->skip_locking) {
> - b = btrfs_root_node(root);
> - level = btrfs_header_level(b);
> - } else {
> - /* we don't know the level of the root node
> -  * until we actually have it read locked
> -  */
> - b = btrfs_read_lock_root_node(root);
> - level = btrfs_header_level(b);
> - 

Re: [PATCH 6/6] Btrfs: remove always true check in unlock_up

[Nikolay Borisov]

On 15.05.2018 20:52, Liu Bo wrote:
> @path->lock[i] is always true at this point.

You must explain why it's true. The reason is since at the beginning of
the for loop the check is performed :

   if (!path->locks[i])
  break;


Codewise it's ok:

Reviewed-by: Nikolay Borisov 

> 
> Signed-off-by: Liu Bo 
> ---
>  fs/btrfs/ctree.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index f7c3f581f647..16d28a4ec54f 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -2330,7 +2330,7 @@ static noinline void unlock_up(struct btrfs_path *path, 
> int level,
>   no_skips = 1;
>  
>   t = path->nodes[i];
> - if (i >= lowest_unlock && i > skip_level && path->locks[i]) {
> + if (i >= lowest_unlock && i > skip_level) {
>   btrfs_tree_unlock_rw(t, path->locks[i]);
>   path->locks[i] = 0;
>   if (write_lock_level &&
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/6] Btrfs: use more straightforward extent_buffer_uptodate

[Nikolay Borisov]

On 15.05.2018 20:52, Liu Bo wrote:
> In read_block_for_search(), it's straightforward to use
> extent_buffer_uptodate() instead since 0 is passed as parent transid to

"instead of the more heavyweight btrfs_buffer_update"

> btrfs_buffer_uptodate(), which means the check for parent transid is not
> needed.
> 
> Signed-off-by: Liu Bo 

Codewise LGTM:

Reviewed-by: Nikolay Borisov 
> ---
>  fs/btrfs/ctree.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index 9fa3d77c98d4..a96d308c51b8 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -2445,7 +2445,7 @@ noinline void btrfs_unlock_up_safe(struct btrfs_path 
> *path, int level)
>* and give up so that our caller doesn't loop forever
>* on our EAGAINs.
>*/
> - if (!btrfs_buffer_uptodate(tmp, 0, 0))
> + if (!extent_buffer_uptodate(tmp))
>   ret = -EIO;
>   free_extent_buffer(tmp);
>   } else {
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/6] Btrfs: remove superfluous free_extent_buffer

[Nikolay Borisov]

On 15.05.2018 20:52, Liu Bo wrote:
> @tmp must be NULL at this point, free_extent_buffer is not needed at all.

You need to explain that this code is executed only if
find_extent_buffer didn't return anything, meaning tmp is null,
otherwise it's hard to review this change without looking at the code.

Codewise it's correct:

Reviewed-by: Nikolay Borisov 

> 
> Signed-off-by: Liu Bo 
> ---
>  fs/btrfs/ctree.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index b3f6f300e492..9fa3d77c98d4 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -2432,7 +2432,6 @@ noinline void btrfs_unlock_up_safe(struct btrfs_path 
> *path, int level)
>   btrfs_unlock_up_safe(p, level + 1);
>   btrfs_set_path_blocking(p);
>  
> - free_extent_buffer(tmp);
>   if (p->reada != READA_NONE)
>   reada_for_search(fs_info, p, level, slot, key->objectid);
>  
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html