All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Sterba <dsterba@suse.cz>
To: Daniel Vacek <neelx@suse.com>
Cc: Chris Mason <clm@fb.com>, Josef Bacik <josef@toxicpanda.com>,
	David Sterba <dsterba@suse.com>,
	linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] btrfs: remove extent buffer's redundant `len` member field
Date: Wed, 30 Apr 2025 15:30:26 +0200	[thread overview]
Message-ID: <20250430133026.GH9140@suse.cz> (raw)
In-Reply-To: <CAPjX3FfBoU9-wYP-JC63K6y8Pzocu0z8cKvXEbjD_NjdxWO+Og@mail.gmail.com>

On Wed, Apr 30, 2025 at 10:21:18AM +0200, Daniel Vacek wrote:
> > The benefit of duplicating the length in each eb is that it's in the
> > same cacheline as the other members that are used for offset
> > calculations or bit manipulations.
> >
> > Going to the fs_info->nodesize may or may not hit a cache, also because
> > it needs to do 2 pointer dereferences, so from that perspective I think
> > it's making it worse.
> 
> I was considering that. Since fs_info is shared for all ebs and other
> stuff like transactions, etc. I think the cache is hot most of the
> time and there will be hardly any performance difference observable.
> Though without benchmarks this is just a speculation (on both sides).

The comparison is between "always access 1 cacheline" and "hope that the
other cacheline is hot", yeah we don't have benchmarks for that but the
first access pattern is not conditional.

> > I don't think we need to do the optimization right now, but maybe in the
> > future if there's a need to add something to eb. Still we can use the
> > remaining 16 bytes up to 256 without making things worse.
> 
> This really depends on configuration. On my laptop (Debian -rt kernel)
> the eb struct is actually 272 bytes as the rt_mutex is significantly
> heavier than raw spin lock. And -rt is a first class citizen nowadays,
> often used in Kubernetes deployments like 5G RAN telco, dpdk and such.
> I think it would be nice to slim the struct below 256 bytes even there
> if that's your aim.

I configured and built RT kernel to see if it's possible to go to 256
bytes on RT and it seems yes with a big sacrifice of removing several
struct members that cache values like folio_size or folio_shift and
generating worse code.

As 272 is a multiple of 16 it's a reasonable size and we don't need to
optimize further. The number of ebs in one slab is 30, with the non-rt
build it's 34, which sounds OK.

  parent reply	other threads:[~2025-04-30 13:30 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-29 15:17 [PATCH] btrfs: remove extent buffer's redundant `len` member field Daniel Vacek
2025-04-29 22:34 ` Qu Wenruo
2025-04-30  8:03 ` David Sterba
2025-04-30  8:21   ` Daniel Vacek
2025-04-30 12:31     ` Daniel Vacek
2025-05-02 10:30       ` David Sterba
2025-05-02 11:23         ` Daniel Vacek
2025-04-30 13:30     ` David Sterba [this message]
2025-04-30 14:13       ` Daniel Vacek
2025-05-02 10:56         ` David Sterba
2025-05-02 12:03           ` Daniel Vacek
2025-05-05 14:10             ` David Sterba
2025-05-05 16:19               ` Daniel Vacek
2025-04-30  8:05 ` Filipe Manana
2025-04-30  8:26   ` Daniel Vacek
2025-04-30  8:34     ` Filipe Manana
2025-04-30  8:50       ` Daniel Vacek
2025-04-30 10:26         ` Filipe Manana
2025-04-30 11:09           ` Johannes Thumshirn
2025-04-30 12:09             ` Daniel Vacek
2025-04-30 12:06           ` Daniel Vacek
2025-04-30 12:33             ` Filipe Manana
2025-04-30 12:53               ` Daniel Vacek
2025-05-02 13:37 ` [PATCH v2 0/2] btrfs: eb struct cleanups Daniel Vacek
2025-05-02 13:37   ` [PATCH v2 1/2] btrfs: remove extent buffer's redundant `len` member field Daniel Vacek
2025-05-02 17:35     ` Boris Burkov
2025-05-05  8:23       ` Daniel Vacek
2025-05-05 11:50     ` [PATCH v3 0/2] btrfs: eb struct cleanups Daniel Vacek
2025-05-05 11:50       ` [PATCH v3 1/2] btrfs: remove extent buffer's redundant `len` member field Daniel Vacek
2025-05-05 15:18         ` David Sterba
2025-05-05 17:53           ` Daniel Vacek
2025-05-13  0:32             ` David Sterba
2025-05-13 10:43               ` Daniel Vacek
2025-05-05 11:50       ` [PATCH v3 2/2] btrfs: rearrange the extent buffer structure members Daniel Vacek
2025-05-02 13:37   ` [PATCH v2 " Daniel Vacek

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=20250430133026.GH9140@suse.cz \
    --to=dsterba@suse.cz \
    --cc=clm@fb.com \
    --cc=dsterba@suse.com \
    --cc=josef@toxicpanda.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=neelx@suse.com \
    /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.