From: Hannes Hering <hannes.hering@linux.vnet.ibm.com>
To: David Howells <dhowells@redhat.com>
Cc: themann@de.ibm.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, ossrosch@linux.vnet.ibm.com,
linuxppc-dev@ozlabs.org, raisch@de.ibm.com, ossthema@de.ibm.com,
osstklei@de.ibm.com, David Miller <davem@davemloft.net>
Subject: Re: [PATCH 2.6.30-rc5] ehea: fix invalid pointer access
Date: Tue, 5 May 2009 15:45:05 +0200 [thread overview]
Message-ID: <200905051545.05877.hannes.hering@linux.vnet.ibm.com> (raw)
In-Reply-To: <26110.1241525994@redhat.com>
On Tuesday 05 May 2009 14:19:54 David Howells wrote:
> In that case, you might want to move the prefetchw() calls in the following:
>
> pref = skb_array[x];
> - prefetchw(pref);
> - prefetchw(pref + EHEA_CACHE_LINE);
> + if (pref) {
> + prefetchw(pref);
> + prefetchw(pref + EHEA_CACHE_LINE);
>
> to before the if-statement. That way the CPU can be attempting the prefetch
> whilst it's chewing over the test and branch. prefetching shouldn't fault on
> a bad address.
>
> David
Hi David,
you are right so far, but actually the prefetch calls on POWER also contain
an if statement to check if the address is valid (i. e. non-zero). We never
have the case that the pref != NULL and pref->data == NULL. And the situation
of pref==NULL is very rare. This means there is no benefit moving our if
statement down from performance perspective if we assume that our if does not
take longer then the if in the prefetch command. We can add an
if(likely(pref) if you like. In fact doing the if statement as we do it now
we actually save the prefetch if statements in case we hit the situation of
pref==NULL.
Regards
Hannes
prev parent reply other threads:[~2009-05-05 13:45 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-05-04 16:02 [PATCH 2.6.30-rc5] ehea: fix invalid pointer access Hannes Hering
2009-05-04 18:07 ` David Miller
2009-05-05 9:11 ` David Howells
2009-05-05 11:19 ` Hannes Hering
2009-05-05 12:19 ` David Howells
2009-05-05 13:45 ` Hannes Hering [this message]
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=200905051545.05877.hannes.hering@linux.vnet.ibm.com \
--to=hannes.hering@linux.vnet.ibm.com \
--cc=davem@davemloft.net \
--cc=dhowells@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@ozlabs.org \
--cc=netdev@vger.kernel.org \
--cc=ossrosch@linux.vnet.ibm.com \
--cc=ossthema@de.ibm.com \
--cc=osstklei@de.ibm.com \
--cc=raisch@de.ibm.com \
--cc=themann@de.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 NNTP newsgroup(s).