From: gregory.clement@free-electrons.com (Gregory CLEMENT)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/4] ARM: mm: kill unused TLB_CAN_READ_FROM_L1_CACHE and use ALT_SMP instead
Date: Wed, 15 May 2013 15:54:14 +0200 [thread overview]
Message-ID: <51939386.7010709@free-electrons.com> (raw)
In-Reply-To: <20130515134117.GH23869@mudshark.cambridge.arm.com>
On 05/15/2013 03:41 PM, Will Deacon wrote:
> On Wed, May 15, 2013 at 02:18:53PM +0100, Gregory CLEMENT wrote:
>> Hi Will,
>
> Hi Gregory,
>
>> On 03/25/2013 07:19 PM, Will Deacon wrote:
>>> Many ARMv7 cores have hardware page table walkers that can read the L1
>>> cache. This is discoverable from the ID_MMFR3 register, although this
>>> can be expensive to access from the low-level set_pte functions and is a
>>> pain to cache, particularly with multi-cluster systems.
>>>
>>> A useful observation is that the multi-processing extensions for ARMv7
>>> require coherent table walks, meaning that we can make use of ALT_SMP
>>> patching in proc-v7-* to patch away the cache flush safely for these
>>> cores.
>>
>> I encountered a regression with 3.10-rc1 on the Armada 370 based boards.
>> With the 3.10-rc1 they hang during auto testy of the xor engine which are
>> mainly DMA transfers. If I revert this patch, it no more hang. I found this
>> by using bisect, it was not obvious at all for me that this patch may have
>> cause this regression.
>
> Is this using dmatest.ko, or a different test program?
No they are self-test from the mv_xor driver
>
>> The issue appear in SMP and in UP. However I think that the PJ4B-v7 used in
>> the Armada 370 are not MP capable.
>
> Ok, so the ALT_UP case should be emitted after patching, correct?
Indeed it was I excepted but I didn't check (I don't know how to do)
>
>> I made some investigation. And in UP if I remove the line:
>> ALT_UP(W(nop))
>
> Did you remove the ALT_SMP case as well? You could also try making the
> ALT_SMP case use W(nop) too and see if it changes anything.
OK I will try it.
>
>> at the begining of the cpu_v7_dcache_clean_are macro located in
>> arch/arm/mm/proc-v7.S
>>
>> Then the kernel boot again. It is not surprising because in this case
>> we found the same generated code that before this patch was applied.
>>
>> now I don't really understand why a W(nop) will cause this issue.
>
> No, that sounds weird. Can you inspect the functions using JTAG after the smp
> patching code has executed?
No I can't: I don't have any JTAG :/
>
>> In SMP mode even with this line removed the kernel hang, but in this case
>> I am not sure of what happen exactly and how the .alt.smp.init section is
>> used.
>
> If you don't have both of the alternatives, things will go wrong.
For my own culture, how ALT_UP and ALT_SMP work in SMP case?
When I disassembled the proc-v7.o, I saw that the SMP variant of the code were
written. How the kernel switch to the UP version of the code?
>
>> I hope you will find some explanation and solution to this bug, because currently
>> the only solution I have is to revert this patch.
>
> Let's not jump to that just yet!
Sure I hope we will find a fix for that.
Thanks,
Gregory
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
next prev parent reply other threads:[~2013-05-15 13:54 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-25 18:19 [PATCH 0/4] TLB and mm-related optimisations Will Deacon
2013-03-25 18:19 ` [PATCH 1/4] ARM: tlb: don't perform inner-shareable invalidation for local TLB ops Will Deacon
2013-03-27 10:34 ` Catalin Marinas
2013-03-27 12:07 ` Will Deacon
2013-03-27 12:30 ` Catalin Marinas
2013-03-27 12:56 ` Will Deacon
2013-03-27 13:40 ` Catalin Marinas
2013-03-27 13:54 ` Will Deacon
2013-03-25 18:19 ` [PATCH 2/4] ARM: tlb: don't perform inner-shareable invalidation for local BP ops Will Deacon
2013-03-27 10:36 ` Catalin Marinas
2013-03-25 18:19 ` [PATCH 3/4] ARM: mm: kill unused TLB_CAN_READ_FROM_L1_CACHE and use ALT_SMP instead Will Deacon
2013-03-27 10:53 ` Catalin Marinas
2013-03-27 12:20 ` Will Deacon
2013-05-15 13:18 ` Gregory CLEMENT
2013-05-15 13:41 ` Will Deacon
2013-05-15 13:54 ` Gregory CLEMENT [this message]
2013-05-15 14:06 ` Will Deacon
2013-05-15 14:46 ` Gregory CLEMENT
2013-05-15 15:04 ` Will Deacon
2013-05-15 15:36 ` Gregory CLEMENT
2013-05-15 15:41 ` Will Deacon
2013-05-15 16:29 ` Gregory CLEMENT
2013-05-15 16:48 ` Will Deacon
2013-05-15 17:16 ` Russell King - ARM Linux
2013-03-25 18:19 ` [PATCH 4/4] ARM: atomics: don't use exclusives for atomic64 read/set with LPAE Will Deacon
2013-03-27 10:57 ` Catalin Marinas
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=51939386.7010709@free-electrons.com \
--to=gregory.clement@free-electrons.com \
--cc=linux-arm-kernel@lists.infradead.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.