All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christophe Leroy <christophe.leroy@csgroup.eu>
To: Guenter Roeck <linux@roeck-us.net>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Paul Mackerras <paulus@samba.org>,
	Michael Ellerman <mpe@ellerman.id.au>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>
Subject: Re: [PATCH v1 3/4] powerpc/code-patching: Use jump_label for testing freed initmem
Date: Thu, 19 May 2022 06:27:40 +0000	[thread overview]
Message-ID: <ab09a24a-acca-a5a8-2e3f-cfa3c704cb65@csgroup.eu> (raw)
In-Reply-To: <20220519021706.GA3526833@roeck-us.net>



Le 19/05/2022 à 04:17, Guenter Roeck a écrit :
> On Tue, Mar 22, 2022 at 04:40:20PM +0100, Christophe Leroy wrote:
>> Once init is done, initmem is freed forever so no need to
>> test system_state at every call to patch_instruction().
>>
>> Use jump_label.
>>
>> This reduces by 2% the time needed to activate ftrace on an 8xx.
>>
> 
> It also causes the qemu mpc8544ds emulation to crash.
> 
> BUG: Unable to handle kernel data access on write at 0xc122eb34
> Faulting instruction address: 0xc001b580
> Oops: Kernel access of bad area, sig: 11 [#1]
> BE PAGE_SIZE=4K MPC8544 DS
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper Not tainted 5.18.0-rc7-next-20220518 #1
> NIP:  c001b580 LR: c001b560 CTR: 00000003
> REGS: c5107dd0 TRAP: 0300   Not tainted  (5.18.0-rc7-next-20220518)
> MSR:  00009000 <EE,ME>  CR: 24000882  XER: 00000000
> DEAR: c122eb34 ESR: 00800000
> GPR00: c001b560 c5107ec0 c5120020 10000000 00000000 00000078 0c000000 cfffffff
> GPR08: c001e9ec 00000001 00000007 00000000 44000882 00000000 c0005178 00000000
> GPR16: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> GPR24: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 c1230000
> NIP [c001b580] free_initmem+0x48/0xa8
> LR [c001b560] free_initmem+0x28/0xa8
> Call Trace:
> [c5107ec0] [c001b560] free_initmem+0x28/0xa8 (unreliable)
> [c5107ee0] [c00051b0] kernel_init+0x38/0x150
> [c5107f00] [c001626c] ret_from_kernel_thread+0x5c/0x64
> Instruction dump:
> 3fe0c123 912a00dc 90010024 48000665 3d20c218 8929fa65 2c090000 41820058
> 813feb34 2c090000 4082003c 39200001 <913feb34> 80010024 3cc0c114 83e1001c
> 
> Reverting this patch fixes the problem.
> 

That's strange.

I was able to reproduce the problem.

Removing the __ro_after_init in front of 
DEFINE_STATIC_KEY_FALSE(init_mem_is_free) fixes the problem.

I can't understand why, mark_readonly() is called after free_initmem().

Christophe

WARNING: multiple messages have this Message-ID (diff)
From: Christophe Leroy <christophe.leroy@csgroup.eu>
To: Guenter Roeck <linux@roeck-us.net>
Cc: "linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
	Paul Mackerras <paulus@samba.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v1 3/4] powerpc/code-patching: Use jump_label for testing freed initmem
Date: Thu, 19 May 2022 06:27:40 +0000	[thread overview]
Message-ID: <ab09a24a-acca-a5a8-2e3f-cfa3c704cb65@csgroup.eu> (raw)
In-Reply-To: <20220519021706.GA3526833@roeck-us.net>



Le 19/05/2022 à 04:17, Guenter Roeck a écrit :
> On Tue, Mar 22, 2022 at 04:40:20PM +0100, Christophe Leroy wrote:
>> Once init is done, initmem is freed forever so no need to
>> test system_state at every call to patch_instruction().
>>
>> Use jump_label.
>>
>> This reduces by 2% the time needed to activate ftrace on an 8xx.
>>
> 
> It also causes the qemu mpc8544ds emulation to crash.
> 
> BUG: Unable to handle kernel data access on write at 0xc122eb34
> Faulting instruction address: 0xc001b580
> Oops: Kernel access of bad area, sig: 11 [#1]
> BE PAGE_SIZE=4K MPC8544 DS
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper Not tainted 5.18.0-rc7-next-20220518 #1
> NIP:  c001b580 LR: c001b560 CTR: 00000003
> REGS: c5107dd0 TRAP: 0300   Not tainted  (5.18.0-rc7-next-20220518)
> MSR:  00009000 <EE,ME>  CR: 24000882  XER: 00000000
> DEAR: c122eb34 ESR: 00800000
> GPR00: c001b560 c5107ec0 c5120020 10000000 00000000 00000078 0c000000 cfffffff
> GPR08: c001e9ec 00000001 00000007 00000000 44000882 00000000 c0005178 00000000
> GPR16: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> GPR24: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 c1230000
> NIP [c001b580] free_initmem+0x48/0xa8
> LR [c001b560] free_initmem+0x28/0xa8
> Call Trace:
> [c5107ec0] [c001b560] free_initmem+0x28/0xa8 (unreliable)
> [c5107ee0] [c00051b0] kernel_init+0x38/0x150
> [c5107f00] [c001626c] ret_from_kernel_thread+0x5c/0x64
> Instruction dump:
> 3fe0c123 912a00dc 90010024 48000665 3d20c218 8929fa65 2c090000 41820058
> 813feb34 2c090000 4082003c 39200001 <913feb34> 80010024 3cc0c114 83e1001c
> 
> Reverting this patch fixes the problem.
> 

That's strange.

I was able to reproduce the problem.

Removing the __ro_after_init in front of 
DEFINE_STATIC_KEY_FALSE(init_mem_is_free) fixes the problem.

I can't understand why, mark_readonly() is called after free_initmem().

Christophe

  reply	other threads:[~2022-05-19  6:27 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-22 15:40 [PATCH v1 0/4] Kill the time spent in patch_instruction() Christophe Leroy
2022-09-27 14:33 ` Christophe Leroy
2022-09-27 14:33 ` Christophe Leroy
2022-03-22 15:40 ` Christophe Leroy
2022-03-22 15:40 ` [PATCH v1 1/4] powerpc/code-patching: Don't call is_vmalloc_or_module_addr() without CONFIG_MODULES Christophe Leroy
2022-09-27 14:33   ` Christophe Leroy
2022-09-27 14:33   ` Christophe Leroy
2022-03-22 15:40   ` Christophe Leroy
2022-03-22 15:40 ` [PATCH v1 2/4] powerpc/code-patching: Speed up page mapping/unmapping Christophe Leroy
2022-09-27 14:33   ` Christophe Leroy
2022-09-27 14:33   ` Christophe Leroy
2022-03-22 15:40   ` Christophe Leroy
2022-03-22 15:40 ` [PATCH v1 3/4] powerpc/code-patching: Use jump_label for testing freed initmem Christophe Leroy
2022-09-27 14:33   ` Christophe Leroy
2022-09-27 14:33   ` Christophe Leroy
2022-03-22 15:40   ` Christophe Leroy
2022-05-19  2:17   ` Guenter Roeck
2022-05-19  2:17     ` Guenter Roeck
2022-05-19  6:27     ` Christophe Leroy [this message]
2022-05-19  6:27       ` Christophe Leroy
2022-05-19  6:53       ` Christophe Leroy
2022-05-19  6:53         ` Christophe Leroy
2022-05-19 17:27         ` Christophe Leroy
2022-05-19 17:27           ` Christophe Leroy
2022-03-22 15:40 ` [PATCH v1 4/4] powerpc/code-patching: Use jump_label to check if poking_init() is done Christophe Leroy
2022-09-27 14:33   ` Christophe Leroy
2022-09-27 14:33   ` Christophe Leroy
2022-03-22 15:40   ` Christophe Leroy
2022-05-15 10:28 ` [PATCH v1 0/4] Kill the time spent in patch_instruction() Michael Ellerman
2022-05-17  6:44   ` Christophe Leroy
2022-05-17 12:37     ` Michael Ellerman
2022-05-31  6:24       ` Christophe Leroy
2022-06-24  7:06         ` Christophe Leroy
2022-09-27 14:33 ` [PATCH v1 1/6] powerpc/code-patching: Use pte_offset_kernel() instead of virt_to_kpte() Christophe Leroy
2022-09-27 14:33   ` Christophe Leroy
2022-09-27 14:33   ` [PATCH v1 2/6] powerpc/code-patching: Remove #ifdef CONFIG_STRICT_KERNEL_RWX Christophe Leroy
2022-09-27 14:33     ` Christophe Leroy
2022-09-27 14:33   ` [PATCH v1 3/6] powerpc/feature-fixups: Refactor entry fixups patching Christophe Leroy
2022-09-27 14:33     ` Christophe Leroy
2022-09-27 14:33   ` [PATCH v1 4/6] powerpc/feature-fixups: Refactor other " Christophe Leroy
2022-09-27 14:33     ` Christophe Leroy
2022-09-27 14:33   ` [PATCH v1 5/6] powerpc/feature-fixups: Do not patch init section after init Christophe Leroy
2022-09-27 14:33     ` Christophe Leroy
2022-09-27 14:33   ` [PATCH v1 6/6] powerpc/code-patching: Remove protection against patching init addresses " Christophe Leroy
2022-09-27 14:33     ` Christophe Leroy
2022-09-27 14:38 ` [PATCH v1 0/4] Kill the time spent in patch_instruction() Christophe Leroy
2022-09-27 14:38   ` Christophe Leroy

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=ab09a24a-acca-a5a8-2e3f-cfa3c704cb65@csgroup.eu \
    --to=christophe.leroy@csgroup.eu \
    --cc=benh@kernel.crashing.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mpe@ellerman.id.au \
    --cc=paulus@samba.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.