Messages in this thread | | | Subject | Re: [RFC][PATCH 2/5] mm: Switch mod_state() to __this_cpu_read() | From | Steven Rostedt <> | Date | Mon, 19 Sep 2011 19:48:09 -0400 |
| |
On Mon, 2011-09-19 at 17:02 -0500, Christoph Lameter wrote: > On Mon, 19 Sep 2011, Steven Rostedt wrote: > > > From: Steven Rostedt <srostedt@redhat.com> > > > > The code in mod_state() is already made to handle the raciness of > > this_cpu_read(). Have the code use __this_cpu_read() instead so > > the debug code does not trigger warnings about it. > > Why would there be a warning triggered? this_cpu_read should take care of > disabling preemption for the read if needed. In fact the fallback case > does do exactly that.
What does disabling preemption for just reading the cpu variable help anywhere? Once you read the variable, and enable preemption, you can migrate. Then that info you have could be useless.
> > I think it would make more sence if __this_cpu_read() could be made to > trigger a warning if used in context where preemption could be off.
Hmm, maybe I should have called it raw_this_cpu_read() then it would probably have better meaning. Just like smp_processor_id() and raw_smp_processor_id().
The point is, most places use this_cpu_read(). Adding a "__" or "raw_" prefix usually means that less checks are done. Think of the copy_from_user(). You have __copy_from_user() that does less checks.
Most of the per_cpu() and get_cpu_var() code was blindly replaced with the this_cpu_*() variants. The original code had preemption disabled checks. Now they don't. Which means if you use the this_cpu_read() and then make some decision on it, that read may be useless and buggy.
We have smp_processor_id() that does the check (and the old per_cpu() and get_cpu_var()). Now we have this_cpu_read() which replaced most of the per_cpu() code in the kernel, making them vulnerable to bugs when preemption is enabled.
-- Steve
| |