Re: [PATCH] btrfs/029: fix wrong usage of name filter

[Misono, Tomohiro]

On 2017/08/30 20:09, Eryu Guan wrote:
> On Wed, Aug 30, 2017 at 04:38:16PM +0900, Misono, Tomohiro wrote:
>> btrfs/029 uses _filter_testdirs() to filter the name of $TEST_DIR and
>> $SCRATCH_MNT directory.
>>
>> In this function, it calls both _filter_test_dir and _filter_scratch
>> concatenapted by pipe. Therefore if $TEST_DIR is a prefix of
>> $SCRATCH_MNT, this filter function gives wrong filtered name for
>> $SCRATCH_MNT and the test fails.
> 
> Sorry, I'm a bit confused, how could $TEST_DIR be a prefix of
> $SCRATCH_MNT? Won't that fail the test at setup time?

I used "/mnt" for $TEST_DIR and "/mnt_scratch" for $SCRATCH_MNT and hit
this problem because "/mnt_scratch" is filtered to "$TEST_DIR_scrach"
instead of "$SCRATCH_MNT".

I think these are valid directory names and other btrfs tests run correctly
with these names.

Thanks,
Tomohiro

--
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-progs: Add zstd support

[Nick Terrell]

Adds zstd support to the btrfs program, and a dependency on libzstd >=
1.0.0.

The patch is also available in my fork of btrfs-progs [1], which passes
Travis-CI. I tested each command that is effected in my test script [2].

I haven't updated Android.mk since I have no way to test it, and am not
certain if it is used.

[1] https://github.com/terrelln/btrfs-progs/tree/devel
[2] https://gist.github.com/terrelln/4136a369c5d10092956781433eed0a23

Signed-off-by: Nick Terrell 
---
 .travis.yml |  9 +++
 Documentation/btrfs-filesystem.asciidoc |  2 +-
 Documentation/btrfs-man5.asciidoc   |  8 +-
 Documentation/btrfs-property.asciidoc   |  2 +-
 INSTALL |  1 +
 Makefile.inc.in |  4 +--
 cmds-filesystem.c   | 16 ++--
 cmds-inspect-dump-super.c   |  2 +-
 cmds-restore.c  | 44 +
 configure.ac|  4 ++-
 ctree.h | 15 ---
 fsfeatures.h|  2 +-
 print-tree.c|  3 +++
 13 files changed, 87 insertions(+), 25 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 2aa44bd..50b3c1c 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -59,6 +59,15 @@ before_install:
  sudo make install;
  cd ../..
 "
+  - "mkdir tmp-zstd;
+ cd tmp-zstd;
+ wget https://github.com/facebook/zstd/archive/v1.3.1.tar.gz;
+ tar xf v1.3.1.tar.gz;
+ cd zstd-1.3.1;
+ make;
+ sudo make install PREFIX=/usr;
+ cd ../..
+"
   - "./autogen.sh && ./configure --disable-documentation && make"

 addons:
diff --git a/Documentation/btrfs-filesystem.asciidoc 
b/Documentation/btrfs-filesystem.asciidoc
index b60ef74..41b3032 100644
--- a/Documentation/btrfs-filesystem.asciidoc
+++ b/Documentation/btrfs-filesystem.asciidoc
@@ -112,7 +112,7 @@ KiB, MiB, GiB, TiB, PiB, or EiB, respectively (case does 
not matter).
 be verbose, print file names as they're submitted for defragmentation
 -c[]
 compress file contents while defragmenting. Optional argument selects the 
compression
-algorithm, 'zlib' (default) or 'lzo'. Currently it's not possible to select no
+algorithm, 'zlib' (default), 'lzo' or 'zstd'. Currently it's not possible to 
select no
 compression. See also section 'EXAMPLES'.
 -r
 defragment files recursively in given directories
diff --git a/Documentation/btrfs-man5.asciidoc 
b/Documentation/btrfs-man5.asciidoc
index 8d9031f..3981435 100644
--- a/Documentation/btrfs-man5.asciidoc
+++ b/Documentation/btrfs-man5.asciidoc
@@ -118,7 +118,7 @@ but a warning is printed if it's more than 300 seconds (5 
minutes).
 (default: off)
 +
 Control BTRFS file data compression.  Type may be specified as 'zlib',
-'lzo' or 'no' (for no compression, used for remounting).  If no type
+'lzo', 'zstd' or 'no' (for no compression, used for remounting).  If no type
 is specified, 'zlib' is used.  If 'compress-force' is specified,
 the compression will allways be attempted, but the data may end up uncompressed
 if the compression would make them larger.
@@ -472,6 +472,12 @@ page size
 the 'lzo' compression has been used on the filesystem, either as a mount option
 or via *btrfs filesystem defrag*.

+*compress_zstd*::
+(since: 4.14)
++
+the 'zstd' compression has been used on the filesystem, either as a mount 
option
+or via *btrfs filesystem defrag*.
+
 *default_subvol*::
 (since: 2.6.34)
 +
diff --git a/Documentation/btrfs-property.asciidoc 
b/Documentation/btrfs-property.asciidoc
index 05ab0bc..7ed6a7d 100644
--- a/Documentation/btrfs-property.asciidoc
+++ b/Documentation/btrfs-property.asciidoc
@@ -43,7 +43,7 @@ read-only flag of subvolume: true or false
 label
 label of device
 compression
-compression setting for an inode: lzo, zlib, or "" (empty string)
+compression setting for an inode: lzo, zlib, zstd, or "" (empty string)

 *list* [-t ] ::
 Lists available properties with their descriptions for the given object.
diff --git a/INSTALL b/INSTALL
index 0465fb0..686cefb 100644
--- a/INSTALL
+++ b/INSTALL
@@ -7,6 +7,7 @@ The Btrfs utility programs require the following 
libraries/tools to build:
 - libblkid - block device id library
 - liblzo2 - LZO data compression library
 - zlib - ZLIB data compression library
+- libzstd - ZSTD data compression library version >= 1.0.0

 For the btrfs-convert utility:

diff --git a/Makefile.inc.in b/Makefile.inc.in
index 3c7bc03..4076add 100644
--- a/Makefile.inc.in
+++ b/Makefile.inc.in
@@ -18,9 +18,9 @@ SUBST_CFLAGS = @CFLAGS@
 SUBST_LDFLAGS = @LDFLAGS@

 LIBS_BASE = @UUID_LIBS@ @BLKID_LIBS@ -L. -pthread
-LIBS_COMP = @ZLIB_LIBS@ @LZO2_LIBS@
+LIBS_COMP = @ZLIB_LIBS@ @LZO2_LIBS@ @ZSTD_LIBS@
 STATIC_LIBS_BASE = @UUID_LIBS_STATIC@ @BLKID_LIBS_STATIC@ -L. -pthread
-STATIC_LIBS_COMP = @ZLIB_LIBS_STATIC@ @LZO2_LIBS_STATIC@
+STATIC_LIBS_COMP = @ZLIB_LIBS_STATIC@ 

[PATCH 3/4 v2] btrfs-progs: mkfs: refactor create_data_reloc_tree

[Yingyi Luo]

From: yingyil 

Add an objectid parameter to make the function a general one for
inserting root items and rename it to create_tree. The change cascades
down to the callchain.

Signed-off-by: yingyil 
---
v2: utilize create_tree() function for creating a subvolume, instead of
create_subvol() in convert/main.c.

 mkfs/main.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/mkfs/main.c b/mkfs/main.c
index afd68bc5..869c11fa 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -320,13 +320,12 @@ static int create_raid_groups(struct btrfs_trans_handle 
*trans,
return ret;
 }
 
-static int create_data_reloc_tree(struct btrfs_trans_handle *trans,
- struct btrfs_root *root)
+static int create_tree(struct btrfs_trans_handle *trans,
+   struct btrfs_root *root, u64 objectid)
 {
struct btrfs_key location;
struct btrfs_root_item root_item;
struct extent_buffer *tmp;
-   u64 objectid = BTRFS_DATA_RELOC_TREE_OBJECTID;
int ret;
 
ret = btrfs_copy_root(trans, root, root->node, , objectid);
@@ -1846,7 +1845,7 @@ raid_groups:
}
}
 
-   ret = create_data_reloc_tree(trans, root);
+   ret = create_tree(trans, root, BTRFS_DATA_RELOC_TREE_OBJECTID);
if (ret) {
error("unable to create data reloc tree: %d", ret);
goto out;
-- 
2.14.1.342.g6490525c54-goog

--
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/4 v2] btrfs-progs: add a parameter to btrfs_link_subvol

[Yingyi Luo]

From: yingyil 

A convert parameter is added as a flag to indicate if btrfs_link_subvol()
is used for btrfs-convert. The change cascades down to the callchain.

Signed-off-by: yingyil 
---
v2: Added a flag for btrfs_link_subvol() function so that it can be used in a
more general way. For example, if mkfs calls this function, it will return
directly with an error code if the subvolume creation fails.

 convert/main.c |  2 +-
 utils.c| 25 +++--
 utils.h|  2 +-
 3 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/convert/main.c b/convert/main.c
index 8f7fec25..19a60782 100644
--- a/convert/main.c
+++ b/convert/main.c
@@ -1192,7 +1192,7 @@ static int do_convert(const char *devname, u32 
convert_flags, u32 nodesize,
task_deinit(ctx.info);
}
 
-   image_root = btrfs_link_subvol(root, subvol_name, 
CONV_IMAGE_SUBVOL_OBJECTID);
+   image_root = btrfs_link_subvol(root, subvol_name, 
CONV_IMAGE_SUBVOL_OBJECTID, 1);
if (!image_root) {
error("unable to link subvolume %s", subvol_name);
goto fail;
diff --git a/utils.c b/utils.c
index 32382328..20ea9e74 100644
--- a/utils.c
+++ b/utils.c
@@ -2576,7 +2576,7 @@ void btrfs_config_init(void)
 }
 
 struct btrfs_root *btrfs_link_subvol(struct btrfs_root *root,
-   const char *base, u64 root_objectid)
+   const char *base, u64 root_objectid, int convert)
 {
struct btrfs_trans_handle *trans;
struct btrfs_fs_info *fs_info = root->fs_info;
@@ -2642,16 +2642,21 @@ struct btrfs_root *btrfs_link_subvol(struct btrfs_root 
*root,
key.type = BTRFS_ROOT_ITEM_KEY;
 
memcpy(buf, base, len);
-   for (i = 0; i < 1024; i++) {
-   ret = btrfs_insert_dir_item(trans, root, buf, len, dirid,
-   , BTRFS_FT_DIR, index);
-   if (ret != -EEXIST)
-   break;
-   len = snprintf(buf, ARRAY_SIZE(buf), "%s%d", base, i);
-   if (len < 1 || len > BTRFS_NAME_LEN) {
-   ret = -EINVAL;
-   break;
+   if (convert) {
+   for (i = 0; i < 1024; i++) {
+   ret = btrfs_insert_dir_item(trans, root, buf, len, 
dirid,
+   , BTRFS_FT_DIR, index);
+   if (ret != -EEXIST)
+   break;
+   len = snprintf(buf, ARRAY_SIZE(buf), "%s%d", base, i);
+   if (len < 1 || len > BTRFS_NAME_LEN) {
+   ret = -EINVAL;
+   break;
+   }
}
+   } else {
+   ret = btrfs_insert_dir_item(trans, root, buf, len, dirid, ,
+   BTRFS_FT_DIR, index);
}
if (ret)
goto fail;
diff --git a/utils.h b/utils.h
index 886590fd..28aa8373 100644
--- a/utils.h
+++ b/utils.h
@@ -171,6 +171,6 @@ unsigned int rand_range(unsigned int upper);
 void init_rand_seed(u64 seed);
 
 struct btrfs_root *btrfs_link_subvol(struct btrfs_root *root, const char *base,
-u64 root_objectid);
+u64 root_objectid, int convert);
 
 #endif
-- 
2.14.1.342.g6490525c54-goog

--
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 4/4 v2] btrfs-progs: mkfs: add subvolume support to mkfs

[Yingyi Luo]

From: yingyil 

Add -S/--subvol [NAME] option to configure. It enables users to create a
subvolume under the toplevel volume and populate the created subvolume
with files from the rootdir specified by -r/--rootdir option.

Signed-off-by: yingyil 
---
v2: Fixed the bug for subvolume creation failure: calling create_tree(), instead
of create_subvol() in convert/main.c since btrfs_make_root_dir() in
create_subvol() is not needed to be called in mkfs.

 mkfs/main.c | 75 -
 1 file changed, 70 insertions(+), 5 deletions(-)

diff --git a/mkfs/main.c b/mkfs/main.c
index 869c11fa..21abc4a4 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -363,6 +363,7 @@ static void print_usage(int ret)
printf("\t-U|--uuid UUID  specify the filesystem UUID (must be 
unique)\n");
printf("  creation:\n");
printf("\t-b|--byte-count SIZEset filesystem size to SIZE (on the 
first device)\n");
+   printf("\t-S|--subvol NAMEcreate a sunvolume with NAME and copy 
files from ROOTDIR to the subvolume\n");
printf("\t-r|--rootdir DIRcopy files from DIR to the image root 
directory\n");
printf("\t-K|--nodiscard  do not perform whole device TRIM\n");
printf("\t-f|--force  force overwrite of existing 
filesystem\n");
@@ -412,6 +413,18 @@ static char *parse_label(const char *input)
return strdup(input);
 }
 
+static char *parse_subvol_name(const char *input)
+{
+   int len = strlen(input);
+
+   if (len >= BTRFS_SUBVOL_NAME_MAX) {
+   error("subvolume name %s is too long (max %d)",
+   input, BTRFS_SUBVOL_NAME_MAX - 1);
+   exit(1);
+   }
+   return strdup(input);
+}
+
 static int add_directory_items(struct btrfs_trans_handle *trans,
   struct btrfs_root *root, u64 objectid,
   ino_t parent_inum, const char *name,
@@ -1418,6 +1431,8 @@ int main(int argc, char **argv)
int discard = 1;
int ssd = 0;
int force_overwrite = 0;
+   char *subvol_name = NULL;
+   int subvol_name_set = 0;
char *source_dir = NULL;
int source_dir_set = 0;
u64 num_of_meta_chunks = 0;
@@ -1444,6 +1459,7 @@ int main(int argc, char **argv)
{ "sectorsize", required_argument, NULL, 's' },
{ "data", required_argument, NULL, 'd' },
{ "version", no_argument, NULL, 'V' },
+   { "subvol", required_argument, NULL, 'S'},
{ "rootdir", required_argument, NULL, 'r' },
{ "nodiscard", no_argument, NULL, 'K' },
{ "features", required_argument, NULL, 'O' },
@@ -1453,7 +1469,7 @@ int main(int argc, char **argv)
{ NULL, 0, NULL, 0}
};
 
-   c = getopt_long(argc, argv, "A:b:fl:n:s:m:d:L:O:r:U:VMKq",
+   c = getopt_long(argc, argv, "A:b:fl:n:s:m:d:L:O:S:r:U:VMKq",
long_options, NULL);
if (c < 0)
break;
@@ -1514,6 +1530,10 @@ int main(int argc, char **argv)
printf("mkfs.btrfs, part of %s\n",
PACKAGE_STRING);
goto success;
+   case 'S':
+   subvol_name = parse_subvol_name(optarg);
+   subvol_name_set = 1;
+   break;
case 'r':
source_dir = optarg;
source_dir_set = 1;
@@ -1534,6 +1554,11 @@ int main(int argc, char **argv)
}
}
 
+   if (subvol_name_set && !source_dir_set) {
+   error("root directory needs to be set");
+   goto error;
+   }
+
if (verbose) {
printf("%s\n", PACKAGE_STRING);
printf("See %s for more information.\n\n", PACKAGE_URL);
@@ -1872,10 +1897,48 @@ raid_groups:
goto out;
}
 
-   ret = make_image(source_dir, root);
-   if (ret) {
-   error("error wihle filling filesystem: %d", ret);
-   goto out;
+   if (subvol_name_set) {
+   u64 dirid, objectid;
+   struct btrfs_root *file_root;
+
+   dirid = 
btrfs_root_dirid(_info->tree_root->root_item);
+   ret = btrfs_find_free_objectid(NULL, 
fs_info->tree_root, dirid, );
+   if (ret) {
+   error("unable to find a free objectid: %d", 
ret);
+   goto out;
+   }
+   trans = 

[PATCH 1/4 v2] btrfs-progs: convert: move link_subvol out of main

[Yingyi Luo]

From: yingyil 

link_subvol() is moved to utils.c and renamed as btrfs_link_subvol().
The change cascades down to the callchain.

Signed-off-by: yingyil 
---
v2: split the original patch to make code movement into a separate patch.

 convert/main.c | 125 +
 utils.c| 123 
 utils.h|   3 ++
 3 files changed, 127 insertions(+), 124 deletions(-)

diff --git a/convert/main.c b/convert/main.c
index 03da9e49..8f7fec25 100644
--- a/convert/main.c
+++ b/convert/main.c
@@ -834,129 +834,6 @@ out:
return ret;
 }
 
-static struct btrfs_root* link_subvol(struct btrfs_root *root,
-   const char *base, u64 root_objectid)
-{
-   struct btrfs_trans_handle *trans;
-   struct btrfs_fs_info *fs_info = root->fs_info;
-   struct btrfs_root *tree_root = fs_info->tree_root;
-   struct btrfs_root *new_root = NULL;
-   struct btrfs_path path;
-   struct btrfs_inode_item *inode_item;
-   struct extent_buffer *leaf;
-   struct btrfs_key key;
-   u64 dirid = btrfs_root_dirid(>root_item);
-   u64 index = 2;
-   char buf[BTRFS_NAME_LEN + 1]; /* for snprintf null */
-   int len;
-   int i;
-   int ret;
-
-   len = strlen(base);
-   if (len == 0 || len > BTRFS_NAME_LEN)
-   return NULL;
-
-   btrfs_init_path();
-   key.objectid = dirid;
-   key.type = BTRFS_DIR_INDEX_KEY;
-   key.offset = (u64)-1;
-
-   ret = btrfs_search_slot(NULL, root, , , 0, 0);
-   if (ret <= 0) {
-   error("search for DIR_INDEX dirid %llu failed: %d",
-   (unsigned long long)dirid, ret);
-   goto fail;
-   }
-
-   if (path.slots[0] > 0) {
-   path.slots[0]--;
-   btrfs_item_key_to_cpu(path.nodes[0], , path.slots[0]);
-   if (key.objectid == dirid && key.type == BTRFS_DIR_INDEX_KEY)
-   index = key.offset + 1;
-   }
-   btrfs_release_path();
-
-   trans = btrfs_start_transaction(root, 1);
-   if (!trans) {
-   error("unable to start transaction");
-   goto fail;
-   }
-
-   key.objectid = dirid;
-   key.offset = 0;
-   key.type =  BTRFS_INODE_ITEM_KEY;
-
-   ret = btrfs_lookup_inode(trans, root, , , 1);
-   if (ret) {
-   error("search for INODE_ITEM %llu failed: %d",
-   (unsigned long long)dirid, ret);
-   goto fail;
-   }
-   leaf = path.nodes[0];
-   inode_item = btrfs_item_ptr(leaf, path.slots[0],
-   struct btrfs_inode_item);
-
-   key.objectid = root_objectid;
-   key.offset = (u64)-1;
-   key.type = BTRFS_ROOT_ITEM_KEY;
-
-   memcpy(buf, base, len);
-   for (i = 0; i < 1024; i++) {
-   ret = btrfs_insert_dir_item(trans, root, buf, len,
-   dirid, , BTRFS_FT_DIR, index);
-   if (ret != -EEXIST)
-   break;
-   len = snprintf(buf, ARRAY_SIZE(buf), "%s%d", base, i);
-   if (len < 1 || len > BTRFS_NAME_LEN) {
-   ret = -EINVAL;
-   break;
-   }
-   }
-   if (ret)
-   goto fail;
-
-   btrfs_set_inode_size(leaf, inode_item, len * 2 +
-btrfs_inode_size(leaf, inode_item));
-   btrfs_mark_buffer_dirty(leaf);
-   btrfs_release_path();
-
-   /* add the backref first */
-   ret = btrfs_add_root_ref(trans, tree_root, root_objectid,
-BTRFS_ROOT_BACKREF_KEY,
-root->root_key.objectid,
-dirid, index, buf, len);
-   if (ret) {
-   error("unable to add root backref for %llu: %d",
-   root->root_key.objectid, ret);
-   goto fail;
-   }
-
-   /* now add the forward ref */
-   ret = btrfs_add_root_ref(trans, tree_root, root->root_key.objectid,
-BTRFS_ROOT_REF_KEY, root_objectid,
-dirid, index, buf, len);
-   if (ret) {
-   error("unable to add root ref for %llu: %d",
-   root->root_key.objectid, ret);
-   goto fail;
-   }
-
-   ret = btrfs_commit_transaction(trans, root);
-   if (ret) {
-   error("transaction commit failed: %d", ret);
-   goto fail;
-   }
-
-   new_root = btrfs_read_fs_root(fs_info, );
-   if (IS_ERR(new_root)) {
-   error("unable to fs read root: %lu", PTR_ERR(new_root));
-   new_root = NULL;
-   }
-fail:
-   btrfs_init_path();
-   return new_root;
-}
-
 static int create_subvol(struct btrfs_trans_handle 

[PATCH 0/4 v2] add subvolume support to mkfs

[Yingyi Luo]

From: yingyil 

Hi all,

Thanks for all the comments you gave to me. I really appriciate it. I've
updated the patch, split it into small ones so that it may be easier
for you to see the changes and review.

One use case of this feature is that it allows easier operations on subvolumes,
such as migrating subvolumes across images. With the current btrfs tool, we have
to do this manually by creating an empty subvolume then moving the files over.
This feature which creates a subvolume for a given directory when making btrfs
images simplifies the process and makes the logic cleaner.

Goffredo: Thanks again for you comments and pointing out the bug. It happened
to be that I skipped the error code checking when I tested the functionality.
The error came from a duplicate call of btrfs_make_root_dir() from
make_root_dir() in mkfs/main.c and create_subvol() in convert/main.c.

strdup() is used to get a returnable copy of the literal string.
FYI, parse_label() used strdup() to parse the input string. I think using
strdup() would make it consistent with the current code.

Extending the feature would be a good idea. I'll try to work on it later.

Best regards,
Yingyi

yingyil (4):
  btrfs-progs: convert: move link_subvol out of main
  btrfs-progs: add a parameter to btrfs_link_subvol
  btrfs-progs: mkfs: refactor create_data_reloc_tree
  btrfs-progs: mkfs: add subvolume support to mkfs

 convert/main.c | 125 +--
 mkfs/main.c|  82 
 utils.c| 128 +
 utils.h|   3 ++
 4 files changed, 205 insertions(+), 133 deletions(-)

-- 
2.14.1.342.g6490525c54-goog

--
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: Remove memory barrier from block_group_cache_done

[Nikolay Borisov]

This memory barrier was introduced in 817d52f8dba2 ("Btrfs: async block group
caching"), but even at that time it's usage was broken since it didn't pair
with anything. There was one situation where the cached member was set to
BTRFS_CACHE_FINISHED in a spinlock region which *might* have acted as a pairing
barrier. Since there is no clear semantics how it's supposed to work better
to just remove it.

Signed-off-by: Nikolay Borisov 
---
 fs/btrfs/extent-tree.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 21c78c48e15b..10c2ef1a9fd2 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -112,7 +112,6 @@ static void space_info_add_old_bytes(struct btrfs_fs_info 
*fs_info,
 static noinline int
 block_group_cache_done(struct btrfs_block_group_cache *cache)
 {
-   smp_mb();
return cache->cached == BTRFS_CACHE_FINISHED ||
cache->cached == BTRFS_CACHE_ERROR;
 }
-- 
2.7.4

--
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/4] btrfs: convert enum btrfs_compression_type to define

[Anand Jain]

On 08/17/2017 07:57 PM, David Sterba wrote:

On Thu, Aug 17, 2017 at 04:33:41AM +0800, Anand Jain wrote:



On 08/16/2017 09:59 PM, David Sterba wrote:

On Sun, Aug 13, 2017 at 12:02:42PM +0800, Anand Jain wrote:

There isn't a huge list to manage the types, which can be managed
with defines. It helps to easily print the types in tracing as well.


We use enums in a lot of places, I'd rather keep it as it is.


   This patch converts all of them, and it was at only one place.


Yeah, but I mean the enum vs define style of constant definition.


   I hope I didn't miss any. Further the next patch 3/4 needs it
   to be define instead of enums, handling enums in the tracing
   isn't as easy as define.


Interesting, in what way are defines better use in tracepoints? I see
eg. show_flush_state using enum btrfs_flush_state in btrfs_flush_space,
the same pattern applies to patch 3/4.


 As of now TRACE_DEFINE_ENUM() for show_flush_state is missing (patch 
sent) which is needed for the state= symbol in the user space as shown 
below,


perf record -e 'btrfs:btrfs_flush_space' -a fill_and_bal /btrfs/sv1 
1 && perf script


::
kworker/u2:4  4220 [000] 19032.858184: btrfs:btrfs_flush_space: (nil)U: 
state=1() flags=4(METADATA) num_bytes=173015040 orig_bytes=173015040 ret=0


(%p does not work in user space, ignore it for now).

sysfs trace is fine.
cat ./trace_pipe
---
kworker/u2:3-4219  [000]  18952.145677: btrfs_flush_space: 
f0918b8d-88a6-4e9f-8ca6-02e2fc290380: state=1(FLUSH_DELAYED_ITEMS_NR) 
flags=4(METADATA) num_bytes

---

 So for BTRFS_COMPRESS.. its either TRADE_DEFINE_ENUM() and enum or 
just #define. I am ok either ways.


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

[PATCH] btrfs: declare TRACE_DEFINE_ENUM for each of show_flush_state enum

[Anand Jain]

So that perf can show the state symbol.

Signed-off-by: Anand Jain 
---
 include/trace/events/btrfs.h | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/include/trace/events/btrfs.h b/include/trace/events/btrfs.h
index d412c49f5a6a..64002f898717 100644
--- a/include/trace/events/btrfs.h
+++ b/include/trace/events/btrfs.h
@@ -27,6 +27,13 @@ struct __btrfs_workqueue;
 struct btrfs_qgroup_extent_record;
 struct btrfs_qgroup;
 
+TRACE_DEFINE_ENUM(FLUSH_DELAYED_ITEMS_NR);
+TRACE_DEFINE_ENUM(FLUSH_DELAYED_ITEMS);
+TRACE_DEFINE_ENUM(FLUSH_DELALLOC);
+TRACE_DEFINE_ENUM(FLUSH_DELALLOC_WAIT);
+TRACE_DEFINE_ENUM(ALLOC_CHUNK);
+TRACE_DEFINE_ENUM(COMMIT_TRANS);
+
 #define show_ref_type(type)\
__print_symbolic(type,  \
{ BTRFS_TREE_BLOCK_REF_KEY, "TREE_BLOCK_REF" }, \
-- 
2.13.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

Re: [PATCH] btrfs/029: fix wrong usage of name filter

[Eryu Guan]

On Wed, Aug 30, 2017 at 04:38:16PM +0900, Misono, Tomohiro wrote:
> btrfs/029 uses _filter_testdirs() to filter the name of $TEST_DIR and
> $SCRATCH_MNT directory.
> 
> In this function, it calls both _filter_test_dir and _filter_scratch
> concatenated by pipe. Therefore if $TEST_DIR is a prefix of
> $SCRATCH_MNT, this filter function gives wrong filtered name for
> $SCRATCH_MNT and the test fails.

Sorry, I'm a bit confused, how could $TEST_DIR be a prefix of
$SCRATCH_MNT? Won't that fail the test at setup time?

Thanks,
Eryu
--
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: resume qgroup rescan on rw remount

[David Sterba]

On Sat, Aug 26, 2017 at 04:39:02PM +1000, Aleksa Sarai wrote:
> On 07/12/2017 03:03 AM, David Sterba wrote:
> > On Mon, Jul 10, 2017 at 04:56:36PM +0300, Nikolay Borisov wrote:
> >> On 10.07.2017 16:12, Nikolay Borisov wrote:
> >>> On  4.07.2017 14:49, Aleksa Sarai wrote:
>  Several distributions mount the "proper root" as ro during initrd and
>  then remount it as rw before pivot_root(2). Thus, if a rescan had been
>  aborted by a previous shutdown, the rescan would never be resumed.
> 
>  This issue would manifest itself as several btrfs ioctl(2)s causing the
>  entire machine to hang when btrfs_qgroup_wait_for_completion was hit
>  (due to the fs_info->qgroup_rescan_running flag being set but the rescan
>  itself not being resumed). Notably, Docker's btrfs storage driver makes
>  regular use of BTRFS_QUOTA_CTL_DISABLE and BTRFS_IOC_QUOTA_RESCAN_WAIT
>  (causing this problem to be manifested on boot for some machines).
> 
>  Cc:  # v3.11+
>  Cc: Jeff Mahoney 
>  Fixes: b382a324b60f ("Btrfs: fix qgroup rescan resume on mount")
>  Signed-off-by: Aleksa Sarai 
> >>>
> >>> Indeed, looking at the code it seems that b382a324b60f ("Btrfs: fix
> >>> qgroup rescan resume on mount") missed adding the qgroup_rescan_resume
> >>> in the remount path. One thing which I couldn't verify though is whether
> >>> reading fs_info->qgroup_flags without any locking is safe from remount
> >>> context.
> >>>
> >>> During remount I don't see any locks taken that prevent operations which
> >>> can modify qgroup_flags.
> >>
> >> Further inspection reveals that the access rules to qgroup_flags are
> >> somewhat broken so this patch doesn't really make things any worse than
> >> they are.
> > 
> > The usage follows a pattern for a bitfield, updated by set_bit/clear_bit
> > etc. The updates to the state or inconsistency is not safe, so some
> > updates could get lost under some circumstances.
> > 
> > Patch added to devel queue, possibly will be submitted to 4.13 so stable
> > can pick it.
> 
> Looks like it wasn't merged in the 4.13 window (so stable hasn't picked 
> it), will this be submitted for 4.14? Thanks.

Sorry this didn't make it to 4.13, the patch is in the queue for 4.14.
--
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/029: fix wrong usage of name filter

[Misono, Tomohiro]

btrfs/029 uses _filter_testdirs() to filter the name of $TEST_DIR and
$SCRATCH_MNT directory.

In this function, it calls both _filter_test_dir and _filter_scratch
concatenated by pipe. Therefore if $TEST_DIR is a prefix of
$SCRATCH_MNT, this filter function gives wrong filtered name for
$SCRATCH_MNT and the test fails.

Fix this by calling _filter_test_dir and _filter_scratch directly.

Signed-off-by: Tomohiro Misono 
---
 tests/btrfs/029 | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/tests/btrfs/029 b/tests/btrfs/029
index c390c95..74fded7 100755
--- a/tests/btrfs/029
+++ b/tests/btrfs/029
@@ -66,19 +66,14 @@ _scratch_mkfs > /dev/null 2>&1
 _scratch_mount
 $XFS_IO_PROG -f -c 'pwrite -S 0x61 0 9000' $SCRATCH_MNT/original >> 
$seqres.full
 
-_filter_testdirs()
-{
-_filter_test_dir | _filter_scratch
-}
-
 _create_reflinks()
 {
 # auto reflink, should fall back to non-reflink
 rm -rf $2
 echo "reflink=auto:"
 cp --reflink=auto $1 $2
-md5sum $1 | _filter_testdirs
-md5sum $2 | _filter_testdirs
+md5sum $1 | _filter_scratch
+md5sum $2 | _filter_test_dir
 
 # always reflink, should fail outright
 rm -rf $2
@@ -86,7 +81,7 @@ _create_reflinks()
 cp --reflink=always $1 $2 >> $seqres.full 2>&1 || echo "cp reflink failed"
 
 # The failed target actually gets created by cp:
-ls $2 | _filter_testdirs
+ls $2 | _filter_test_dir
 }
 
 echo "test reflinks across different devices"
-- 
2.9.5

--
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: remove BTRFS_FS_QUOTA_DISABLING flag

[Misono, Tomohiro]

Currently, "btrfs quota enable" would fail after "btrfs quota disable" on
the first time with syslog output "qgroup_rescan_init failed with -22", but
it would succeed on the second time.

When "quota disable" is called, BTRFS_FS_QUOTA_DISABLING flag bit will be
set in fs_info->flags in btrfs_quota_disable(), but it will not be droppd
in btrfs_run_qgroups() (which is called in btrfs_commit_transaction())
because quota_root has already been freed. If "quota enable" is called
after that, both BTRFS_FS_QUOTA_DISABLING and BTRFS_FS_QUOTA_ENABLED flag
would be dropped in the btrfs_run_qgroups() since quota_root is not NULL.
This leads to the failure of "quota enable" on the first time.

BTRFS_FS_QUOTA_DISABLING flag is not used outside of "quota disable"
context and is equivalent to whether quota_root is NULL or not.
btrfs_run_qgroups() checks whether quota_root is NULL or not in the first
place. 

So, let's remove BTRFS_FS_QUOTA_DISABLING flag.

Signed-off-by: Tomohiro Misono 
---
 fs/btrfs/ctree.h  | 1 -
 fs/btrfs/qgroup.c | 4 
 2 files changed, 5 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 3f3eb7b..49501c0 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -709,7 +709,6 @@ struct btrfs_delayed_root;
 #define BTRFS_FS_OPEN  5
 #define BTRFS_FS_QUOTA_ENABLED 6
 #define BTRFS_FS_QUOTA_ENABLING7
-#define BTRFS_FS_QUOTA_DISABLING   8
 #define BTRFS_FS_UPDATE_UUID_TREE_GEN  9
 #define BTRFS_FS_CREATING_FREE_SPACE_TREE  10
 #define BTRFS_FS_BTREE_ERR 11
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 4ce351e..f0b111e 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -807,7 +807,6 @@ static int btrfs_clean_quota_tree(struct btrfs_trans_handle 
*trans,
}
ret = 0;
 out:
-   set_bit(BTRFS_FS_QUOTA_DISABLING, >fs_info->flags);
btrfs_free_path(path);
return ret;
 }
@@ -954,7 +953,6 @@ int btrfs_quota_disable(struct btrfs_trans_handle *trans,
if (!fs_info->quota_root)
goto out;
clear_bit(BTRFS_FS_QUOTA_ENABLED, _info->flags);
-   set_bit(BTRFS_FS_QUOTA_DISABLING, _info->flags);
btrfs_qgroup_wait_for_completion(fs_info, false);
spin_lock(_info->qgroup_lock);
quota_root = fs_info->quota_root;
@@ -2087,8 +2085,6 @@ int btrfs_run_qgroups(struct btrfs_trans_handle *trans,
 
if (test_and_clear_bit(BTRFS_FS_QUOTA_ENABLING, _info->flags))
set_bit(BTRFS_FS_QUOTA_ENABLED, _info->flags);
-   if (test_and_clear_bit(BTRFS_FS_QUOTA_DISABLING, _info->flags))
-   clear_bit(BTRFS_FS_QUOTA_ENABLED, _info->flags);
 
spin_lock(_info->qgroup_lock);
while (!list_empty(_info->dirty_qgroups)) {
-- 
2.9.5


--
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/029: fix wrong usage of name filter

[Misono, Tomohiro]

Sorry, this patch contains leading spaces, I will resend this soon.

On 2017/08/30 11:44, Misono, Tomohiro wrote:
> btrfs/029 uses _filter_testdirs() to filter the name of $TEST_DIR and
> $SCRATCH_MNT directory.
> 
> In this function, it calls both _filter_test_dir and _filter_scratch
> concatenated by pipe. Therefore if $TEST_DIR is a prefix of
> $SCRATCH_MNT, this filter function gives wrong filtered name for
> $SCRATCH_MNT and the test fails.
> 
> Fix this by calling _filter_test_dir and _filter_scratch directly.
> 
> Signed-off-by: Tomohiro Misono 
> ---
>   tests/btrfs/029 | 11 +++
>   1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/tests/btrfs/029 b/tests/btrfs/029
> index c390c95..74fded7 100755
> --- a/tests/btrfs/029
> +++ b/tests/btrfs/029
> @@ -66,19 +66,14 @@ _scratch_mkfs > /dev/null 2>&1
>   _scratch_mount
>   $XFS_IO_PROG -f -c 'pwrite -S 0x61 0 9000' $SCRATCH_MNT/original >> 
> $seqres.full
> 
> -_filter_testdirs()
> -{
> -_filter_test_dir | _filter_scratch
> -}
> -
>   _create_reflinks()
>   {
>   # auto reflink, should fall back to non-reflink
>   rm -rf $2
>   echo "reflink=auto:"
>   cp --reflink=auto $1 $2
> -md5sum $1 | _filter_testdirs
> -md5sum $2 | _filter_testdirs
> +md5sum $1 | _filter_scratch
> +md5sum $2 | _filter_test_dir
> 
>   # always reflink, should fail outright
>   rm -rf $2
> @@ -86,7 +81,7 @@ _create_reflinks()
>   cp --reflink=always $1 $2 >> $seqres.full 2>&1 || echo "cp reflink 
> failed"
> 
>   # The failed target actually gets created by cp:
> -ls $2 | _filter_testdirs
> +ls $2 | _filter_test_dir
>   }
> 
>   echo "test reflinks across different devices"
> 

--
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: remove BTRFS_FS_QUOTA_DISABLING flag

[Misono, Tomohiro]

Sorry, this patch contains leading spaces, I will resend this soon.

On 2017/08/30 10:51, Misono, Tomohiro wrote:
> Currently, "btrfs quota enable" would fail after "btrfs quota disable" on
> the first time with syslog output "qgroup_rescan_init failed with -22", but
> it would succeed on the second time.
> 
> When "quota disable" is called, BTRFS_FS_QUOTA_DISABLING flag bit will be
> set in fs_info->flags in btrfs_quota_disable(), but it will not be droppd
> in btrfs_run_qgroups() (which is called in btrfs_commit_transaction())
> because quota_root has already been freed. If "quota enable" is called
> after that, both BTRFS_FS_QUOTA_DISABLING and BTRFS_FS_QUOTA_ENABLED flag
> would be dropped in the btrfs_run_qgroups() since quota_root is not NULL.
> This leads to the failure of "quota enable" on the first time.
> 
> BTRFS_FS_QUOTA_DISABLING flag is not used outside of "quota disable"
> context and is equivalent to whether quota_root is NULL or not.
> btrfs_run_qgroups() checks whether quota_root is NULL or not in the first
> place.
> 
> So, let's remove BTRFS_FS_QUOTA_DISABLING flag.
> 
> Signed-off-by: Tomohiro Misono 
> ---
>   fs/btrfs/ctree.h  | 1 -
>   fs/btrfs/qgroup.c | 4 
>   2 files changed, 5 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 3f3eb7b..49501c0 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -709,7 +709,6 @@ struct btrfs_delayed_root;
>   #define BTRFS_FS_OPEN   5
>   #define BTRFS_FS_QUOTA_ENABLED  6
>   #define BTRFS_FS_QUOTA_ENABLING 7
> -#define BTRFS_FS_QUOTA_DISABLING 8
>   #define BTRFS_FS_UPDATE_UUID_TREE_GEN   9
>   #define BTRFS_FS_CREATING_FREE_SPACE_TREE   10
>   #define BTRFS_FS_BTREE_ERR  11
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index 4ce351e..f0b111e 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -807,7 +807,6 @@ static int btrfs_clean_quota_tree(struct 
> btrfs_trans_handle *trans,
>   }
>   ret = 0;
>   out:
> - set_bit(BTRFS_FS_QUOTA_DISABLING, >fs_info->flags);
>   btrfs_free_path(path);
>   return ret;
>   }
> @@ -954,7 +953,6 @@ int btrfs_quota_disable(struct btrfs_trans_handle 
> *trans,
>   if (!fs_info->quota_root)
>   goto out;
>   clear_bit(BTRFS_FS_QUOTA_ENABLED, _info->flags);
> - set_bit(BTRFS_FS_QUOTA_DISABLING, _info->flags);
>   btrfs_qgroup_wait_for_completion(fs_info, false);
>   spin_lock(_info->qgroup_lock);
>   quota_root = fs_info->quota_root;
> @@ -2087,8 +2085,6 @@ int btrfs_run_qgroups(struct btrfs_trans_handle 
> *trans,
> 
>   if (test_and_clear_bit(BTRFS_FS_QUOTA_ENABLING, _info->flags))
>   set_bit(BTRFS_FS_QUOTA_ENABLED, _info->flags);
> - if (test_and_clear_bit(BTRFS_FS_QUOTA_DISABLING, _info->flags))
> - clear_bit(BTRFS_FS_QUOTA_ENABLED, _info->flags);
> 
>   spin_lock(_info->qgroup_lock);
>   while (!list_empty(_info->dirty_qgroups)) {
> 

--
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