[PATCH v2 2/3] btrfs-progs: test: fix how to make test files in fsck-tests 013

[Tsutomu Itoh]

In my test environment, following error was occurred because the size
of /lib/modules/`uname -r`/* is larger than 1GB.

# make test-fsck
[TEST]   fsck-tests.sh
[TEST/fsck]   013-extent-tree-rebuild
failed: cp -aR /lib/modules/4.9.0-rc5/ /test/btrfs-progs/tests/mnt
test failed for case 013-extent-tree-rebuild
Makefile:272: recipe for target 'test-fsck' failed
make: *** [test-fsck] Error 1
#

In this test case, 'generate_dataset small' is enough for making the
test files, so I will use 'generate_dataset' instead of 'cp'.

For this, move 'generate_dataset()' from 'common.convert' to 'common'.

Signed-off-by: Tsutomu Itoh 
---
v2: change how to make test files
---
 tests/common | 89 
 tests/common.convert | 89 
 tests/fsck-tests/013-extent-tree-rebuild/test.sh |  2 +-
 3 files changed, 90 insertions(+), 90 deletions(-)

diff --git a/tests/common b/tests/common
index c61962d..8b760f7 100644
--- a/tests/common
+++ b/tests/common
@@ -309,6 +309,95 @@ check_kernel_support()
return 0
 }
 
+# how many files to create.
+DATASET_SIZE=50
+
+generate_dataset() {
+
+   dataset_type="$1"
+   dirpath=$TEST_MNT/$dataset_type
+   run_check $SUDO_HELPER mkdir -p "$dirpath"
+
+   case $dataset_type in
+   small)
+   for num in $(seq 1 "$DATASET_SIZE"); do
+   run_check $SUDO_HELPER dd if=/dev/urandom 
of="$dirpath/$dataset_type.$num" bs=10K \
+   count=1 >/dev/null 2>&1
+   done
+   ;;
+
+   hardlink)
+   for num in $(seq 1 "$DATASET_SIZE"); do
+   run_check $SUDO_HELPER touch 
$dirpath/$dataset_type.$num
+   run_check $SUDO_HELPER ln 
"$dirpath/$dataset_type.$num" "$dirpath/hlink.$num"
+   done
+   ;;
+
+   fast_symlink)
+   for num in $(seq 1 "$DATASET_SIZE"); do
+   run_check $SUDO_HELPER touch 
$dirpath/$dataset_type.$num
+   run_check cd "$dirpath" && \
+   $SUDO_HELPER ln -s "$dataset_type.$num" 
"$dirpath/slink.$num" && \
+   cd /
+   done
+   ;;
+
+   brokenlink)
+   for num in $(seq 1 "$DATASET_SIZE"); do
+   run_check $SUDO_HELPER ln -s 
"$dirpath/$dataset_type.$num" "$dirpath/blink.$num"
+   done
+   ;;
+
+   perm)
+   for modes in 777 775 755 750 700 666 664 644 640 600 
444 440 400 000\
+   1777 1775 1755 1750 1700 1666 1664 1644 1640 
1600 1444 1440 1400 1000   \
+   2777 2775 2755 2750 2700 2666 2664 2644 2640 
2600 2444 2440 2400 2000   \
+   4777 4775 4755 4750 4700 4666 4664 4644 4640 
4600  4440 4400 4000; do
+   if [[ "$modes" == *9* ]] || [[ "$modes" == *8* 
]]
+   then
+   continue;
+   else
+   run_check $SUDO_HELPER touch 
"$dirpath/$dataset_type.$modes"
+   run_check $SUDO_HELPER chmod "$modes" 
"$dirpath/$dataset_type.$modes"
+   fi
+   done
+   ;;
+
+   sparse)
+   for num in $(seq 1 "$DATASET_SIZE"); do
+   run_check $SUDO_HELPER dd if=/dev/urandom 
of="$dirpath/$dataset_type.$num" bs=10K \
+   count=1 >/dev/null 2>&1
+   run_check $SUDO_HELPER truncate -s 500K 
"$dirpath/$dataset_type.$num"
+   run_check $SUDO_HELPER dd if=/dev/urandom 
of="$dirpath/$dataset_type.$num" bs=10K \
+   oflag=append conv=notrunc count=1 >/dev/null 
2>&1
+   run_check $SUDO_HELPER truncate -s 800K 
"$dirpath/$dataset_type.$num"
+   done
+   ;;
+
+   acls)
+   for num in $(seq 1 "$DATASET_SIZE"); do
+   run_check $SUDO_HELPER touch 
"$dirpath/$dataset_type.$num"
+   run_check $SUDO_HELPER setfacl -m "u:root:x" 
"$dirpath/$dataset_type.$num"
+   run_check $SUDO_HELPER setfattr -n user.foo -v 
"bar$num" "$dirpath/$dataset_type.$num"
+   done
+   ;;
+
+   fifo)
+   for num in $(seq 1 "$DATASET_SIZE"); do
+  

Re: [PATCH] btrfs: Ensure proper sector alignment for btrfs_free_reserved_data_space

[Qu Wenruo]

At 11/19/2016 10:52 AM, Jeff Mahoney wrote:

From: Jeff Mahoney 
Subject: btrfs: Ensure proper sector alignment for
 btrfs_free_reserved_data_space
References: bsc#1005666
Patch-mainline: Submitted 18 Nov 2016, linux-btrfs

This fixes the WARN_ON on BTRFS_I(inode)->reserved_extents in
btrfs_destroy_inode and the WARN_ON on nonzero delalloc bytes on umount
with qgroups enabled.

I was able to reproduce this by setting up a small (~500kb) quota limit
and writing a file one byte at a time until I hit the limit.  The warnings
would all hit on umount.

The root cause is that we would reserve a block-sized range in both
the reservation and the quota in btrfs_check_data_free_space, but if we
encountered a problem (like e.g. EDQUOT), we would only release the single
byte in the qgroup reservation.


Good catch.

> That caused an iotree state split, which

increased the number of outstanding extents, in turn disallowing releasing
the metadata reservation.

Signed-off-by: Jeff Mahoney 


Reviewed-by: Qu Wenruo 

What about adding more assert/warn_on to reservation/qgroup freeing and 
allocation functions?


It would help much more to detect similar problems.

Thanks,
Qu


---
 fs/btrfs/extent-tree.c |7 +++
 1 file changed, 7 insertions(+)

--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -3822,6 +3822,13 @@ void btrfs_free_reserved_data_space_noqu
  */
 void btrfs_free_reserved_data_space(struct inode *inode, u64 start, u64 len)
 {
+   struct btrfs_root *root = BTRFS_I(inode)->root;
+
+   /* Make sure the range is aligned to sectorsize */
+   len = round_up(start + len, root->sectorsize) -
+ round_down(start, root->sectorsize);
+   start = round_down(start, root->sectorsize);
+
btrfs_free_reserved_data_space_noquota(inode, start, len);
btrfs_qgroup_free_data(inode, start, len);
 }





--
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/3] btrfs-progs: test: expand size of test device of fsck-tests 013

[Tsutomu Itoh]

Hi, Qu and David,

On 2016/11/19 3:28, David Sterba wrote:
> On Fri, Nov 18, 2016 at 02:36:28PM +0800, Qu Wenruo wrote:
>>
>>
>> On 11/18/2016 01:47 PM, Tsutomu Itoh wrote:
>>> In my test environment, size of /lib/modules/`uname -r`/* is
>>> larger than 1GB, so test data can not copy to testdev.
>>> Therefore we need expand the size of testdev.
>>
>> IMHO the correct fix is to enhance populate_fs() to fill the fs into a 
>> given size, other than using the unreliable copy.
> 
> Yeah that would be better, the contents of the modules dir could be
> anything, but was enough to use for initial testing.

Thanks for your comments. I'll post v2 patch.

Thanks,
Tsutomu

--
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: Ensure proper sector alignment for btrfs_free_reserved_data_space

[Jeff Mahoney]

On 11/20/16 10:50 AM, Liu Bo wrote:
> On Fri, Nov 18, 2016 at 09:52:40PM -0500, Jeff Mahoney wrote:
>> From: Jeff Mahoney 
>> Subject: btrfs: Ensure proper sector alignment for
>>  btrfs_free_reserved_data_space
>> References: bsc#1005666
>> Patch-mainline: Submitted 18 Nov 2016, linux-btrfs
>>
>> This fixes the WARN_ON on BTRFS_I(inode)->reserved_extents in
>> btrfs_destroy_inode and the WARN_ON on nonzero delalloc bytes on umount
>> with qgroups enabled.
>>
>> I was able to reproduce this by setting up a small (~500kb) quota limit
>> and writing a file one byte at a time until I hit the limit.  The warnings
>> would all hit on umount.
>>
>> The root cause is that we would reserve a block-sized range in both
>> the reservation and the quota in btrfs_check_data_free_space, but if we
>> encountered a problem (like e.g. EDQUOT), we would only release the single
>> byte in the qgroup reservation.  That caused an iotree state split, which
>> increased the number of outstanding extents, in turn disallowing releasing
>> the metadata reservation.
>>
>> Signed-off-by: Jeff Mahoney 
>> ---
>>  fs/btrfs/extent-tree.c |7 +++
>>  1 file changed, 7 insertions(+)
>>
>> --- a/fs/btrfs/extent-tree.c
>> +++ b/fs/btrfs/extent-tree.c
>> @@ -3822,6 +3822,13 @@ void btrfs_free_reserved_data_space_noqu
>>   */
>>  void btrfs_free_reserved_data_space(struct inode *inode, u64 start, u64 len)
>>  {
>> +struct btrfs_root *root = BTRFS_I(inode)->root;
>> +
>> +/* Make sure the range is aligned to sectorsize */
>> +len = round_up(start + len, root->sectorsize) -
>> +  round_down(start, root->sectorsize);
>> +start = round_down(start, root->sectorsize);
>> +
>>  btrfs_free_reserved_data_space_noquota(inode, start, len);
>>  btrfs_qgroup_free_data(inode, start, len);
> 
> The patch looks reasonable, but I'm afraid btrfs_fallocate can be
> affected since in btrfs_fallocate(), btrfs_qgroup_reserve_data() takes
> 'cur_offset' and 'last_byte - cur_offset' which are possible unaligned
> to root->sectorsize, but if any errors occur during allocation,
> btrfs_qgroup_free_data() in btrfs_free_reserved_data_space() is gonna
> free aligned range and it ends up a negative qgroup value.

Ok, yeah.  I was thinking about this later that evening but hadn't
gotten a chance to dig back into it.  I think the biggest thing is that
the handling of space reservation and qgroups is way too complicated.
It seems nearly impossible for new bugs *not* to sneak in whenever we
touch them.

For this particular bit, though, both cur_offset and last_byte are
sector aligned in btrfs_fallocate, at least in the current mainline
HEAD.  I think fixing up the alignment in the reservation and qgroups
routines is probably the wrong way to do it.  Instead, we should expect
the callers to handle the alignment properly and complain very loudly if
they fail to do that.  I started in on patches to do that after I
submitted the one above, but wanted to get the fix sent first.

-Jeff

-- 
Jeff Mahoney
SUSE Labs



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] btrfs: Ensure proper sector alignment for btrfs_free_reserved_data_space

[Liu Bo]

On Fri, Nov 18, 2016 at 09:52:40PM -0500, Jeff Mahoney wrote:
> From: Jeff Mahoney 
> Subject: btrfs: Ensure proper sector alignment for
>  btrfs_free_reserved_data_space
> References: bsc#1005666
> Patch-mainline: Submitted 18 Nov 2016, linux-btrfs
> 
> This fixes the WARN_ON on BTRFS_I(inode)->reserved_extents in
> btrfs_destroy_inode and the WARN_ON on nonzero delalloc bytes on umount
> with qgroups enabled.
> 
> I was able to reproduce this by setting up a small (~500kb) quota limit
> and writing a file one byte at a time until I hit the limit.  The warnings
> would all hit on umount.
> 
> The root cause is that we would reserve a block-sized range in both
> the reservation and the quota in btrfs_check_data_free_space, but if we
> encountered a problem (like e.g. EDQUOT), we would only release the single
> byte in the qgroup reservation.  That caused an iotree state split, which
> increased the number of outstanding extents, in turn disallowing releasing
> the metadata reservation.
> 
> Signed-off-by: Jeff Mahoney 
> ---
>  fs/btrfs/extent-tree.c |7 +++
>  1 file changed, 7 insertions(+)
> 
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -3822,6 +3822,13 @@ void btrfs_free_reserved_data_space_noqu
>   */
>  void btrfs_free_reserved_data_space(struct inode *inode, u64 start, u64 len)
>  {
> + struct btrfs_root *root = BTRFS_I(inode)->root;
> +
> + /* Make sure the range is aligned to sectorsize */
> + len = round_up(start + len, root->sectorsize) -
> +   round_down(start, root->sectorsize);
> + start = round_down(start, root->sectorsize);
> +
>   btrfs_free_reserved_data_space_noquota(inode, start, len);
>   btrfs_qgroup_free_data(inode, start, len);

The patch looks reasonable, but I'm afraid btrfs_fallocate can be
affected since in btrfs_fallocate(), btrfs_qgroup_reserve_data() takes
'cur_offset' and 'last_byte - cur_offset' which are possible unaligned
to root->sectorsize, but if any errors occur during allocation,
btrfs_qgroup_free_data() in btrfs_free_reserved_data_space() is gonna
free aligned range and it ends up a negative qgroup value.

Thanks,

-liubo

>  }
> 
> 
> -- 
> Jeff Mahoney
> SUSE Labs
> --
> 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: degraded BTRFS RAID 1 not mountable: open_ctree failed, unable to find block group for 0

[Niccolò Belli]

On giovedì 17 novembre 2016 21:20:56 CET, Austin S. Hemmelgarn wrote:

On 2016-11-17 15:05, Chris Murphy wrote:

I think the wiki should be updated to reflect that raid1 and raid10
are mostly OK. I think it's grossly misleading to consider either as
green/OK when a single degraded read write mount creates single chunks
that will then prevent a subsequent degraded read write mount. And
also the lack of various notifications of device faultiness I think
make it less than OK also. It's not in the "do not use" category but
it should be in the middle ground status so users can make informed
decisions.


It's worth pointing out also regarding this:
* This is handled sanely in recent kernels (the check got 
changed from per-fs to per-chunk, so you still have a usable FS 
if all the single chunks are only on devices you still have).
* This is only an issue with filesystems with exactly two 
disks.  If a 3+ disk raid1 FS goes degraded, you still generate 
raid1 chunks.
* There are a couple of other cases where raid1 mode falls flat 
on it's face (lots of I/O errors in a short span of time with 
compression enabled can cause a kernel panic for example).
* raid10 has some other issues of it's own (you lose two 
devices, your filesystem is dead, which shouldn't be the case 
100% of the time (if you lose different parts of each mirror, 
BTRFS _should_ be able to recover, it just doesn't do so right 
now)).


As far as the failed device handling issues, those are a 
problem with BTRFS in general, not just raid1 and raid10, so I 
wouldn't count those against raid1 and raid10.


Everything you mentioned should be in the wiki IMHO. Knowledge is power.
--
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