lkml.org 
[lkml]   [2020]   [Jul]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
SubjectRE: Kernel crash due to memory corruption with v5.4.26-rt17 and PowerPC e500
Date
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:bigeasy@linutronix.de]
Sent: Monday, July 06, 2020 6:50 PM
To: Mark Marshall <markmarshall14@gmail.com>
Cc: linux-rt-users <linux-rt-users@vger.kernel.org>; Mark Marshall <mark.marshall@omicronenergy.com>; Thomas Graziadei <thomas.graziadei@omicronenergy.com>; Thomas Gleixner <tglx@linutronix.de>; linux-kernel@vger.kernel.org; rostedt@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
[unhandled content-type:application/octet-stream]
\
 
 \ /
  Last update: 2020-07-10 13:09    [W:0.083 / U:0.028 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site