Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754914AbbKXUX7 (ORCPT ); Tue, 24 Nov 2015 15:23:59 -0500 Received: from mail-ig0-f170.google.com ([209.85.213.170]:34751 "EHLO mail-ig0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754468AbbKXUXy (ORCPT ); Tue, 24 Nov 2015 15:23:54 -0500 MIME-Version: 1.0 In-Reply-To: <20151123225823.GI19072@mtj.duckdns.org> References: <1447853127-3461-1-git-send-email-pmladek@suse.com> <1447853127-3461-10-git-send-email-pmladek@suse.com> <20151123225823.GI19072@mtj.duckdns.org> Date: Tue, 24 Nov 2015 12:23:53 -0800 X-Google-Sender-Auth: GP2x3mOGY_8AVmeT4oX-xPPvK38 Message-ID: Subject: Re: [PATCH v3 09/22] kthread: Allow to cancel kthread work From: Linus Torvalds To: Tejun Heo Cc: Petr Mladek , Andrew Morton , Oleg Nesterov , Ingo Molnar , Peter Zijlstra , Steven Rostedt , "Paul E. McKenney" , Josh Triplett , Thomas Gleixner , Jiri Kosina , Borislav Petkov , Michal Hocko , linux-mm , Vlastimil Babka , Linux API , Linux Kernel Mailing List Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1468 Lines: 41 On Mon, Nov 23, 2015 at 2:58 PM, Tejun Heo 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/