lkml.org 
[lkml]   [2020]   [Aug]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: Kernel crash due to memory corruption with v5.4.26-rt17 and PowerPC e500
From
Date
Hi Sebastian,

any progress on your side?

Do you think the patch could be applied for the next versions?

Regards,
Thomas

On Fri, 2020-07-10 at 10:59 +0000, Thomas Graziadei wrote:
> 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

\
 
 \ /
  Last update: 2020-08-12 14:58    [W:0.061 / U:0.652 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site