i386: Increase MALLOC_ALIGNMENT to 16 [BZ #21120]

Message ID 20170629173030.GA25414@intel.com
State Superseded
Headers

Commit Message

Lu, Hongjiu June 29, 2017, 5:30 p.m. UTC
  GCC 7 changed the definition of max_align_t on i386:

https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=9b5c49ef97e63cc63f1ffa13baf771368105ebe2

As a result, glibc malloc no longer returns memory blocks which are as
aligned as max_align_t requires.

This causes malloc/tst-malloc-thread-fail to fail with an error like this
one:

error: allocation function 0, size 144 not aligned to 16

This patch increases the malloc alignment to 16 for i386.

Tested on i386 with GCC 7 and on x86-64.  OK for master?

H.J.
---
	[BZ #21120]
	* sysdeps/generic/malloc-alignment.h: New file.
	* sysdeps/i386/malloc-alignment.h: Likewise.
	* sysdeps/generic/malloc-machine.h: Include <malloc-alignment.h>.
---
 sysdeps/generic/malloc-alignment.h | 24 ++++++++++++++++++++++++
 sysdeps/generic/malloc-machine.h   |  1 +
 sysdeps/i386/malloc-alignment.h    | 24 ++++++++++++++++++++++++
 3 files changed, 49 insertions(+)
 create mode 100644 sysdeps/generic/malloc-alignment.h
 create mode 100644 sysdeps/i386/malloc-alignment.h
  

Comments

Carlos O'Donell June 29, 2017, 5:55 p.m. UTC | #1
On 06/29/2017 01:30 PM, H.J. Lu wrote:
> GCC 7 changed the definition of max_align_t on i386:
> 
> https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=9b5c49ef97e63cc63f1ffa13baf771368105ebe2
> 
> As a result, glibc malloc no longer returns memory blocks which are as
> aligned as max_align_t requires.
> 
> This causes malloc/tst-malloc-thread-fail to fail with an error like this
> one:
> 
> error: allocation function 0, size 144 not aligned to 16
> 
> This patch increases the malloc alignment to 16 for i386.
> 
> Tested on i386 with GCC 7 and on x86-64.  OK for master?
> 
> H.J.
> ---
> 	[BZ #21120]
> 	* sysdeps/generic/malloc-alignment.h: New file.
> 	* sysdeps/i386/malloc-alignment.h: Likewise.
> 	* sysdeps/generic/malloc-machine.h: Include <malloc-alignment.h>.

Please use malloc-machine.h which was the previous header that provided
machine-dependent malloc definitions. That way we remain consistent across
releases and make it easier to backport such changes without adding a new
header.

OK with that change.
  
H.J. Lu June 29, 2017, 6:06 p.m. UTC | #2
On Thu, Jun 29, 2017 at 10:55 AM, Carlos O'Donell <carlos@redhat.com> wrote:
> On 06/29/2017 01:30 PM, H.J. Lu wrote:
>> GCC 7 changed the definition of max_align_t on i386:
>>
>> https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=9b5c49ef97e63cc63f1ffa13baf771368105ebe2
>>
>> As a result, glibc malloc no longer returns memory blocks which are as
>> aligned as max_align_t requires.
>>
>> This causes malloc/tst-malloc-thread-fail to fail with an error like this
>> one:
>>
>> error: allocation function 0, size 144 not aligned to 16
>>
>> This patch increases the malloc alignment to 16 for i386.
>>
>> Tested on i386 with GCC 7 and on x86-64.  OK for master?
>>
>> H.J.
>> ---
>>       [BZ #21120]
>>       * sysdeps/generic/malloc-alignment.h: New file.
>>       * sysdeps/i386/malloc-alignment.h: Likewise.
>>       * sysdeps/generic/malloc-machine.h: Include <malloc-alignment.h>.
>
> Please use malloc-machine.h which was the previous header that provided
> machine-dependent malloc definitions. That way we remain consistent across
> releases and make it easier to backport such changes without adding a new
> header.

It won't work too well for Hurd since we have

./sysdeps/generic/malloc-machine.h
./sysdeps/mach/hurd/malloc-machine.h
./sysdeps/nptl/malloc-machine.h

What will Hurd/i386 get?   malloc-alignment.h handles it automatically.

> OK with that change.
>
> --
> Cheers,
> Carlos.
  
Carlos O'Donell June 29, 2017, 6:11 p.m. UTC | #3
On 06/29/2017 02:06 PM, H.J. Lu wrote:
> On Thu, Jun 29, 2017 at 10:55 AM, Carlos O'Donell <carlos@redhat.com> wrote:
>> On 06/29/2017 01:30 PM, H.J. Lu wrote:
>>> GCC 7 changed the definition of max_align_t on i386:
>>>
>>> https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=9b5c49ef97e63cc63f1ffa13baf771368105ebe2
>>>
>>> As a result, glibc malloc no longer returns memory blocks which are as
>>> aligned as max_align_t requires.
>>>
>>> This causes malloc/tst-malloc-thread-fail to fail with an error like this
>>> one:
>>>
>>> error: allocation function 0, size 144 not aligned to 16
>>>
>>> This patch increases the malloc alignment to 16 for i386.
>>>
>>> Tested on i386 with GCC 7 and on x86-64.  OK for master?
>>>
>>> H.J.
>>> ---
>>>       [BZ #21120]
>>>       * sysdeps/generic/malloc-alignment.h: New file.
>>>       * sysdeps/i386/malloc-alignment.h: Likewise.
>>>       * sysdeps/generic/malloc-machine.h: Include <malloc-alignment.h>.
>>
>> Please use malloc-machine.h which was the previous header that provided
>> machine-dependent malloc definitions. That way we remain consistent across
>> releases and make it easier to backport such changes without adding a new
>> header.
> 
> It won't work too well for Hurd since we have
> 
> ./sysdeps/generic/malloc-machine.h
> ./sysdeps/mach/hurd/malloc-machine.h
> ./sysdeps/nptl/malloc-machine.h
> 
> What will Hurd/i386 get?   malloc-alignment.h handles it automatically.

If your patch made Hurd/i386 use MALLOC_ALIGNMENT of 16 then a new patch
using malloc-machine.h would set MALLOC_ALIGNMENT to 16 in
systeps/mach/hurd/malloc-machine.h?
  
H.J. Lu June 29, 2017, 6:43 p.m. UTC | #4
On Thu, Jun 29, 2017 at 11:11 AM, Carlos O'Donell <carlos@redhat.com> wrote:
> On 06/29/2017 02:06 PM, H.J. Lu wrote:
>> On Thu, Jun 29, 2017 at 10:55 AM, Carlos O'Donell <carlos@redhat.com> wrote:
>>> On 06/29/2017 01:30 PM, H.J. Lu wrote:
>>>> GCC 7 changed the definition of max_align_t on i386:
>>>>
>>>> https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=9b5c49ef97e63cc63f1ffa13baf771368105ebe2
>>>>
>>>> As a result, glibc malloc no longer returns memory blocks which are as
>>>> aligned as max_align_t requires.
>>>>
>>>> This causes malloc/tst-malloc-thread-fail to fail with an error like this
>>>> one:
>>>>
>>>> error: allocation function 0, size 144 not aligned to 16
>>>>
>>>> This patch increases the malloc alignment to 16 for i386.
>>>>
>>>> Tested on i386 with GCC 7 and on x86-64.  OK for master?
>>>>
>>>> H.J.
>>>> ---
>>>>       [BZ #21120]
>>>>       * sysdeps/generic/malloc-alignment.h: New file.
>>>>       * sysdeps/i386/malloc-alignment.h: Likewise.
>>>>       * sysdeps/generic/malloc-machine.h: Include <malloc-alignment.h>.
>>>
>>> Please use malloc-machine.h which was the previous header that provided
>>> machine-dependent malloc definitions. That way we remain consistent across
>>> releases and make it easier to backport such changes without adding a new
>>> header.
>>
>> It won't work too well for Hurd since we have
>>
>> ./sysdeps/generic/malloc-machine.h
>> ./sysdeps/mach/hurd/malloc-machine.h
>> ./sysdeps/nptl/malloc-machine.h
>>
>> What will Hurd/i386 get?   malloc-alignment.h handles it automatically.
>
> If your patch made Hurd/i386 use MALLOC_ALIGNMENT of 16 then a new patch
> using malloc-machine.h would set MALLOC_ALIGNMENT to 16 in
> systeps/mach/hurd/malloc-machine.h?
>

This assumes that mach/hurd == i386.  Also I don't like define
MALLOC_ALIGNMENT to 16 for i386 in 2 different places.
Since

./sysdeps/generic/malloc-machine.h
./sysdeps/mach/hurd/malloc-machine.h
./sysdeps/nptl/malloc-machine.h

malloc-machine.h is not pure processor specific. It is also OS
specific.  I prefer to define MALLOC_ALIGNMENT to 16 for i386
in a processor specific header file.
  
Carlos O'Donell June 29, 2017, 8:03 p.m. UTC | #5
On 06/29/2017 02:43 PM, H.J. Lu wrote:
> On Thu, Jun 29, 2017 at 11:11 AM, Carlos O'Donell <carlos@redhat.com> wrote:
>> On 06/29/2017 02:06 PM, H.J. Lu wrote:
>>> On Thu, Jun 29, 2017 at 10:55 AM, Carlos O'Donell <carlos@redhat.com> wrote:
>>>> On 06/29/2017 01:30 PM, H.J. Lu wrote:
>>>>> GCC 7 changed the definition of max_align_t on i386:
>>>>>
>>>>> https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=9b5c49ef97e63cc63f1ffa13baf771368105ebe2
>>>>>
>>>>> As a result, glibc malloc no longer returns memory blocks which are as
>>>>> aligned as max_align_t requires.
>>>>>
>>>>> This causes malloc/tst-malloc-thread-fail to fail with an error like this
>>>>> one:
>>>>>
>>>>> error: allocation function 0, size 144 not aligned to 16
>>>>>
>>>>> This patch increases the malloc alignment to 16 for i386.
>>>>>
>>>>> Tested on i386 with GCC 7 and on x86-64.  OK for master?
>>>>>
>>>>> H.J.
>>>>> ---
>>>>>       [BZ #21120]
>>>>>       * sysdeps/generic/malloc-alignment.h: New file.
>>>>>       * sysdeps/i386/malloc-alignment.h: Likewise.
>>>>>       * sysdeps/generic/malloc-machine.h: Include <malloc-alignment.h>.
>>>>
>>>> Please use malloc-machine.h which was the previous header that provided
>>>> machine-dependent malloc definitions. That way we remain consistent across
>>>> releases and make it easier to backport such changes without adding a new
>>>> header.
>>>
>>> It won't work too well for Hurd since we have
>>>
>>> ./sysdeps/generic/malloc-machine.h
>>> ./sysdeps/mach/hurd/malloc-machine.h
>>> ./sysdeps/nptl/malloc-machine.h
>>>
>>> What will Hurd/i386 get?   malloc-alignment.h handles it automatically.
>>
>> If your patch made Hurd/i386 use MALLOC_ALIGNMENT of 16 then a new patch
>> using malloc-machine.h would set MALLOC_ALIGNMENT to 16 in
>> systeps/mach/hurd/malloc-machine.h?
>>
> 
> This assumes that mach/hurd == i386.  Also I don't like define
> MALLOC_ALIGNMENT to 16 for i386 in 2 different places.
> Since
> 
> ./sysdeps/generic/malloc-machine.h
> ./sysdeps/mach/hurd/malloc-machine.h
> ./sysdeps/nptl/malloc-machine.h
> 
> malloc-machine.h is not pure processor specific. It is also OS
> specific.  I prefer to define MALLOC_ALIGNMENT to 16 for i386
> in a processor specific header file. 

Add a sysdeps/mach/hurd/i386, which is an os/machine directory.

Similarly for i386?
  
H.J. Lu June 29, 2017, 8:10 p.m. UTC | #6
On Thu, Jun 29, 2017 at 1:03 PM, Carlos O'Donell <carlos@redhat.com> wrote:
> On 06/29/2017 02:43 PM, H.J. Lu wrote:
>> On Thu, Jun 29, 2017 at 11:11 AM, Carlos O'Donell <carlos@redhat.com> wrote:
>>> On 06/29/2017 02:06 PM, H.J. Lu wrote:
>>>> On Thu, Jun 29, 2017 at 10:55 AM, Carlos O'Donell <carlos@redhat.com> wrote:
>>>>> On 06/29/2017 01:30 PM, H.J. Lu wrote:
>>>>>> GCC 7 changed the definition of max_align_t on i386:
>>>>>>
>>>>>> https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=9b5c49ef97e63cc63f1ffa13baf771368105ebe2
>>>>>>
>>>>>> As a result, glibc malloc no longer returns memory blocks which are as
>>>>>> aligned as max_align_t requires.
>>>>>>
>>>>>> This causes malloc/tst-malloc-thread-fail to fail with an error like this
>>>>>> one:
>>>>>>
>>>>>> error: allocation function 0, size 144 not aligned to 16
>>>>>>
>>>>>> This patch increases the malloc alignment to 16 for i386.
>>>>>>
>>>>>> Tested on i386 with GCC 7 and on x86-64.  OK for master?
>>>>>>
>>>>>> H.J.
>>>>>> ---
>>>>>>       [BZ #21120]
>>>>>>       * sysdeps/generic/malloc-alignment.h: New file.
>>>>>>       * sysdeps/i386/malloc-alignment.h: Likewise.
>>>>>>       * sysdeps/generic/malloc-machine.h: Include <malloc-alignment.h>.
>>>>>
>>>>> Please use malloc-machine.h which was the previous header that provided
>>>>> machine-dependent malloc definitions. That way we remain consistent across
>>>>> releases and make it easier to backport such changes without adding a new
>>>>> header.
>>>>
>>>> It won't work too well for Hurd since we have
>>>>
>>>> ./sysdeps/generic/malloc-machine.h
>>>> ./sysdeps/mach/hurd/malloc-machine.h
>>>> ./sysdeps/nptl/malloc-machine.h
>>>>
>>>> What will Hurd/i386 get?   malloc-alignment.h handles it automatically.
>>>
>>> If your patch made Hurd/i386 use MALLOC_ALIGNMENT of 16 then a new patch
>>> using malloc-machine.h would set MALLOC_ALIGNMENT to 16 in
>>> systeps/mach/hurd/malloc-machine.h?
>>>
>>
>> This assumes that mach/hurd == i386.  Also I don't like define
>> MALLOC_ALIGNMENT to 16 for i386 in 2 different places.
>> Since
>>
>> ./sysdeps/generic/malloc-machine.h
>> ./sysdeps/mach/hurd/malloc-machine.h
>> ./sysdeps/nptl/malloc-machine.h
>>
>> malloc-machine.h is not pure processor specific. It is also OS
>> specific.  I prefer to define MALLOC_ALIGNMENT to 16 for i386
>> in a processor specific header file.
>
> Add a sysdeps/mach/hurd/i386, which is an os/machine directory.
>
> Similarly for i386?
>

Are you suggesting 3 i386 header files to define MALLOC_ALIGNMENT?

1. generic i386.
2. nptl i386.
3. hurd i386.

That is very weird.
  
H.J. Lu June 30, 2017, 11:54 a.m. UTC | #7
On Thu, Jun 29, 2017 at 1:10 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, Jun 29, 2017 at 1:03 PM, Carlos O'Donell <carlos@redhat.com> wrote:
>> On 06/29/2017 02:43 PM, H.J. Lu wrote:
>>> On Thu, Jun 29, 2017 at 11:11 AM, Carlos O'Donell <carlos@redhat.com> wrote:
>>>> On 06/29/2017 02:06 PM, H.J. Lu wrote:
>>>>> On Thu, Jun 29, 2017 at 10:55 AM, Carlos O'Donell <carlos@redhat.com> wrote:
>>>>>> On 06/29/2017 01:30 PM, H.J. Lu wrote:
>>>>>>> GCC 7 changed the definition of max_align_t on i386:
>>>>>>>
>>>>>>> https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=9b5c49ef97e63cc63f1ffa13baf771368105ebe2
>>>>>>>
>>>>>>> As a result, glibc malloc no longer returns memory blocks which are as
>>>>>>> aligned as max_align_t requires.
>>>>>>>
>>>>>>> This causes malloc/tst-malloc-thread-fail to fail with an error like this
>>>>>>> one:
>>>>>>>
>>>>>>> error: allocation function 0, size 144 not aligned to 16
>>>>>>>
>>>>>>> This patch increases the malloc alignment to 16 for i386.
>>>>>>>
>>>>>>> Tested on i386 with GCC 7 and on x86-64.  OK for master?
>>>>>>>
>>>>>>> H.J.
>>>>>>> ---
>>>>>>>       [BZ #21120]
>>>>>>>       * sysdeps/generic/malloc-alignment.h: New file.
>>>>>>>       * sysdeps/i386/malloc-alignment.h: Likewise.
>>>>>>>       * sysdeps/generic/malloc-machine.h: Include <malloc-alignment.h>.
>>>>>>
>>>>>> Please use malloc-machine.h which was the previous header that provided
>>>>>> machine-dependent malloc definitions. That way we remain consistent across
>>>>>> releases and make it easier to backport such changes without adding a new
>>>>>> header.
>>>>>
>>>>> It won't work too well for Hurd since we have
>>>>>
>>>>> ./sysdeps/generic/malloc-machine.h
>>>>> ./sysdeps/mach/hurd/malloc-machine.h
>>>>> ./sysdeps/nptl/malloc-machine.h
>>>>>
>>>>> What will Hurd/i386 get?   malloc-alignment.h handles it automatically.
>>>>
>>>> If your patch made Hurd/i386 use MALLOC_ALIGNMENT of 16 then a new patch
>>>> using malloc-machine.h would set MALLOC_ALIGNMENT to 16 in
>>>> systeps/mach/hurd/malloc-machine.h?
>>>>
>>>
>>> This assumes that mach/hurd == i386.  Also I don't like define
>>> MALLOC_ALIGNMENT to 16 for i386 in 2 different places.
>>> Since
>>>
>>> ./sysdeps/generic/malloc-machine.h
>>> ./sysdeps/mach/hurd/malloc-machine.h
>>> ./sysdeps/nptl/malloc-machine.h
>>>
>>> malloc-machine.h is not pure processor specific. It is also OS
>>> specific.  I prefer to define MALLOC_ALIGNMENT to 16 for i386
>>> in a processor specific header file.
>>
>> Add a sysdeps/mach/hurd/i386, which is an os/machine directory.
>>
>> Similarly for i386?
>>
>
> Are you suggesting 3 i386 header files to define MALLOC_ALIGNMENT?
>
> 1. generic i386.
> 2. nptl i386.
> 3. hurd i386.
>
> That is very weird.

malloc-machine.h is an internal header and was never installed.  Adding
another internal header makes no differences for backport in this case.
FWIW, I backported my malloc-alignment.h patch to glibc 2.25:

https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/hjl/pr21120/2.25

and 2.24:

https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/hjl/pr21120/2.24

I built and checked them for i686 with GCC 7.  The biggest challenge is to
support GCC 7, not the header file itself.
  
Carlos O'Donell June 30, 2017, 1:05 p.m. UTC | #8
On 06/30/2017 07:54 AM, H.J. Lu wrote:
> On Thu, Jun 29, 2017 at 1:10 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> Are you suggesting 3 i386 header files to define MALLOC_ALIGNMENT?
>>
>> 1. generic i386.
>> 2. nptl i386.
>> 3. hurd i386.
>>
>> That is very weird.

I hope my explanation below makes the difference clearer.

> I built and checked them for i686 with GCC 7.  The biggest challenge is to
> support GCC 7, not the header file itself.
 
We have an existing malloc-machine.h for this purpose.

The design question is:

(a) Should we add a new file malloc-alignment.h to specify a new parameter.
    - Adds new file malloc-alignment.h.
    - 2 new files.

(b) Should we use existing malloc-machine.h to include the new parameter?
    - Extends existing malloc-machine.h to new sysdep dirs.
    - 3 new files.

You are arguing for (a) and I argue for (b).

Use 1 less file but require a new header.
Use 1 more file but use existing headers.

I argue that it's simpler and easier for developers if we avoid additional headers
and use existing headers.

I'm open hear a comment from someone else.
  
H.J. Lu June 30, 2017, 1:35 p.m. UTC | #9
On Fri, Jun 30, 2017 at 6:05 AM, Carlos O'Donell <carlos@redhat.com> wrote:
> On 06/30/2017 07:54 AM, H.J. Lu wrote:
>> On Thu, Jun 29, 2017 at 1:10 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> Are you suggesting 3 i386 header files to define MALLOC_ALIGNMENT?
>>>
>>> 1. generic i386.
>>> 2. nptl i386.
>>> 3. hurd i386.
>>>
>>> That is very weird.
>
> I hope my explanation below makes the difference clearer.
>
>> I built and checked them for i686 with GCC 7.  The biggest challenge is to
>> support GCC 7, not the header file itself.
>
> We have an existing malloc-machine.h for this purpose.
>
> The design question is:
>
> (a) Should we add a new file malloc-alignment.h to specify a new parameter.
>     - Adds new file malloc-alignment.h.
>     - 2 new files.

And define MALLOC_ALIGNMENT for i386 in one place.

>
> (b) Should we use existing malloc-machine.h to include the new parameter?
>     - Extends existing malloc-machine.h to new sysdep dirs.
>     - 3 new files.

And define  MALLOC_ALIGNMENT for i386 in 3 difference places.
If we need to support a new OS or threading library, we need to
define MALLOC_ALIGNMENT for i386 in another place.

> You are arguing for (a) and I argue for (b).
>
> Use 1 less file but require a new header.
> Use 1 more file but use existing headers.
>
> I argue that it's simpler and easier for developers if we avoid additional headers
> and use existing headers.
>

Because your suggestion defines MALLOC_ALIGNMENT for i386 in
more than one place, I argue it is more complex and more error
prone than a single MALLOC_ALIGNMENT definition for i386.  With
my approach, it is fixed once for all.
  
Carlos O'Donell June 30, 2017, 1:52 p.m. UTC | #10
On 06/30/2017 09:35 AM, H.J. Lu wrote:
> On Fri, Jun 30, 2017 at 6:05 AM, Carlos O'Donell <carlos@redhat.com> wrote:
>> On 06/30/2017 07:54 AM, H.J. Lu wrote:
>>> On Thu, Jun 29, 2017 at 1:10 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>> Are you suggesting 3 i386 header files to define MALLOC_ALIGNMENT?
>>>>
>>>> 1. generic i386.
>>>> 2. nptl i386.
>>>> 3. hurd i386.
>>>>
>>>> That is very weird.
>>
>> I hope my explanation below makes the difference clearer.
>>
>>> I built and checked them for i686 with GCC 7.  The biggest challenge is to
>>> support GCC 7, not the header file itself.
>>
>> We have an existing malloc-machine.h for this purpose.
>>
>> The design question is:
>>
>> (a) Should we add a new file malloc-alignment.h to specify a new parameter.
>>     - Adds new file malloc-alignment.h.
>>     - 2 new files.
> 
> And define MALLOC_ALIGNMENT for i386 in one place.
> 
>>
>> (b) Should we use existing malloc-machine.h to include the new parameter?
>>     - Extends existing malloc-machine.h to new sysdep dirs.
>>     - 3 new files.
> 
> And define  MALLOC_ALIGNMENT for i386 in 3 difference places.
> If we need to support a new OS or threading library, we need to
> define MALLOC_ALIGNMENT for i386 in another place.
> 
>> You are arguing for (a) and I argue for (b).
>>
>> Use 1 less file but require a new header.
>> Use 1 more file but use existing headers.
>>
>> I argue that it's simpler and easier for developers if we avoid additional headers
>> and use existing headers.
>>
> 
> Because your suggestion defines MALLOC_ALIGNMENT for i386 in
> more than one place, I argue it is more complex and more error
> prone than a single MALLOC_ALIGNMENT definition for i386.  With
> my approach, it is fixed once for all.

Can't they all equally just include 'sysdeps/i386/malloc-machine.h'?

/* Include the base i386 definitions.  */
#include <sysdeps/i386/malloc-machine.h>

So you have one definition in one place?

In sysdeps/nptl/malloc-machine.h we already have:
#include <sysdeps/generic/malloc-machine.h>

So we already use a layered approach to avoid multiple definitions.
  
Joseph Myers June 30, 2017, 2:31 p.m. UTC | #11
On Fri, 30 Jun 2017, Carlos O'Donell wrote:

> (a) Should we add a new file malloc-alignment.h to specify a new parameter.
>     - Adds new file malloc-alignment.h.
>     - 2 new files.

My preference is to add headers like this and so reduce the need for 
#ifndef (that is, have a generic malloc-alignment that includes the 
definition that's currently guarded with #ifndef).
  

Patch

diff --git a/sysdeps/generic/malloc-alignment.h b/sysdeps/generic/malloc-alignment.h
new file mode 100644
index 0000000..193e827
--- /dev/null
+++ b/sysdeps/generic/malloc-alignment.h
@@ -0,0 +1,24 @@ 
+/* Define MALLOC_ALIGNMENT for malloc.  Generic version.
+   Copyright (C) 2017 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef _GENERIC_MALLOC_ALIGNMENT_H
+#define _GENERIC_MALLOC_ALIGNMENT_H
+
+/* Use the default MALLOC_ALIGNMENT.  */
+
+#endif /* !defined(_GENERIC_MALLOC_ALIGNMENT_H) */
diff --git a/sysdeps/generic/malloc-machine.h b/sysdeps/generic/malloc-machine.h
index 21aa9fc..4491b90 100644
--- a/sysdeps/generic/malloc-machine.h
+++ b/sysdeps/generic/malloc-machine.h
@@ -21,6 +21,7 @@ 
 #define _GENERIC_MALLOC_MACHINE_H
 
 #include <atomic.h>
+#include <malloc-alignment.h>
 
 #ifndef atomic_full_barrier
 # define atomic_full_barrier() __asm ("" ::: "memory")
diff --git a/sysdeps/i386/malloc-alignment.h b/sysdeps/i386/malloc-alignment.h
new file mode 100644
index 0000000..f72f7a8
--- /dev/null
+++ b/sysdeps/i386/malloc-alignment.h
@@ -0,0 +1,24 @@ 
+/* Define MALLOC_ALIGNMENT for malloc.  i386 version.
+   Copyright (C) 2017 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef _I386_MALLOC_ALIGNMENT_H
+#define _I386_MALLOC_ALIGNMENT_H
+
+#define MALLOC_ALIGNMENT 16
+
+#endif /* !defined(_I386_MALLOC_ALIGNMENT_H) */