From: Thomas Graziadei <thomas.graziadei@omicronenergy.com>
To: "'Sebastian Andrzej Siewior'" <bigeasy@linutronix.de>,
Mark Marshall <markmarshall14@gmail.com>
Cc: linux-rt-users <linux-rt-users@vger.kernel.org>,
Mark Marshall <mark.marshall@omicronenergy.com>,
Thomas Gleixner <tglx@linutronix.de>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"rostedt@goodmis.org" <rostedt@goodmis.org>
Subject: RE: Kernel crash due to memory corruption with v5.4.26-rt17 and PowerPC e500
Date: Fri, 10 Jul 2020 10:59:00 +0000 [thread overview]
Message-ID: <eeeae7d9-c028-47f0-ab5b-7983e32f36cb@EXC03-ATKLA.omicron.at> (raw)
In-Reply-To: <20200706165004.7m57fvspmwnjcjxh@linutronix.de>
[-- Attachment #1: Type: text/plain, Size: 4133 bytes --]
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
[-- Attachment #2: check.patch --]
[-- Type: application/octet-stream, Size: 437 bytes --]
diff --git a/fs/exec.c b/fs/exec.c
index 77603ceed51f9..1310fb4d5f0d4 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1037,8 +1037,12 @@ static int exec_mmap(struct mm_struct *mm)
task_lock(tsk);
active_mm = tsk->active_mm;
membarrier_exec_mmap(mm);
+
+ preempt_disable();
tsk->mm = mm;
tsk->active_mm = mm;
+ preempt_enable();
+
activate_mm(active_mm, mm);
tsk->mm->vmacache_seqnum = 0;
vmacache_flush(tsk);
next prev parent reply other threads:[~2020-07-10 11:08 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-04 9:40 Kernel crash due to memory corruption with v5.4.26-rt17 and PowerPC e500 Mark Marshall
2020-05-29 13:14 ` Sebastian Andrzej Siewior
2020-05-29 15:38 ` Mark Marshall
2020-05-29 16:15 ` Sebastian Andrzej Siewior
2020-05-29 16:37 ` Sebastian Andrzej Siewior
2020-07-06 16:50 ` Sebastian Andrzej Siewior
2020-07-10 10:59 ` Thomas Graziadei [this message]
2020-08-12 12:45 ` Thomas Graziadei
2020-08-19 7:11 ` 'Sebastian Andrzej Siewior'
2020-09-01 7:41 ` 'Sebastian Andrzej Siewior'
2020-05-29 19:03 ` Mark Marshall
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=eeeae7d9-c028-47f0-ab5b-7983e32f36cb@EXC03-ATKLA.omicron.at \
--to=thomas.graziadei@omicronenergy.com \
--cc=bigeasy@linutronix.de \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rt-users@vger.kernel.org \
--cc=mark.marshall@omicronenergy.com \
--cc=markmarshall14@gmail.com \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).