Received-SPF: pass (google.com: domain of linux-kernel+bounces-9659-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <20231221154702.2267684-1-guoren@kernel.org> <20231221154702.2267684-5-guoren@kernel.org>
 <ZYUZ8QUJxCL93Fgv@LeoBras>
In-Reply-To: <ZYUZ8QUJxCL93Fgv@LeoBras>
From: Guo Ren <guoren@kernel.org>
Date: Fri, 22 Dec 2023 19:25:16 +0800
Message-ID: <CAJF2gTTX7AjexoRUw=ZS34HEWno7H3zvnXA+Q7TYsztNU=k2cg@mail.gmail.com>
Subject: Re: [PATCH V2 4/4] riscv: mm: Optimize TASK_SIZE definition
To: Leonardo Bras <leobras@redhat.com>
Cc: linux-kernel@vger.kernel.org, paul.walmsley@sifive.com, palmer@dabbelt.com, 
	alexghiti@rivosinc.com, charlie@rivosinc.com, xiao.w.wang@intel.com, 
	david@redhat.com, panqinglin2020@iscas.ac.cn, rick.p.edgecombe@intel.com, 
	willy@infradead.org, bjorn@rivosinc.com, conor.dooley@microchip.com, 
	cleger@rivosinc.com, linux-riscv@lists.infradead.org, 
	Guo Ren <guoren@linux.alibaba.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, Dec 22, 2023 at 1:09=E2=80=AFPM Leonardo Bras <leobras@redhat.com> =
wrote:
>
> On Thu, Dec 21, 2023 at 10:47:01AM -0500, guoren@kernel.org wrote:
> > From: Guo Ren <guoren@linux.alibaba.com>
> >
> > Unify the TASK_SIZE definition with VA_BITS for better readability.
> > Add COMPAT mode user address space info in the comment.
> >
> > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > Signed-off-by: Guo Ren <guoren@kernel.org>
> > ---
> >  arch/riscv/include/asm/pgtable.h | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/=
pgtable.h
> > index e415582276ec..d165ddae3b42 100644
> > --- a/arch/riscv/include/asm/pgtable.h
> > +++ b/arch/riscv/include/asm/pgtable.h
> > @@ -866,6 +866,7 @@ static inline pte_t pte_swp_clear_exclusive(pte_t p=
te)
> >   * Note that PGDIR_SIZE must evenly divide TASK_SIZE.
> >   * Task size is:
> >   * -        0x9fc00000       (~2.5GB) for RV32.
> > + * -        0x80000000       (   2GB) for RV64 compat mode
> >   * -      0x4000000000       ( 256GB) for RV64 using SV39 mmu
> >   * -    0x800000000000       ( 128TB) for RV64 using SV48 mmu
> >   * - 0x100000000000000       (  64PB) for RV64 using SV57 mmu
> > @@ -877,11 +878,11 @@ static inline pte_t pte_swp_clear_exclusive(pte_t=
 pte)
> >   * Similarly for SV57, bits 63=E2=80=9357 must be equal to bit 56.
> >   */
> >  #ifdef CONFIG_64BIT
> > -#define TASK_SIZE_64 (PGDIR_SIZE * PTRS_PER_PGD / 2)
> > +#define TASK_SIZE_64 (UL(1) << (VA_BITS - 1))
>
> Checked for l5, l4 and l3, and it seems a correct replacement.
>
> >
> >  #ifdef CONFIG_COMPAT
> > -#define TASK_SIZE_32 (_AC(0x80000000, UL))
> > -#define TASK_SIZE    (test_thread_flag(TIF_32BIT) ? \
> > +#define TASK_SIZE_32 (UL(1) << (VA_BITS_SV32 - 1))
>
> Oh, much better. Thanks for removing the magic number :)
>
> > +#define TASK_SIZE    (is_compat_task() ? \
> >                        TASK_SIZE_32 : TASK_SIZE_64)
I would remove is_compat_task() in the next version because your patch
contains that.

> >  #else
> >  #define TASK_SIZE    TASK_SIZE_64
> > --
> > 2.40.1
> >
>
> That's much more readable IMO now. Thanks!
>
> FWIW:
> Reviewed-by: Leonardo Bras <leobras@redhat.com>
>


--=20
Best Regards
 Guo Ren