From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.9 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id CE3D2C4363A for ; Tue, 27 Oct 2020 19:19:25 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id C99E120756 for ; Tue, 27 Oct 2020 19:19:24 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="CJT35Ob+"; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="wGSJTne4" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C99E120756 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linutronix.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 0503B6B007D; Tue, 27 Oct 2020 15:19:24 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 000E26B007E; Tue, 27 Oct 2020 15:19:23 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E32606B0081; Tue, 27 Oct 2020 15:19:23 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0026.hostedemail.com [216.40.44.26]) by kanga.kvack.org (Postfix) with ESMTP id B715D6B007D for ; Tue, 27 Oct 2020 15:19:23 -0400 (EDT) Received: from smtpin10.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 587268249980 for ; Tue, 27 Oct 2020 19:19:23 +0000 (UTC) X-FDA: 77418669006.10.laugh13_200c4562727e Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin10.hostedemail.com (Postfix) with ESMTP id 3986D16A07F for ; Tue, 27 Oct 2020 19:19:23 +0000 (UTC) X-HE-Tag: laugh13_200c4562727e X-Filterd-Recvd-Size: 4754 Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) by imf42.hostedemail.com (Postfix) with ESMTP for ; Tue, 27 Oct 2020 19:19:22 +0000 (UTC) From: Thomas Gleixner DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1603826359; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=p0CmGDDPRiNeoZmas3WC/BuGgJ2L+HXBpzjZ1xtym5Y=; b=CJT35Ob+9BEk+pzK1G7vADh+AXYUOI8dtu3AGqSy9ZsGLcD2f5cOqxQJFNoOUnUjECe2gl htxY0g/9FXlzESqidLgtrXkLsbnH6TNLr8JIj14QVKb+wUu1FnCjGEC5bBiUUVv5JVBK5d ZoNqh/Wk66DPC3dGS+DmYjj3EtbbiAEQpxqjAJEVt92nItu2yGYMivm1fhY02ZgMUNIVHt KpivOpCLzQvhXeUnEz80jvkuEcWElefx/AwEvI4wPZZLRiYYr5ll+dCMEne4HZ/SN9X+8p kxLBGEYTIDcTNR+3xmE2stq8iLaaK8wpg5ot74EE2QmsR+IiMp2i3lJNu4yB6w== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1603826359; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=p0CmGDDPRiNeoZmas3WC/BuGgJ2L+HXBpzjZ1xtym5Y=; b=wGSJTne4di+ga8Wi8QvPkEJ2X0GUE7zBPXVCZRhU81Ff7SeNCoGnI0oEcsqf0KuxFxASCy Ri/lIxZ/Eol2J2BA== To: Petr Mladek , qiang.zhang@windriver.com Cc: tj@kernel.org, akpm@linux-foundation.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, Peter Zijlstra Subject: Re: [PATCH] kthread_worker: re-set CPU affinities if CPU come online In-Reply-To: <20201027163925.GE31882@alley> References: <20201026065213.30477-1-qiang.zhang@windriver.com> <20201027163925.GE31882@alley> Date: Tue, 27 Oct 2020 20:19:19 +0100 Message-ID: <87a6w71npk.fsf@nanos.tec.linutronix.de> MIME-Version: 1.0 Content-Type: text/plain X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: Petr, On Tue, Oct 27 2020 at 17:39, Petr Mladek wrote: > On Mon 2020-10-26 14:52:13, qiang.zhang@windriver.com wrote: >> From: Zqiang >> >> When someone CPU offlined, the 'kthread_worker' which bind this CPU, >> will run anywhere, if this CPU online, recovery of 'kthread_worker' >> affinity by cpuhp notifiers. > > I am not familiar with CPU hotplug notifiers. I rather add Thomas and > Peter into Cc. Thanks! >> +static int kworker_cpu_online(unsigned int cpu, struct hlist_node *node) >> +{ >> + struct kthread_worker *worker = hlist_entry(node, struct kthread_worker, cpuhp_node); > > The code here looks correct. > > JFYI, I was curious why many cpuhp callbacks used hlist_entry_safe(). > But they did not check for NULL. Hence the _safe() variant did > not really prevented any crash. > > I seems that it was a cargo-cult programming. cpuhp_invoke_callback() calls > simple hlist_for_each(). If I get it correctly, the operations are > synchronized by cpus_read_lock()/cpus_write_lock() and _safe variant > really is not needed. Correct. >> +static __init int kthread_worker_hotplug_init(void) >> +{ >> + int ret; >> + >> + ret = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN, "kthread-worker/online", >> + kworker_cpu_online, NULL); The dynamic hotplug states run late. What's preventing work to be queued on such a worker before it is bound to the CPU again? Nothing at all. Moving the hotplug state early does not help either because this cannot happen _before_ the CPUHP_AP_ONLINE state. After that it's already too late because that's after interrupts have been reenabled on the upcoming CPU. Depending on the interrupt routing an interrupt hitting the upcoming CPU might queue work before the state is reached. Work might also be queued via a timer before rebind happens. The only current user (powerclamp) has it's own hotplug handling and stops the thread and creates a new one when the CPU comes online. So that's not a problem. But in general this _is_ a problem. There is also no mechanism to ensure that work on a CPU bound worker has been drained before the CPU goes offline and that work on the outgoing CPU cannot be queued after a certain point in the hotplug state machine. CPU bound kernel threads have special properties. You can access per CPU variables without further protection. This blows up in your face once the worker thread is unbound after a hotplug operation. So the proposed patch is duct tape and papers over the underlying design problem. Either this is fixed in a way which ensures operation on the bound CPU under all circumstances or it needs to be documented that users have to have their own hotplug handling similar to what powerclamp does. Thanks, tglx