btrfs balance 4.0 regression?

btrfs balance 4.0 regression?

[Josh Boyer]

Hi Omar and Chris,

We have a bug reported [1] against 4.0 saying that btrfs balance is
broken.  The reporter found a revert patch that Omar sent [2] to
revert commit 2f0810880.  Looking in Linus' latest tree, I don't see
that revert and I don't immediately see a patch to fix the issue Omar
reported either.

Do either of you know if this is still an issue?  If not, which commit
was it fixed by?

josh

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1217191
[2] https://patchwork.kernel.org/patch/6238111/

Re: btrfs balance 4.0 regression?

[Duncan]

Josh Boyer posted on Thu, 14 May 2015 08:43:25 -0400 as excerpted:

> Hi Omar and Chris,
> 
> We have a bug reported [1] against 4.0 saying that btrfs balance is
> broken.  The reporter found a revert patch that Omar sent [2] to revert
> commit 2f0810880.  Looking in Linus' latest tree, I don't see that
> revert and I don't immediately see a patch to fix the issue Omar
> reported either.
> 
> Do either of you know if this is still an issue?  If not, which commit
> was it fixed by?
> 
> josh
> 
> [1] https://bugzilla.redhat.com/show_bug.cgi?id=1217191
> [2] https://patchwork.kernel.org/patch/6238111/

Still an issue, officially as of dev comments a day or two ago, at least.

>From various comments including from Chris Mason directly, the devs are 
aware of it, but (from a non-dev list-regular perspective) there's a 
seeming reluctance to simply apply the revert patch.  Not being a dev I 
can't explain why tho I can speculate that the patch is logically correct 
and simply triggers this other bug.  But further patches have yet to 
appear.

Part of the problem may be a bit of confusion as some of the devs 
evidently thought the revert patch fixed the problem and hadn't been 
worrying about it until others pointed out the revert hadn't been applied 
and the problem thus remained.

So as of now, the choice appears to be broken balance-convert with the 
current code, or broken ext*-convert with that patch reverted.  Both 
cases aren't entirely common, so I guess it's up to you which you want to 
break ATM.  For the list, however, current status is revert not applied, 
and balance-convert known to be broken.

Non-convert balance should however continue to work just fine.

Beyond that... well, maybe we'll get a bit more explanation or a new 
patch off this thread. =:^)

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

Re: btrfs balance 4.0 regression?

[Omar Sandoval]

On Fri, May 15, 2015 at 12:15:06AM +0000, Duncan wrote:
> Josh Boyer posted on Thu, 14 May 2015 08:43:25 -0400 as excerpted:
> 
> > Hi Omar and Chris,
> > 
> > We have a bug reported [1] against 4.0 saying that btrfs balance is
> > broken.  The reporter found a revert patch that Omar sent [2] to revert
> > commit 2f0810880.  Looking in Linus' latest tree, I don't see that
> > revert and I don't immediately see a patch to fix the issue Omar
> > reported either.
> > 
> > Do either of you know if this is still an issue?  If not, which commit
> > was it fixed by?
> > 
> > josh
> > 
> > [1] https://bugzilla.redhat.com/show_bug.cgi?id=1217191
> > [2] https://patchwork.kernel.org/patch/6238111/
> 
> Still an issue, officially as of dev comments a day or two ago, at least.

Yup, Chris says he has a proper fix but it hasn't hit the list yet.

> From various comments including from Chris Mason directly, the devs are 
> aware of it, but (from a non-dev list-regular perspective) there's a 
> seeming reluctance to simply apply the revert patch.  Not being a dev I 
> can't explain why tho I can speculate that the patch is logically correct 
> and simply triggers this other bug.  But further patches have yet to 
> appear.
> 
> Part of the problem may be a bit of confusion as some of the devs 
> evidently thought the revert patch fixed the problem and hadn't been 
> worrying about it until others pointed out the revert hadn't been applied 
> and the problem thus remained.
> 
> So as of now, the choice appears to be broken balance-convert with the 
> current code, or broken ext*-convert with that patch reverted.  Both 
> cases aren't entirely common, so I guess it's up to you which you want to 
> break ATM.

Actually, ext4 convert is broken anyways (with irrelevant output
elided):

# mkfs.ext4 -F /dev/vdb
# btrfs-convert /dev/vdb
# mount /dev/vdb /mnt
# btrfs fi df /mnt
Data, single: total=2.64GiB, used=163.70MiB		<- single
System, single: total=32.00MiB, used=16.00KiB		<- single
Metadata, single: total=1.33GiB, used=37.13MiB		<- single
GlobalReserve, single: total=16.00MiB, used=0.00B	<- single
# btrfs device add -f /dev/vdc /mnt
# btrfs balance start -dconvert=raid1 -mconvert=raid1 /mnt
Done, had to relocate 9 out of 9 chunks
# btrfs fi df /mnt
Data, single: total=832.00MiB, used=200.55MiB		<- still single
System, single: total=32.00MiB, used=16.00KiB		<- still single
Metadata, single: total=256.00MiB, used=368.00KiB	<- still single
GlobalReserve, single: total=16.00MiB, used=0.00B	<- still single

So the balance succeeds unlike before the commit that caused the
regression, but the profile is still single, which defeats the purpose.

-- 
Omar

Re: btrfs balance 4.0 regression?

[Chris Murphy]

On Thu, May 14, 2015 at 6:33 PM, Omar Sandoval <osandov@osandov.com> wrote:
>
>
> Yup, Chris says he has a proper fix but it hasn't hit the list yet.
>
>
> Actually, ext4 convert is broken anyways (with irrelevant output
> elided):


I'm curious how this bug ended up in mainline. Isn't there an XFS test
for both balance+convert and ext4 convert? If not, shouldn't there be?
It's not a data loss bug but Btrfs is in a transitional stretch where
functionality loss bugs are no longer minor. (I'd look but I'm lazy
and xfs tests doesn't appear to be indexed.)


-- 
Chris Murphy

Re: btrfs balance 4.0 regression?

[Omar Sandoval]

On Thu, May 14, 2015 at 05:33:29PM -0700, Omar Sandoval wrote:
> On Fri, May 15, 2015 at 12:15:06AM +0000, Duncan wrote:
> > Josh Boyer posted on Thu, 14 May 2015 08:43:25 -0400 as excerpted:
> > 
> > > Hi Omar and Chris,
> > > 
> > > We have a bug reported [1] against 4.0 saying that btrfs balance is
> > > broken.  The reporter found a revert patch that Omar sent [2] to revert
> > > commit 2f0810880.  Looking in Linus' latest tree, I don't see that
> > > revert and I don't immediately see a patch to fix the issue Omar
> > > reported either.
> > > 
> > > Do either of you know if this is still an issue?  If not, which commit
> > > was it fixed by?
> > > 
> > > josh
> > > 
> > > [1] https://bugzilla.redhat.com/show_bug.cgi?id=1217191
> > > [2] https://patchwork.kernel.org/patch/6238111/
> > 
> > Still an issue, officially as of dev comments a day or two ago, at least.
> 
> Yup, Chris says he has a proper fix but it hasn't hit the list yet.
> 
> > From various comments including from Chris Mason directly, the devs are 
> > aware of it, but (from a non-dev list-regular perspective) there's a 
> > seeming reluctance to simply apply the revert patch.  Not being a dev I 
> > can't explain why tho I can speculate that the patch is logically correct 
> > and simply triggers this other bug.  But further patches have yet to 
> > appear.
> > 
> > Part of the problem may be a bit of confusion as some of the devs 
> > evidently thought the revert patch fixed the problem and hadn't been 
> > worrying about it until others pointed out the revert hadn't been applied 
> > and the problem thus remained.
> > 
> > So as of now, the choice appears to be broken balance-convert with the 
> > current code, or broken ext*-convert with that patch reverted.  Both 
> > cases aren't entirely common, so I guess it's up to you which you want to 
> > break ATM.
> 
> Actually, ext4 convert is broken anyways (with irrelevant output
> elided):

(I realize that I was being a bit too alarmist here. Reposting a message
from another thread clarifying.)

"""
Just to clarify, reverting 2f0810880f082fa8ba66ab2c33b02e4ff9770a5e does
not break ext4 conversion. If you revert it, you can btrfs-convert, do a
btrfs balance to finalize the conversion, then do another btrfs balance
-dconvert=... -mconvert=... to convert the profile. I should have been
clearer in that other thread: conversion from ext4 to Btrfs works, its
just that the commit that caused the regression did not actually
accomplish what it set out to do: allow converting the data/metadata
profile of a freshly btrfs-converted ext4 filesystem.
"""


-- 
Omar