All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Sterba <dsterba@suse.cz>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	David Sterba <dsterba@suse.com>,
	linux-btrfs@vger.kernel.org,
	Nathan Chancellor <nathan@kernel.org>,
	Nick Desaulniers <nick.desaulniers+lkml@gmail.com>,
	linux-kbuild@vger.kernel.org
Subject: Re: [PATCH 2/2] btrfs: send: make use of -fms-extensions for defining struct fs_path
Date: Wed, 22 Oct 2025 07:24:43 +0200	[thread overview]
Message-ID: <20251022052443.GP13776@suse.cz> (raw)
In-Reply-To: <CAHk-=wgHLkpQAEDpA9pwXp_oteWkdcs-56m7rnQD=Th0N2sW9g@mail.gmail.com>

On Mon, Oct 20, 2025 at 09:48:25AM -1000, Linus Torvalds wrote:
> On Mon, 20 Oct 2025 at 04:22, Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:
> >
> > +struct __fs_path {
> > +       char *start;
> > +       char *end;
> > +
> > +       char *buf;
> > +       unsigned short buf_len:15;
> > +       unsigned short reversed:1;
> > +};
> > +static_assert(sizeof(struct __fs_path) < 256);
> >  struct fs_path {
> > +       struct __fs_path;
> > +       /*
> > +        * Average path length does not exceed 200 bytes, we'll have
> > +        * better packing in the slab and higher chance to satisfy
> > +        * an allocation later during send.
> > +        */
> > +       char inline_buf[256 - sizeof(struct __fs_path)];
> >  };
> 
> It strikes me that this won't pack as well as it used to before the change.
> 
> On 64-bit architectrures, 'struct __fs_path' will be 8-byte aligned
> due to the pointers in it, and that means that the size of it will
> also be aligned: it will be 32 bytes in size.
> 
> So you'll get 256-32 bytes of inline_buf.
> 
> And it *used* to be that 'inline_buf[]' was packed righ after the
> 16-bit buf_len / reversed bits, so it used to get an extra six bytes.
> 
> I think it could be fixed with a "__packed" thing on that inner
> struct, but that also worries me a bit because we'd certainly never
> want the compiler to generate the code for unaligned accesses (on the
> broken architectures that would do that). You'd then have to mark the
> containing structure as being aligned to make compilers generate good
> code.
> 
> So either you lose some inline buffer space, or you end up having to
> add extra packing stuff. Either way is a bit of a bother.

For the inline path buffer losing 6 bytes is acceptable, I did some
stats on my system and full paths under /.snapshots from snapper are all
uner 200 bytes, lengths on another random data partition goes up to 380.
Users can of course have path of any length but there's a fallback and
the allocations are done in the steps of slab bucket sizes so it's
reasonably effective.

  reply	other threads:[~2025-10-22  5:24 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-20 14:22 [PATCH 0/2] Kbuild: enable -fms-extensions, make btrfs the first user Rasmus Villemoes
2025-10-20 14:22 ` [PATCH 1/2] Kbuild: enable -fms-extensions Rasmus Villemoes
2025-10-22 16:15   ` Nathan Chancellor
2025-10-22 20:35     ` Rasmus Villemoes
2025-10-22 21:11       ` Nathan Chancellor
2025-10-23 12:40         ` Nathan Chancellor
2025-10-23 14:17           ` Dave Kleikamp
2025-10-23 16:45             ` Nathan Chancellor
2025-10-20 14:22 ` [PATCH 2/2] btrfs: send: make use of -fms-extensions for defining struct fs_path Rasmus Villemoes
2025-10-20 19:48   ` Linus Torvalds
2025-10-22  5:24     ` David Sterba [this message]
2025-10-22  5:30 ` [PATCH 0/2] Kbuild: enable -fms-extensions, make btrfs the first user David Sterba
2025-10-22 16:17   ` Nathan Chancellor

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251022052443.GP13776@suse.cz \
    --to=dsterba@suse.cz \
    --cc=dsterba@suse.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=nathan@kernel.org \
    --cc=nick.desaulniers+lkml@gmail.com \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.