From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5F52C42AA5 for ; Mon, 13 Jan 2025 10:54:46 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.133.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736765688; cv=none; b=hKkE8E8n4rEubGqJ+vkC9S3NFeFuJEfb5Sr0q923HOIiWZFfL/4UGgM/iQhGhKv+4FyYSglb1YK9L+gtpwr+dieAdPxV2z+P0UDgc1Vu41vRFcQBcrOW01bvZkT6D5sH+i7Kn8vjp7Na3YRyUFcKdyJJfXvvWqIJ0+Ara+Rv9d8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736765688; c=relaxed/simple; bh=vpCMVeYACO/WiU6OLCRTvkReA7r+aw6O5Qst42e9TxM=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=sxPQOf91Kb8jCmliFmZyyTmusRgyWlJya7xOuEdi25JCJt2PG4prnlxDPdIyFTz1C9ViJM05v0cAiiU6XJ1hL0EdfWwvVquyuTx1sAPAlome9jOG9AvxnWYst3YQnKOCi0/oXIoFEWVQOhlbAYkuoS5BH5TE8+K66e9k1psYwKM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=JVPwPe/y; arc=none smtp.client-ip=170.10.133.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="JVPwPe/y" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1736765685; 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=GwF+M0vOaZmvGT6MnBX+m/vCvtGV+clKeelTy7SqR0A=; b=JVPwPe/yG2eBDDnBatZ9JNkZTye8AJHT9ALXGZuLKoc8voYoGPMtpC6kQtv/agLuvDszYF iES9wsxUKzn9TWWwF3u2rZ2ZBkxrLCI+ZJAIMliPHXhTr64lKwP0D6mZBjOoIA8jpymDQp z/2rwEi8fmeLblTE/pu0Aj+a4k/hVb8= Received: from mail-qk1-f198.google.com (mail-qk1-f198.google.com [209.85.222.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-542-ySrdIXrqPZqG9J0wUUgxSQ-1; Mon, 13 Jan 2025 05:54:43 -0500 X-MC-Unique: ySrdIXrqPZqG9J0wUUgxSQ-1 X-Mimecast-MFC-AGG-ID: ySrdIXrqPZqG9J0wUUgxSQ Received: by mail-qk1-f198.google.com with SMTP id af79cd13be357-7b6ecd22efbso721214085a.0 for ; Mon, 13 Jan 2025 02:54:43 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736765683; x=1737370483; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=GwF+M0vOaZmvGT6MnBX+m/vCvtGV+clKeelTy7SqR0A=; b=hORBDbiQ+/RLBiqfGUy+YiPHzMQdUReDGGHGy+BAZMsXCFj7TwlH8q9D++9L9fs1M9 L1kTjfYxrcyXCgE39q1bK/U93OkTjwh3Y4Q9xMbsLYybhwQqdxmrnSvAgD/iaPvu6tMj atlC+8XQt0JnCd2kGnrz39hx4L2EgEbdmS5EcCVCs08OtjxyfJ+4JiLgj9NaWNum1ljv wzvOnrdPYHhu/1nvzYWdkvSapXfpGmk12/rB5HhSmCypfqqLdL/JcLiiGfy7moBRuaYP 54UwfQFSCT/igYGMQXPdO/lykKzL/9RgfDjoFQTDSC2BHc6gKpl0f737Rs0DTXgM0aBl 050w== X-Forwarded-Encrypted: i=1; AJvYcCVYeILf6JIAq8wPS5HGYY15QJiWNEVx4FH8QrcpyENCso+1U1b1jlN7dqx5gBnWoQSi3CkzUkoHyLy5NtM=@vger.kernel.org X-Gm-Message-State: AOJu0YwCQeqphVBh3roLL5JaxLOCPxfA3oG/sU6HxAnBHjPyeC6tdRr6 9IXyW4m6yWII1ODCcfIRdoHgIMzbFUYBNTwJ95ytjzNxOV5Yw5wwNG/mgs7+llUbwrT4LlseZit IHHaO8RHywUViTaY5aRRzCH4huHqqtg31v/9slVgoQP02eV9LiHjdZz2pHa2RbA== X-Gm-Gg: ASbGncvTeWE4dDA1wA/cQqBnRwrbRvxR4GTIm1CaxDKEkUmyG2HHHs0J1zmgZaM8LSt 9adukl/bGDGxZMRQdfbAPUtUBtPJw9YGR2ucUblh0RAHskF+wU/kWlD0wMn5PXhOhY/CbjuVdt1 GnOGPuuZuosHHXPddqz1liGW+QriZhmdDnu25RX/jJtuZamK2nuYqDKKqti+DKDDS6+XqiBdtgp yyqhuSb2gRrptsOEa0TcMsLaOYG5i+PsiWgBEDJqMPSlQalip+K1WDVMAM53UciZDPTKPuSsXX9 ur6gnOPHmw== X-Received: by 2002:a05:620a:2886:b0:7b6:d241:3f07 with SMTP id af79cd13be357-7bce11d87dcmr2512044085a.15.1736765683104; Mon, 13 Jan 2025 02:54:43 -0800 (PST) X-Google-Smtp-Source: AGHT+IG7R1qf3EAmP/TMdHObxijL4Ay8qqAg2ZcZD+1uqeL3t3FkcipWHO7z1dCAHAxoyUN6ULmi6A== X-Received: by 2002:a05:620a:2886:b0:7b6:d241:3f07 with SMTP id af79cd13be357-7bce11d87dcmr2512041685a.15.1736765682728; Mon, 13 Jan 2025 02:54:42 -0800 (PST) Received: from jlelli-thinkpadt14gen4.remote.csb ([151.29.92.51]) by smtp.gmail.com with ESMTPSA id af79cd13be357-7bce350e155sm476021585a.97.2025.01.13.02.54.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 13 Jan 2025 02:54:41 -0800 (PST) Date: Mon, 13 Jan 2025 11:54:36 +0100 From: Juri Lelli To: Doug Berger Cc: Ingo Molnar , Peter Zijlstra , Vincent Guittot , Dietmar Eggemann , Steven Rostedt , Ben Segall , Mel Gorman , Valentin Schneider , Florian Fainelli , linux-kernel@vger.kernel.org Subject: Re: [RFC] sched/deadline: only mark active cpu as free Message-ID: References: <20250110233010.2339521-1-opendmb@gmail.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20250110233010.2339521-1-opendmb@gmail.com> Hi Doug, On 10/01/25 15:30, Doug Berger wrote: > There is a hazard in the deadline scheduler where an offlined CPU > can have its free_cpus bit left set in the def_root_domain when > the schedutil cpufreq governor is used. This can allow a deadline > thread to be pushed to the runqueue of a powered down CPU which > breaks scheduling. > > This commit works around the issue by only setting the free_cpus > bit for a CPU when it is "active". It is likely that the ordering > of sched_set_rq_online() and set_cpu_active() at the end of the > sched_cpu_deactivate() function should be revisited if this > approach has merit. > > Signed-off-by: Doug Berger > --- > > Coffee is recommended before proceeding. > > While stress testing CPU hotplug on a quad-core arm64 architecture > system I encountered a deadlock. My specific deadlock appears to be > dependent on the system having three or more cores and using the > sched-util cpufreq governor which uses a deadline scheduled thread > named "sugov:n" where n is the CPU number. > > The scenario I observe is as follows: > Initially, CPU0 and CPU1 are active and CPU2 and CPU3 have been > previously offlined so their runqueues are attached to the > def_root_domain. > 1) A hot plug is initiated on CPU2. > 2) The cpuhp/2 thread invokes the cpufreq governor driver during > the CPUHP_AP_ONLINE_DYN step. > 3) The sched util cpufreq governor creates the "sugov:2" thread to > execute on CPU2 with the deadline scheduler. > 4) The deadline scheduler clears the free_cpus mask for CPU2 within > the def_root_domain when "sugov:2" is scheduled. > 5) When the "sugov:2" thread blocks, cpudl_clear() gets called to > clear the deadline which sets the free_cpus mask for CPU2 within > the def_root_domain. > 6) When cpuhp/2 reaches the CPUHP_AP_ACTIVE step a new scheduling > domain is created to include CPU0, CPU1, and CPU2. > o detach_destroy_domains() invokes rq_attach_root() for CPU0 and > CPU1 which offlines their runqueues and detaches their current > dynamic scheduling domain (clearing their deadline free_cpus > bits there) and attaches the def_root_domain and onlines their > runqueus (setting their deadline free_cpus bits there). > o build_sched_domains() invokes rq_attach_root() for CPU0, CPU1, > and CPU2. > - Since only CPU0 and CPU1 are online in the def_root_domain > set_rq_offline() is only called for them to offline their > runqueues and detach the def_root_domain (clearing their > deadline free_cpus bits there). > - The free_cpus bit for CPU2 in def_root_domain is allowed to > remain set. > - The newly created dynamic scheduling domain is attached to > CPU0, CPU1, and CPU2 runqueues and set_rq_online() is used > to online their runqueues (setting their deadline free_cpus > bits there). > 7) The cpuhp/2 thread also invokes sched_set_rq_online() in the > CPUHP_AP_ACTIVE step, but since the runqueues are already online > essentially nothing happens. > 8) Some time later CPU2 is hot unplugged. > 9) At the CPUHP_AP_ACTIVE step, cpuhp/2 marks CPU2 not active and > invokes balance_push_set() for CPU2 which migrates "sugov:2" to > a different CPU through fallback. > 10) Also at this step, cpuhp/2 invokes sched_set_rq_offline() for > CPU2 which takes its runqueue offline and clears its deadline > free_cpus bit in the current dynamic scheduling domain. > 11) Also at this step, cpuhp/2 updates the scheduling domain to > remove CPU2. > o detach_destroy_domains() invokes rq_attach_root() for CPU0, > CPU1, and CPU2 to move them back to the def_root_domain. > - Since only CPU0 and CPU1 are online in the current dynamic > scheduling domain (CPU2 was removed at 10 above), > set_rq_offline() is only called for them to clear their > deadline free_cpus bits. > - The def_root_domain is attached to CPU0, CPU1, and CPU2 > runqueues and since only CPU0 and CPU1 are marked active > set_rq_online() is used to online their runqueues (setting > their deadline free_cpus bits there). > - The free_cpus bit for CPU2 in def_root_domain is allowed > to remain set. > o build_sched_domains() invokes rq_attach_root() for CPU0 and > CPU1 which offlines their runqueues (clearing their deadline > free_cpus bits in def_root_domain), attaches a new dynamic > scheduling domain, and onlines their runqueus (setting their > deadline free_cpus bits there). > 12) The cpuhp/2 thread invokes the cpufreq governor driver during > the CPUHP_AP_ONLINE_DYN step which attempts to stop the > "sugov:2" kthread by calling kthread_flush_worker() followed by > kthread_stop(). > 13) The "sugov:2" thread can likely be successfully deadline > scheduled on CPU0 or CPU1 to allow the cpuhp/2 thread to > complete offlining CPU2 and power it off. > 14) Some time later CPU1 is hot unplugged. > 15) At the CPUHP_AP_ACTIVE step, cpuhp/1 marks CPU1 not active and > invokes balance_push_set() for CPU1 which migrates "sugov:1" to > CPU0 through fallback. > 16) Also at this step, cpuhp/1 invokes sched_set_rq_offline() for > CPU1 which takes its runqueue offline and clears its deadline > free_cpus bit in the current dynamic scheduling domain. > 17) Also at this step, cpuhp/1 updates the scheduling domain to > remove CPU1. > o detach_destroy_domains() invokes rq_attach_root() for CPU0 > and CPU1 to move them back to the def_root_domain. > - Since only CPU0 is online in the current dynamic scheduling > domain (CPU1 was removed at 16 above), set_rq_offline() is > only called for it to clear its deadline free_cpus bit. > - The def_root_domain is attached to CPU0 and CPU1 runqueues > and since only CPU0 is marked active set_rq_online() is > used to online its runqueue (setting its deadline free_cpus > bits there). > - The free_cpus bit for CPU1 is untouched in def_root_domain. > - The free_cpus bit for CPU2 in def_root_domain remains set > from the preceeding sequence. > 18) If CPU0 executes the "sugov:0" deadline thread at this time it > may see that the "sugov:1" deadline thread is also on its > runqueue and may call push_dl_task() to attempt to push it to a > different CPU. > 19) The effort to find a later runqueue will find the stale > free_cpus bit of CPU2 in the currently attached def_root_domain > and will migrate the "sugov:1" thread to the runqueue of the > powered down CPU2 where it can never get scheduled. > 20) The cpuhp/1 thread invokes the cpufreq governor driver during > the CPUHP_AP_ONLINE_DYN step which attempts to stop the > "sugov:1" kthread by calling kthread_flush_worker() followed by > kthread_stop(). Since "sugov:1" never gets scheduled cpuhp/1 > remains blocked on completion events. > > Steps 1-13 amount to setting a trap by allowing a free_cpus bit in > the deadline scheduler def_root_domain to remain set for a CPU that > is powered off. The trap can be sprung during the narrow timing > hazard when the def_root_domain is transitionally attached while > changing scheduling domains if the deadline scheduler pushes a > queued task to the powered off CPU. > > This problem appears to have been initially introduced by commit > 120455c514f7 ("sched: Fix hotplug vs CPU bandwidth control") which > moved the set_rq_offline() handling from sched_cpu_dying() to > sched_cpu_deactivate(). The original sequence allowed the free_cpus > bit to be forcibly cleared in the def_root_domains after all of the > scheduler dust settled. The new location makes the > sched_set_rq_offline() essentially meaningless for the deadline > scheduler since the managing of changed scheduling domains happens > later. > > There are likely many different approaches to address this issue > and I'm hopeful that somone more familiar with the scheduler than > I can propose a better solution than the one suggested here. > > Thank you for reading this far. Any advice is appreciated. Thanks a lot for the detailed analysis! I actually fear that the issue is due to the cpudl_clear_freecpu() call in rq_offline_dl() being racy, as we don't hold cp->lock while calling that. So, I think your solution below might be almost correct. I am thinking we should do something similar in cpudl_set() and remove cpudl_ {set,clear}_freecpu() calls altogether. What do you think? If agree, care to update your patch please? :) Best, Juri > kernel/sched/cpudeadline.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/kernel/sched/cpudeadline.c b/kernel/sched/cpudeadline.c > index 95baa12a1029..6896bbe0e9ae 100644 > --- a/kernel/sched/cpudeadline.c > +++ b/kernel/sched/cpudeadline.c > @@ -195,7 +195,8 @@ void cpudl_clear(struct cpudl *cp, int cpu) > cp->elements[cpu].idx = IDX_INVALID; > cpudl_heapify(cp, old_idx); > > - cpumask_set_cpu(cpu, cp->free_cpus); > + if (cpu_active(cpu)) > + cpumask_set_cpu(cpu, cp->free_cpus); > } > raw_spin_unlock_irqrestore(&cp->lock, flags); > } > -- > 2.34.1 >