Re: [PATCH/RFC] m68k: Add full preempt support

From: Michael Schmitz <schmitzmic@gmail.com>
To: Geert Uytterhoeven <geert@linux-m68k.org>,
	Greg Ungerer <gerg@linux-m68k.org>,
	Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Matthew Wilcox <willy@infradead.org>
Cc: linux-m68k@lists.linux-m68k.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH/RFC] m68k: Add full preempt support
Date: Tue, 17 Oct 2023 09:33:40 +1300	[thread overview]
Message-ID: <7286cd41-4f06-8afc-b8c5-acc617771daf@gmail.com> (raw)
In-Reply-To: <CAMuHMdUQ72KOPw5vxNfhjoTR-SsaELeKneBmyQPYEWa_o5OZZA@mail.gmail.com>

Hi Geert,

On 17/10/23 01:23, Geert Uytterhoeven wrote:
> On Fri, Oct 6, 2023 at 4:49 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>> Currently, a Preemptible Kernel is only supported on the Coldfire CPU
>> family.  Extend preempt support to the full Classic M68K CPU family
>> (68020+ with MMU, and 68000-derivatives without MMU).
>>
>> Make sure preemption is disabled in loops involving cache and TLB
>> flushing.
>>
>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> ---
>> Tested on ARAnyM, using none/voluntary/full preemption.
>> Compile-tested on 68000.
>>
>> I don't know if all of the preempt_{dis,en}able() pairs are really
>> needed, or if we need more of them.  I just followed what some other
>> architectures are doing.
>>
>> Without the preempt_{dis,en}able() pairs, the kernel panics on ARAnyM while
>> starting early userspace (Debian 4.0), cfr. below.
>> Does full preemption work on Coldfire without?
>> Thanks for your comments!
>>
>>      BUG: Bad rss-counter state mm:93ade98e type:MM_FILEPAGES val:-7
>>      BUG: Bad rss-counter state mm:93ade98e type:MM_ANONPAGES val:-1
>>      Kernel panic - not syncing: table already free!
> Ran into one of these again, once, so probably we need more
> preempt_{dis,en}able() pairs, but where?
>
>      BUG: Bad rss-counter state mm:9bb416c6 type:MM_FILEPAGES val:-20
>      BUG: Bad rss-counter state mm:9bb416c6 type:MM_ANONPAGES val:-6

Is that significant here? In particular, does this condition cause a 
fault signal sent to the associated process?

If I read the mmput() code right, this is just a warning happening 
during process exit. The stack trace suggests that process exit was 
forced by a signal.

Maybe we should disable preemption until signal handling is complete?

Your patch to entry.S does not check for pending signals before 
preempting the kernel on return from exception or interrupt ...

Cheers,

     Michael

>      Kernel panic - not syncing: table already free!
>      CPU: 0 PID: 480 Comm: grep Not tainted
> 6.6.0-rc6-atari-01887-g1ab3ae298649 #1601
>      Stack from 018c9c8c:
>              018c9c8c 0048a940 0048a940 00000000 00000001 003c0caa
> 0048a940 003b9b96
>              00000001 00000001 01a58000 01a62100 01a5b000 000cf316
> 018c9dd2 00006440
>              0047d67f 80039000 c0000000 01a58b00 80039000 80000000
> c0000000 01a62100
>              01a5b000 000cfd0e 01a58b00 00000001 ffffffff 018c9db2
> 00000000 000cfb36
>              80000000 01a60a68 01a60a68 003aa02e 000dbd0a 000d557c
> 80000000 80038fff
>              80038fff bfffffff 000cfd8e 018c9dd2 80000000 80039000
> 00000000 c0000000
>      Call Trace: [<003c0caa>] dump_stack+0xc/0x10
>       [<003b9b96>] panic+0xd8/0x256
>       [<000cf316>] pmd_offset.isra.0+0x0/0x20
>       [<00006440>] free_pointer_table+0x68/0x124
>       [<000cfd0e>] free_pgd_range+0x1d8/0x1ea
>       [<000cfb36>] free_pgd_range+0x0/0x1ea
>       [<003aa02e>] mas_find+0x0/0x4c
>       [<000dbd0a>] unlink_anon_vmas+0x0/0x108
>       [<000d557c>] unlink_file_vma+0x0/0x40
>       [<000cfd8e>] free_pgtables+0x6e/0x80
>       [<00045eec>] up_read+0x0/0x62
>       [<000d63fc>] exit_mmap+0x118/0x1ee
>       [<0000c001>] asinbig+0x25/0x34
>       [<0000ffff>] EXPTBL+0x2eb/0x400
>       [<0000ffff>] EXPTBL+0x2eb/0x400
>       [<00021f10>] __mmput+0x18/0xae
>       [<00025cce>] do_exit+0x2b4/0x7ba
>       [<00026316>] do_group_exit+0x1e/0x96
>       [<0002e354>] get_signal+0x19c/0x694
>       [<000032fc>] test_ti_thread_flag+0x0/0x14
>       [<0002d6b6>] force_sig_info_to_task+0xb0/0xea
>       [<00003caa>] do_notify_resume+0x36/0x4a0
>       [<0002d970>] force_sig_fault_to_task+0x32/0x3e
>       [<0002d990>] force_sig_fault+0x14/0x1a
>       [<0000597c>] send_fault_sig+0x24/0x84
>       [<00002632>] do_signal_return+0x10/0x1a
>       [<00088000>] adjust_reg_min_max_vals+0x106/0x140c
>
>      ---[ end Kernel panic - not syncing: table already free! ]---
>
> Gr{oetje,eeting}s,
>
>                          Geert
>