diff mbox series

[RFC,V3,31/43] rv64ilp32_abi: maple_tree: Use BITS_PER_LONG instead of CONFIG_64BIT

Message ID 20250325121624.523258-32-guoren@kernel.org (mailing list archive)
State New
Headers show
Series rv64ilp32_abi: Build CONFIG_64BIT kernel-self with ILP32 ABI | expand

Commit Message

Guo Ren March 25, 2025, 12:16 p.m. UTC
From: "Guo Ren (Alibaba DAMO Academy)" <guoren@kernel.org>

The Maple tree algorithm uses ulong type for each element. The
number of slots is based on BITS_PER_LONG for RV64ILP32 ABI, so
use BITS_PER_LONG instead of CONFIG_64BIT.

Signed-off-by: Guo Ren (Alibaba DAMO Academy) <guoren@kernel.org>
---
 include/linux/maple_tree.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Liam R. Howlett March 25, 2025, 7:09 p.m. UTC | #1
* guoren@kernel.org <guoren@kernel.org> [250325 08:24]:
> From: "Guo Ren (Alibaba DAMO Academy)" <guoren@kernel.org>
> 
> The Maple tree algorithm uses ulong type for each element. The
> number of slots is based on BITS_PER_LONG for RV64ILP32 ABI, so
> use BITS_PER_LONG instead of CONFIG_64BIT.
> 
> Signed-off-by: Guo Ren (Alibaba DAMO Academy) <guoren@kernel.org>
> ---
>  include/linux/maple_tree.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/maple_tree.h b/include/linux/maple_tree.h
> index cbbcd18d4186..ff6265b6468b 100644
> --- a/include/linux/maple_tree.h
> +++ b/include/linux/maple_tree.h
> @@ -24,7 +24,7 @@
>   *
>   * Nodes in the tree point to their parent unless bit 0 is set.
>   */
> -#if defined(CONFIG_64BIT) || defined(BUILD_VDSO32_64)
> +#if (BITS_PER_LONG == 64) || defined(BUILD_VDSO32_64)

This will break my userspace testing, if you do not update the testing as
well.  This can be found in tools/testing/radix-tree.  Please also look
at the Makefile as well since it will generate a build flag for the
userspace.

This raises other concerns as the code is found with a grep command, so
I'm not sure why it was missed and if anything else is missed?

If you consider this email to be the (unasked) question about what to do
here, then please CC me, the maintainer of the files including the one
you are updating here.

Thank you,
Liam
Guo Ren March 27, 2025, 12:47 p.m. UTC | #2
On Wed, Mar 26, 2025 at 3:10 AM Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
>
> * guoren@kernel.org <guoren@kernel.org> [250325 08:24]:
> > From: "Guo Ren (Alibaba DAMO Academy)" <guoren@kernel.org>
> >
> > The Maple tree algorithm uses ulong type for each element. The
> > number of slots is based on BITS_PER_LONG for RV64ILP32 ABI, so
> > use BITS_PER_LONG instead of CONFIG_64BIT.
> >
> > Signed-off-by: Guo Ren (Alibaba DAMO Academy) <guoren@kernel.org>
> > ---
> >  include/linux/maple_tree.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/linux/maple_tree.h b/include/linux/maple_tree.h
> > index cbbcd18d4186..ff6265b6468b 100644
> > --- a/include/linux/maple_tree.h
> > +++ b/include/linux/maple_tree.h
> > @@ -24,7 +24,7 @@
> >   *
> >   * Nodes in the tree point to their parent unless bit 0 is set.
> >   */
> > -#if defined(CONFIG_64BIT) || defined(BUILD_VDSO32_64)
> > +#if (BITS_PER_LONG == 64) || defined(BUILD_VDSO32_64)
>
> This will break my userspace testing, if you do not update the testing as
> well.  This can be found in tools/testing/radix-tree.  Please also look
> at the Makefile as well since it will generate a build flag for the
> userspace.
I think you are talking about the following:
============================================================
../shared/shared.mk:
ifndef LONG_BIT
LONG_BIT := $(shell getconf LONG_BIT)
endif

generated/bit-length.h: FORCE
        @mkdir -p generated
        @if ! grep -qws CONFIG_$(LONG_BIT)BIT generated/bit-length.h; then   \
                echo "Generating $@";                                        \
                echo "#define CONFIG_$(LONG_BIT)BIT 1" > $@;                 \
                echo "#define CONFIG_PHYS_ADDR_T_$(LONG_BIT)BIT 1" >> $@;    \
        fi

$ grep CONFIG_64BIT * -r -A 2
generated/bit-length.h:#define CONFIG_64BIT 1
generated/bit-length.h-#define CONFIG_PHYS_ADDR_T_64BIT 1
--
maple.c:#if defined(CONFIG_64BIT)
maple.c-static noinline void __init check_erase2_testset(struct maple_tree *mt,
maple.c-                const unsigned long *set, unsigned long size)
--
maple.c:#if CONFIG_64BIT
maple.c-        MT_BUG_ON(mt, data_end != mas_data_end(&mas));
maple.c-#endif
--
maple.c:#if CONFIG_64BIT
maple.c-        MT_BUG_ON(mt, data_end - 2 != mas_data_end(&mas));
maple.c-#endif
--
maple.c:#if CONFIG_64BIT
maple.c-        MT_BUG_ON(mt, data_end - 4 != mas_data_end(&mas));
maple.c-#endif
--
maple.c:#if defined(CONFIG_64BIT)
maple.c-        /* Captures from VMs that found previous errors */
maple.c-        mt_init_flags(&tree, 0);
============================================================

First, we don't introduce rv64ilp32-abi user space, which means these
testing codes can't run on rv64ilp32-abi userspace currently. So, the
problem you mentioned doesn't exist.

Second, CONFIG_32BIT is determined by LONG_BIT, so there's no issue in
maple.c with future rv64ilp32-abi userspace.
That means rv64ilp32-abi userspace would use CONFIG_32BIT to test
radix-tree. It's okay.

>
> This raises other concerns as the code is found with a grep command, so
> I'm not sure why it was missed and if anything else is missed?
>
> If you consider this email to be the (unasked) question about what to do
> here, then please CC me, the maintainer of the files including the one
> you are updating here.
>
> Thank you,
> Liam
>
diff mbox series

Patch

diff --git a/include/linux/maple_tree.h b/include/linux/maple_tree.h
index cbbcd18d4186..ff6265b6468b 100644
--- a/include/linux/maple_tree.h
+++ b/include/linux/maple_tree.h
@@ -24,7 +24,7 @@ 
  *
  * Nodes in the tree point to their parent unless bit 0 is set.
  */
-#if defined(CONFIG_64BIT) || defined(BUILD_VDSO32_64)
+#if (BITS_PER_LONG == 64) || defined(BUILD_VDSO32_64)
 /* 64bit sizes */
 #define MAPLE_NODE_SLOTS	31	/* 256 bytes including ->parent */
 #define MAPLE_RANGE64_SLOTS	16	/* 256 bytes */