public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Steven Munroe <munroesj@linux.vnet.ibm.com>
To: Florian Weimer <fweimer@redhat.com>
Cc: Rajalakshmi Srinivasaraghavan <raji@linux.vnet.ibm.com>,
	libc-alpha@sourceware.org
Subject: Re: [PATCH] powerpc: Use aligned stores in memset
Date: Tue, 12 Sep 2017 13:38:00 -0000	[thread overview]
Message-ID: <1505223476.12360.14.camel@oc7878010663> (raw)
In-Reply-To: <d7115391-1e52-5ecb-dce6-57895aaed268@redhat.com>

On Tue, 2017-09-12 at 12:30 +0200, Florian Weimer wrote:
> 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:
> 

..snip

> 
> This means that GCC introduced an unaligned store, no matter how memset
> was implemented.
> 
C will do what ever the programmer wants. We can not stop that. 

And in user mode and cache coherent memory this is not a problem as
Adhemerval explained.

So we are not going to degrade the performance of general applications
for a tiny subset of specialized device drivers. Those guy have to know
what they are doing.

But in the library (like libc) that might be called from a user mode
device driver (Xorg for example) and access Cache inhibited memory the
memcpy implementation has to check alignment and size and using the
correct instructions for each case.

That is what we are doing here. 


> 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.
> 
Yes device driver writers have to know what they are doing.

> 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.
> 
We are just trying to make the mem* safe (not segfault or alignment
check) if used correctly. 

The definition of correctly is a bit fluid. I personally disagree with
the Xorg folks but so far they have refused to bend...


> Thanks,
> Florian
> 


  parent reply	other threads:[~2017-09-12 13:38 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-18  5:13 Rajalakshmi Srinivasaraghavan
2017-08-18  6:21 ` Florian Weimer
2017-08-18  6:51   ` Rajalakshmi Srinivasaraghavan
2017-08-18  9:10     ` Florian Weimer
2017-08-18 12:13       ` Adhemerval Zanella
2017-09-12 10:30       ` Florian Weimer
2017-09-12 12:18         ` Zack Weinberg
2017-09-12 13:57           ` Steven Munroe
2017-09-12 14:37           ` Joseph Myers
2017-09-12 15:06             ` Zack Weinberg
2017-09-12 17:09           ` Florian Weimer
2017-09-12 13:38         ` Steven Munroe [this message]
2017-09-12 14:08           ` Florian Weimer
2017-09-12 14:16             ` Steven Munroe
2017-09-12 17:04               ` Florian Weimer
2017-09-12 19:21                 ` Steven Munroe
2017-09-12 19:45                   ` Florian Weimer
2017-09-12 20:25                     ` Steven Munroe
2017-09-13 13:12         ` Tulio Magno Quites Machado Filho
2017-09-18 13:54           ` Florian Weimer
2017-10-03 18:29             ` Adhemerval Zanella
2017-10-05 12:13               ` Rajalakshmi Srinivasaraghavan
2017-11-08 18:52               ` Tulio Magno Quites Machado Filho
2017-12-08 19:52                 ` [PATCHv2] powerpc: POWER8 memcpy optimization for cached memory Tulio Magno Quites Machado Filho
2017-12-08 20:06                   ` Florian Weimer
2017-12-11 12:44                     ` Tulio Magno Quites Machado Filho
2017-12-11 20:09                       ` Adhemerval Zanella
2017-12-10  7:11                   ` Rajalakshmi Srinivasaraghavan
2017-12-11 19:48                     ` Tulio Magno Quites Machado Filho
2017-08-18  6:25 ` [PATCH] powerpc: Use aligned stores in memset Andrew Pinski
2017-08-21  2:20 ` Tulio Magno Quites Machado Filho

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1505223476.12360.14.camel@oc7878010663 \
    --to=munroesj@linux.vnet.ibm.com \
    --cc=fweimer@redhat.com \
    --cc=libc-alpha@sourceware.org \
    --cc=raji@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).