Re: [PATCH v9 1/3] riscv: mm: modify pte format for Svnapot

From: Conor Dooley <conor@kernel.org>
To: Andrew Jones <ajones@ventanamicro.com>,
	Qinglin Pan <panqinglin2020@iscas.ac.cn>
Cc: linux-riscv@lists.infradead.org, palmer@dabbelt.com, jeff@riscv.org
Subject: Re: [PATCH v9 1/3] riscv: mm: modify pte format for Svnapot
Date: Fri, 09 Dec 2022 15:33:25 +0100	[thread overview]
Message-ID: <CD44ACE9-43EC-4865-8604-1C7876D5EEAE@kernel.org> (raw)
In-Reply-To: <20221209074229.iexrvp5hkuq3qz2l@kamzik>

On 9 December 2022 08:42:29 GMT+01:00, Andrew Jones <ajones@ventanamicro.com> wrote:
>On Fri, Dec 09, 2022 at 01:34:42AM +0800, Qinglin Pan wrote:
>...
>> > > > +#ifdef CONFIG_RISCV_ISA_SVNAPOT
>> > > > +#define HUGE_MAX_HSTATE        (2 + (NAPOT_ORDER_MAX -
>> > > > NAPOT_CONT_ORDER_BASE))
>> > > > +#else
>> > > > +#define HUGE_MAX_HSTATE        2
>> > > > +#endif
>> > > 
>> > > This is coming from a place of *complete* ignorance:
>> > > If CONFIG_RISCV_ISA_SVNAPOT is enabled in the kernel but there is no
>> > > support for it on a platform is it okay to change the value of
>> > > HUGE_MAX_HSTATE? Apologies if I've missed something obvious.
>> > 
>> > :(
>> > You are right. If CONFIG_RISCV_ISA_SVNAPOT is enabled, HUGE_MAX_HSTATE
>> > will always be 3 whether has_svnapot() is true or false. I will try to
>> > fix this.
>> 
>> I am really expecting that HUGE_MAX_HSTATE can change according to the
>> platform situation when CONFIG_RISCV_ISA_SVNAPOT is enabled. But it
>> seems impossible to achive this with a minor patch. Because this value
>> is used as some static allocated arrays' length (for example, hstates in
>> mm/hugetlb.c:52) in arch-independent code :(
>> 
>> This immuture HUGE_MAX_HSTATE value will cause no functional error but
>> it really makes some objects used only partially when
>> CONFIG_RISCV_ISA_SVNAPOT is enabled but the platform has no svnapot
>> support :( I am not sure whether this patch can be accepted with this
>> feature? Or any other helpful idea to fix this problem?
>> 
>> Please let me know if I ignore any information about it.
>
>I agree that the only problem appears to a waste of memory, particularly

Caveat about ignorance still applies, how much of a waste of memory are we talking?

>if cgroups are enabled. I don't see any easy solution either. Maybe a
>warning in the Kconfig text would be sufficient?

I think that entirely depends on how wasteful it is.
If it's minor, sure?

Thanks,
Conor.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv