Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
MIME-Version: 1.0
References: <20230807012654.55951-1-zhangshida@kylinos.cn> <20230817173123.GD2247938@mit.edu>
In-Reply-To: <20230817173123.GD2247938@mit.edu>
From:   Stephen Zhang <starzhangzsd@gmail.com>
Date:   Sat, 19 Aug 2023 10:38:02 +0800
Message-ID: <CANubcdW1z4QqDsHuuTqGxa_P40uG2s6LRk=pCkk_zL0TmJULrg@mail.gmail.com>
Subject: Re: [PATCH] ext4: Modify the rec_len helpers to accommodate future cases
To:     "Theodore Ts'o" <tytso@mit.edu>
Cc:     adilger.kernel@dilger.ca, linux-ext4@vger.kernel.org,
        linux-kernel@vger.kernel.org, zhangshida@kylinos.cn,
        Andreas Dilger <adilger@dilger.ca>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Theodore Ts'o <tytso@mit.edu> =E4=BA=8E2023=E5=B9=B48=E6=9C=8818=E6=97=A5=
=E5=91=A8=E4=BA=94 01:31=E5=86=99=E9=81=93=EF=BC=9A
>
> On Mon, Aug 07, 2023 at 09:26:54AM +0800, zhangshida wrote:
> > From: Shida Zhang <zhangshida@kylinos.cn>
> >
> > Following Andreas' suggestion, it is time to adapt these helpers
> > to handle larger records during runtime, especially in preparation
> > for the eventual support of ext4 with a block size greater than
> > PAGE_SIZE.
>
> Is there a reason for landing this now?  We don't have support for
> block_size > PAGE_SIZE yet, and this patch doesn't come for free, at
> least not systems with page_size < 64k.  These inline functions are
> *very* hot and get used in a large number of places.  Have you looked
> to see what it might do to text size of the ext4 code?  And whether
> the expansion to the icache might actually impact performance on CPU
> bound workloads with very large directories?
>
> I will note that there are some opportunities to optimize how often we
> use ext4_rec_len_from_disk.  For example, it gets called from
> ext4_check_dir_entry(), and often the callers of that function will
> need the directory record length.  So having ext4_check_dir_entry()
> optionally fill in the rec_len via a passed-in pointer might be
> worthwhile.

Yep, the best way to do it is to leave it unmerged until it is necessary.
At the same time, I will try to eliminate these regression concerns based
on these suggestions.

Cheers,
Shida

>
> Cheers,
>
>                                                 - Ted