public inbox for libc-ports@sourceware.org
 help / color / mirror / Atom feed
From: "Steve Ellcey " <sellcey@mips.com>
To: <libc-ports@sourceware.org>
Subject: [patch, mips, memmove] Remove memmove's use of memcpy on MIPS
Date: Fri, 25 Jan 2013 18:17:00 -0000	[thread overview]
Message-ID: <8eea9547-6de4-482b-833c-f552df9283d4@EXCHHUB01.MIPS.com> (raw)

While doing some other testing Maciej Rozycki found a bug in the MIPS
memmove.  A few weeks ago (after the release of 2.17) I updated the MIPS
memcpy.S to use the prepare-to-store prefetch instruction.  This makes
memcpy (particularly long memcpy's) much faster.  The problem is that the
prepare-to-store prefetch is not a 'real' prefetch, it doesn't read the
memory into the cache but, if the data is not already in cache, it creates a
cache line with all zeros and assumes that the user will write to the entire
line.  If you try to read from that cache line or if you don't write to the
entire cache line before it is flushed you will get bad results.

By setting MEMCPY_OK_FOR_FWD_MEMMOVE in memmove.c, we are causing memmove
to call memcpy for overlapping forward memmoves and while the MIPS memcpy
does correctly read the source bytes before writing them, it may do a
(prepare-to-store) prefetch on data in the source buffer before reading it.
This will corrupt the data it is trying to read and give bad results.

Rather then use a slower safer prefetch for memcpy, which would also fix
this problem,  I would like to remove the setting of MEMCPY_OK_FOR_FWD_MEMMOVE
in memmove.c.

OK for checkin?

Steve Ellcey
sellcey@mips.com



2013-01-25  Steve Ellcey  <sellcey@mips.com>

	* sysdeps/mips/memmove.c: Do not set MEMCPY_OK_FOR_FWD_MEMMOVE.



diff --git a/ports/sysdeps/mips/memmove.c b/ports/sysdeps/mips/memmove.c
index fd36859..ae312d1 100644
--- a/ports/sysdeps/mips/memmove.c
+++ b/ports/sysdeps/mips/memmove.c
@@ -18,6 +18,8 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-/* MIPS' implementation of memcpy is safe to use for memmove.  */
-#define MEMCPY_OK_FOR_FWD_MEMMOVE 1
+/* MIPS' implementation of memcpy is not safe to use for memmove
+   due to the use of the prepare-to-store prefetch so we do not
+   define MEMCPY_OK_FOR_FWD_MEMMOVE.  */
+
 #include <string/memmove.c>

             reply	other threads:[~2013-01-25 18:17 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-25 18:17 Steve Ellcey  [this message]
2013-01-25 18:55 ` Chris Metcalf
2013-01-25 19:29   ` Steve Ellcey
2013-01-25 20:16 ` Joseph S. Myers
2013-01-25 21:29   ` Steve Ellcey
2013-01-25 23:15     ` Joseph S. Myers
2013-01-26  2:09     ` Maxim Kuvyrkov

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=8eea9547-6de4-482b-833c-f552df9283d4@EXCHHUB01.MIPS.com \
    --to=sellcey@mips.com \
    --cc=libc-ports@sourceware.org \
    /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).