lkml.org 
[lkml]   [2015]   [Nov]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v3 09/22] kthread: Allow to cancel kthread work
From
On Mon, Nov 23, 2015 at 2:58 PM, Tejun Heo <tj@kernel.org> wrote:
>
> And the timer can do (ignoring the multiple worker support, do we even
> need that?)
>
> while (!trylock(worker)) {
> if (work->canceling)
> return;
> cpu_relax();
> }

No no no!

People, you need to learn that code like the above is *not*
acceptable. It's busy-looping on a spinlock, and constantly trying to
*write* to the spinlock.

It will literally crater performance on a multi-socket SMP system if
it ever triggers. We're talking 10x slowdowns, and absolutely
unacceptable cache coherency traffic.

These kinds of loops absolutely *have* to have the read-only part. The
"cpu_relax()" above needs to be a loop that just tests the lock state
by *reading* it, so the cpu_relax() needs to be replaced with
something like

while (spin_is_locked(lock)) cpu_relax();

instead (possibly just "spin_unlock_wait()" - but the explicit loop
might be worth it if you then want to check the "canceling" flag
independently of the lock state too).

In general, it's very dangerous to try to cook up your own locking
rules. People *always* get it wrong.

Linus


\
 
 \ /
  Last update: 2015-11-24 22:01    [W:0.133 / U:0.128 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site