[GIT PULL] bcachefs fixes for 6.11-rc6

[GIT PULL] bcachefs fixes for 6.11-rc6

[Kent Overstreet]

Hi Linus, just a couple small ones.

the data corruption in the buffered write path is troubling; inode lock
should not have been able to cause that...

Cheers,
Kent

The following changes since commit 49aa7830396bce33b00fa7ee734c35de36521138:

  bcachefs: Fix rebalance_work accounting (2024-08-24 10:16:21 -0400)

are available in the Git repository at:

  git://evilpiepirate.org/bcachefs.git tags/bcachefs-2024-08-21

for you to fetch changes up to 3d3020c461936009dc58702e267ff67b0076cbf2:

  bcachefs: Mark more errors as autofix (2024-08-31 19:27:01 -0400)

----------------------------------------------------------------
bcachefs fixes for 6.11-rc6

- Fix a rare data corruption in the rebalance path, caught as a nonce
  inconsistency on encrypted filesystems
- Revert lockless buffered write path
- Mark more errors as autofix

----------------------------------------------------------------
Kent Overstreet (4):
      bcachefs: Fix failure to return error in data_update_index_update()
      bcachefs: Fix bch2_extents_match() false positive
      bcachefs: Revert lockless buffered IO path
      bcachefs: Mark more errors as autofix

 fs/bcachefs/data_update.c      |   1 +
 fs/bcachefs/errcode.h          |   1 -
 fs/bcachefs/extents.c          |  23 ++++++-
 fs/bcachefs/fs-io-buffered.c   | 149 +++++++++++------------------------------
 fs/bcachefs/sb-errors_format.h |  10 +--
 5 files changed, 68 insertions(+), 116 deletions(-)

Re: [GIT PULL] bcachefs fixes for 6.11-rc6

[Linus Torvalds]

On Sun, 1 Sept 2024 at 14:44, Kent Overstreet <kent.overstreet@linux.dev> wrote:
>
>   git://evilpiepirate.org/bcachefs.git tags/bcachefs-2024-08-21

Hmm. Your git host is often slow, but now it seems to be even worse than usual.

I blamed my phone hotspot at first, but everything else is fine.

evilpiepirate.org takes a few minutes, and then I get a "Connection
reset by peer" error.

Maybe kick that machine? Or even just use something like a github mirror?

               Linus

Re: [GIT PULL] bcachefs fixes for 6.11-rc6

[Kent Overstreet]

On Sun, Sep 01, 2024 at 03:13:39PM GMT, Linus Torvalds wrote:
> On Sun, 1 Sept 2024 at 14:44, Kent Overstreet <kent.overstreet@linux.dev> wrote:
> >
> >   git://evilpiepirate.org/bcachefs.git tags/bcachefs-2024-08-21
> 
> Hmm. Your git host is often slow, but now it seems to be even worse than usual.
> 
> I blamed my phone hotspot at first, but everything else is fine.
> 
> evilpiepirate.org takes a few minutes, and then I get a "Connection
> reset by peer" error.
> 
> Maybe kick that machine? Or even just use something like a github mirror?

odd, everything looks quiet on it, but it was acting funny earlier today

I do have a github mirror though: 
https://github.com/koverstreet/bcachefs/ tags/bcachefs-2024-08-21

Re: [GIT PULL] bcachefs fixes for 6.11-rc6

[Linus Torvalds]

On Sun, 1 Sept 2024 at 15:19, Kent Overstreet <kent.overstreet@linux.dev> wrote:
>
> odd, everything looks quiet on it, but it was acting funny earlier today
>
> I do have a github mirror though:
> https://github.com/koverstreet/bcachefs/ tags/bcachefs-2024-08-21

That works. Thanks,

                  Linus

Re: [GIT PULL] bcachefs fixes for 6.11-rc6

[pr-tracker-bot]

The pull request you sent on Sat, 31 Aug 2024 22:44:08 -0400:

> git://evilpiepirate.org/bcachefs.git tags/bcachefs-2024-08-21

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/a4c763129fbcc7da5d3134ea95f9577f25bc637d

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html

Re: [GIT PULL] bcachefs fixes for 6.11-rc6

[Geert Uytterhoeven]

Hi Kent,

Replying here, as there is (again) no patch email to reply to to report issues.

noreply@ellerman.id.au is reporting several build failures[1] in linux-next:

    fs/bcachefs/sb-members.c: In function ‘bch2_sb_member_alloc’:
    fs/bcachefs/sb-members.c:503:2: error: a label can only be part of
a statement and a declaration is not a statement
      503 |  unsigned nr_devices = max_t(unsigned, dev_idx + 1,
c->sb.nr_devices);
          |  ^~~~~~~~
    fs/bcachefs/sb-members.c:505:2: error: expected expression before ‘struct’
      505 |  struct bch_sb_field_members_v2 *mi =
bch2_sb_field_get(c->disk_sb.sb, members_v2);
          |  ^~~~~~

Apparently this fails with gcc-10 and older, but builds with gcc-11
and gcc-12.

The failure is due to commit 4e7795eda4459bf3 ("bcachefs:
bch2_sb_member_alloc()"), which is nowhere to be found on
lore.kernel.org.  Please stop committing private unreviewed patches
to linux-next, as several people have asked before.
Thank you!

[1] http://kisskb.ellerman.id.au/kisskb/branch/linux-next/head/6804f0edbe7747774e6ae60f20cec4ee3ad7c187/

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [GIT PULL] bcachefs fixes for 6.11-rc6

[Nathan Chancellor]

On Tue, Sep 03, 2024 at 03:53:56PM +0200, Geert Uytterhoeven wrote:
> Hi Kent,
> 
> Replying here, as there is (again) no patch email to reply to to report issues.
> 
> noreply@ellerman.id.au is reporting several build failures[1] in linux-next:
> 
>     fs/bcachefs/sb-members.c: In function ‘bch2_sb_member_alloc’:
>     fs/bcachefs/sb-members.c:503:2: error: a label can only be part of
> a statement and a declaration is not a statement
>       503 |  unsigned nr_devices = max_t(unsigned, dev_idx + 1,
> c->sb.nr_devices);
>           |  ^~~~~~~~
>     fs/bcachefs/sb-members.c:505:2: error: expected expression before ‘struct’
>       505 |  struct bch_sb_field_members_v2 *mi =
> bch2_sb_field_get(c->disk_sb.sb, members_v2);
>           |  ^~~~~~
> 
> Apparently this fails with gcc-10 and older, but builds with gcc-11
> and gcc-12.

Just noting this also happens with clang. Depending on the version, it
is either a hard error like this or a warning (that gets upgraded to an
error with CONFIG_WERROR).

Clang 15:

  fs/bcachefs/sb-members.c:503:2: error: expected expression
          unsigned nr_devices = max_t(unsigned, dev_idx + 1, c->sb.nr_devices);
          ^
  fs/bcachefs/sb-members.c:507:42: error: use of undeclared identifier 'nr_devices'
                                       le16_to_cpu(mi->member_bytes) * nr_devices, sizeof(u64));
                                                                       ^
  include/linux/math.h:37:22: note: expanded from macro 'DIV_ROUND_UP'
  #define DIV_ROUND_UP __KERNEL_DIV_ROUND_UP
                       ^
  fs/bcachefs/sb-members.c:513:30: error: use of undeclared identifier 'nr_devices'
          c->disk_sb.sb->nr_devices = nr_devices;
                                      ^

Clang 19:

  fs/bcachefs/sb-members.c:503:2: error: label followed by a declaration is a C23 extension [-Werror,-Wc23-extensions]
    503 |         unsigned nr_devices = max_t(unsigned, dev_idx + 1, c->sb.nr_devices);
        |         ^

Cheers,
Nathan

Re: [GIT PULL] bcachefs fixes for 6.11-rc6

[Kent Overstreet]

On Tue, Sep 03, 2024 at 03:53:56PM GMT, Geert Uytterhoeven wrote:
> Hi Kent,
> 
> Replying here, as there is (again) no patch email to reply to to report issues.
> 
> noreply@ellerman.id.au is reporting several build failures[1] in linux-next:
> 
>     fs/bcachefs/sb-members.c: In function ‘bch2_sb_member_alloc’:
>     fs/bcachefs/sb-members.c:503:2: error: a label can only be part of
> a statement and a declaration is not a statement
>       503 |  unsigned nr_devices = max_t(unsigned, dev_idx + 1,
> c->sb.nr_devices);
>           |  ^~~~~~~~
>     fs/bcachefs/sb-members.c:505:2: error: expected expression before ‘struct’
>       505 |  struct bch_sb_field_members_v2 *mi =
> bch2_sb_field_get(c->disk_sb.sb, members_v2);
>           |  ^~~~~~
> 
> Apparently this fails with gcc-10 and older, but builds with gcc-11
> and gcc-12.

Thanks for the report - it's fixed now (thanks, Hongbo)

> The failure is due to commit 4e7795eda4459bf3 ("bcachefs:
> bch2_sb_member_alloc()"), which is nowhere to be found on
> lore.kernel.org.  Please stop committing private unreviewed patches
> to linux-next, as several people have asked before.

They're still in git; I'd suggest just doing a git send-email and
tweaking the output if you want to start a review on a patch you find.

There's been some discussions in filesystem land about how/when we want
patches to hit the list - I'm not a huge fan of the patch bombs that
drown everything else out on the list, which is what it would be if I
did mail everything.

But if the email workflow is really what you want, and if it's going to
be generating useful review (list activity is growing...), I could be
convinced...

We're getting past the "just fix all the stupid shit" phase, and my
output is (I hope) trending toward something more stustainable, with a
stream of more _interesting_ patches to talk about, so - yeah, it's
starting to sound more reasonable, if that's what people want.

My priority is just going to be on fostering _useful_ technical
discussion. If the only reason you're wanting patches on the list is
because of trivial shit automated tests can and do catch - that's not a
win, to me. If I start posting patch series and we seem to be learning
from it, I'll stick with it.

Re: [GIT PULL] bcachefs fixes for 6.11-rc6

[Geert Uytterhoeven]

Hi Kent,

On Wed, Sep 4, 2024 at 8:55 PM Kent Overstreet
<kent.overstreet@linux.dev> wrote:
> On Tue, Sep 03, 2024 at 03:53:56PM GMT, Geert Uytterhoeven wrote:
> > Replying here, as there is (again) no patch email to reply to to report issues.
> >
> > noreply@ellerman.id.au is reporting several build failures[1] in linux-next:

[...]

> > The failure is due to commit 4e7795eda4459bf3 ("bcachefs:
> > bch2_sb_member_alloc()"), which is nowhere to be found on
> > lore.kernel.org.  Please stop committing private unreviewed patches
> > to linux-next, as several people have asked before.
>
> They're still in git; I'd suggest just doing a git send-email and
> tweaking the output if you want to start a review on a patch you find.
>
> There's been some discussions in filesystem land about how/when we want
> patches to hit the list - I'm not a huge fan of the patch bombs that
> drown everything else out on the list, which is what it would be if I
> did mail everything.
>
> But if the email workflow is really what you want, and if it's going to
> be generating useful review (list activity is growing...), I could be
> convinced...
>
> We're getting past the "just fix all the stupid shit" phase, and my
> output is (I hope) trending toward something more stustainable, with a
> stream of more _interesting_ patches to talk about, so - yeah, it's
> starting to sound more reasonable, if that's what people want.
>
> My priority is just going to be on fostering _useful_ technical
> discussion. If the only reason you're wanting patches on the list is
> because of trivial shit automated tests can and do catch - that's not a
> win, to me. If I start posting patch series and we seem to be learning
> from it, I'll stick with it.

Please follow the standard procedure. Posting patches is actually a
requirement for your branch being part of linux-next:

    You will need to ensure that the patches/commits in your tree/series have
    been:
         * submitted under GPL v2 (or later) and include the Contributor's
            Signed-off-by,
         * posted to the relevant mailing list,
         * reviewed by you (or another maintainer of your subsystem tree),
         * successfully unit tested, and
         * destined for the current or next Linux merge window.

     https://lore.kernel.org/all/20240807080423.45efb506@canb.auug.org.au/

noreply@ellerman.id.au told me you broke 32-bit builds (again):

    ERROR: modpost: "__udivdi3" [fs/bcachefs/bcachefs.ko] undefined!
    http://kisskb.ellerman.id.au/kisskb/buildresult/15230339/
    http://kisskb.ellerman.id.au/kisskb/buildresult/15230439/

I have bisected this to commit 7abab864a198fbb6 ("bcachefs: Progress
indicator for extents_to_backpointers"), which was not posted to
a public mailing list archived by lore (again), and does a plain
64-by-size_t division, which should use div64_ul() instead.

--- a/fs/bcachefs/backpointers.c
+++ b/fs/bcachefs/backpointers.c
@@ -810,7 +810,7 @@ static inline void progress_init(struct
progress_indicator_state *s,

                u64 v;
                bch2_accounting_mem_read(c,
disk_accounting_pos_to_bpos(&acc), &v, 1);
-               s->nodes_total += v / btree_sectors(c);
+               s->nodes_total += div64_ul(v, btree_sectors(c));

        }
 }

Thanks for complying!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds