jemalloc: allow on MIPS64

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(-)

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
>

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
<div dir="auto"><div><br><div class="gmail_quote"><div dir="ltr">On Sat, Jan 26, 2019, 00:57 Arnout Vandecappelle &lt;<a href="mailto:arnout@mind.be">arnout@mind.be</a> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
<br>
On 25/01/2019 21:50, Thomas De Schampheleire wrote:<br>
&gt; From: Thomas De Schampheleire &lt;<a href="mailto:thomas.de_schampheleire@nokia.com" target="_blank" rel="noreferrer">thomas.de_schampheleire@nokia.com</a>&gt;<br>
&gt; <br>
&gt; jemalloc uses architecture #ifdefs to determine LG_QUANTUM and gives an<br>
&gt; error when an unsupported architecture is used.<br>
&gt; For this reason, Buildroot commit 3baf996c6a2b57ffaaa4627c1e04ff67c30e9754<br>
&gt; introduced BR2_PACKAGE_JEMALLOC_ARCH_SUPPORTS.<br>
&gt; <br>
&gt; In the jemalloc sources, &#39;mips&#39; is checked via &#39;__mips__&#39; which is set both<br>
&gt; for 32-bit as 64-bit MIPS (including MIPS64 n32).<br>
&gt; However, the Buildroot arch selection only includes 32-bit MIPS via BR2_mips<br>
&gt; and BR2_mipsel.<br>
&gt; <br>
&gt; Update the arch selection to support MIPS64.<br>
<br>
 OK, it builds like that, but is it really going to work correctly? Most arches<br>
define it as 4, only 32-bit ARM, or1k (which is always 32-bit) and mips define<br>
it as 3. So is 3 really the correct value for mips64?<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">Well we are using it under MIPS64/n32 so the same value as MIPS32 is fine.</div><div dir="auto">I haven&#39;t actually tested on n64.</div><div dir="auto"><br></div><div dir="auto">I don&#39;t know how this value is used, and whether a bad value is just suboptimal or actually broken.</div><div dir="auto"><br></div><div dir="auto">The fact that there is no general difference between eg i386 and x86_64, indicates there is no automatic difference between 32bit and 64bit.</div><div dir="auto"><br></div><div dir="auto">If you prefer I can change the patch to check for n32 specifically.</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
 Note that a value can also be passed with --with-lg-quantum config option for<br>
unsupported arches.<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">Yes, but we&#39;d need to create a config symbol just for that. If we can solve it without, I think it would be better.</div><div dir="auto"><br></div><div dir="auto">Best regards</div><div dir="auto">Thomas </div></div>

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