Re: [PATCH 0/3] Btrfs: fix free space tree bitmaps+tests on big-endian systems

[David Sterba]

On Thu, Aug 18, 2016 at 01:33:27PM -0700, Omar Sandoval wrote:
> > Omar, looks like we need to make the patched kernel refuse to mount free
> > space trees without a new incompat bit set.  That way there won't be any
> > surprises for the people that have managed to get a free space tree saved.
> > Can it please printk a message about clearing the tree and mounting again?
> > 
> > -chris
> 
> Sorry it took me a month to get around to this, I tried to implement
> this a couple of ways but I really don't like it. Basically, when we see
> that we're missing the compat bit, we have to assume that the free space
> tree was created with the same endianness that we're running on now.
> That could lead to a false positive if, say, we created the filesystem
> on a little-endian machine with an old kernel but are using it on a
> big-endian system, or a false negative if it was created on a big-endian
> machine with an old kernel but we're using it on a little-endian
> machine.
> 
> There's also the question of making it a compat bit vs an incompat bit.
> An incompat bit makes sure that we don't break the filesystem by
> mounting it on an old big-endian kernel, but needlessly breaks
> backwards-compatibility for little-endian.
> 
> I'd be much happier if we could just pretend this never happened. Here's
> the patch, anyways, for the sake of completeness. Chris, what do you
> think?

Here's my proposal how to resolve this:

* we have reports from people using intel hw that FST works fine for
  them, so here I don't see any need to introduce incompat bits

* there are few users on bigendian machines, they need to update kernel
  to store the correct layout of FST bitmap

* as majority of user will never hit the BE/LE problem, I'd focus on
  advising how to reset the free space tree and let anybody update (ie.
  pretend this hasn't happened)

I don't think we absolutelly must introduce the incompat bit and prevent
a disaster, because there are very few users affected.
--
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/3] Btrfs: fix free space tree bitmaps+tests on big-endian systems

[Anatoly Pugachev]

On Sat, Aug 27, 2016 at 3:56 AM, Omar Sandoval  wrote:
> On Fri, Aug 26, 2016 at 02:06:29PM +0300, Anatoly Pugachev wrote:
>>
>> I can't load btrfs module with this patch applied to 4.8.0-rc3+ (git
>> v4.8-rc3-39-g61c0457)
>> on "modprobe btrfs" i'm getting the following in the logs and module
>> does not load:
>>
>> Btrfs loaded, crc32c=crc32c-generic, debug=on, assert=on
>> BTRFS: selftest: sectorsize: 8192  nodesize: 8192
>> BTRFS: selftest: Running btrfs free space cache tests
>> BTRFS: selftest: Running extent only tests
>> BTRFS: selftest: Running bitmap only tests
>> BTRFS: selftest: Running bitmap and extent tests
>> BTRFS: selftest: Running space stealing from bitmap to extent
>> BTRFS: selftest: Free space cache tests finished
>> BTRFS: selftest: Running extent buffer operation tests
>> BTRFS: selftest: Running btrfs_split_item tests
>> BTRFS: selftest: Running extent I/O tests
>> BTRFS: selftest: Running find delalloc tests
>> BTRFS: selftest: Running extent buffer bitmap tests
>> BTRFS: selftest: Setting straddling pages failed
>> BTRFS: selftest: Extent I/O tests finished
>
> Is this with the whole patchset + this patch? You still need the patch
> set for this to actually work, the extra patch is just some extra
> checks.

Omar,

sorry, I don't get what do you mean by the whole patchset ? I used git
kernel (git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git)
and applied patch from your mail from August 18,
https://www.spinics.net/lists/linux-btrfs/msg58023.html

Tell me what patches do I need or point to btrfs git repo with all
patches included and I'll test it.

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

Re: [PATCH 0/3] Btrfs: fix free space tree bitmaps+tests on big-endian systems

[Omar Sandoval]

On Fri, Aug 26, 2016 at 02:06:29PM +0300, Anatoly Pugachev wrote:
> On Thu, Aug 18, 2016 at 11:33 PM, Omar Sandoval  wrote:
> > On Tue, Jul 19, 2016 at 03:25:16PM -0400, Chris Mason wrote:
> >> On 07/19/2016 12:06 PM, Chandan Rajendra wrote:
> >>
> >> Omar, looks like we need to make the patched kernel refuse to mount free
> >> space trees without a new incompat bit set.  That way there won't be any
> >> surprises for the people that have managed to get a free space tree saved.
> >> Can it please printk a message about clearing the tree and mounting again?
> >>
> > Sorry it took me a month to get around to this, I tried to implement
> > this a couple of ways but I really don't like it. Basically, when we see
> > that we're missing the compat bit, we have to assume that the free space
> > tree was created with the same endianness that we're running on now.
> > That could lead to a false positive if, say, we created the filesystem
> > on a little-endian machine with an old kernel but are using it on a
> > big-endian system, or a false negative if it was created on a big-endian
> > machine with an old kernel but we're using it on a little-endian
> > machine.
> >
> > There's also the question of making it a compat bit vs an incompat bit.
> > An incompat bit makes sure that we don't break the filesystem by
> > mounting it on an old big-endian kernel, but needlessly breaks
> > backwards-compatibility for little-endian.
> >
> > I'd be much happier if we could just pretend this never happened. Here's
> > the patch, anyways, for the sake of completeness. Chris, what do you
> > think?
> 
> Omar,
> 
> I can't load btrfs module with this patch applied to 4.8.0-rc3+ (git
> v4.8-rc3-39-g61c0457)
> on "modprobe btrfs" i'm getting the following in the logs and module
> does not load:
> 
> Btrfs loaded, crc32c=crc32c-generic, debug=on, assert=on
> BTRFS: selftest: sectorsize: 8192  nodesize: 8192
> BTRFS: selftest: Running btrfs free space cache tests
> BTRFS: selftest: Running extent only tests
> BTRFS: selftest: Running bitmap only tests
> BTRFS: selftest: Running bitmap and extent tests
> BTRFS: selftest: Running space stealing from bitmap to extent
> BTRFS: selftest: Free space cache tests finished
> BTRFS: selftest: Running extent buffer operation tests
> BTRFS: selftest: Running btrfs_split_item tests
> BTRFS: selftest: Running extent I/O tests
> BTRFS: selftest: Running find delalloc tests
> BTRFS: selftest: Running extent buffer bitmap tests
> BTRFS: selftest: Setting straddling pages failed
> BTRFS: selftest: Extent I/O tests finished

Is this with the whole patchset + this patch? You still need the patch
set for this to actually work, the extra patch is just some extra
checks.

-- 
Omar
--
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/3] Btrfs: fix free space tree bitmaps+tests on big-endian systems

[Anatoly Pugachev]

On Thu, Aug 18, 2016 at 11:33 PM, Omar Sandoval  wrote:
> On Tue, Jul 19, 2016 at 03:25:16PM -0400, Chris Mason wrote:
>> On 07/19/2016 12:06 PM, Chandan Rajendra wrote:
>>
>> Omar, looks like we need to make the patched kernel refuse to mount free
>> space trees without a new incompat bit set.  That way there won't be any
>> surprises for the people that have managed to get a free space tree saved.
>> Can it please printk a message about clearing the tree and mounting again?
>>
> Sorry it took me a month to get around to this, I tried to implement
> this a couple of ways but I really don't like it. Basically, when we see
> that we're missing the compat bit, we have to assume that the free space
> tree was created with the same endianness that we're running on now.
> That could lead to a false positive if, say, we created the filesystem
> on a little-endian machine with an old kernel but are using it on a
> big-endian system, or a false negative if it was created on a big-endian
> machine with an old kernel but we're using it on a little-endian
> machine.
>
> There's also the question of making it a compat bit vs an incompat bit.
> An incompat bit makes sure that we don't break the filesystem by
> mounting it on an old big-endian kernel, but needlessly breaks
> backwards-compatibility for little-endian.
>
> I'd be much happier if we could just pretend this never happened. Here's
> the patch, anyways, for the sake of completeness. Chris, what do you
> think?

Omar,

I can't load btrfs module with this patch applied to 4.8.0-rc3+ (git
v4.8-rc3-39-g61c0457)
on "modprobe btrfs" i'm getting the following in the logs and module
does not load:

Btrfs loaded, crc32c=crc32c-generic, debug=on, assert=on
BTRFS: selftest: sectorsize: 8192  nodesize: 8192
BTRFS: selftest: Running btrfs free space cache tests
BTRFS: selftest: Running extent only tests
BTRFS: selftest: Running bitmap only tests
BTRFS: selftest: Running bitmap and extent tests
BTRFS: selftest: Running space stealing from bitmap to extent
BTRFS: selftest: Free space cache tests finished
BTRFS: selftest: Running extent buffer operation tests
BTRFS: selftest: Running btrfs_split_item tests
BTRFS: selftest: Running extent I/O tests
BTRFS: selftest: Running find delalloc tests
BTRFS: selftest: Running extent buffer bitmap tests
BTRFS: selftest: Setting straddling pages failed
BTRFS: selftest: Extent I/O tests finished
--
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/3] Btrfs: fix free space tree bitmaps+tests on big-endian systems

[Omar Sandoval]

On Tue, Jul 19, 2016 at 03:25:16PM -0400, Chris Mason wrote:
> On 07/19/2016 12:06 PM, Chandan Rajendra wrote:
> > On Monday, July 18, 2016 03:31:04 PM Omar Sandoval wrote:
> > > Yeah, this should definitely not work. It's possible that things are
> > > just silently failing and getting corrupted if the module isn't built
> > > with CONFIG_BTRFS_ASSERT, but btrfsck v4.6.1 + my patch should catch
> > > that.
> > > 
> > > Chandan, is fsx creating enough fragmentation to trigger the switch to
> > > bitmaps? You can check with `btrfs inspect dump-tree`; there should be
> > > FREE_SPACE_BITMAP items. If there are only FREE_SPACE_EXTENT items, then
> > > it's not testing the right code path.
> > > 
> > > I have a script here [1] that I've been using to test the free space
> > > tree. When I ran it with `--check` on MIPS, it failed on the old kernel
> > > and passed with this series. If you stick a return after the call to
> > > `unlink_every_other_file()`, you'll get a nice, fragmented filesystem to
> > > feed to xfstests, as well.
> > 
> > You are right, There were only FREE_SPACE_EXTENT items in the filesystem 
> > that
> > was operated on by fsx. I executed fragment_free_space_tree.py to create a
> > filesystem with FREE_SPACE_BITMAP items. When such a filesystem is created
> > with the unpatched kernel, later mounted on a patched kernel and fsx 
> > executed
> > on it, I see that we fail assertion statements in free-space-tree.c. For 
> > e.g.
> > 
> > BTRFS error (device loop0): incorrect extent count for 289406976; counted 
> > 8186, expected 8192
> > BTRFS: assertion failed: 0, file: 
> > /root/repos/linux/fs/btrfs/free-space-tree.c, line: 1485
> > 
> 
> Omar, looks like we need to make the patched kernel refuse to mount free
> space trees without a new incompat bit set.  That way there won't be any
> surprises for the people that have managed to get a free space tree saved.
> Can it please printk a message about clearing the tree and mounting again?
> 
> -chris

Sorry it took me a month to get around to this, I tried to implement
this a couple of ways but I really don't like it. Basically, when we see
that we're missing the compat bit, we have to assume that the free space
tree was created with the same endianness that we're running on now.
That could lead to a false positive if, say, we created the filesystem
on a little-endian machine with an old kernel but are using it on a
big-endian system, or a false negative if it was created on a big-endian
machine with an old kernel but we're using it on a little-endian
machine.

There's also the question of making it a compat bit vs an incompat bit.
An incompat bit makes sure that we don't break the filesystem by
mounting it on an old big-endian kernel, but needlessly breaks
backwards-compatibility for little-endian.

I'd be much happier if we could just pretend this never happened. Here's
the patch, anyways, for the sake of completeness. Chris, what do you
think?

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 2fe8f89..f50b3e0 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -246,7 +246,8 @@ struct btrfs_super_block {
  * Compat flags that we support.  If any incompat flags are set other than the
  * ones specified below then we will fail to mount
  */
-#define BTRFS_FEATURE_COMPAT_SUPP  0ULL
+#define BTRFS_FEATURE_COMPAT_SUPP  \
+   (BTRFS_FEATURE_COMPAT_FREE_SPACE_TREE_LE)
 #define BTRFS_FEATURE_COMPAT_SAFE_SET  0ULL
 #define BTRFS_FEATURE_COMPAT_SAFE_CLEAR0ULL
 
@@ -3538,6 +3539,62 @@ static inline int __btrfs_fs_compat_ro(struct 
btrfs_fs_info *fs_info, u64 flag)
return !!(btrfs_super_compat_ro_flags(disk_super) & flag);
 }
 
+#define btrfs_set_fs_compat(__fs_info, opt) \
+   __btrfs_set_fs_compat((__fs_info), BTRFS_FEATURE_COMPAT_##opt)
+
+static inline void __btrfs_set_fs_compat(struct btrfs_fs_info *fs_info,
+u64 flag)
+{
+   struct btrfs_super_block *disk_super;
+   u64 features;
+
+   disk_super = fs_info->super_copy;
+   features = btrfs_super_compat_flags(disk_super);
+   if (!(features & flag)) {
+   spin_lock(_info->super_lock);
+   features = btrfs_super_compat_flags(disk_super);
+   if (!(features & flag)) {
+   features |= flag;
+   btrfs_set_super_compat_flags(disk_super, features);
+   btrfs_info(fs_info, "setting %llu feature flag", flag);
+   }
+   spin_unlock(_info->super_lock);
+   }
+}
+
+#define btrfs_clear_fs_compat(__fs_info, opt) \
+   __btrfs_clear_fs_compat((__fs_info), BTRFS_FEATURE_COMPAT_##opt)
+
+static inline void __btrfs_clear_fs_compat(struct btrfs_fs_info *fs_info,
+ u64 flag)
+{
+   struct btrfs_super_block *disk_super;
+   u64 features;
+
+   disk_super = fs_info->super_copy;
+   features = 

Re: [PATCH 0/3] Btrfs: fix free space tree bitmaps+tests on big-endian systems

[Anatoly Pugachev]

On Fri, Jul 15, 2016 at 2:31 AM, Omar Sandoval  wrote:
> From: Omar Sandoval 
>
> So it turns out that the free space tree bitmap handling has always been
> broken on big-endian systems. Totally my bad.
>
> Patch 1 fixes this. Technically, it's a disk format change for
> big-endian systems, but it never could have worked before, so I won't go
> through the trouble of any incompat bits. If you've somehow been using
> space_cache=v2 on a big-endian system (I doubt anyone is), you're going
> to want to mount with nospace_cache to clear it and wait for this to go
> in.
>
> Patch 2 fixes a similar error in the sanity tests (it's the same as the
> v2 I posted here [1]) and patch 3 expands the sanity tests to catch the
> oversight that patch 1 fixes.
>
> Applies to v4.7-rc7. No regressions in xfstests, and the sanity tests
> pass on x86_64 and MIPS.

Omar,

can you please upstream this patch or update it for current git kernel ?
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] Btrfs: fix free space tree bitmaps+tests on big-endian systems

[Chris Mason]

On 07/19/2016 12:06 PM, Chandan Rajendra wrote:

On Monday, July 18, 2016 03:31:04 PM Omar Sandoval wrote:

Yeah, this should definitely not work. It's possible that things are
just silently failing and getting corrupted if the module isn't built
with CONFIG_BTRFS_ASSERT, but btrfsck v4.6.1 + my patch should catch
that.

Chandan, is fsx creating enough fragmentation to trigger the switch to
bitmaps? You can check with `btrfs inspect dump-tree`; there should be
FREE_SPACE_BITMAP items. If there are only FREE_SPACE_EXTENT items, then
it's not testing the right code path.

I have a script here [1] that I've been using to test the free space
tree. When I ran it with `--check` on MIPS, it failed on the old kernel
and passed with this series. If you stick a return after the call to
`unlink_every_other_file()`, you'll get a nice, fragmented filesystem to
feed to xfstests, as well.


You are right, There were only FREE_SPACE_EXTENT items in the filesystem that
was operated on by fsx. I executed fragment_free_space_tree.py to create a
filesystem with FREE_SPACE_BITMAP items. When such a filesystem is created
with the unpatched kernel, later mounted on a patched kernel and fsx executed
on it, I see that we fail assertion statements in free-space-tree.c. For e.g.

BTRFS error (device loop0): incorrect extent count for 289406976; counted 8186, 
expected 8192
BTRFS: assertion failed: 0, file: /root/repos/linux/fs/btrfs/free-space-tree.c, 
line: 1485



Omar, looks like we need to make the patched kernel refuse to mount free 
space trees without a new incompat bit set.  That way there won't be any 
surprises for the people that have managed to get a free space tree 
saved.  Can it please printk a message about clearing the tree and 
mounting again?


-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: [PATCH 0/3] Btrfs: fix free space tree bitmaps+tests on big-endian systems

[Chandan Rajendra]

On Monday, July 18, 2016 03:31:04 PM Omar Sandoval wrote:
> On Mon, Jul 18, 2016 at 02:43:26PM -0400, Chris Mason wrote:
> > 
> > 
> > On 07/17/2016 08:19 AM, Chandan Rajendra wrote:
> > > On Friday, July 15, 2016 12:15:15 PM Omar Sandoval wrote:
> > > > On Fri, Jul 15, 2016 at 12:34:10PM +0530, Chandan Rajendra wrote:
> > > > > On Thursday, July 14, 2016 07:47:04 PM Chris Mason wrote:
> > > > > > On 07/14/2016 07:31 PM, Omar Sandoval wrote:
> > > > > > > From: Omar Sandoval 
> > > > > > > 
> > > > > > > So it turns out that the free space tree bitmap handling has 
> > > > > > > always been
> > > > > > > broken on big-endian systems. Totally my bad.
> > > > > > > 
> > > > > > > Patch 1 fixes this. Technically, it's a disk format change for
> > > > > > > big-endian systems, but it never could have worked before, so I 
> > > > > > > won't go
> > > > > > > through the trouble of any incompat bits. If you've somehow been 
> > > > > > > using
> > > > > > > space_cache=v2 on a big-endian system (I doubt anyone is), you're 
> > > > > > > going
> > > > > > > to want to mount with nospace_cache to clear it and wait for this 
> > > > > > > to go
> > > > > > > in.
> > > > > > > 
> > > > > > > Patch 2 fixes a similar error in the sanity tests (it's the same 
> > > > > > > as the
> > > > > > > v2 I posted here [1]) and patch 3 expands the sanity tests to 
> > > > > > > catch the
> > > > > > > oversight that patch 1 fixes.
> > > > > > > 
> > > > > > > Applies to v4.7-rc7. No regressions in xfstests, and the sanity 
> > > > > > > tests
> > > > > > > pass on x86_64 and MIPS.
> > > > > > 
> > > > > > Thanks for fixing this up Omar.  Any big endian friends want to try 
> > > > > > this
> > > > > > out in extended testing and make sure we've nailed it down?
> > > > > > 
> > > > > 
> > > > > Hi Omar & Chris,
> > > > > 
> > > > > I will run fstests with this patchset applied on ppc64 BE and inform 
> > > > > you about
> > > > > the results.
> > > > > 
> > > > 
> > > > Thanks, Chandan! I set up my xfstests for space_cache=v2 by doing:
> > > > 
> > > > mkfs.btrfs "$TEST_DEV"
> > > > mount -o space_cache=v2 "$TEST_DEV" "$TEST_DIR"
> > > > umount "$TEST_DEV"
> > > > 
> > > > and adding
> > > > 
> > > > export MOUNT_OPTIONS="-o space_cache=v2"
> > > > 
> > > > to local.config. btrfsck also needs the patch here [1].
> > > > 
> > > > 
> > > 
> > > Hi,
> > > 
> > > I did execute the fstests tests suite on ppc64 BE as per above 
> > > configuration
> > > and there were no new regressions. Also, I did execute fsx (via 
> > > generic/127)
> > > thrice on the same filesystem instance,
> > > 1. With the unpatched kernel and later
> > > 2. With the patched kernel and again
> > > 3. With the unpatched kernel
> > > ... there were no new regressions when executing the above steps.
> > 
> > Thanks Chandan!  But I'm a little confused.  If the patch is helping, we
> > should be storing bitmaps wrong on disk unpatched.  There should be problems
> > going back and forth.
> > 
> > -chris
> 
> Yeah, this should definitely not work. It's possible that things are
> just silently failing and getting corrupted if the module isn't built
> with CONFIG_BTRFS_ASSERT, but btrfsck v4.6.1 + my patch should catch
> that.
> 
> Chandan, is fsx creating enough fragmentation to trigger the switch to
> bitmaps? You can check with `btrfs inspect dump-tree`; there should be
> FREE_SPACE_BITMAP items. If there are only FREE_SPACE_EXTENT items, then
> it's not testing the right code path.
> 
> I have a script here [1] that I've been using to test the free space
> tree. When I ran it with `--check` on MIPS, it failed on the old kernel
> and passed with this series. If you stick a return after the call to
> `unlink_every_other_file()`, you'll get a nice, fragmented filesystem to
> feed to xfstests, as well.

You are right, There were only FREE_SPACE_EXTENT items in the filesystem that
was operated on by fsx. I executed fragment_free_space_tree.py to create a
filesystem with FREE_SPACE_BITMAP items. When such a filesystem is created
with the unpatched kernel, later mounted on a patched kernel and fsx executed
on it, I see that we fail assertion statements in free-space-tree.c. For e.g.

BTRFS error (device loop0): incorrect extent count for 289406976; counted 8186, 
expected 8192
BTRFS: assertion failed: 0, file: /root/repos/linux/fs/btrfs/free-space-tree.c, 
line: 1485

-- 
chandan

--
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/3] Btrfs: fix free space tree bitmaps+tests on big-endian systems

[Omar Sandoval]

On Mon, Jul 18, 2016 at 02:43:26PM -0400, Chris Mason wrote:
> 
> 
> On 07/17/2016 08:19 AM, Chandan Rajendra wrote:
> > On Friday, July 15, 2016 12:15:15 PM Omar Sandoval wrote:
> > > On Fri, Jul 15, 2016 at 12:34:10PM +0530, Chandan Rajendra wrote:
> > > > On Thursday, July 14, 2016 07:47:04 PM Chris Mason wrote:
> > > > > On 07/14/2016 07:31 PM, Omar Sandoval wrote:
> > > > > > From: Omar Sandoval 
> > > > > > 
> > > > > > So it turns out that the free space tree bitmap handling has always 
> > > > > > been
> > > > > > broken on big-endian systems. Totally my bad.
> > > > > > 
> > > > > > Patch 1 fixes this. Technically, it's a disk format change for
> > > > > > big-endian systems, but it never could have worked before, so I 
> > > > > > won't go
> > > > > > through the trouble of any incompat bits. If you've somehow been 
> > > > > > using
> > > > > > space_cache=v2 on a big-endian system (I doubt anyone is), you're 
> > > > > > going
> > > > > > to want to mount with nospace_cache to clear it and wait for this 
> > > > > > to go
> > > > > > in.
> > > > > > 
> > > > > > Patch 2 fixes a similar error in the sanity tests (it's the same as 
> > > > > > the
> > > > > > v2 I posted here [1]) and patch 3 expands the sanity tests to catch 
> > > > > > the
> > > > > > oversight that patch 1 fixes.
> > > > > > 
> > > > > > Applies to v4.7-rc7. No regressions in xfstests, and the sanity 
> > > > > > tests
> > > > > > pass on x86_64 and MIPS.
> > > > > 
> > > > > Thanks for fixing this up Omar.  Any big endian friends want to try 
> > > > > this
> > > > > out in extended testing and make sure we've nailed it down?
> > > > > 
> > > > 
> > > > Hi Omar & Chris,
> > > > 
> > > > I will run fstests with this patchset applied on ppc64 BE and inform 
> > > > you about
> > > > the results.
> > > > 
> > > 
> > > Thanks, Chandan! I set up my xfstests for space_cache=v2 by doing:
> > > 
> > > mkfs.btrfs "$TEST_DEV"
> > > mount -o space_cache=v2 "$TEST_DEV" "$TEST_DIR"
> > > umount "$TEST_DEV"
> > > 
> > > and adding
> > > 
> > > export MOUNT_OPTIONS="-o space_cache=v2"
> > > 
> > > to local.config. btrfsck also needs the patch here [1].
> > > 
> > > 
> > 
> > Hi,
> > 
> > I did execute the fstests tests suite on ppc64 BE as per above configuration
> > and there were no new regressions. Also, I did execute fsx (via generic/127)
> > thrice on the same filesystem instance,
> > 1. With the unpatched kernel and later
> > 2. With the patched kernel and again
> > 3. With the unpatched kernel
> > ... there were no new regressions when executing the above steps.
> 
> Thanks Chandan!  But I'm a little confused.  If the patch is helping, we
> should be storing bitmaps wrong on disk unpatched.  There should be problems
> going back and forth.
> 
> -chris

Yeah, this should definitely not work. It's possible that things are
just silently failing and getting corrupted if the module isn't built
with CONFIG_BTRFS_ASSERT, but btrfsck v4.6.1 + my patch should catch
that.

Chandan, is fsx creating enough fragmentation to trigger the switch to
bitmaps? You can check with `btrfs inspect dump-tree`; there should be
FREE_SPACE_BITMAP items. If there are only FREE_SPACE_EXTENT items, then
it's not testing the right code path.

I have a script here [1] that I've been using to test the free space
tree. When I ran it with `--check` on MIPS, it failed on the old kernel
and passed with this series. If you stick a return after the call to
`unlink_every_other_file()`, you'll get a nice, fragmented filesystem to
feed to xfstests, as well.

1: 
https://github.com/osandov/osandov-linux/blob/master/scripts/fragment_free_space_tree.py

-- 
Omar
--
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/3] Btrfs: fix free space tree bitmaps+tests on big-endian systems

[Chris Mason]

On 07/17/2016 08:19 AM, Chandan Rajendra wrote:

On Friday, July 15, 2016 12:15:15 PM Omar Sandoval wrote:

On Fri, Jul 15, 2016 at 12:34:10PM +0530, Chandan Rajendra wrote:

On Thursday, July 14, 2016 07:47:04 PM Chris Mason wrote:

On 07/14/2016 07:31 PM, Omar Sandoval wrote:

From: Omar Sandoval 

So it turns out that the free space tree bitmap handling has always been
broken on big-endian systems. Totally my bad.

Patch 1 fixes this. Technically, it's a disk format change for
big-endian systems, but it never could have worked before, so I won't go
through the trouble of any incompat bits. If you've somehow been using
space_cache=v2 on a big-endian system (I doubt anyone is), you're going
to want to mount with nospace_cache to clear it and wait for this to go
in.

Patch 2 fixes a similar error in the sanity tests (it's the same as the
v2 I posted here [1]) and patch 3 expands the sanity tests to catch the
oversight that patch 1 fixes.

Applies to v4.7-rc7. No regressions in xfstests, and the sanity tests
pass on x86_64 and MIPS.


Thanks for fixing this up Omar.  Any big endian friends want to try this
out in extended testing and make sure we've nailed it down?



Hi Omar & Chris,

I will run fstests with this patchset applied on ppc64 BE and inform you about
the results.



Thanks, Chandan! I set up my xfstests for space_cache=v2 by doing:

mkfs.btrfs "$TEST_DEV"
mount -o space_cache=v2 "$TEST_DEV" "$TEST_DIR"
umount "$TEST_DEV"

and adding

export MOUNT_OPTIONS="-o space_cache=v2"

to local.config. btrfsck also needs the patch here [1].




Hi,

I did execute the fstests tests suite on ppc64 BE as per above configuration
and there were no new regressions. Also, I did execute fsx (via generic/127)
thrice on the same filesystem instance,
1. With the unpatched kernel and later
2. With the patched kernel and again
3. With the unpatched kernel
... there were no new regressions when executing the above steps.


Thanks Chandan!  But I'm a little confused.  If the patch is helping, we 
should be storing bitmaps wrong on disk unpatched.  There should be 
problems going back and forth.


-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: [PATCH 0/3] Btrfs: fix free space tree bitmaps+tests on big-endian systems

[Chandan Rajendra]

On Friday, July 15, 2016 12:15:15 PM Omar Sandoval wrote:
> On Fri, Jul 15, 2016 at 12:34:10PM +0530, Chandan Rajendra wrote:
> > On Thursday, July 14, 2016 07:47:04 PM Chris Mason wrote:
> > > On 07/14/2016 07:31 PM, Omar Sandoval wrote:
> > > > From: Omar Sandoval 
> > > >
> > > > So it turns out that the free space tree bitmap handling has always been
> > > > broken on big-endian systems. Totally my bad.
> > > >
> > > > Patch 1 fixes this. Technically, it's a disk format change for
> > > > big-endian systems, but it never could have worked before, so I won't go
> > > > through the trouble of any incompat bits. If you've somehow been using
> > > > space_cache=v2 on a big-endian system (I doubt anyone is), you're going
> > > > to want to mount with nospace_cache to clear it and wait for this to go
> > > > in.
> > > >
> > > > Patch 2 fixes a similar error in the sanity tests (it's the same as the
> > > > v2 I posted here [1]) and patch 3 expands the sanity tests to catch the
> > > > oversight that patch 1 fixes.
> > > >
> > > > Applies to v4.7-rc7. No regressions in xfstests, and the sanity tests
> > > > pass on x86_64 and MIPS.
> > > 
> > > Thanks for fixing this up Omar.  Any big endian friends want to try this 
> > > out in extended testing and make sure we've nailed it down?
> > >
> > 
> > Hi Omar & Chris,
> > 
> > I will run fstests with this patchset applied on ppc64 BE and inform you 
> > about
> > the results.
> > 
> 
> Thanks, Chandan! I set up my xfstests for space_cache=v2 by doing:
> 
> mkfs.btrfs "$TEST_DEV"
> mount -o space_cache=v2 "$TEST_DEV" "$TEST_DIR"
> umount "$TEST_DEV"
> 
> and adding
> 
> export MOUNT_OPTIONS="-o space_cache=v2"
> 
> to local.config. btrfsck also needs the patch here [1].
> 
> 

Hi,

I did execute the fstests tests suite on ppc64 BE as per above configuration
and there were no new regressions. Also, I did execute fsx (via generic/127)
thrice on the same filesystem instance,
1. With the unpatched kernel and later
2. With the patched kernel and again
3. With the unpatched kernel
... there were no new regressions when executing the above steps.

Tested-by: Chandan Rajendra 

-- 
chandan

--
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/3] Btrfs: fix free space tree bitmaps+tests on big-endian systems

[Omar Sandoval]

On Fri, Jul 15, 2016 at 12:34:10PM +0530, Chandan Rajendra wrote:
> On Thursday, July 14, 2016 07:47:04 PM Chris Mason wrote:
> > On 07/14/2016 07:31 PM, Omar Sandoval wrote:
> > > From: Omar Sandoval 
> > >
> > > So it turns out that the free space tree bitmap handling has always been
> > > broken on big-endian systems. Totally my bad.
> > >
> > > Patch 1 fixes this. Technically, it's a disk format change for
> > > big-endian systems, but it never could have worked before, so I won't go
> > > through the trouble of any incompat bits. If you've somehow been using
> > > space_cache=v2 on a big-endian system (I doubt anyone is), you're going
> > > to want to mount with nospace_cache to clear it and wait for this to go
> > > in.
> > >
> > > Patch 2 fixes a similar error in the sanity tests (it's the same as the
> > > v2 I posted here [1]) and patch 3 expands the sanity tests to catch the
> > > oversight that patch 1 fixes.
> > >
> > > Applies to v4.7-rc7. No regressions in xfstests, and the sanity tests
> > > pass on x86_64 and MIPS.
> > 
> > Thanks for fixing this up Omar.  Any big endian friends want to try this 
> > out in extended testing and make sure we've nailed it down?
> >
> 
> Hi Omar & Chris,
> 
> I will run fstests with this patchset applied on ppc64 BE and inform you about
> the results.
> 

Thanks, Chandan! I set up my xfstests for space_cache=v2 by doing:

mkfs.btrfs "$TEST_DEV"
mount -o space_cache=v2 "$TEST_DEV" "$TEST_DIR"
umount "$TEST_DEV"

and adding

export MOUNT_OPTIONS="-o space_cache=v2"

to local.config. btrfsck also needs the patch here [1].

Thanks again.

1: http://thread.gmane.org/gmane.comp.file-systems.btrfs/58382

-- 
Omar
--
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/3] Btrfs: fix free space tree bitmaps+tests on big-endian systems

[Chandan Rajendra]

On Thursday, July 14, 2016 07:47:04 PM Chris Mason wrote:
> On 07/14/2016 07:31 PM, Omar Sandoval wrote:
> > From: Omar Sandoval 
> >
> > So it turns out that the free space tree bitmap handling has always been
> > broken on big-endian systems. Totally my bad.
> >
> > Patch 1 fixes this. Technically, it's a disk format change for
> > big-endian systems, but it never could have worked before, so I won't go
> > through the trouble of any incompat bits. If you've somehow been using
> > space_cache=v2 on a big-endian system (I doubt anyone is), you're going
> > to want to mount with nospace_cache to clear it and wait for this to go
> > in.
> >
> > Patch 2 fixes a similar error in the sanity tests (it's the same as the
> > v2 I posted here [1]) and patch 3 expands the sanity tests to catch the
> > oversight that patch 1 fixes.
> >
> > Applies to v4.7-rc7. No regressions in xfstests, and the sanity tests
> > pass on x86_64 and MIPS.
> 
> Thanks for fixing this up Omar.  Any big endian friends want to try this 
> out in extended testing and make sure we've nailed it down?
>

Hi Omar & Chris,

I will run fstests with this patchset applied on ppc64 BE and inform you about
the results.

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

-- 
chandan

--
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/3] Btrfs: fix free space tree bitmaps+tests on big-endian systems

[Chris Mason]

On 07/14/2016 07:31 PM, Omar Sandoval wrote:

From: Omar Sandoval 

So it turns out that the free space tree bitmap handling has always been
broken on big-endian systems. Totally my bad.

Patch 1 fixes this. Technically, it's a disk format change for
big-endian systems, but it never could have worked before, so I won't go
through the trouble of any incompat bits. If you've somehow been using
space_cache=v2 on a big-endian system (I doubt anyone is), you're going
to want to mount with nospace_cache to clear it and wait for this to go
in.

Patch 2 fixes a similar error in the sanity tests (it's the same as the
v2 I posted here [1]) and patch 3 expands the sanity tests to catch the
oversight that patch 1 fixes.

Applies to v4.7-rc7. No regressions in xfstests, and the sanity tests
pass on x86_64 and MIPS.


Thanks for fixing this up Omar.  Any big endian friends want to try this 
out in extended testing and make sure we've nailed it down?


-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