[Buildroot] [PATCH] jemalloc: allow on MIPS64

[Buildroot] [PATCH] jemalloc: allow on MIPS64

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

jemalloc uses architecture #ifdefs to determine LG_QUANTUM and gives an
error when an unsupported architecture is used.
For this reason, Buildroot commit 3baf996c6a2b57ffaaa4627c1e04ff67c30e9754
introduced BR2_PACKAGE_JEMALLOC_ARCH_SUPPORTS.

In the jemalloc sources, 'mips' is checked via '__mips__' which is set both
for 32-bit as 64-bit MIPS (including MIPS64 n32).
However, the Buildroot arch selection only includes 32-bit MIPS via BR2_mips
and BR2_mipsel.

Update the arch selection to support MIPS64.

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 package/jemalloc/Config.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/jemalloc/Config.in b/package/jemalloc/Config.in
index c3fd8e34b8..edb582d203 100644
--- a/package/jemalloc/Config.in
+++ b/package/jemalloc/Config.in
@@ -3,7 +3,7 @@ config BR2_PACKAGE_JEMALLOC_ARCH_SUPPORTS
 	default y if BR2_arm || BR2_armeb
 	default y if BR2_aarch64 || BR2_aarch64_be
 	default y if BR2_i386 || BR2_x86_64
-	default y if BR2_mips || BR2_mipsel
+	default y if BR2_mips || BR2_mipsel || BR2_mips64 || BR2_mips64el
 	default y if BR2_sparc64
 	default y if BR2_powerpc
 	default y if BR2_sh4 || BR2sh4eb || BR2_sh4a || BR2_sh4aeb
-- 
2.19.2

[Buildroot] [PATCH] jemalloc: allow on MIPS64

[Arnout Vandecappelle]

On 25/01/2019 21:50, Thomas De Schampheleire wrote:
> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> 
> jemalloc uses architecture #ifdefs to determine LG_QUANTUM and gives an
> error when an unsupported architecture is used.
> For this reason, Buildroot commit 3baf996c6a2b57ffaaa4627c1e04ff67c30e9754
> introduced BR2_PACKAGE_JEMALLOC_ARCH_SUPPORTS.
> 
> In the jemalloc sources, 'mips' is checked via '__mips__' which is set both
> for 32-bit as 64-bit MIPS (including MIPS64 n32).
> However, the Buildroot arch selection only includes 32-bit MIPS via BR2_mips
> and BR2_mipsel.
> 
> Update the arch selection to support MIPS64.

 OK, it builds like that, but is it really going to work correctly? Most arches
define it as 4, only 32-bit ARM, or1k (which is always 32-bit) and mips define
it as 3. So is 3 really the correct value for mips64?

 Note that a value can also be passed with --with-lg-quantum config option for
unsupported arches.

 Regards,
 Arnout


> 
> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> ---
>  package/jemalloc/Config.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/package/jemalloc/Config.in b/package/jemalloc/Config.in
> index c3fd8e34b8..edb582d203 100644
> --- a/package/jemalloc/Config.in
> +++ b/package/jemalloc/Config.in
> @@ -3,7 +3,7 @@ config BR2_PACKAGE_JEMALLOC_ARCH_SUPPORTS
>  	default y if BR2_arm || BR2_armeb
>  	default y if BR2_aarch64 || BR2_aarch64_be
>  	default y if BR2_i386 || BR2_x86_64
> -	default y if BR2_mips || BR2_mipsel
> +	default y if BR2_mips || BR2_mipsel || BR2_mips64 || BR2_mips64el
>  	default y if BR2_sparc64
>  	default y if BR2_powerpc
>  	default y if BR2_sh4 || BR2sh4eb || BR2_sh4a || BR2_sh4aeb
> 

[Buildroot] [PATCH] jemalloc: allow on MIPS64

[Thomas De Schampheleire]

On Sat, Jan 26, 2019, 00:57 Arnout Vandecappelle <arnout@mind.be wrote:

>
>
> On 25/01/2019 21:50, Thomas De Schampheleire wrote:
> > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> >
> > jemalloc uses architecture #ifdefs to determine LG_QUANTUM and gives an
> > error when an unsupported architecture is used.
> > For this reason, Buildroot commit
> 3baf996c6a2b57ffaaa4627c1e04ff67c30e9754
> > introduced BR2_PACKAGE_JEMALLOC_ARCH_SUPPORTS.
> >
> > In the jemalloc sources, 'mips' is checked via '__mips__' which is set
> both
> > for 32-bit as 64-bit MIPS (including MIPS64 n32).
> > However, the Buildroot arch selection only includes 32-bit MIPS via
> BR2_mips
> > and BR2_mipsel.
> >
> > Update the arch selection to support MIPS64.
>
>  OK, it builds like that, but is it really going to work correctly? Most
> arches
> define it as 4, only 32-bit ARM, or1k (which is always 32-bit) and mips
> define
> it as 3. So is 3 really the correct value for mips64?
>

Well we are using it under MIPS64/n32 so the same value as MIPS32 is fine.
I haven't actually tested on n64.

I don't know how this value is used, and whether a bad value is just
suboptimal or actually broken.

The fact that there is no general difference between eg i386 and x86_64,
indicates there is no automatic difference between 32bit and 64bit.

If you prefer I can change the patch to check for n32 specifically.


>  Note that a value can also be passed with --with-lg-quantum config option
> for
> unsupported arches.
>

Yes, but we'd need to create a config symbol just for that. If we can solve
it without, I think it would be better.

Best regards
Thomas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20190126/7d73db3d/attachment.html>

[Buildroot] [PATCH] jemalloc: allow on MIPS64

[Arnout Vandecappelle]

 Hi Thomas,

On 26/01/2019 08:16, Thomas De Schampheleire wrote:
> 
> On Sat, Jan 26, 2019, 00:57 Arnout Vandecappelle <arnout@mind.be
> <mailto:arnout@mind.be> wrote:
> 
> 
> 
>     On 25/01/2019 21:50, Thomas De Schampheleire wrote:
>     > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com
>     <mailto:thomas.de_schampheleire@nokia.com>>
>     >
>     > jemalloc uses architecture #ifdefs to determine LG_QUANTUM and gives an
>     > error when an unsupported architecture is used.
>     > For this reason, Buildroot commit 3baf996c6a2b57ffaaa4627c1e04ff67c30e9754
>     > introduced BR2_PACKAGE_JEMALLOC_ARCH_SUPPORTS.
>     >
>     > In the jemalloc sources, 'mips' is checked via '__mips__' which is set both
>     > for 32-bit as 64-bit MIPS (including MIPS64 n32).
>     > However, the Buildroot arch selection only includes 32-bit MIPS via BR2_mips
>     > and BR2_mipsel.
>     >
>     > Update the arch selection to support MIPS64.
> 
>     ?OK, it builds like that, but is it really going to work correctly? Most arches
>     define it as 4, only 32-bit ARM, or1k (which is always 32-bit) and mips define
>     it as 3. So is 3 really the correct value for mips64?
> 
> 
> Well we are using it under MIPS64/n32 so the same value as MIPS32 is fine.
> I haven't actually tested on n64.
> 
> I don't know how this value is used, and whether a bad value is just suboptimal
> or actually broken.

 I did a little more research, and it looks like a bad value is just suboptimal.

 I found for example that Redis forces LG_QUANTUM to 3, because that matches
better with the sizes they use.

 If I understand correctly, an LG_QUANTUM of 3 should always give a smaller
memory footprint (probably at the cost of some performance, I guess).


> The fact that there is no general difference between eg i386 and x86_64,
> indicates there is no automatic difference between 32bit and 64bit.
> 
> If you prefer I can change the patch to check for n32 specifically.

 No that would just over-complicate things.


>     ?Note that a value can also be passed with --with-lg-quantum config option for
>     unsupported arches.
> 
> 
> Yes, but we'd need to create a config symbol just for that. If we can solve it
> without, I think it would be better.

 What I meant is that we could remove the arch dependency entirely and instead
pass --with-lg-quantum=3 for any arch which is not one of the supported ones (or
indeed for any arch at all).

 That said, this simple patch fixes it for you. If anyone needs to enable
jemalloc on one of the more exotic architectures, they can do it and do a
runtime test with it. So I've applied to master, thanks.

 Regards,
 Arnout