RE: [PATCH v2] Btrfs: remove unnecessary code of chunk_root assignment in btrfs_read_chunk_tree.
Hi, Sean Fu
> -Original Message-
> From: Sean Fu [mailto:fxinr...@gmail.com]
> Sent: Tuesday, September 06, 2016 11:51 AM
> To: dste...@suse.com
> Cc: c...@fb.com; anand.j...@oracle.com; fdman...@suse.com;
> zhao...@cn.fujitsu.com; linux-ker...@vger.kernel.org;
> linux-btrfs@vger.kernel.org; Sean Fu
> Subject: [PATCH v2] Btrfs: remove unnecessary code of chunk_root assignment
> in btrfs_read_chunk_tree.
>
> The input argument root is already set with "fs_info->chunk_root".
> "chunk_root = fs_info->chunk_root = btrfs_alloc_root(fs_info)" in caller
> "open_ctree".
> "root->fs_info = fs_info” in "btrfs_alloc_root".
>
> Signed-off-by: Sean Fu
> ---
> Changes in v2:
> - Renaming root to chunk_root to make it clear.
>
> fs/btrfs/volumes.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 366b335..eb3d04a 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -6591,7 +6591,7 @@ out_short_read:
> return -EIO;
> }
>
> -int btrfs_read_chunk_tree(struct btrfs_root *root)
> +int btrfs_read_chunk_tree(struct btrfs_root *chunk_root)
Maybe you also need to modify function body to make it
pass compile, and the header file also need to be modified.
BTW, Qu Wenruo give us a better way in reply, we
can use fs_info directly.
Thanks
Zhaolei
> {
> struct btrfs_path *path;
> struct extent_buffer *leaf;
> @@ -6600,8 +6600,6 @@ int btrfs_read_chunk_tree(struct btrfs_root *root)
> int ret;
> int slot;
>
> - root = root->fs_info->chunk_root;
> -
> path = btrfs_alloc_path();
> if (!path)
> return -ENOMEM;
> --
> 2.6.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
Re: [PATCH] fstests: common: Enhance _exclude_scratch_mount_option to handle multiply options and generic fs type
At 09/06/2016 12:20 PM, Eryu Guan wrote:
On Mon, Sep 05, 2016 at 03:13:33PM +0800, Qu Wenruo wrote:
Enhance _exclude_scratch_mount_option() function to get real mount
options from $MOUNT_OPTIONS.
This seems unnecessarily complex to me.
Considering the fact that we didn't specify any other options except -o,
yes, it's a quite complex than original one.
But, it's still needed for case like "-oopt3" can't be handled by "grep
-w", as there is no space between "-o" and "opt3".
Now it can understand and extract real mount option from string like
"-o opt1,opt2 -oopt3".
Furthermore, it doesn't use grep method, which can lead to false alert
for options like inode_cache and noinode_cache.
It now do compare with the first n characters of the prohibited list,
so it can handle "data=" and above "no" prefix well.
I think we can fix it by adding "-w" option to grep, and replacing
"data=" with "data", "=" seems not necessary.
In that case, "-oopt3" can't be handled by "-w" option.
And add a new parameter, 'fstype' for _exclude_scratch_mount_option().
So for generic test cases, it can still prohibit mount options for given
fs(mainly for btrfs though)
This requires every caller of this helper provides an additional fstype
argument, and in most cases this argument is not useful (generic or
current FSTYP). If btrfs needs to be handled differently, how about
checking the fstype in the test and adding additional mount option rules
if fstype is btrfs?
Currently, only 2 callers in fact. ext4/271 and xfs/134.
So the cost is still quite low to add a 'fstype'.
The main object is, to info reviewer or testcase writer which fs type
needs special handling.
Considering not every contributor will add comment about excluded mount
options, and in case generic test cases needs to exclude one mount
option for given fstype, it will be quite hard to find the reason.
So with new fstype parameter, at least we known which fstype is to be blame.
(Although, it will be btrfs for most of time)
Finally, allow it to accept multiple options at the same time.
No need for multiple _exclude_scratch_mount_option lines now
So _exclude_scratch_mount_option is simply:
# skip test if MOUNT_OPTIONS contains the given mount options
_exclude_scratch_mount_option()
{
for opt in $*; do
if echo $MOUNT_OPTIONS | grep -qw "$opt"; then
_notrun "mount option \"$opt\" not allowed in this test"
fi
done
}
(Note that the comment in current code is wrong, MKFS_OPTIONS should be
MOUNT_OPTIONS)
Yes, I also noted that and changed it to MOUNT_OPTIONS.
Thanks,
Qu
What do you and/or other people think?
Thanks,
Eryu
Signed-off-by: Qu Wenruo
---
common/rc | 41 ++---
tests/ext4/271 | 6 ++
tests/xfs/134 | 3 +--
3 files changed, 41 insertions(+), 9 deletions(-)
diff --git a/common/rc b/common/rc
index 04039a4..8fca637 100644
--- a/common/rc
+++ b/common/rc
@@ -3183,11 +3183,46 @@ _require_cloner()
_notrun "cloner binary not present at $CLONER_PROG"
}
-# skip test if MKFS_OPTIONS contains the given string
+# Normalize mount options from global $MOUNT_OPTIONS
+# Will convert "-o options1,options2 -ooptions3" into
+# "options1 options2 options3" for easy access
+_normalize_mount_options()
+{
+ echo $MOUNT_OPTIONS | sed -n 's/-o\s*\(\S*\)/\1/gp' |\
+ sed 's/,/ /g'
+}
+
+# Helper for _exclude_scratch_mount_option , check if exclusive mount option $1
+# is in the custom mount options
+_check_one_exclusive_option()
+{
+ excl_opt=$1
+ shift
+ mnt_opts=$*
+ excl_len=${#excl_opt}
+
+ for n in $mnt_opts; do
+ # Handle mount option like "data="
+ sub_str=$(expr substr $n 1 ${excl_len})
+ if [ $sub_str == $excl_opt ]; then
+ _notrun "mount option \"$n\" not allowed in this test"
+ fi
+ done
+}
+
+# skip test if custom MOUNT_OPTIONS contains prohibited mount
+# options
_exclude_scratch_mount_option()
{
- if echo $MOUNT_OPTIONS | grep -q "$1"; then
- _notrun "mount option \"$1\" not allowed in this test"
+ fstype=$1
+ shift
+
+ normalized=$(_normalize_mount_options)
+ if [ $fstype == "generic" -o $fstype = $FSTYP ]; then
+ while [ $# -gt 1 ]; do
+ _check_one_exclusive_option $1 $normalized
+ shift
+ done
fi
}
diff --git a/tests/ext4/271 b/tests/ext4/271
index 8674090..f3a99b7 100755
--- a/tests/ext4/271
+++ b/tests/ext4/271
@@ -41,10 +41,8 @@ _supported_os Linux
_require_scratch
# this test needs no journal to be loaded, skip on journal related mount
# options, otherwise mount would fail with "-o noload" mount option
-_exclude_scratch_mount_option "data="
-_exclude_scratch_mount_option "commit="
-_exclude_scratch_mount_option "journal_checksum"
Re: [PATCH] fstests: common: Enhance _exclude_scratch_mount_option to handle multiply options and generic fs type
On Mon, Sep 05, 2016 at 03:13:33PM +0800, Qu Wenruo wrote:
> Enhance _exclude_scratch_mount_option() function to get real mount
> options from $MOUNT_OPTIONS.
This seems unnecessarily complex to me.
>
> Now it can understand and extract real mount option from string like
> "-o opt1,opt2 -oopt3".
> Furthermore, it doesn't use grep method, which can lead to false alert
> for options like inode_cache and noinode_cache.
> It now do compare with the first n characters of the prohibited list,
> so it can handle "data=" and above "no" prefix well.
I think we can fix it by adding "-w" option to grep, and replacing
"data=" with "data", "=" seems not necessary.
>
> And add a new parameter, 'fstype' for _exclude_scratch_mount_option().
> So for generic test cases, it can still prohibit mount options for given
> fs(mainly for btrfs though)
This requires every caller of this helper provides an additional fstype
argument, and in most cases this argument is not useful (generic or
current FSTYP). If btrfs needs to be handled differently, how about
checking the fstype in the test and adding additional mount option rules
if fstype is btrfs?
>
> Finally, allow it to accept multiple options at the same time.
> No need for multiple _exclude_scratch_mount_option lines now
So _exclude_scratch_mount_option is simply:
# skip test if MOUNT_OPTIONS contains the given mount options
_exclude_scratch_mount_option()
{
for opt in $*; do
if echo $MOUNT_OPTIONS | grep -qw "$opt"; then
_notrun "mount option \"$opt\" not allowed in this test"
fi
done
}
(Note that the comment in current code is wrong, MKFS_OPTIONS should be
MOUNT_OPTIONS)
What do you and/or other people think?
Thanks,
Eryu
>
> Signed-off-by: Qu Wenruo
> ---
> common/rc | 41 ++---
> tests/ext4/271 | 6 ++
> tests/xfs/134 | 3 +--
> 3 files changed, 41 insertions(+), 9 deletions(-)
>
> diff --git a/common/rc b/common/rc
> index 04039a4..8fca637 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -3183,11 +3183,46 @@ _require_cloner()
> _notrun "cloner binary not present at $CLONER_PROG"
> }
>
> -# skip test if MKFS_OPTIONS contains the given string
> +# Normalize mount options from global $MOUNT_OPTIONS
> +# Will convert "-o options1,options2 -ooptions3" into
> +# "options1 options2 options3" for easy access
> +_normalize_mount_options()
> +{
> + echo $MOUNT_OPTIONS | sed -n 's/-o\s*\(\S*\)/\1/gp' |\
> + sed 's/,/ /g'
> +}
> +
> +# Helper for _exclude_scratch_mount_option , check if exclusive mount option
> $1
> +# is in the custom mount options
> +_check_one_exclusive_option()
> +{
> + excl_opt=$1
> + shift
> + mnt_opts=$*
> + excl_len=${#excl_opt}
> +
> + for n in $mnt_opts; do
> + # Handle mount option like "data="
> + sub_str=$(expr substr $n 1 ${excl_len})
> + if [ $sub_str == $excl_opt ]; then
> + _notrun "mount option \"$n\" not allowed in this test"
> + fi
> + done
> +}
> +
> +# skip test if custom MOUNT_OPTIONS contains prohibited mount
> +# options
> _exclude_scratch_mount_option()
> {
> - if echo $MOUNT_OPTIONS | grep -q "$1"; then
> - _notrun "mount option \"$1\" not allowed in this test"
> + fstype=$1
> + shift
> +
> + normalized=$(_normalize_mount_options)
> + if [ $fstype == "generic" -o $fstype = $FSTYP ]; then
> + while [ $# -gt 1 ]; do
> + _check_one_exclusive_option $1 $normalized
> + shift
> + done
> fi
> }
>
> diff --git a/tests/ext4/271 b/tests/ext4/271
> index 8674090..f3a99b7 100755
> --- a/tests/ext4/271
> +++ b/tests/ext4/271
> @@ -41,10 +41,8 @@ _supported_os Linux
> _require_scratch
> # this test needs no journal to be loaded, skip on journal related mount
> # options, otherwise mount would fail with "-o noload" mount option
> -_exclude_scratch_mount_option "data="
> -_exclude_scratch_mount_option "commit="
> -_exclude_scratch_mount_option "journal_checksum"
> -_exclude_scratch_mount_option "journal_async_commit"
> +_exclude_scratch_mount_option ext4 "data=" "commit=" "journal_checksum" \
> +"journal_async_commit"
>
> rm -f $seqres.full
> _scratch_mkfs_sized $((128 * 1024 * 1024)) >> $seqres.full 2>&1
> diff --git a/tests/xfs/134 b/tests/xfs/134
> index b3a1107..ca4a73d 100755
> --- a/tests/xfs/134
> +++ b/tests/xfs/134
> @@ -51,8 +51,7 @@ _supported_os Linux IRIX
> _require_test
> _require_xfs_quota
> # we can't run with group quotas
> -_exclude_scratch_mount_option "gquota"
> -_exclude_scratch_mount_option "grpquota"
> +_exclude_scratch_mount_option xfs "gquota" "grpquota"
>
> dir=$SCRATCH_MNT/project
>
> --
> 2.7.4
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majord...@vg
Re: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment in btrfs_read_chunk_tree.
On 9/5/16 11:05 PM, Jeff Mahoney wrote: > On 9/5/16 3:56 AM, Qu Wenruo wrote: >> >> >> At 09/05/2016 09:19 AM, Zhao Lei wrote: >>> Hi, Sean Fu >>> From: Sean Fu [mailto:fxinr...@gmail.com] Sent: Sunday, September 04, 2016 7:54 PM To: dste...@suse.com Cc: c...@fb.com; anand.j...@oracle.com; fdman...@suse.com; zhao...@cn.fujitsu.com; linux-btrfs@vger.kernel.org; linux-ker...@vger.kernel.org; Sean Fu Subject: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment in btrfs_read_chunk_tree. The input argument root is already set with "fs_info->chunk_root". "chunk_root = fs_info->chunk_root = btrfs_alloc_root(fs_info)" in caller "open_ctree". “root->fs_info = fs_info” in "btrfs_alloc_root". >>> The root argument of this function means "any root". >>> And the function is designed getting chunk root from >>> "any root" in head. >>> >>> Since there is only one caller of this function, >>> and the caller always send chunk_root as root argument in >>> current code, we can remove above conversion, >>> and I suggest renaming root to chunk_root to make it clear, >>> something like: >>> >>> - btrfs_read_chunk_tree(struct btrfs_root *root) >>> + btrfs_read_chunk_tree(struct btrfs_root *chunk_root) >> >> Since root is only used to get fs_info->chunk_root, why not use fs_info >> directly? > > Weird. Exactly this was a part of my fs_info patchset. I guess I need > to go back and check what else is missing. Actually, most of this didn't land. Pretty much anything that's a root ->fs_info conversion is in there. -Jeff > -Jeff > > >> Thanks, >> Qu >> >>> >>> Thanks >>> Zhaolei >>> Signed-off-by: Sean Fu --- fs/btrfs/volumes.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 366b335..384a6d2 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -6600,8 +6600,6 @@ int btrfs_read_chunk_tree(struct btrfs_root *root) int ret; int slot; -root = root->fs_info->chunk_root; - path = btrfs_alloc_path(); if (!path) return -ENOMEM; -- 2.6.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 >>> >> >> >> -- >> 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 >> > > -- Jeff Mahoney SUSE Labs signature.asc Description: OpenPGP digital signature
Re: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment in btrfs_read_chunk_tree.
On 9/5/16 3:56 AM, Qu Wenruo wrote: > > > At 09/05/2016 09:19 AM, Zhao Lei wrote: >> Hi, Sean Fu >> >>> From: Sean Fu [mailto:fxinr...@gmail.com] >>> Sent: Sunday, September 04, 2016 7:54 PM >>> To: dste...@suse.com >>> Cc: c...@fb.com; anand.j...@oracle.com; fdman...@suse.com; >>> zhao...@cn.fujitsu.com; linux-btrfs@vger.kernel.org; >>> linux-ker...@vger.kernel.org; Sean Fu >>> Subject: [PATCH] Btrfs: remove unnecessary code of chunk_root >>> assignment in >>> btrfs_read_chunk_tree. >>> >>> The input argument root is already set with "fs_info->chunk_root". >>> "chunk_root = fs_info->chunk_root = btrfs_alloc_root(fs_info)" in caller >>> "open_ctree". >>> “root->fs_info = fs_info” in "btrfs_alloc_root". >>> >> The root argument of this function means "any root". >> And the function is designed getting chunk root from >> "any root" in head. >> >> Since there is only one caller of this function, >> and the caller always send chunk_root as root argument in >> current code, we can remove above conversion, >> and I suggest renaming root to chunk_root to make it clear, >> something like: >> >> - btrfs_read_chunk_tree(struct btrfs_root *root) >> + btrfs_read_chunk_tree(struct btrfs_root *chunk_root) > > Since root is only used to get fs_info->chunk_root, why not use fs_info > directly? Weird. Exactly this was a part of my fs_info patchset. I guess I need to go back and check what else is missing. -Jeff > Thanks, > Qu > >> >> Thanks >> Zhaolei >> >>> Signed-off-by: Sean Fu >>> --- >>> fs/btrfs/volumes.c | 2 -- >>> 1 file changed, 2 deletions(-) >>> >>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c >>> index 366b335..384a6d2 100644 >>> --- a/fs/btrfs/volumes.c >>> +++ b/fs/btrfs/volumes.c >>> @@ -6600,8 +6600,6 @@ int btrfs_read_chunk_tree(struct btrfs_root *root) >>> int ret; >>> int slot; >>> >>> -root = root->fs_info->chunk_root; >>> - >>> path = btrfs_alloc_path(); >>> if (!path) >>> return -ENOMEM; >>> -- >>> 2.6.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 >> > > > -- > 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 > -- Jeff Mahoney SUSE Labs signature.asc Description: OpenPGP digital signature
Re: btrfs send extremely slow (almost stuck)
At 09/06/2016 05:29 AM, Oliver Freyermuth wrote: Am 05.09.2016 um 07:21 schrieb Qu Wenruo: Did you get the half way send stream? Luckily, yes! If the send stream has something, please use "--no-data" option to send the subvolume again to get the metadata only dump, and upload it for debug. Also the metadata-only dump fails with the same ioctl error (-2: No such file or directory). So I could only upload the stream up the occurence of that failure... Also, please paste "btrfs-debug-tree -t " output for debug. WARN: above "btrfs-debug-tree" command will contain file names. You could use the following sed to wipe filename: "btrfs-debug-tree -t 5 /dev/sda6 | sed "s/name:.*//" This indeed runs through without failure. It seems though that "btrfs send --no-data" which contains full metadata anyways contains all filenames (just from a quick look with 'strings'). I can probably not remove these without invalidating the stream, though... So I'd not like to upload this to some public location. Not a problem. You can try this branch of btrfs-progs: https://github.com/adam900710/btrfs-progs/tree/dump_send_stream Which adds a new subcommand "btrfs inspect dump-send". That command will dump all metadata for a send stream, like: -- ./btrfs ins dump-send < /tmp/output subvol: ./ro_snap uuid: 356a747f-b42f-1f4e-911d-fa5259f037f7, transid: 8 chown: ./ro_snap/ gid: 0, uid: 0 chmod: ./ro_snap/ mode: 755 utimes: ./ro_snap/ mkdir: ./ro_snap/o257-7-0 rename: ./ro_snap/o257-7-0 to ./ro_snap/etc utimes: ./ro_snap/ chown: ./ro_snap/etc gid: 0, uid: 0 chmod: ./ro_snap/etc mode: 755 utimes: ./ro_snap/etc mkfile: ./ro_snap/o258-7-0 rename: ./ro_snap/o258-7-0 to ./ro_snap/etc/hostname .. -- Where /tmp/output is a send stream. In that case you can mask all your file name. But your idea to locate the inode seems good enough for debugging though. However, you gave me an idea. I had a look at the output of running the file created by "btrfs send --no-data" piping that through "strings". This revealed the last files which btrfs send was able to treat before running into the ioctl failure. Indeed, this is my thunderbird profile directory, always a place with a lot of activity. Now the interesting part begins: Since of course I have a backup of this directory, I decided to move that profile to another FS and back. Turns out I can not run rm -rf ~/.thunderbird since it claims "directory not empty". Kernel log does no bug-on or OOPS or anything like that. That's reproducible not only in the snapshots, but also in my "home" subvolume for this folder. "stat -c %s" of the supposed-to-be-empty profile directory reveals indeed: 2482 In this case, before reverting to a backup, would you please run a "btrfsck check" and paste the output? Further more, your btrfs-debug-tree dump should provide more help for this case. With btrfs-debug-tree dump, at least we can find what's going wrong and causing the rm -rf failure. So I guess I should refresh my backups soon and either run "btrfs check --repair" or, if that fails, redo the FS... Likely btrfs check --repair will fail for me since (due to duperemove usage) I'll for sure also be hit by https://bugzilla.kernel.org/show_bug.cgi?id=155791 since I'm still using 4.7.1 so I'd like to update to 4.7.2 before trying out that repair strategy. I sadly can't do that in the next few days since I actively need the machine in question, so I'll rename that folder and restore just that from backup for now. Is the debug-information still of interest? If so, I can share it (but would not post it publicly to the list since many filenames are in there...). It weighs in at about 2 x 80 MiB after xz compression. Yes, debug dump is quite helpful. Better with your .thunderbird inode number. (ls -aldi .thunderbird can give the inode number, the first number) For debug tree filename problem, feel free to wipe the filename with the sed pipe I mentioned in previous mail. IIRC it should wipe all possible filename. Or is there anything else I can try safely? Despite debug-tree dump, which is sometimes overkilled, "btrfs check" (default in read-only mode) with v4.6.1 will help a lot. It will locate the direct problem very quickly and save us quite sometime to manually check the debug tree dump. (I assume your send problem is not related to send, but corrupted fs tree) Although it needs to be run on unmounted fs, so you may need to enter single user mode or use a liveCD/USB to do it. Thanks, Qu Thanks a lot in any case and cheers, Oliver Thanks, Qu -- 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://
RE: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment in btrfs_read_chunk_tree.
Hi, Qu Wenruo > From: Qu Wenruo [mailto:quwen...@cn.fujitsu.com] > Sent: Monday, September 05, 2016 3:57 PM > To: Zhao Lei ; 'Sean Fu' ; > dste...@suse.com > Cc: c...@fb.com; anand.j...@oracle.com; fdman...@suse.com; > linux-btrfs@vger.kernel.org; linux-ker...@vger.kernel.org > Subject: Re: [PATCH] Btrfs: remove unnecessary code of chunk_root > assignment in btrfs_read_chunk_tree. > > > > At 09/05/2016 09:19 AM, Zhao Lei wrote: > > Hi, Sean Fu > > > >> From: Sean Fu [mailto:fxinr...@gmail.com] > >> Sent: Sunday, September 04, 2016 7:54 PM > >> To: dste...@suse.com > >> Cc: c...@fb.com; anand.j...@oracle.com; fdman...@suse.com; > >> zhao...@cn.fujitsu.com; linux-btrfs@vger.kernel.org; > >> linux-ker...@vger.kernel.org; Sean Fu > >> Subject: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment > in > >> btrfs_read_chunk_tree. > >> > >> The input argument root is already set with "fs_info->chunk_root". > >> "chunk_root = fs_info->chunk_root = btrfs_alloc_root(fs_info)" in caller > >> "open_ctree". > >> “root->fs_info = fs_info” in "btrfs_alloc_root". > >> > > The root argument of this function means "any root". > > And the function is designed getting chunk root from > > "any root" in head. > > > > Since there is only one caller of this function, > > and the caller always send chunk_root as root argument in > > current code, we can remove above conversion, > > and I suggest renaming root to chunk_root to make it clear, > > something like: > > > > - btrfs_read_chunk_tree(struct btrfs_root *root) > > + btrfs_read_chunk_tree(struct btrfs_root *chunk_root) > > Since root is only used to get fs_info->chunk_root, why not use fs_info > directly? > Good idea. Thanks Zhaolei > Thanks, > Qu > > > > > Thanks > > Zhaolei > > > >> Signed-off-by: Sean Fu > >> --- > >> fs/btrfs/volumes.c | 2 -- > >> 1 file changed, 2 deletions(-) > >> > >> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > >> index 366b335..384a6d2 100644 > >> --- a/fs/btrfs/volumes.c > >> +++ b/fs/btrfs/volumes.c > >> @@ -6600,8 +6600,6 @@ int btrfs_read_chunk_tree(struct btrfs_root > *root) > >>int ret; > >>int slot; > >> > >> - root = root->fs_info->chunk_root; > >> - > >>path = btrfs_alloc_path(); > >>if (!path) > >>return -ENOMEM; > >> -- > >> 2.6.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 > > -- 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: btrfs send extremely slow (almost stuck)
Oliver Freyermuth posted on Mon, 05 Sep 2016 23:29:08 +0200 as excerpted: > However, you gave me an idea. I had a look at the output of running the > file created by "btrfs send --no-data" piping that through "strings". > This revealed the last files which btrfs send was able to treat before > running into the ioctl failure. > Indeed, this is my thunderbird profile directory, always a place with a > lot of activity. > > Now the interesting part begins: Since of course I have a backup of this > directory, I decided to move that profile to another FS and back. > Turns out I can not run rm -rf ~/.thunderbird since it claims "directory > not empty". Kernel log does no bug-on or OOPS or anything like that. > > That's reproducible not only in the snapshots, but also in my "home" > subvolume for this folder. > > "stat -c %s" of the supposed-to-be-empty profile directory reveals > indeed: > 2482 > > So I guess I should refresh my backups soon and either run "btrfs check > --repair" or, if that fails, redo the FS... > Likely btrfs check --repair will fail for me since (due to duperemove > usage) I'll for sure also be hit by > https://bugzilla.kernel.org/show_bug.cgi?id=155791 since I'm still using > 4.7.1 so I'd like to update to 4.7.2 before trying out that repair > strategy. > > I sadly can't do that in the next few days since I actively need the > machine in question, so I'll rename that folder and restore just that > from backup for now. > > Is the debug-information still of interest? If so, I can share it (but > would not post it publicly to the list since many filenames are in > there...). > It weighs in at about 2 x 80 MiB after xz compression. > > Or is there anything else I can try safely? I had something very similar happen here a few weeks ago, except with my firefox profile dir (I don't run thunderbird, preferring claws-mail, but I do run firefox as my browser). My use-case does neither snapshots nor send/receive, however, so it was just the single root subvolume (5). But there was supposedly a file in that dir according to bash's tab-completion, that would neither list, nor rm, which meant the dir couldn't rm -r either. (Interestingly enough, rm -i asked if I wanted to rm "weird file" whatever, and weird it indeed was! ) So I immediately copied all the normal files to a new dir, and deleted the normal files from the problem dir, leaving only the weird one. Then I renamed the problem dir in ordered to be able to rename the new dir (with the good files) back to the name firefox expected. Then I decided to see what I could do with the renamed dir. I believe I rebooted (or umount/mount cycled the filesystem) as well. I think I had to use the magic-sysrq m/remount-ro key as it refused to umount even from systemd emergency mode. But here's the interesting part. At least after the rename and a reboot, it *DID* let me delete (using mc) the dir! I honestly didn't expect it'd let me, but it did. So I'd try that. After copying all the good files out and renaming the dir out of the way, so you can rename the dir you copied the good files into back into place, reboot (or umount and mount again if possible), possibly by going to single-user or emergency mode first and using magic srq remount-ro to force it, if necessary, before rebooting. Then try to delete the dir again, and see if it will. The difference, however, is that I didn't have any snapshots/subvolumes or other reflinks to the "weird" file, only the one normal hardlink. So even if it's the same thing, I'm not sure if it'll work for you given the multiple snapshot reflinks to the file, as it did for me with just the one. So it might not work at all for you, or might work but you have to delete it in each snapshot, or deleting it in one might delete it in all (which would be weird, but it's already a weird file we're dealing with, so who knows...), I don't know which. And that of course assumes it's even the same basic bug and would behave as it did for me if you had no snapshots. That was with kernel 4.7.0 (which I'm still running, I'll be upgrading to 4.8 rcs pretty soon now) I believe. If not, then it was late in the 4.7 rc cycle or possibly 4.6.0, but it was definitely not older than that. -- Duncan - List replies preferred. No HTML msgs. "Every nonfree program has a lord, a master -- and if you use the program, he is your master." Richard Stallman -- 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/3] btrfs-progs: Introduce new send-dump object
At 08/24/2016 08:49 PM, David Sterba wrote: On Wed, Aug 17, 2016 at 08:42:48AM +0800, Qu Wenruo wrote: At 08/16/2016 10:31 PM, David Sterba wrote: On Mon, Aug 01, 2016 at 02:29:42PM +0800, Qu Wenruo wrote: Introduce send-dump.[ch] which implements a new btrfs_send_ops to exam and output all operations inside a send stream. It has a better output format than the old and no longer compilable send-test tool, but still tries to be script friendly. Provides the basis for later "inspect-internal dump-send" command. I'd rather put that into the receive subcommand, as it takes the stream as argument and it's closer to the send/receive commands, but that's a minor thing. I'm OK to put it into receive command. Although unlike normal receive usage, it doesn't need the parameter, and make to be optional. That's a good point and would make the syntax work in two modes, but we do that in ohter commands and I hope it will not be confusing. Exmaples: Normal receive: btrfs receive -f file /mount Dump the stream: btrs receive --dump -f file Sorry for the late reply. I'm OK for the new syntax. If no other problem I'll begin to change the syntax and rebase it to devel. Thanks, Qu -- 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: btrfs send extremely slow (almost stuck)
Am 05.09.2016 um 07:21 schrieb Qu Wenruo: > Did you get the half way send stream? Luckily, yes! > If the send stream has something, please use "--no-data" option to send the > subvolume again to get the metadata only dump, and upload it for debug. Also the metadata-only dump fails with the same ioctl error (-2: No such file or directory). So I could only upload the stream up the occurence of that failure... > > Also, please paste "btrfs-debug-tree -t " output for debug. > WARN: above "btrfs-debug-tree" command will contain file names. > You could use the following sed to wipe filename: > > "btrfs-debug-tree -t 5 /dev/sda6 | sed "s/name:.*//" This indeed runs through without failure. It seems though that "btrfs send --no-data" which contains full metadata anyways contains all filenames (just from a quick look with 'strings'). I can probably not remove these without invalidating the stream, though... So I'd not like to upload this to some public location. However, you gave me an idea. I had a look at the output of running the file created by "btrfs send --no-data" piping that through "strings". This revealed the last files which btrfs send was able to treat before running into the ioctl failure. Indeed, this is my thunderbird profile directory, always a place with a lot of activity. Now the interesting part begins: Since of course I have a backup of this directory, I decided to move that profile to another FS and back. Turns out I can not run rm -rf ~/.thunderbird since it claims "directory not empty". Kernel log does no bug-on or OOPS or anything like that. That's reproducible not only in the snapshots, but also in my "home" subvolume for this folder. "stat -c %s" of the supposed-to-be-empty profile directory reveals indeed: 2482 So I guess I should refresh my backups soon and either run "btrfs check --repair" or, if that fails, redo the FS... Likely btrfs check --repair will fail for me since (due to duperemove usage) I'll for sure also be hit by https://bugzilla.kernel.org/show_bug.cgi?id=155791 since I'm still using 4.7.1 so I'd like to update to 4.7.2 before trying out that repair strategy. I sadly can't do that in the next few days since I actively need the machine in question, so I'll rename that folder and restore just that from backup for now. Is the debug-information still of interest? If so, I can share it (but would not post it publicly to the list since many filenames are in there...). It weighs in at about 2 x 80 MiB after xz compression. Or is there anything else I can try safely? Thanks a lot in any case and cheers, Oliver > > Thanks, > Qu > -- 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: Post btrfs-convert verify permissions and acls
Signed-off-by: Lakshmipathi.G
---
tests/common.convert | 95 +++-
1 file changed, 94 insertions(+), 1 deletion(-)
diff --git a/tests/common.convert b/tests/common.convert
index 4e3d49c..67c99b1 100644
--- a/tests/common.convert
+++ b/tests/common.convert
@@ -123,6 +123,38 @@ convert_test_gen_checksums() {
count=1 >/dev/null 2>&1
run_check_stdout $SUDO_HELPER find $TEST_MNT -type f ! -name 'image'
-exec md5sum {} \+ > "$CHECKSUMTMP"
}
+# list $TEST_MNT data set file permissions.
+# $1: path where the permissions will be stored
+convert_test_perm() {
+ local PERMTMP
+ PERMTMP="$1"
+ FILES_LIST=$(mktemp --tmpdir btrfs-progs-convert.fileslistXX)
+
+ run_check $SUDO_HELPER dd if=/dev/zero of=$TEST_MNT/test bs=$nodesize \
+ count=1 >/dev/null 2>&1
+ run_check_stdout $SUDO_HELPER find $TEST_MNT -type f ! -name 'image'
-fprint $FILES_LIST
+ #Fix directory entries order.
+ sort $FILES_LIST -o $FILES_LIST
+ for file in `cat $FILES_LIST` ;do
+ run_check_stdout $SUDO_HELPER getfacl --absolute-names $file >>
"$PERMTMP"
+ done
+ rm $FILES_LIST
+}
+# list acls of files on $TEST_MNT
+# $1: path where the acls will be stored
+convert_test_acl() {
+ local ACLSTMP
+ ACLTMP="$1"
+ FILES_LIST=$(mktemp --tmpdir btrfs-progs-convert.fileslistXX)
+
+ run_check_stdout $SUDO_HELPER find $TEST_MNT/acls -type f -fprint
$FILES_LIST
+ #Fix directory entries order.
+ sort $FILES_LIST -o $FILES_LIST
+ for file in `cat $FILES_LIST`;do
+ run_check_stdout $SUDO_HELPER getfattr --absolute-names -d
$file >> "$ACLTMP"
+ done
+ rm $FILES_LIST
+}
# do conversion with given features and nodesize, fsck afterwards
# $1: features, argument of -O, can be empty
@@ -133,15 +165,68 @@ convert_test_do_convert() {
run_check $TOP/btrfs-show-super -Ffa $TEST_DEV
}
+# post conversion check, verify file permissions.
+# $1: file with ext permissions.
+convert_test_post_check_permissions() {
+ local EXT_PERMTMP
+ local BTRFS_PERMTMP
+
+ EXT_PERMTMP="$1"
+ BTRFS_PERMTMP=$(mktemp --tmpdir btrfs-progs-convert.permXX)
+ convert_test_perm "$BTRFS_PERMTMP"
+
+ btrfs_perm=`md5sum $BTRFS_PERMTMP | cut -f1 -d' '`
+ ext_perm=`md5sum $EXT_PERMTMP | cut -f1 -d' '`
+
+ if [ "$btrfs_perm" != "$ext_perm" ];
+ then
+ btrfs_perm_file=`md5sum $BTRFS_PERMTMP | cut -f2 -d' '`
+ ext_perm_file=`md5sum $EXT_PERMTMP | cut -f2 -d' '`
+ _fail "file permission failed. Mismatched
BTRFS:$btrfs_perm_file:$btrfs_perm EXT:$ext_perm_file:$ext_perm"
+ fi
+
+ rm $BTRFS_PERMTMP
+}
+# post conversion check, compare BTRFS file acls against EXT.
+# $1: file with ext acls.
+convert_test_post_check_acl() {
+ local EXT_ACLTMP
+ local BTRFS_ACLTMP
+
+ EXT_ACLTMP="$1"
+ BTRFS_ACLTMP=$(mktemp --tmpdir btrfs-progs-convert.aclsXXX)
+ convert_test_acl "$BTRFS_ACLTMP"
+
+ btrfs_acl=`md5sum $BTRFS_ACLTMP | cut -f1 -d' '`
+ ext_acl=`md5sum $EXT_ACLTMP | cut -f1 -d' '`
+
+ if [ "$btrfs_acl" != "$ext_acl" ]
+ then
+ btrfs_acl_file=`md5sum $BTRFS_ACLTMP | cut -f2 -d' '`
+ ext_acl_file=`md5sum $EXT_ACLTMP | cut -f2 -d' '`
+ _fail "file acl failed. Mismatched
BTRFS:$btrfs_acl_file:$btrfs_acl EXT:$ext_acl_file:$ext_acl"
+ fi
+
+ rm $BTRFS_ACLTMP
+}
# post conversion checks, verify md5sums
# $1: file with checksums
+# $2: file with permissions.
+# $3: file with acl entries.
convert_test_post_check() {
local CHECKSUMTMP
+ local EXT_PERMTMP
+ local EXT_ACLTMP
+
CHECKSUMTMP="$1"
+ EXT_PERMTMP="$2"
+ EXT_ACLTMP="$3"
run_check_mount_test_dev
run_check_stdout $SUDO_HELPER md5sum -c "$CHECKSUMTMP" |
grep -q 'FAILED' && _fail "file validation failed"
+ convert_test_post_check_permissions "$EXT_PERMTMP"
+ convert_test_post_check_acl "$EXT_ACLTMP"
run_check_umount_test_dev
}
@@ -161,6 +246,8 @@ convert_test() {
local nodesize
local msg
local CHECKSUMTMP
+ local EXT_PERMTMP
+ local EXT_ACLTMP
features="$1"
msg="$2"
@@ -170,13 +257,19 @@ convert_test() {
convert_test_prep_fs "$@"
populate_fs
CHECKSUMTMP=$(mktemp --tmpdir btrfs-progs-convert.XX)
+ EXT_PERMTMP=$(mktemp --tmpdir btrfs-progs-convert.permXX)
+ EXT_ACLTMP=$(mktemp --tmpdir btrfs-progs-convert.aclsXXX)
convert_test_gen_checksums "$CHECKSUMTMP"
+ convert_test_perm "$EXT_PERMTMP"
+ convert_test_acl "$EXT_ACLTMP"
run_check_umount_test_dev
convert_test_do_convert "$features" "$nodesize"
- convert_test_post_check "$CHECKSUMTMP"
+ convert_test_pos
Re: [PATCH 1/2] btrfs-progs: mkfs: Warn user for minimal RAID5/6 devices setup
On Fri, Sep 02, 2016 at 11:59:07AM +1000, Steven Haigh wrote: > Is it worthwhile adding a note that RAID5 / RAID6 may very well eat your > data at this stage? This depends on the kernel implementation, so it needs to take that into account. There's a patchset in the works to automatically select mkfs features based on kernel so a warning for features like raid56 will be built on top of 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 0/7] Kill the btree inode
On Fri, Sep 02, 2016 at 03:39:59PM -0400, Josef Bacik wrote: > In order to provide a better way to do subpage blocksizes we need to stop > allocating pages from a per fs btree inode and instead allocate our own pages. > This work depends on 3 generic patches that I've sent previously > > remove mapping from balance_dirty_pages*() > writeback: allow for dirty metadata accounting > writeback: introduce super_operations->write_metadata What's the status of the patches? I don't see them in current linux-next. I can put them to our for-next for the time being so we can test the btree_inode removal. > This is a pretty big change but ultimately makes extent_buffer reclaim much > cleaner and will make the sub-pagesize blocksize work significantly cleaner. > I've been hammering on this for a few weeks now and seems to be pretty solid. The preparatory patches are ok. The core patch is quite large, although there are simple transformations to the new eb_info, the remaining changes are still quite big for a review. But I don't see a way how to split it, it's basically an all-or-nothing change. Overall it looks ok so I'll put it to for-next. -- 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 BUG_ON in btrfs_mark_buffer_dirty
On Fri, Sep 2, 2016 at 8:35 PM, Liu Bo wrote:
> This can only happen with CONFIG_BTRFS_FS_CHECK_INTEGRITY=y.
>
> Commit 1ba98d0 ("Btrfs: detect corruption when non-root leaf has zero item")
> assumes that a leaf is its root when leaf->bytenr == btrfs_root_bytenr(root),
> however, we should not use btrfs_root_bytenr(root) since it's mainly got
> updated during committing transaction. So the check can fail when doing
> COW on this leaf while it is a root.
>
> This changes to use "if (leaf == btrfs_root_node(root))" instead, just like
> how we check whether leaf is a root in __btrfs_cow_block().
>
> Reported-by: Jeff Mahoney
> Signed-off-by: Liu Bo
Hi Bo,
Even with this patch applied against latest branch for-linus-4.8, at
least on a build with CONFIG_BTRFS_FS_CHECK_INTEGRITY=y,
the issue still happens for me when running fsstress with balance in parallel:
[ 366.998044] BTRFS: device fsid 69759b3a-96ae-467d-aa63-364144e73a28
devid 1 transid 3 /dev/sdc
[ 367.023652] BTRFS info (device sdc): turning on discard
[ 367.025036] BTRFS info (device sdc): disk space caching is enabled
[ 367.026360] BTRFS info (device sdc): has skinny extents
[ 367.069415] BTRFS info (device sdc): creating UUID tree
[ 367.133704] BTRFS info (device sdc): relocating block group 29360128 flags 36
[ 367.142196] BTRFS info (device sdc): found 2 extents
[ 367.147932] BTRFS info (device sdc): relocating block group 20971520 flags 34
[ 367.157947] BTRFS info (device sdc): found 1 extents
[ 367.162649] BTRFS info (device sdc): relocating block group 12582912 flags 1
[ 367.182872] BTRFS info (device sdc): found 1 extents
[ 367.189932] BTRFS info (device sdc): found 1 extents
[ 367.200983] BTRFS info (device sdc): relocating block group
1270874112 flags 1
[ 367.235862] BTRFS critical (device sdc): corrupt leaf, non-root
leaf's nritems is 0: block=1103937536,root=5, slot=0
[ 367.238223] BTRFS info (device sdc): leaf 1103937536 total ptrs 0
free space 3995
[ 367.239371] BTRFS: assertion failed: 0, file: fs/btrfs/disk-io.c, line: 4059
[ 367.240321] [ cut here ]
[ 367.241245] kernel BUG at fs/btrfs/ctree.h:3367!
[ 367.241961] invalid opcode: [#1] PREEMPT SMP
[ 367.242685] Modules linked in: btrfs crc32c_generic xor raid6_pq
acpi_cpufreq tpm_tis tpm sg i2c_piix4 i2c_core psmouse ppdev processor
evdev parport_pc parport serio_raw pcspkr button loop autofs4 ext4
crc16 jbd2 mbcache sr_mod cdrom sd_mod ata_generic virtio_scsi
ata_piix libata virtio_pci virtio_ring e1000 virtio scsi_mod floppy
[last unloaded: btrfs]
[ 367.244302] CPU: 11 PID: 3451 Comm: fdm-stress Not tainted
4.7.0-rc6-btrfs-next-34+ #1
[ 367.244302] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.9.1-0-gb3ef39f-prebuilt.qemu-project.org 04/01/2014
[ 367.244302] task: 880183ef8bc0 ti: 880183fe task.ti:
880183fe
[ 367.244302] RIP: 0010:[] []
assfail.constprop.42+0x1c/0x1e [btrfs]
[ 367.244302] RSP: 0018:880183fe3c78 EFLAGS: 00010296
[ 367.244302] RAX: 0040 RBX: 880222a66ab0 RCX: 0001
[ 367.244302] RDX: 810a0a23 RSI: 814a82cb RDI:
[ 367.244302] RBP: 880183fe3c78 R08: 0001 R09:
[ 367.244302] R10: 880183fe3b70 R11: 82f3650d R12: 880183e8e000
[ 367.244302] R13: 8800b3e7d000 R14: 0a59 R15: 0017
[ 367.244302] FS: 7f0b85310700() GS:88023f4c()
knlGS:
[ 367.244302] CS: 0010 DS: ES: CR0: 80050033
[ 367.244302] CR2: 7f0b7800ea28 CR3: 00022da9b000 CR4: 06e0
[ 367.244302] Stack:
[ 367.244302] 880183fe3ca0 a03e51e3 0023
880222a66ab0
[ 367.244302] 880183885bb8 880183fe3d38 a03c9540
83fe3d08
[ 367.244302] 8800b3e7d2d8 1000 880183fe3e7f
880183ff8000
[ 367.244302] Call Trace:
[ 367.244302] [] btrfs_mark_buffer_dirty+0xdf/0xe5 [btrfs]
[ 367.244302] [] split_leaf+0x47c/0x566 [btrfs]
[ 367.244302] [] btrfs_search_slot+0x5df/0x755 [btrfs]
[ 367.244302] [] ? slab_post_alloc_hook+0x42/0x52
[ 367.244302] [] btrfs_insert_empty_items+0x5d/0xa6 [btrfs]
[ 367.244302] [] btrfs_symlink+0x17f/0x34f [btrfs]
[ 367.244302] [] vfs_symlink+0x51/0x6e
[ 367.244302] [] SYSC_symlinkat+0x6d/0xb2
[ 367.244302] [] ? trace_hardirqs_on_thunk+0x1a/0x1c
[ 367.244302] [] SyS_symlink+0x16/0x18
[ 367.244302] [] entry_SYSCALL_64_fastpath+0x18/0xa8
[ 367.244302] [] ? time_hardirqs_off+0x9/0x14
[ 367.244302] [] ? trace_hardirqs_off_caller+0x1f/0xaa
[ 367.244302] Code: 89 83 88 00 00 00 31 c0 5b 41 5c 41 5d 5d c3 55
89 f1 48 c7 c2 14 8b 46 a0 48 89 fe 48 c7 c7 27 8c 46 a0 48 89 e5 e8
e5 2f cc e0 <0f> 0b 55 89 f1 48 c7 c2 03 a2 46 a0 48 89 fe 48 c7 c7 dc
a3 46
[ 367.244302] RIP [] assfail.constprop.42+0x1c/0x1e [btrfs]
[ 367.244302] RSP
[ 367.289039] ---[ end trace a3e4ce9819ed383b ]---
thanks
> ---
> fs/
Re: [PATCH] Btrfs: don't leak reloc root nodes on error
On Fri, Sep 02, 2016 at 06:08:35PM -0700, Liu Bo wrote:
> On Fri, Sep 02, 2016 at 03:25:43PM -0400, Josef Bacik wrote:
> > We don't track the reloc roots in any sort of normal way, so the only way
> > the
> > root/commit_root nodes get free'd is if the relocation finishes
> > successfully and
> > the reloc root is deleted. Fix this by free'ing them in free_reloc_roots.
> > Thanks,
>
> Looks good.
>
> >
> > Signed-off-by: Josef Bacik
> > ---
> > fs/btrfs/relocation.c | 4
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> > index 7fc6ea7..62dfc2c 100644
> > --- a/fs/btrfs/relocation.c
> > +++ b/fs/btrfs/relocation.c
> > @@ -2349,6 +2349,10 @@ void free_reloc_roots(struct list_head *list)
> > while (!list_empty(list)) {
> > reloc_root = list_entry(list->next, struct btrfs_root,
> > root_list);
> > + free_extent_buffer(reloc_root->node);
> > + free_extent_buffer(reloc_root->commit_root);
> > + reloc_root->node = NULL;
> > + reloc_root->commit_root = NULL;
>
> What about reloc_root itself?
> > __del_reloc_root(reloc_root);
It's deleted at the end of __del_reloc_root
--
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: gazillions of Incorrect local/global backref count
On Mon, 2016-09-05 at 09:27 +0200, David Sterba wrote: > As others replied, it's a false positive. There's a fix on the way, > once > it's done I'll release 4.7.2. Yeah... thanks again for confirming... and sorry that I've missed the obvious earlier post :-/ Best wishes, Chris. smime.p7s Description: S/MIME cryptographic signature
Re: Security implications of btrfs receive?
Graham Cobb posted on Mon, 05 Sep 2016 10:59:30 +0100 as excerpted: > Lastly, even if receive is designed to be very secure, it is possible > that it could trigger/use code paths in the btrfs kernel code which are > not normally used during normal file operations and so could trigger > bugs not normally seen. Has any work been done on testing for that (for > example tests using malicious streams, including ones which btrfs-send > cannot generate)? As a btrfs user and list regular (not a dev) I'll only answer this part, as it's the part I know an answer to. =:^) Btrfs in general is not fuzz- or malicious-content resistant, yet. In general, btrfs is considered stabilizing, but not yet fully stable, and fuzzer-related bug reports, as others, are taken and worked on, but the emphasis has been primarily on getting things working and bugs fixed in general, not yet on security hardening of any sort, so no claims as to btrfs hardening or resistance to malicious content can be made at this point, except that it's known to be pretty soft in that regard ATM. As I said, fuzz-generated bugs are accepted and fixed, but I don't know that the intent is to ever "harden" btrfs in that regard, more to simply make it resilient to corruptions in general. There has been, for instance, some discussion of attacks by simply leaving maliciously engineered btrfs thumb drives around to be inserted and automounted, but the attitude seems to be once they have physical access to plug them in, hardening is an exercise in futility, so the object isn't to prevent that attack vector, but rather, to make btrfs more resilient to normal (as opposed to deliberate) corruption that may occur, including that which is easiest to /find/ by fuzzing, but which may "just happen" in the real world, as well. Of course that's not in the specific scope of receive, but I'd put it in the same boat. IOW, treat potential send clients much as you would people with accounts on the machine. If you wouldn't trust them with a basic shell account, don't trust their send-streams either. Meanwhile, the stabilizing but not fully stable and mature status also means backups are even more strongly recommended than they would be with a fully stable filesystem. Which means, to the extent that backups can mitigate the issue, they'd certainly be prudent and may to that extent solve the practical issue. However, as always, if it's not backed up and you lose it, you've simply lost the low-value data that wasn't of enough value to you to be worth the hassle of backup, defined by your actions as such, regardless of any words claiming the contrary. To the extent that you can trust your people as much as your backups, great, but not having those backups really /is/ defining that data as not worth the hassle, regardless of whether it's lost to malicious attack or to hardware/ software/wetware bug. -- Duncan - List replies preferred. No HTML msgs. "Every nonfree program has a lord, a master -- and if you use the program, he is your master." Richard Stallman -- 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
Btrfs progs release 4.7.2
Hi, btrfs-progs 4.7 have been released. There's one notable fix, for the false warning from checker about backref mismatches, first reported by Chris Murphy (https://bugzilla.kernel.org/show_bug.cgi?id=155791). Update to 4.7.2 is required before using --repair. The other part are current fixes resulting from fuzzing. Changes: * check: * urgent fix: false report of backref mismatches; do not --repair last unaffected version 4.6.1 (code reverted to that state) * fuzzing and fixes * added more sanity checks for various structures * testing images added * build: udev compatibility: do not install .rules on version < 190 * other: * dump-super: do not crash on garbage value in csum_type * minor improvements in messages and help strings * documentation: * filesystem features Tarballs: https://www.kernel.org/pub/linux/kernel/people/kdave/btrfs-progs/ Git: git://git.kernel.org/pub/scm/linux/kernel/git/kdave/btrfs-progs.git Shortlog: David Sterba (16): btrfs-progs: tests README: fuzzed images btrfs-progs: docs: describe filesystem features btrfs-progs: drop permission arg from non-creating open btrfs-progs: dump-super: detect invalid checksum type btrfs-progs: tests: add 015-dump-super-garbage btrfs-progs: print help test to stdout btrfs-progs: image: more verbose syntax erors for -t and -c btrfs-progs: tests: add fuzzed image for invalid sub_stripe value btrfs-progs: tests: add fuzzed image for invalid sys_array and stripe_len btrfs-progs: tests: add fuzzed image for invalid chunk sectorsize btrfs-progs: tests: add fuzzed image for heap overflow while checking chunk items btrfs-progs: build: add ASAN to debugging features Revert "btrfs-progs: check: switch to iterating over the backref_tree" Revert "btrfs-progs: check: supplement extent backref list with rbtree" btrfs-progs: update CHANGES for 4.7.2 Btrfs progs v4.7.2 Jeff Mahoney (1): btrfs-progs: build: only install udev rules for udev >= 190 Qu Wenruo (12): btrfs-progs: Enhance and export print_objectid function btrfs-progs: Enhance and export print_key_type function btrfs-progs: check: ignore invalid key in invalid root btrfs-progs: Do extra chunk check before processing chunk item btrfs-progs: fuzz-test: Add image for wrong chunk item in root tree btrfs-progs: check: do early check for read_tree_block btrfs-progs: fuzz-test: Add image for unaligned tree block ptr btrfs-progs: fsck: Check drop level before walking through fs tree btrfs-progs: fuzz-test: Add test case for invalid drop level btrfs-progs: fsck: Check bytenr alignment for extent item btrfs-progs: fsck: Avoid abort and BUG_ON in add_tree_backref btrfs-progs: fuzz-test: Add test case for unaligned extent item -- 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
Security implications of btrfs receive?
Does anyone know of a security analysis of btrfs receive? I assume that just using btrfs receive requires root (is that so?). But I was thinking of setting up a backup server which would receive snapshots from various client systems, each in their own path, and I wondered how much the security of the backup server (and other clients' backups) was dependent on the security of the client. Does the "path" argument of btrfs-receive mean that *all* operations are confined to that path? For example, if a UUID or transid is sent which refers to an entity outside the path will that other entity be affected or used? Is it possible for a file to be created containing shared extents from outside the path? Is it possible to confuse/affect filesystem metadata which would affect the integrity of subvolumes or files outside the path or prevent other clients from doing something legitimate? Do the answers change if the --chroot option is given? I am confused about the -m option -- does that mean that the root mount point has to be visible in the chroot? Lastly, even if receive is designed to be very secure, it is possible that it could trigger/use code paths in the btrfs kernel code which are not normally used during normal file operations and so could trigger bugs not normally seen. Has any work been done on testing for that (for example tests using malicious streams, including ones which btrfs-send cannot generate)? I am just wondering whether any work has been done/published on this area. Regards Graham -- 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: BTRFS constantly reports "No space left on device" even with a huge unallocated space
Just like what Wang has mentioned, would you please paste all the output of the contents of /sys/fs/btrfs//allocation? It's recommended to use "grep . -IR " to get all the data as it will show the file name. Thanks, Qu At 09/03/2016 03:25 AM, Ronan Arraes Jardim Chagas wrote: Hi guys! Jeff was right. I had the problem again today and quotas are disabled now. I couldn't get any useful message in log this time. Look at the metadata: btrfs fi usage / Overall: Device size: 1.26TiB Device allocated: 43.07GiB Device unallocated:1.21TiB Device missing: 0.00B Used: 41.94GiB Free (estimated): 1.21TiB (min: 622.46GiB) Data ratio: 1.00 Metadata ratio: 2.00 Global reserve: 352.00MiB (used: 0.00B) Data,single: Size:40.01GiB, Used:39.94GiB /dev/sda6 40.01GiB Metadata,DUP: Size:1.50GiB, Used:1.00GiB /dev/sda6 3.00GiB System,DUP: Size:32.00MiB, Used:16.00KiB /dev/sda6 64.00MiB Unallocated: /dev/sda6 1.21TiB Any ideas to help me? Regards, Ronan Arraes -- 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 unnecessary code of chunk_root assignment in btrfs_read_chunk_tree.
At 09/05/2016 09:19 AM, Zhao Lei wrote: Hi, Sean Fu From: Sean Fu [mailto:fxinr...@gmail.com] Sent: Sunday, September 04, 2016 7:54 PM To: dste...@suse.com Cc: c...@fb.com; anand.j...@oracle.com; fdman...@suse.com; zhao...@cn.fujitsu.com; linux-btrfs@vger.kernel.org; linux-ker...@vger.kernel.org; Sean Fu Subject: [PATCH] Btrfs: remove unnecessary code of chunk_root assignment in btrfs_read_chunk_tree. The input argument root is already set with "fs_info->chunk_root". "chunk_root = fs_info->chunk_root = btrfs_alloc_root(fs_info)" in caller "open_ctree". “root->fs_info = fs_info” in "btrfs_alloc_root". The root argument of this function means "any root". And the function is designed getting chunk root from "any root" in head. Since there is only one caller of this function, and the caller always send chunk_root as root argument in current code, we can remove above conversion, and I suggest renaming root to chunk_root to make it clear, something like: - btrfs_read_chunk_tree(struct btrfs_root *root) + btrfs_read_chunk_tree(struct btrfs_root *chunk_root) Since root is only used to get fs_info->chunk_root, why not use fs_info directly? Thanks, Qu Thanks Zhaolei Signed-off-by: Sean Fu --- fs/btrfs/volumes.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 366b335..384a6d2 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -6600,8 +6600,6 @@ int btrfs_read_chunk_tree(struct btrfs_root *root) int ret; int slot; - root = root->fs_info->chunk_root; - path = btrfs_alloc_path(); if (!path) return -ENOMEM; -- 2.6.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 -- 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: does btrfs-receive use/compare the checksums from the btrfs-send side?
At 09/04/2016 12:29 PM, Christoph Anton Mitterer wrote: On Mon, 2016-08-29 at 16:25 +0800, Qu Wenruo wrote: Send will generate checksum for each command. What does "command" mean here? Or better said how much data is secured with one CRC32? Command is one send command stream, containing all needed info for a operation, like subvolume command, containing UUID,tranid, chown command, containing gid/uid, chmod command, containing mode, utimes command, containting acm times and a lot of others. For how much data is secured by 1 CRC32, it depends on the size of the command. Normal command is quite small, but the exception would be write command. More than 48K bytes can be secured by one CRC32. For send stream, it's CRC32 for the whole command. And this is verified then on the receiving end? Yes. Wouldn't it be useful (if this technically possibly) to use the checksums directly from the sent blocks? That way one could also catch any errors on the receiving side, that occurred after the checksum from the receive was verified (e.g. memory errors). And couldn't one do something similar locally, when btrfs copies blocks? At least something like this would seem to me like the most native way: - One want's checksum protection - One copies data - One has already checksums for that data You can try my dump-send command branch, to verify how send/receive works: https://github.com/adam900710/btrfs-progs/tree/dump_send_stream With several try, you could find at least the following reasons: 1) Not all data has checksum Only non-inlined data has checksum. Inlined data has no checksum (protected by leaf checksum then) 2) Send doesn't following sectorsize unit for non-inlined data Just create a 6K file, send the subvolume out, and use dump-send to exam it. You'll find that, send stream contains exactly 6K data, not 8K (2* 4K, which 4K is sectorsize). While for data checksum, that are all in sectorsize unit. 3) We need to protect the whole command, not file data only. Even write command contains metadata info, like the offset and length of the write. Since we need to protect the whole command, why not introduce the complexity to use TWO CRC32 for meta and data? => thus that should be used, as it's the most canonical version of a checksum for that data... anything that is newly calculated could in the best case just be good, and in the worst add new errors (unnoticed),... e.g. when memory is broken and the new checksum is calculated over that. However most bugs are not caused by memory corruption, but humans. So the command checksum design seems quite good for me though. It's unified, simple structure and expandable. Thanks, Qu Cheers, Chris. -- 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: gazillions of Incorrect local/global backref count
On Sun, Sep 04, 2016 at 06:50:53AM +0200, Christoph Anton Mitterer wrote: > Hey. > > I just did a btrfs check on my notebooks root fs, with: > $ uname -a > Linux heisenberg 4.7.0-1-amd64 #1 SMP Debian 4.7.2-1 (2016-08-28) > x86_64 GNU/Linux > $ btrfs --version > btrfs-progs v4.7.1 As others replied, it's a false positive. There's a fix on the way, once it's done I'll release 4.7.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] fstests: common: Enhance _exclude_scratch_mount_option to handle multiply options and generic fs type
Enhance _exclude_scratch_mount_option() function to get real mount
options from $MOUNT_OPTIONS.
Now it can understand and extract real mount option from string like
"-o opt1,opt2 -oopt3".
Furthermore, it doesn't use grep method, which can lead to false alert
for options like inode_cache and noinode_cache.
It now do compare with the first n characters of the prohibited list,
so it can handle "data=" and above "no" prefix well.
And add a new parameter, 'fstype' for _exclude_scratch_mount_option().
So for generic test cases, it can still prohibit mount options for given
fs(mainly for btrfs though)
Finally, allow it to accept multiple options at the same time.
No need for multiple _exclude_scratch_mount_option lines now
Signed-off-by: Qu Wenruo
---
common/rc | 41 ++---
tests/ext4/271 | 6 ++
tests/xfs/134 | 3 +--
3 files changed, 41 insertions(+), 9 deletions(-)
diff --git a/common/rc b/common/rc
index 04039a4..8fca637 100644
--- a/common/rc
+++ b/common/rc
@@ -3183,11 +3183,46 @@ _require_cloner()
_notrun "cloner binary not present at $CLONER_PROG"
}
-# skip test if MKFS_OPTIONS contains the given string
+# Normalize mount options from global $MOUNT_OPTIONS
+# Will convert "-o options1,options2 -ooptions3" into
+# "options1 options2 options3" for easy access
+_normalize_mount_options()
+{
+ echo $MOUNT_OPTIONS | sed -n 's/-o\s*\(\S*\)/\1/gp' |\
+ sed 's/,/ /g'
+}
+
+# Helper for _exclude_scratch_mount_option , check if exclusive mount option $1
+# is in the custom mount options
+_check_one_exclusive_option()
+{
+ excl_opt=$1
+ shift
+ mnt_opts=$*
+ excl_len=${#excl_opt}
+
+ for n in $mnt_opts; do
+ # Handle mount option like "data="
+ sub_str=$(expr substr $n 1 ${excl_len})
+ if [ $sub_str == $excl_opt ]; then
+ _notrun "mount option \"$n\" not allowed in this test"
+ fi
+ done
+}
+
+# skip test if custom MOUNT_OPTIONS contains prohibited mount
+# options
_exclude_scratch_mount_option()
{
- if echo $MOUNT_OPTIONS | grep -q "$1"; then
- _notrun "mount option \"$1\" not allowed in this test"
+ fstype=$1
+ shift
+
+ normalized=$(_normalize_mount_options)
+ if [ $fstype == "generic" -o $fstype = $FSTYP ]; then
+ while [ $# -gt 1 ]; do
+ _check_one_exclusive_option $1 $normalized
+ shift
+ done
fi
}
diff --git a/tests/ext4/271 b/tests/ext4/271
index 8674090..f3a99b7 100755
--- a/tests/ext4/271
+++ b/tests/ext4/271
@@ -41,10 +41,8 @@ _supported_os Linux
_require_scratch
# this test needs no journal to be loaded, skip on journal related mount
# options, otherwise mount would fail with "-o noload" mount option
-_exclude_scratch_mount_option "data="
-_exclude_scratch_mount_option "commit="
-_exclude_scratch_mount_option "journal_checksum"
-_exclude_scratch_mount_option "journal_async_commit"
+_exclude_scratch_mount_option ext4 "data=" "commit=" "journal_checksum" \
+ "journal_async_commit"
rm -f $seqres.full
_scratch_mkfs_sized $((128 * 1024 * 1024)) >> $seqres.full 2>&1
diff --git a/tests/xfs/134 b/tests/xfs/134
index b3a1107..ca4a73d 100755
--- a/tests/xfs/134
+++ b/tests/xfs/134
@@ -51,8 +51,7 @@ _supported_os Linux IRIX
_require_test
_require_xfs_quota
# we can't run with group quotas
-_exclude_scratch_mount_option "gquota"
-_exclude_scratch_mount_option "grpquota"
+_exclude_scratch_mount_option xfs "gquota" "grpquota"
dir=$SCRATCH_MNT/project
--
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
