Re: [PATCH] powerpc/64s/hash: Fix hash_preload running with interrupts enabled
On Mon, 27 Jul 2020 16:09:47 +1000, Nicholas Piggin wrote:
> Commit 2f92447f9f96 ("powerpc/book3s64/hash: Use the pte_t address from the
> caller") removed the local_irq_disable from hash_preload, but it was
> required for more than just the page table walk: the hash pte busy bit is
> effectively a lock which may be taken in interrupt context, and the local
> update flag test must not be preempted before it's used.
>
> This solves apparent lockups with perf interrupting __hash_page_64K. If
> get_perf_callchain then also takes a hash fault on the same page while it
> is already locked, it will loop forever taking hash faults, which looks like
> this:
>
> [...]
Applied to powerpc/fixes.
[1/1] powerpc/64s/hash: Fix hash_preload running with interrupts enabled
https://git.kernel.org/powerpc/c/909adfc66b9a1db21b5e8733e9ebfa6cd5135d74
cheers
Re: [PATCH] powerpc/64s/hash: Fix hash_preload running with interrupts enabled
> On 28-Jul-2020, at 6:14 AM, Michael Ellerman wrote:
>
> Athira Rajeev writes:
>>> On 27-Jul-2020, at 6:05 PM, Michael Ellerman wrote:
>>>
>>> Athira Rajeev writes:
> On 27-Jul-2020, at 11:39 AM, Nicholas Piggin wrote:
>
> Commit 2f92447f9f96 ("powerpc/book3s64/hash: Use the pte_t address from
> the
> caller") removed the local_irq_disable from hash_preload, but it was
> required for more than just the page table walk: the hash pte busy bit is
> effectively a lock which may be taken in interrupt context, and the local
> update flag test must not be preempted before it's used.
>
> This solves apparent lockups with perf interrupting __hash_page_64K. If
> get_perf_callchain then also takes a hash fault on the same page while it
> is already locked, it will loop forever taking hash faults, which looks
> like
> this:
>
> cpu 0x49e: Vector: 100 (System Reset) at [c0001a4f7d70]
> pc: c0072dc8: hash_page_mm+0x8/0x800
> lr: c000c5a4: do_hash_page+0x24/0x38
> sp: c0002ac1cc69ac70
> msr: 80081033
> current = 0xc0002ac1cc602e00
> paca= 0xc0001de1f280 irqmask: 0x03 irq_happened: 0x01
> pid = 20118, comm = pread2_processe
> Linux version 5.8.0-rc6-00345-g1fad14f18bc6
> 49e:mon> t
> [c0002ac1cc69ac70] c000c5a4 do_hash_page+0x24/0x38 (unreliable)
> --- Exception: 300 (Data Access) at c008fa60
> __copy_tofrom_user_power7+0x20c/0x7ac
> [link register ] c0335d10 copy_from_user_nofault+0xf0/0x150
> [c0002ac1cc69af70] c00032bf9fa3c880 (unreliable)
> [c0002ac1cc69afa0] c0109df0 read_user_stack_64+0x70/0xf0
> [c0002ac1cc69afd0] c0109fcc perf_callchain_user_64+0x15c/0x410
> [c0002ac1cc69b060] c0109c00 perf_callchain_user+0x20/0x40
> [c0002ac1cc69b080] c031c6cc get_perf_callchain+0x25c/0x360
> [c0002ac1cc69b120] c0316b50 perf_callchain+0x70/0xa0
> [c0002ac1cc69b140] c0316ddc perf_prepare_sample+0x25c/0x790
> [c0002ac1cc69b1a0] c0317350 perf_event_output_forward+0x40/0xb0
> [c0002ac1cc69b220] c0306138 __perf_event_overflow+0x88/0x1a0
> [c0002ac1cc69b270] c010cf70 record_and_restart+0x230/0x750
> [c0002ac1cc69b620] c010d69c perf_event_interrupt+0x20c/0x510
> [c0002ac1cc69b730] c0027d9c
> performance_monitor_exception+0x4c/0x60
> [c0002ac1cc69b750] c000b2f8
> performance_monitor_common_virt+0x1b8/0x1c0
> --- Exception: f00 (Performance Monitor) at c00cb5b0
> pSeries_lpar_hpte_insert+0x0/0x160
> [link register ] c00846f0 __hash_page_64K+0x210/0x540
> [c0002ac1cc69ba50] (unreliable)
> [c0002ac1cc69bb00] c0073ae0 update_mmu_cache+0x390/0x3a0
> [c0002ac1cc69bb70] c037f024 wp_page_copy+0x364/0xce0
> [c0002ac1cc69bc20] c038272c do_wp_page+0xdc/0xa60
> [c0002ac1cc69bc70] c03857bc handle_mm_fault+0xb9c/0x1b60
> [c0002ac1cc69bd50] c006c434 __do_page_fault+0x314/0xc90
> [c0002ac1cc69be20] c000c5c8 handle_page_fault+0x10/0x2c
> --- Exception: 300 (Data Access) at 7fff8c861fe8
> SP (76b19660) is in userspace
>
> Reported-by: Athira Rajeev
> Reported-by: Anton Blanchard
> Reviewed-by: Aneesh Kumar K.V
> Fixes: 2f92447f9f96 ("powerpc/book3s64/hash: Use the pte_t address from
> the
> caller")
> Signed-off-by: Nicholas Piggin
Hi,
Tested with the patch and it fixes the lockups I was seeing with my test
run.
Thanks for the fix.
Tested-by: Athira Rajeev
>>>
>>> Thanks for testing.
>>>
>>> What test are you running?
>>
>> Hi Michael
>>
>> I was running “perf record” and Unixbench tests (
>> https://github.com/kdlucas/byte-unixbench ) in parallel where we were
>> getting soft lockups
>>
>> 1. Perf command run:
>> # perf record -a -g -c 1000 -o sleep 60
>>
>> 2. Unixbench tests
>> # Run -q -c spawn
>
> Thanks, I can reproduce it with that.
Sure Michael
>
> cheers
Re: [PATCH] powerpc/64s/hash: Fix hash_preload running with interrupts enabled
Athira Rajeev writes:
>> On 27-Jul-2020, at 6:05 PM, Michael Ellerman wrote:
>>
>> Athira Rajeev writes:
On 27-Jul-2020, at 11:39 AM, Nicholas Piggin wrote:
Commit 2f92447f9f96 ("powerpc/book3s64/hash: Use the pte_t address from the
caller") removed the local_irq_disable from hash_preload, but it was
required for more than just the page table walk: the hash pte busy bit is
effectively a lock which may be taken in interrupt context, and the local
update flag test must not be preempted before it's used.
This solves apparent lockups with perf interrupting __hash_page_64K. If
get_perf_callchain then also takes a hash fault on the same page while it
is already locked, it will loop forever taking hash faults, which looks
like
this:
cpu 0x49e: Vector: 100 (System Reset) at [c0001a4f7d70]
pc: c0072dc8: hash_page_mm+0x8/0x800
lr: c000c5a4: do_hash_page+0x24/0x38
sp: c0002ac1cc69ac70
msr: 80081033
current = 0xc0002ac1cc602e00
paca= 0xc0001de1f280 irqmask: 0x03 irq_happened: 0x01
pid = 20118, comm = pread2_processe
Linux version 5.8.0-rc6-00345-g1fad14f18bc6
49e:mon> t
[c0002ac1cc69ac70] c000c5a4 do_hash_page+0x24/0x38 (unreliable)
--- Exception: 300 (Data Access) at c008fa60
__copy_tofrom_user_power7+0x20c/0x7ac
[link register ] c0335d10 copy_from_user_nofault+0xf0/0x150
[c0002ac1cc69af70] c00032bf9fa3c880 (unreliable)
[c0002ac1cc69afa0] c0109df0 read_user_stack_64+0x70/0xf0
[c0002ac1cc69afd0] c0109fcc perf_callchain_user_64+0x15c/0x410
[c0002ac1cc69b060] c0109c00 perf_callchain_user+0x20/0x40
[c0002ac1cc69b080] c031c6cc get_perf_callchain+0x25c/0x360
[c0002ac1cc69b120] c0316b50 perf_callchain+0x70/0xa0
[c0002ac1cc69b140] c0316ddc perf_prepare_sample+0x25c/0x790
[c0002ac1cc69b1a0] c0317350 perf_event_output_forward+0x40/0xb0
[c0002ac1cc69b220] c0306138 __perf_event_overflow+0x88/0x1a0
[c0002ac1cc69b270] c010cf70 record_and_restart+0x230/0x750
[c0002ac1cc69b620] c010d69c perf_event_interrupt+0x20c/0x510
[c0002ac1cc69b730] c0027d9c performance_monitor_exception+0x4c/0x60
[c0002ac1cc69b750] c000b2f8
performance_monitor_common_virt+0x1b8/0x1c0
--- Exception: f00 (Performance Monitor) at c00cb5b0
pSeries_lpar_hpte_insert+0x0/0x160
[link register ] c00846f0 __hash_page_64K+0x210/0x540
[c0002ac1cc69ba50] (unreliable)
[c0002ac1cc69bb00] c0073ae0 update_mmu_cache+0x390/0x3a0
[c0002ac1cc69bb70] c037f024 wp_page_copy+0x364/0xce0
[c0002ac1cc69bc20] c038272c do_wp_page+0xdc/0xa60
[c0002ac1cc69bc70] c03857bc handle_mm_fault+0xb9c/0x1b60
[c0002ac1cc69bd50] c006c434 __do_page_fault+0x314/0xc90
[c0002ac1cc69be20] c000c5c8 handle_page_fault+0x10/0x2c
--- Exception: 300 (Data Access) at 7fff8c861fe8
SP (76b19660) is in userspace
Reported-by: Athira Rajeev
Reported-by: Anton Blanchard
Reviewed-by: Aneesh Kumar K.V
Fixes: 2f92447f9f96 ("powerpc/book3s64/hash: Use the pte_t address from the
caller")
Signed-off-by: Nicholas Piggin
>>>
>>>
>>> Hi,
>>>
>>> Tested with the patch and it fixes the lockups I was seeing with my test
>>> run.
>>> Thanks for the fix.
>>>
>>> Tested-by: Athira Rajeev
>>
>> Thanks for testing.
>>
>> What test are you running?
>
> Hi Michael
>
> I was running “perf record” and Unixbench tests (
> https://github.com/kdlucas/byte-unixbench ) in parallel where we were getting
> soft lockups
>
> 1. Perf command run:
> # perf record -a -g -c 1000 -o sleep 60
>
> 2. Unixbench tests
> # Run -q -c spawn
Thanks, I can reproduce it with that.
cheers
Re: [PATCH] powerpc/64s/hash: Fix hash_preload running with interrupts enabled
> On 27-Jul-2020, at 6:05 PM, Michael Ellerman wrote:
>
> Athira Rajeev writes:
>>> On 27-Jul-2020, at 11:39 AM, Nicholas Piggin wrote:
>>>
>>> Commit 2f92447f9f96 ("powerpc/book3s64/hash: Use the pte_t address from the
>>> caller") removed the local_irq_disable from hash_preload, but it was
>>> required for more than just the page table walk: the hash pte busy bit is
>>> effectively a lock which may be taken in interrupt context, and the local
>>> update flag test must not be preempted before it's used.
>>>
>>> This solves apparent lockups with perf interrupting __hash_page_64K. If
>>> get_perf_callchain then also takes a hash fault on the same page while it
>>> is already locked, it will loop forever taking hash faults, which looks like
>>> this:
>>>
>>> cpu 0x49e: Vector: 100 (System Reset) at [c0001a4f7d70]
>>> pc: c0072dc8: hash_page_mm+0x8/0x800
>>> lr: c000c5a4: do_hash_page+0x24/0x38
>>> sp: c0002ac1cc69ac70
>>> msr: 80081033
>>> current = 0xc0002ac1cc602e00
>>> paca= 0xc0001de1f280 irqmask: 0x03 irq_happened: 0x01
>>> pid = 20118, comm = pread2_processe
>>> Linux version 5.8.0-rc6-00345-g1fad14f18bc6
>>> 49e:mon> t
>>> [c0002ac1cc69ac70] c000c5a4 do_hash_page+0x24/0x38 (unreliable)
>>> --- Exception: 300 (Data Access) at c008fa60
>>> __copy_tofrom_user_power7+0x20c/0x7ac
>>> [link register ] c0335d10 copy_from_user_nofault+0xf0/0x150
>>> [c0002ac1cc69af70] c00032bf9fa3c880 (unreliable)
>>> [c0002ac1cc69afa0] c0109df0 read_user_stack_64+0x70/0xf0
>>> [c0002ac1cc69afd0] c0109fcc perf_callchain_user_64+0x15c/0x410
>>> [c0002ac1cc69b060] c0109c00 perf_callchain_user+0x20/0x40
>>> [c0002ac1cc69b080] c031c6cc get_perf_callchain+0x25c/0x360
>>> [c0002ac1cc69b120] c0316b50 perf_callchain+0x70/0xa0
>>> [c0002ac1cc69b140] c0316ddc perf_prepare_sample+0x25c/0x790
>>> [c0002ac1cc69b1a0] c0317350 perf_event_output_forward+0x40/0xb0
>>> [c0002ac1cc69b220] c0306138 __perf_event_overflow+0x88/0x1a0
>>> [c0002ac1cc69b270] c010cf70 record_and_restart+0x230/0x750
>>> [c0002ac1cc69b620] c010d69c perf_event_interrupt+0x20c/0x510
>>> [c0002ac1cc69b730] c0027d9c performance_monitor_exception+0x4c/0x60
>>> [c0002ac1cc69b750] c000b2f8
>>> performance_monitor_common_virt+0x1b8/0x1c0
>>> --- Exception: f00 (Performance Monitor) at c00cb5b0
>>> pSeries_lpar_hpte_insert+0x0/0x160
>>> [link register ] c00846f0 __hash_page_64K+0x210/0x540
>>> [c0002ac1cc69ba50] (unreliable)
>>> [c0002ac1cc69bb00] c0073ae0 update_mmu_cache+0x390/0x3a0
>>> [c0002ac1cc69bb70] c037f024 wp_page_copy+0x364/0xce0
>>> [c0002ac1cc69bc20] c038272c do_wp_page+0xdc/0xa60
>>> [c0002ac1cc69bc70] c03857bc handle_mm_fault+0xb9c/0x1b60
>>> [c0002ac1cc69bd50] c006c434 __do_page_fault+0x314/0xc90
>>> [c0002ac1cc69be20] c000c5c8 handle_page_fault+0x10/0x2c
>>> --- Exception: 300 (Data Access) at 7fff8c861fe8
>>> SP (76b19660) is in userspace
>>>
>>> Reported-by: Athira Rajeev
>>> Reported-by: Anton Blanchard
>>> Reviewed-by: Aneesh Kumar K.V
>>> Fixes: 2f92447f9f96 ("powerpc/book3s64/hash: Use the pte_t address from the
>>> caller")
>>> Signed-off-by: Nicholas Piggin
>>
>>
>> Hi,
>>
>> Tested with the patch and it fixes the lockups I was seeing with my test run.
>> Thanks for the fix.
>>
>> Tested-by: Athira Rajeev
>
> Thanks for testing.
>
> What test are you running?
Hi Michael
I was running “perf record” and Unixbench tests (
https://github.com/kdlucas/byte-unixbench ) in parallel where we were getting
soft lockups
1. Perf command run:
# perf record -a -g -c 1000 -o sleep 60
2. Unixbench tests
# Run -q -c spawn
Wtth the fix, perf completes successfully.
Thanks
Athira
>
> cheers
Re: [PATCH] powerpc/64s/hash: Fix hash_preload running with interrupts enabled
Athira Rajeev writes:
>> On 27-Jul-2020, at 11:39 AM, Nicholas Piggin wrote:
>>
>> Commit 2f92447f9f96 ("powerpc/book3s64/hash: Use the pte_t address from the
>> caller") removed the local_irq_disable from hash_preload, but it was
>> required for more than just the page table walk: the hash pte busy bit is
>> effectively a lock which may be taken in interrupt context, and the local
>> update flag test must not be preempted before it's used.
>>
>> This solves apparent lockups with perf interrupting __hash_page_64K. If
>> get_perf_callchain then also takes a hash fault on the same page while it
>> is already locked, it will loop forever taking hash faults, which looks like
>> this:
>>
>> cpu 0x49e: Vector: 100 (System Reset) at [c0001a4f7d70]
>>pc: c0072dc8: hash_page_mm+0x8/0x800
>>lr: c000c5a4: do_hash_page+0x24/0x38
>>sp: c0002ac1cc69ac70
>> msr: 80081033
>> current = 0xc0002ac1cc602e00
>> paca= 0xc0001de1f280 irqmask: 0x03 irq_happened: 0x01
>>pid = 20118, comm = pread2_processe
>> Linux version 5.8.0-rc6-00345-g1fad14f18bc6
>> 49e:mon> t
>> [c0002ac1cc69ac70] c000c5a4 do_hash_page+0x24/0x38 (unreliable)
>> --- Exception: 300 (Data Access) at c008fa60
>> __copy_tofrom_user_power7+0x20c/0x7ac
>> [link register ] c0335d10 copy_from_user_nofault+0xf0/0x150
>> [c0002ac1cc69af70] c00032bf9fa3c880 (unreliable)
>> [c0002ac1cc69afa0] c0109df0 read_user_stack_64+0x70/0xf0
>> [c0002ac1cc69afd0] c0109fcc perf_callchain_user_64+0x15c/0x410
>> [c0002ac1cc69b060] c0109c00 perf_callchain_user+0x20/0x40
>> [c0002ac1cc69b080] c031c6cc get_perf_callchain+0x25c/0x360
>> [c0002ac1cc69b120] c0316b50 perf_callchain+0x70/0xa0
>> [c0002ac1cc69b140] c0316ddc perf_prepare_sample+0x25c/0x790
>> [c0002ac1cc69b1a0] c0317350 perf_event_output_forward+0x40/0xb0
>> [c0002ac1cc69b220] c0306138 __perf_event_overflow+0x88/0x1a0
>> [c0002ac1cc69b270] c010cf70 record_and_restart+0x230/0x750
>> [c0002ac1cc69b620] c010d69c perf_event_interrupt+0x20c/0x510
>> [c0002ac1cc69b730] c0027d9c performance_monitor_exception+0x4c/0x60
>> [c0002ac1cc69b750] c000b2f8
>> performance_monitor_common_virt+0x1b8/0x1c0
>> --- Exception: f00 (Performance Monitor) at c00cb5b0
>> pSeries_lpar_hpte_insert+0x0/0x160
>> [link register ] c00846f0 __hash_page_64K+0x210/0x540
>> [c0002ac1cc69ba50] (unreliable)
>> [c0002ac1cc69bb00] c0073ae0 update_mmu_cache+0x390/0x3a0
>> [c0002ac1cc69bb70] c037f024 wp_page_copy+0x364/0xce0
>> [c0002ac1cc69bc20] c038272c do_wp_page+0xdc/0xa60
>> [c0002ac1cc69bc70] c03857bc handle_mm_fault+0xb9c/0x1b60
>> [c0002ac1cc69bd50] c006c434 __do_page_fault+0x314/0xc90
>> [c0002ac1cc69be20] c000c5c8 handle_page_fault+0x10/0x2c
>> --- Exception: 300 (Data Access) at 7fff8c861fe8
>> SP (76b19660) is in userspace
>>
>> Reported-by: Athira Rajeev
>> Reported-by: Anton Blanchard
>> Reviewed-by: Aneesh Kumar K.V
>> Fixes: 2f92447f9f96 ("powerpc/book3s64/hash: Use the pte_t address from the
>> caller")
>> Signed-off-by: Nicholas Piggin
>
>
> Hi,
>
> Tested with the patch and it fixes the lockups I was seeing with my test run.
> Thanks for the fix.
>
> Tested-by: Athira Rajeev
Thanks for testing.
What test are you running?
cheers
Re: [PATCH] powerpc/64s/hash: Fix hash_preload running with interrupts enabled
> On 27-Jul-2020, at 11:39 AM, Nicholas Piggin wrote:
>
> Commit 2f92447f9f96 ("powerpc/book3s64/hash: Use the pte_t address from the
> caller") removed the local_irq_disable from hash_preload, but it was
> required for more than just the page table walk: the hash pte busy bit is
> effectively a lock which may be taken in interrupt context, and the local
> update flag test must not be preempted before it's used.
>
> This solves apparent lockups with perf interrupting __hash_page_64K. If
> get_perf_callchain then also takes a hash fault on the same page while it
> is already locked, it will loop forever taking hash faults, which looks like
> this:
>
> cpu 0x49e: Vector: 100 (System Reset) at [c0001a4f7d70]
>pc: c0072dc8: hash_page_mm+0x8/0x800
>lr: c000c5a4: do_hash_page+0x24/0x38
>sp: c0002ac1cc69ac70
> msr: 80081033
> current = 0xc0002ac1cc602e00
> paca= 0xc0001de1f280 irqmask: 0x03 irq_happened: 0x01
>pid = 20118, comm = pread2_processe
> Linux version 5.8.0-rc6-00345-g1fad14f18bc6
> 49e:mon> t
> [c0002ac1cc69ac70] c000c5a4 do_hash_page+0x24/0x38 (unreliable)
> --- Exception: 300 (Data Access) at c008fa60
> __copy_tofrom_user_power7+0x20c/0x7ac
> [link register ] c0335d10 copy_from_user_nofault+0xf0/0x150
> [c0002ac1cc69af70] c00032bf9fa3c880 (unreliable)
> [c0002ac1cc69afa0] c0109df0 read_user_stack_64+0x70/0xf0
> [c0002ac1cc69afd0] c0109fcc perf_callchain_user_64+0x15c/0x410
> [c0002ac1cc69b060] c0109c00 perf_callchain_user+0x20/0x40
> [c0002ac1cc69b080] c031c6cc get_perf_callchain+0x25c/0x360
> [c0002ac1cc69b120] c0316b50 perf_callchain+0x70/0xa0
> [c0002ac1cc69b140] c0316ddc perf_prepare_sample+0x25c/0x790
> [c0002ac1cc69b1a0] c0317350 perf_event_output_forward+0x40/0xb0
> [c0002ac1cc69b220] c0306138 __perf_event_overflow+0x88/0x1a0
> [c0002ac1cc69b270] c010cf70 record_and_restart+0x230/0x750
> [c0002ac1cc69b620] c010d69c perf_event_interrupt+0x20c/0x510
> [c0002ac1cc69b730] c0027d9c performance_monitor_exception+0x4c/0x60
> [c0002ac1cc69b750] c000b2f8
> performance_monitor_common_virt+0x1b8/0x1c0
> --- Exception: f00 (Performance Monitor) at c00cb5b0
> pSeries_lpar_hpte_insert+0x0/0x160
> [link register ] c00846f0 __hash_page_64K+0x210/0x540
> [c0002ac1cc69ba50] (unreliable)
> [c0002ac1cc69bb00] c0073ae0 update_mmu_cache+0x390/0x3a0
> [c0002ac1cc69bb70] c037f024 wp_page_copy+0x364/0xce0
> [c0002ac1cc69bc20] c038272c do_wp_page+0xdc/0xa60
> [c0002ac1cc69bc70] c03857bc handle_mm_fault+0xb9c/0x1b60
> [c0002ac1cc69bd50] c006c434 __do_page_fault+0x314/0xc90
> [c0002ac1cc69be20] c000c5c8 handle_page_fault+0x10/0x2c
> --- Exception: 300 (Data Access) at 7fff8c861fe8
> SP (76b19660) is in userspace
>
> Reported-by: Athira Rajeev
> Reported-by: Anton Blanchard
> Reviewed-by: Aneesh Kumar K.V
> Fixes: 2f92447f9f96 ("powerpc/book3s64/hash: Use the pte_t address from the
> caller")
> Signed-off-by: Nicholas Piggin
Hi,
Tested with the patch and it fixes the lockups I was seeing with my test run.
Thanks for the fix.
Tested-by: Athira Rajeev
> ---
> arch/powerpc/kernel/exceptions-64s.S | 14 +++---
> arch/powerpc/mm/book3s64/hash_utils.c | 25 +
> arch/powerpc/perf/core-book3s.c | 6 ++
> 3 files changed, 42 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/kernel/exceptions-64s.S
> b/arch/powerpc/kernel/exceptions-64s.S
> index 0fc8bad878b2..446e54c3f71e 100644
> --- a/arch/powerpc/kernel/exceptions-64s.S
> +++ b/arch/powerpc/kernel/exceptions-64s.S
> @@ -3072,10 +3072,18 @@ do_hash_page:
> ori r0,r0,DSISR_BAD_FAULT_64S@l
> and.r0,r5,r0/* weird error? */
> bne-handle_page_fault /* if not, try to insert a HPTE */
> +
> + /*
> + * If we are in an "NMI" (e.g., an interrupt when soft-disabled), then
> + * don't call hash_page, just fail the fault. This is required to
> + * prevent re-entrancy problems in the hash code, namely perf
> + * interrupts hitting while something holds H_PAGE_BUSY, and taking a
> + * hash fault. See the comment in hash_preload().
> + */
> ld r11, PACA_THREAD_INFO(r13)
> - lwz r0,TI_PREEMPT(r11) /* If we're in an "NMI" */
> - andis. r0,r0,NMI_MASK@h/* (i.e. an irq when soft-disabled) */
> - bne 77f /* then don't call hash_page now */
> + lwz r0,TI_PREEMPT(r11)
> + andis. r0,r0,NMI_MASK@h
> + bne 77f
>
> /*
>* r3 contains the trap number
> diff --git a/arch/powerpc/mm/book3s64/hash_utils.c
> b/arch/powerpc/mm/book3s64/hash_utils.c
> index 468169e33c86..9b9f92ad0e7a 100644
> --- a/arch/powerpc/mm/book3s64/hash_utils.c
> +++ b/arch/powerpc/mm/book3s64/hash_utils.c
> @@
[PATCH] powerpc/64s/hash: Fix hash_preload running with interrupts enabled
Commit 2f92447f9f96 ("powerpc/book3s64/hash: Use the pte_t address from the
caller") removed the local_irq_disable from hash_preload, but it was
required for more than just the page table walk: the hash pte busy bit is
effectively a lock which may be taken in interrupt context, and the local
update flag test must not be preempted before it's used.
This solves apparent lockups with perf interrupting __hash_page_64K. If
get_perf_callchain then also takes a hash fault on the same page while it
is already locked, it will loop forever taking hash faults, which looks like
this:
cpu 0x49e: Vector: 100 (System Reset) at [c0001a4f7d70]
pc: c0072dc8: hash_page_mm+0x8/0x800
lr: c000c5a4: do_hash_page+0x24/0x38
sp: c0002ac1cc69ac70
msr: 80081033
current = 0xc0002ac1cc602e00
paca= 0xc0001de1f280 irqmask: 0x03 irq_happened: 0x01
pid = 20118, comm = pread2_processe
Linux version 5.8.0-rc6-00345-g1fad14f18bc6
49e:mon> t
[c0002ac1cc69ac70] c000c5a4 do_hash_page+0x24/0x38 (unreliable)
--- Exception: 300 (Data Access) at c008fa60
__copy_tofrom_user_power7+0x20c/0x7ac
[link register ] c0335d10 copy_from_user_nofault+0xf0/0x150
[c0002ac1cc69af70] c00032bf9fa3c880 (unreliable)
[c0002ac1cc69afa0] c0109df0 read_user_stack_64+0x70/0xf0
[c0002ac1cc69afd0] c0109fcc perf_callchain_user_64+0x15c/0x410
[c0002ac1cc69b060] c0109c00 perf_callchain_user+0x20/0x40
[c0002ac1cc69b080] c031c6cc get_perf_callchain+0x25c/0x360
[c0002ac1cc69b120] c0316b50 perf_callchain+0x70/0xa0
[c0002ac1cc69b140] c0316ddc perf_prepare_sample+0x25c/0x790
[c0002ac1cc69b1a0] c0317350 perf_event_output_forward+0x40/0xb0
[c0002ac1cc69b220] c0306138 __perf_event_overflow+0x88/0x1a0
[c0002ac1cc69b270] c010cf70 record_and_restart+0x230/0x750
[c0002ac1cc69b620] c010d69c perf_event_interrupt+0x20c/0x510
[c0002ac1cc69b730] c0027d9c performance_monitor_exception+0x4c/0x60
[c0002ac1cc69b750] c000b2f8
performance_monitor_common_virt+0x1b8/0x1c0
--- Exception: f00 (Performance Monitor) at c00cb5b0
pSeries_lpar_hpte_insert+0x0/0x160
[link register ] c00846f0 __hash_page_64K+0x210/0x540
[c0002ac1cc69ba50] (unreliable)
[c0002ac1cc69bb00] c0073ae0 update_mmu_cache+0x390/0x3a0
[c0002ac1cc69bb70] c037f024 wp_page_copy+0x364/0xce0
[c0002ac1cc69bc20] c038272c do_wp_page+0xdc/0xa60
[c0002ac1cc69bc70] c03857bc handle_mm_fault+0xb9c/0x1b60
[c0002ac1cc69bd50] c006c434 __do_page_fault+0x314/0xc90
[c0002ac1cc69be20] c000c5c8 handle_page_fault+0x10/0x2c
--- Exception: 300 (Data Access) at 7fff8c861fe8
SP (76b19660) is in userspace
Reported-by: Athira Rajeev
Reported-by: Anton Blanchard
Reviewed-by: Aneesh Kumar K.V
Fixes: 2f92447f9f96 ("powerpc/book3s64/hash: Use the pte_t address from the
caller")
Signed-off-by: Nicholas Piggin
---
arch/powerpc/kernel/exceptions-64s.S | 14 +++---
arch/powerpc/mm/book3s64/hash_utils.c | 25 +
arch/powerpc/perf/core-book3s.c | 6 ++
3 files changed, 42 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/exceptions-64s.S
b/arch/powerpc/kernel/exceptions-64s.S
index 0fc8bad878b2..446e54c3f71e 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -3072,10 +3072,18 @@ do_hash_page:
ori r0,r0,DSISR_BAD_FAULT_64S@l
and.r0,r5,r0/* weird error? */
bne-handle_page_fault /* if not, try to insert a HPTE */
+
+ /*
+* If we are in an "NMI" (e.g., an interrupt when soft-disabled), then
+* don't call hash_page, just fail the fault. This is required to
+* prevent re-entrancy problems in the hash code, namely perf
+* interrupts hitting while something holds H_PAGE_BUSY, and taking a
+* hash fault. See the comment in hash_preload().
+*/
ld r11, PACA_THREAD_INFO(r13)
- lwz r0,TI_PREEMPT(r11) /* If we're in an "NMI" */
- andis. r0,r0,NMI_MASK@h/* (i.e. an irq when soft-disabled) */
- bne 77f /* then don't call hash_page now */
+ lwz r0,TI_PREEMPT(r11)
+ andis. r0,r0,NMI_MASK@h
+ bne 77f
/*
* r3 contains the trap number
diff --git a/arch/powerpc/mm/book3s64/hash_utils.c
b/arch/powerpc/mm/book3s64/hash_utils.c
index 468169e33c86..9b9f92ad0e7a 100644
--- a/arch/powerpc/mm/book3s64/hash_utils.c
+++ b/arch/powerpc/mm/book3s64/hash_utils.c
@@ -1559,6 +1559,7 @@ static void hash_preload(struct mm_struct *mm, pte_t
*ptep, unsigned long ea,
pgd_t *pgdir;
int rc, ssize, update_flags = 0;
unsigned long access = _PAGE_PRESENT | _PAGE_READ | (is_exec ?
_PAGE_EXEC : 0);
+
