Re: [PATCH] powerpc: Use aligned stores in memset

[Florian Weimer <fweimer@redhat.com>]

On 08/18/2017 11:10 AM, Florian Weimer wrote:
> On 08/18/2017 08:51 AM, Rajalakshmi Srinivasaraghavan wrote:
>>
>>
>> On 08/18/2017 11:51 AM, Florian Weimer wrote:
>>> On 08/18/2017 07:11 AM, Rajalakshmi Srinivasaraghavan wrote:
>>>>     * sysdeps/powerpc/powerpc64/power8/memset.S: Store byte by byte
>>>>     for unaligned inputs if size is less than 8.
>>>
>>> This makes me rather nervous.  powerpc64le was supposed to have
>>> reasonable efficient unaligned loads and stores.  GCC happily generates
>>> them, too.
>>
>> This is meant ONLY for caching inhibited accesses.  Caching Inhibited
>> accesses are required to be Guarded and properly aligned.
> 
> The intent is to support memset for such memory regions, right?  This
> change is insufficient.  You have to fix GCC as well because it will
> inline memset of unaligned pointers, like this:

Here's a more complete example:


#include <assert.h>
#include <stdio.h>
#include <string.h>

typedef long __attribute__ ((aligned(1))) long_unaligned;

__attribute__ ((noinline, noclone, weak))
void
clear (long_unaligned *p)
{
  memset (p, 0, sizeof (*p));
}

struct data
{
  char misalign;
  long_unaligned data;
};

int
main (void)
{
  struct data *data = malloc (sizeof (*data));
  assert (data != NULL);
  long_unaligned *p = &data->data;
  printf ("pointer: %p\n", p);
  clear (p);
  return 0;
}

The clear function compiles to:

typedef long __attribute__ ((aligned(1))) long_unaligned;

void
clear (long_unaligned *p)
{
  memset (p, 0, sizeof (*p));
}

At run time, I get:

pointer: 0x10003c10011

This means that GCC introduced an unaligned store, no matter how memset
was implemented.

I could not find the manual which has the requirement that the mem*
functions do not use unaligned accesses.  Unless they are worded in a
very peculiar way, right now, the GCC/glibc combination does not comply
with a requirement that memset & Co. can be used for device memory access.

Furthermore, I find it very peculiar that over-reading device memory is
acceptable.  Some memory-mapped devices behave strangely if memory
locations are read out of order or multiple times, and the current glibc
implementation accesses locations which are outside the specified object
boundaries.

So I think the implementation constraint on the mem* functions is wrong.
 It leads to a slower implementation of the mem* function for most of
userspace which does not access device memory, and even for device
memory, it is probably not what you want.

Thanks,
Florian