On Mon, 12 Mar 2018 14:05:52 -0500 "Steven J. Hill"
wrote:
> Attempting to hotplug CPUs with CONFIG_VM_EVENT_COUNTERS enabled
> can cause vmstat_update() to report a BUG due to preemption not
> being disabled around smp_processor_id(). Discovered on Ubiquiti
> EdgeRouter Pro with Cavium Octeon II processor.
>
> BUG: using smp_processor_id() in preemptible [] code:
> kworker/1:1/269
> caller is vmstat_update+0x50/0xa0
> CPU: 0 PID: 269 Comm: kworker/1:1 Not tainted
> 4.16.0-rc4-Cavium-Octeon-9-gf83bbd5-dirty #1
> Workqueue: mm_percpu_wq vmstat_update
> Stack : 00260026 10009ce0 0001
> 0005 800118000800
> 00bf 00bf 766d737461745f75
> 83ad 0007 0800
> 818d 0001 81818a70
> 8115bbb0 818a
> 0005 8144dc50
> 80008898 800088983b30 0088 813d3054
> 83ace622 00be
> 00be 81121fb4
> ...
> Call Trace:
> [] show_stack+0x94/0x128
> [] dump_stack+0xa4/0xe0
> [] check_preemption_disabled+0x118/0x120
> [] vmstat_update+0x50/0xa0
> [] process_one_work+0x144/0x348
> [] worker_thread+0x150/0x4b8
> [] kthread+0x110/0x140
> [] ret_from_kernel_thread+0x14/0x1c
>
> ...
>
> --- a/mm/vmstat.c
> +++ b/mm/vmstat.c
> @@ -1839,9 +1839,11 @@ 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();
> }
> }
hm, I suspect this warning is a false-positive. vmstat_update() is
called from a workqueue and so vmstat_update()'s execution is pinned to
a particular CPU, so smp_processor_id()'s return will not be
invalidated by a preemption-induced CPU switch. Unless the workqueue
code changed more than I think it did ;)
The patch is OK and will work, but I wonder if a better fix is to use
raw_smp_processor_id() and to add a little comment explaining why it's
needed and is safe.
