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$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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