Re: [PATCH v5 0/9] Merge arm64/riscv hugetlbfs contpte support

[Alexandre Ghiti <alexghiti@rivosinc.com>]

Hi Will,

On Thu, May 8, 2025 at 2:30 PM Will Deacon <will@kernel.org> wrote:
>
> Hi folks,
>
> On Mon, May 05, 2025 at 06:08:50PM +0200, Alexandre Ghiti wrote:
> > On 29/04/2025 16:09, Ryan Roberts wrote:
> > > On 07/04/2025 13:04, Alexandre Ghiti wrote:
> > > > Can someone from arm64 review this? I think it's preferable to share the same
> > > > implementation between riscv and arm64.
> > > I've been thinking about this for a while and had some conversations internally.
> > > This patchset has both pros and cons.
> > >
> > > In the pros column, it increases code reuse in an area that has had quite of few
> > > bugs popping up lately; so this would bring more eyes and hopefully higher
> > > quality in the long run.
> > >
> > > But in the cons column, we have seen HW errata in similar areas in the past and
> > > I'm nervous that by hoisting this code to mm, we make it harder to workaround
> > > any future errata. Additionally I can imagine that this change could make it
> > > harder to support future Arm architecture enhancements.
> > >
> > > I appreciate the cons are not strong *technical* arguments but nevertheless they
> > > are winning out in this case; My opinion is that we should keep the arm64
> > > implementations of huge_pte_ (and contpte_ too - I know you have a separate
> > > series for this) private to arm64.
> > >
> > > Sorry about that.
> > >
> > > > The end goal is the support of mTHP using svnapot on riscv, which we want soon,
> > > > so if that patchset does not gain any traction, I'll just copy/paste the arm64
> > > > implementation into riscv.
> > > This copy/paste approach would be my preference.
> >
> >
> > I have to admit that I disagree with this approach, the riscv and arm64
> > implementations are *exactly* the same so it sounds weird to duplicate code,
> > the pros you mention outweigh the cons.
> >
> > Unless I'm missing something about the erratas? To me, that's easily fixed
> > by providing arch specific overrides no? Can you describe what sort of
> > erratas would not fit then?
>
> If we start with the common implementation you have here, nothing
> prevents us from forking the code in future if the architectures diverge
> so I'd be inclined to merge this series and see how we get on. However,
> one thing I *do* think we need to ensure is that the relevant folks from
> both arm64 (i.e. Ryan) and riscv (i.e. Alexandre) are cc'd on changes to
> the common code. Otherwise, it's going to be a step backwards in terms
> of maintainability.
>
> Could we add something to MAINTAINERS so that the new file picks you both
> up as reviewers?

I'm adding Lorenzo as he is cleaning the mm MAINTAINERS entries.

@Lorenzo: should we add a new section "CONTPTE" for this? FYI, hugetlb
is the first patchset, I have another patchset to merge THP contpte
support [1] as well so the "HUGETLB" section does not seem to be a
good fit.

[1] https://lore.kernel.org/linux-riscv/20240508191931.46060-1-alexghiti@rivosinc.com/

Thanks,

Alex

>
> Will