Hi Sebastian, thanks for looking into this.
We could reproduce the issue with QEMU. At runtime you need to set mdev as the kernel's hotplug client (/proc/sys/kernel/hotplug) and give it a dummy /etc/mdev.conf like (.* 1:1 777). Then just do a loop and insmod/rmmod crc4.ko and crc7.ko. Swapping the mm assignment did not work -> exception after 1900 iterations Your second suggestion with check.patch (attached to this email for completeness, only protecting the exec_mmap function) did not work eighter -> exception after 2600 iterations Your third suggestion (a modification to the original revert) enclosed in this e-mail does seem to work. Still no problems after 30000 iterations. By the way, as noticed in your kernel config, we would be quite interested in a gcc 9 compiler for our platform. Is there a mainline/maintained version or fork for this or another possibility to get it? Regards, Thomas -----Original Message----- From: Sebastian Andrzej Siewior [mailto:bige...@linutronix.de] Sent: Monday, July 06, 2020 6:50 PM To: Mark Marshall <markmarshal...@gmail.com> Cc: linux-rt-users <linux-rt-us...@vger.kernel.org>; Mark Marshall <mark.marsh...@omicronenergy.com>; Thomas Graziadei <thomas.grazia...@omicronenergy.com>; Thomas Gleixner <t...@linutronix.de>; linux-kernel@vger.kernel.org; rost...@goodmis.org Subject: Re: Kernel crash due to memory corruption with v5.4.26-rt17 and PowerPC e500 On 2020-05-29 18:37:22 [+0200], To Mark Marshall wrote: > On 2020-05-29 18:15:18 [+0200], To Mark Marshall wrote: > > In order to get it back into the RT queue I need to understand why > > it is required. What exactly is it fixing. Let me stare at for a > > little… > > it used to be local_irq_disable() which then became preempt_disable() > local_irq_disable() due to ARM's limitation. Any luck on your side? I *think* if you swap the mm assignment in exec_mmap() then it should be gone. Basically: | tsk->active_mm = mm; | tsk->mm = mm; However I think to apply something like this: diff --git a/fs/exec.c b/fs/exec.c --- a/fs/exec.c +++ b/fs/exec.c @@ -1035,11 +1035,15 @@ static int exec_mmap(struct mm_struct *mm) } } task_lock(tsk); + + task_lock_mm(); active_mm = tsk->active_mm; membarrier_exec_mmap(mm); tsk->mm = mm; tsk->active_mm = mm; activate_mm(active_mm, mm); + task_unlock_mm(); + tsk->mm->vmacache_seqnum = 0; vmacache_flush(tsk); task_unlock(tsk); diff --git a/include/linux/sched/task.h b/include/linux/sched/task.h --- a/include/linux/sched/task.h +++ b/include/linux/sched/task.h @@ -176,4 +176,31 @@ static inline void task_unlock(struct task_struct *p) spin_unlock(&p->alloc_lock); } +#ifdef CONFIG_PREEMPT_RT +/* + * Protects ->mm and ->active_mm. + * Avoids scheduling so switch_mm() or enter_lazy_tlb() will not read +the + * members while they are updated. + */ +static inline void task_lock_mm(void) +{ + preempt_disable(); +} + +static inline void task_unlock_mm(void) { + preempt_enable(); +} + +#else + +static inline void task_lock_mm(void) +{ +} + +static inline void task_unlock_mm(void) { } #endif + #endif /* _LINUX_SCHED_TASK_H */ diff --git a/mm/mmu_context.c b/mm/mmu_context.c --- a/mm/mmu_context.c +++ b/mm/mmu_context.c @@ -25,6 +25,7 @@ void use_mm(struct mm_struct *mm) struct task_struct *tsk = current; task_lock(tsk); + task_lock_mm(); active_mm = tsk->active_mm; if (active_mm != mm) { mmgrab(mm); @@ -32,6 +33,7 @@ void use_mm(struct mm_struct *mm) } tsk->mm = mm; switch_mm(active_mm, mm, tsk); + task_unlock_mm(); task_unlock(tsk); #ifdef finish_arch_post_lock_switch finish_arch_post_lock_switch(); @@ -55,10 +57,12 @@ void unuse_mm(struct mm_struct *mm) struct task_struct *tsk = current; task_lock(tsk); + task_lock_mm(); sync_mm_rss(mm); tsk->mm = NULL; /* active_mm is still 'mm' */ enter_lazy_tlb(mm, tsk); + task_unlock_mm(); task_unlock(tsk); } EXPORT_SYMBOL_GPL(unuse_mm); -- 2.27.0 > > > Best regards, > > > Mark Sebastian
check.patch
Description: check.patch
