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: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
