mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Matt Helsley <matthltc@us.ibm.com>
To: David Miller <davem@davemloft.net>
Cc: zaitcev@redhat.com, erikj@sgi.com, guillaume.thouvenin@bull.net,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] connector: Some fixes for ia64 unaligned access errors
Date: Mon, 11 Dec 2006 19:09:16 -0800	[thread overview]
Message-ID: <1165892956.24721.129.camel@localhost.localdomain> (raw)
In-Reply-To: <20061211.175050.55478586.davem@davemloft.net>

On Mon, 2006-12-11 at 17:50 -0800, David Miller wrote:
> From: Pete Zaitcev <zaitcev@redhat.com>
> Date: Mon, 11 Dec 2006 17:29:07 -0800
> 
> > On Mon, 11 Dec 2006 15:52:47 -0800, Matt Helsley <matthltc@us.ibm.com> wrote:
> >
> > > 	I'm shocked memcpy() introduces 8-byte stores that violate architecture
> > > alignment rules. Is there any chance this a bug in ia64's memcpy()
> > > implementation? I've tried to read it but since I'm not familiar with
> > > ia64 asm I can't make out significant parts of it in
> > > arch/ia64/lib/memcpy.S.
> >
> > The arch/ia64/lib/memcpy.S is probably fine, it must be gcc doing
> > an inline substitution of a well-known function.
> >
> > A commenter on my blog mentioned seeing the same thing in the past.
> > (http://zaitcev.livejournal.com/107185.html?thread=128945#t128945)
> >
> > It's possible that applying (void *) cast to the first argument of memcpy
> > would disrupt this optimization. But since we have a well understood
> > patch by Erik, which only adds a penalty of 32 bytes of stack waste
> > and 32 bytes of memcpy, I thought it best not to bother with heaping
> > workarounds.
> 
> Yes GCC can assume the object is aligned because of the type
> of the argument to memcpy().

Hmm, that GCC assumption conflicts with the prototypes of memcpy() I've
seen.

	Does the code really check the type or just the size argument? If the
latter then I don't think assuming alignment is correct -- we could be
copying a non-nul-terminated string that happens to be a power of 2 in
length.

Cheers,
	-Matt Helsley


  reply	other threads:[~2006-12-12  3:09 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-12-07 23:22 Erik Jacobson
2006-12-09  3:20 ` Pete Zaitcev
2006-12-09  7:47   ` Matt Helsley
2006-12-09 21:09   ` Erik Jacobson
2006-12-10  2:34     ` Pete Zaitcev
2006-12-11 23:52       ` Matt Helsley
2006-12-12  1:29         ` Pete Zaitcev
2006-12-12  1:50           ` David Miller
2006-12-12  3:09             ` Matt Helsley [this message]
2006-12-12  3:41               ` David Miller
2006-12-12  2:07           ` Chen, Kenneth W
2006-12-11 23:52 ` Matt Helsley
2006-12-12 17:54   ` Erik Jacobson
2006-12-13  0:45     ` Andrew Morton
2006-12-13  2:31       ` Erik Jacobson
2006-12-13  2:38         ` Andrew Morton
2006-12-13  6:08           ` Erik Jacobson

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=1165892956.24721.129.camel@localhost.localdomain \
    --to=matthltc@us.ibm.com \
    --cc=davem@davemloft.net \
    --cc=erikj@sgi.com \
    --cc=guillaume.thouvenin@bull.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=zaitcev@redhat.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

Powered by JetHome