[linux-yocto] [PATCH] Revert mm/vmstat.c: fix vmstat_update() preemption BUG

[Bruce Ashfield]

On 2018-07-24 9:47 PM, He Zhe wrote:
> 
> 
> On 2018年07月25日 04:07, Bruce Ashfield wrote:
>> On 2018-07-23 10:07 PM, He Zhe wrote:
>>>
>>>
>>> On 2018年07月24日 03:21, Bruce Ashfield wrote:
>>>> Which version/branches is this for ?
>>>>
>>>
>>> This is for linux-yocto rt branches. linux-yocto-4.12 does not need this
>>> fix, since the commit, see below, causing the issue have not been included.
>>
>> Rather than me guessing. Which versions are you looking at ? I've got
>> active -stable updates for many of the linux-yocto trees, so any number
>> could be impacted.
> 
> I'm looking at
> git://git.yoctoproject.org/linux-yocto
> v4.14/standard/preempt-rt/base
> v4.14/standard/preempt-rt/intel
> 
> I want to fix
> http://git.yoctoproject.org/cgit/cgit.cgi/linux-yocto/commit/?h=v4.14/standard/preempt-rt/base&id=08e9dbd5184e4e059adf1cc77b5dc08eca314a77
> by porting
> https://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-stable-rt.git/commit/?h=v4.14-rt&id=ac7768148a10fe7e934a7a510cc067883163f115
> 
> 

thanks for the details. This is now merged.

Bruce

> Thanks,
> Zhe
> 
> 
>>
>> Bruce
>>
>>>
>>> git://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-stable-rt.git
>>>
>>> commit 08e9dbd5184e4e059adf1cc77b5dc08eca314a77
>>> Author:     Steven J. Hill <steven.hill at cavium.com>
>>> AuthorDate: Wed Mar 28 16:01:09 2018 -0700
>>> Commit:     Greg Kroah-Hartman <gregkh at linuxfoundation.org>
>>> CommitDate: Wed May 30 07:52:21 2018 +0200
>>>
>>>       mm/vmstat.c: fix vmstat_update() preemption BUG
>>>
>>>
>>> Thanks,
>>> Zhe
>>>
>>>> Given the author, I'm assuming it is for the -rt branches and also
>>>> for 4.12 .. but I'd like a confirmation of that.
>>>>
>>>> Bruce
>>>>
>>>> On 2018-07-20 4:13 AM, zhe.he at windriver.com wrote:
>>>>> From: Sebastian Andrzej Siewior <bigeasy at linutronix.de>
>>>>>
>>>>> commit 97731753d44d5efcb95b994dc952c0e8195b3e96 upstream
>>>>>
>>>>> This patch reverts commit c7f26ccfb2c3 ("mm/vmstat.c: fix
>>>>> vmstat_update() preemption BUG").
>>>>> Steven saw a "using smp_processor_id() in preemptible" message and
>>>>> added a preempt_disable() section around it to keep it quiet. This is
>>>>> not the right thing to do it does not fix the real problem.
>>>>>
>>>>> vmstat_update() is invoked by a kworker on a specific CPU. This worker
>>>>> it bound to this CPU. The name of the worker was "kworker/1:1" so it
>>>>> should have been a worker which was bound to CPU1. A worker which can
>>>>> run on any CPU would have a `u' before the first digit.
>>>>>
>>>>> smp_processor_id() can be used in a preempt-enabled region as long as
>>>>> the task is bound to a single CPU which is the case here. If it could
>>>>> run on an arbitrary CPU then this is the problem we have an should seek
>>>>> to resolve.
>>>>> Not only this smp_processor_id() must not be migrated to another CPU but
>>>>> also refresh_cpu_vm_stats() which might access wrong per-CPU variables.
>>>>> Not to mention that other code relies on the fact that such a worker
>>>>> runs on one specific CPU only.
>>>>>
>>>>> Therefore I revert that commit and we should look instead what broke the
>>>>> affinity mask of the kworker.
>>>>>
>>>>> Cc: Steven J. Hill <steven.hill at cavium.com>
>>>>> Cc: Tejun Heo <htejun at gmail.com>
>>>>> Cc: Andrew Morton <akpm at linux-foundation.org>
>>>>> Signed-off-by: Sebastian Andrzej Siewior <bigeasy at linutronix.de>
>>>>> Signed-off-by: Steven Rostedt (VMware) <rostedt at goodmis.org>
>>>>>
>>>>> Backport this to fix following serious crash for preempt-rt kernel
>>>>>
>>>>> BUG: scheduling while atomic: kworker/3:1/91/0x00000002
>>>>> Preemption disabled at:
>>>>> [<ffffffff8437c37d>] vmstat_update+0x2d/0xb0
>>>>> CPU: 3 PID: 91 Comm: kworker/3:1 Not tainted 4.14.48-rt29-yocto-preempt-rt #1
>>>>> Hardware name: Intel Corporation Broadwell Client platform/Basking Ridge, BIOS BDW-E2R1.86C.0118.R01.1503110618 03/11/2015
>>>>> Workqueue: mm_percpu_wq vmstat_update
>>>>> Call Trace:
>>>>>     dump_stack+0x4f/0x6b
>>>>>     ? vmstat_update+0x2d/0xb0
>>>>>     __schedule_bug.cold.24+0x7d/0x9a
>>>>>     __schedule+0x45a/0x6c0
>>>>>     ? task_blocks_on_rt_mutex+0x173/0x300
>>>>>     schedule+0x3d/0xd0
>>>>>     rt_spin_lock_slowlock_locked+0x118/0x2a0
>>>>>     rt_spin_lock_slowlock+0x48/0x60
>>>>>     rt_spin_lock+0x3f/0x50
>>>>>     queue_delayed_work_on+0x63/0x100
>>>>>     vmstat_update+0x6e/0xb0
>>>>>     process_one_work+0x1c3/0x430
>>>>>     worker_thread+0x32/0x440
>>>>>     kthread+0x127/0x140
>>>>>     ? process_one_work+0x430/0x430
>>>>>     ? kthread_create_on_node+0x40/0x40
>>>>>     ret_from_fork+0x35/0x40
>>>>>
>>>>> Signed-off-by: He Zhe <zhe.he at windriver.com>
>>>>> ---
>>>>>     mm/vmstat.c | 2 --
>>>>>     1 file changed, 2 deletions(-)
>>>>>
>>>>> diff --git a/mm/vmstat.c b/mm/vmstat.c
>>>>> index 84fd6eb..0d17b8f 100644
>>>>> --- a/mm/vmstat.c
>>>>> +++ b/mm/vmstat.c
>>>>> @@ -1782,11 +1782,9 @@ static void vmstat_update(struct work_struct *w)
>>>>>              * to occur in the future. Keep on running the
>>>>>              * update worker thread.
>>>>>              */
>>>>> -        preempt_disable();
>>>>>             queue_delayed_work_on(smp_processor_id(), mm_percpu_wq,
>>>>>                     this_cpu_ptr(&vmstat_work),
>>>>>                     round_jiffies_relative(sysctl_stat_interval));
>>>>> -        preempt_enable();
>>>>>         }
>>>>>     }
>>>>>    
>>>>
>>>>
>>>
>>
>>
>