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 6DAD0156C76 for ; Thu, 16 Jan 2025 10:13:54 +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=1737022436; cv=none; b=dpDhkeYrDFkI0908TYS42xCPnqGgpH0dZQuleKzOjrMsXq9Sk/j+fRDAZ+mzA1VQ/RsZ/fnmpEiyH5l4QjDY03W77kK8IaykT8BtxmVDI2c0AMyEkBbuWZamdeJvq6U+Qbqrvkxr1yU94W0nB/so8nLL6dcetsJ1JzDjRZIHuBY= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737022436; c=relaxed/simple; bh=sPUgMC24j/P/80OrBLLh5WGVm8pQ9vG2tKmuDU0d+O4=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=fwP4GkX9qlNdO70XAIJyx7vZgac72eZZAMOf/CDt74otLea5KAyDIo39wdeSy2/KnI/SnY/zHYnjdk0/c3NAx5JCkoU4P6oSQX0nXe263f8QbKrDtnqgVqRR9fxHQaGZ1vMpgmEDfOTWDVMBC1IDi5JMYP2Ybt+2m6vl200tM+k= 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=J488uEOo; 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="J488uEOo" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1737022433; 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=4fKOnMPhET6AlGGkon+EyIpuds9+73owJj43ZSVQ/D4=; b=J488uEOoxYCRlray45PyhIRWKxDYYXU0/teSH89q99CMT6zUKVNv9Hu20ZH0jbUw5Z9Ace BH6oV6FAUCmLKpb6Ae3gMFAy0wdYEEqvs607aR1VkIHjWiV1lqaK6jvJABYL9+RxxTN2/H eNmuFXbwqvXb4jV6Hm0XxLgqAO7Zwgs= Received: from mail-qv1-f69.google.com (mail-qv1-f69.google.com [209.85.219.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-628-zgdSKAjLNQy5ujSOCUJtdA-1; Thu, 16 Jan 2025 05:13:52 -0500 X-MC-Unique: zgdSKAjLNQy5ujSOCUJtdA-1 X-Mimecast-MFC-AGG-ID: zgdSKAjLNQy5ujSOCUJtdA Received: by mail-qv1-f69.google.com with SMTP id 6a1803df08f44-6d899291908so23662376d6.0 for ; Thu, 16 Jan 2025 02:13:52 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1737022431; x=1737627231; 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=4fKOnMPhET6AlGGkon+EyIpuds9+73owJj43ZSVQ/D4=; b=eOxlM9JAej9jpxYfaOvQoXfXY045fuEO2SoOrwoKKuebF44CyebBWpTHl3auapddAa mgBE5ByHb3ZBYc/xJ28En2IavJ0hiuqZsZdR+vV1aM1HJsY/qEhqidN5l/Xhff76Cpd4 m9e6kCM8Bu+GSQ9Ot3CeytqckuRUPZz86WhfICOQw6IHuNiGcDR2TVHXoc7o9Yj5zXpL 2dTf4CzjEPQgi98kC0ZQ/4E+fu4EzPqUpOry7Sdf7fU4f+8/NAtvOZ4zPn+4998a+6xl 1Ag19+DNhxxRq+qw9lXdABgWIBc74Z6XTn0QAavH/Bs9+MDnWcl+YgU0gxsZRQ3W7VI8 1bhw== X-Forwarded-Encrypted: i=1; AJvYcCWnTwr2MA6GwvBAMHodqAfoLTzpDI4LXeXriPBcCV1MbaupM4UUBdotz/GGm0wk+yPjUFSoAnRI3fbe+w0=@vger.kernel.org X-Gm-Message-State: AOJu0YxGRKLfDroq99HGxUuUVeE4SrRSg+ATk26mjd3YeZyxETk48ANe 0vvUBtnSNAMNjecU0PPy8sbJYlqWxy0JaPTi+wqt6vC10u9lrZfs/Pesb6EdccB1qzLX1gq04zh SMYGkYZ1CPbGtiWeUsNcyW1+7QWCqDOaCB+qt5z+RkN8CCjXnC5lhXbchXPeOXQ== X-Gm-Gg: ASbGnctHume08mqVEmmJkyQ3WYGqJsG1Q1u89CRDI0VuQpQziTsb2VEru/t7j82LVDp p3YmMYxQ0hum5hs/VLG8K+0OeCjDRDGsiD05ZxqbtZ+xPifbjhapa9vaBK7mh5c1xlBuSVS1gR6 3AwKtxbcsw5+HgTg33vU2tpGs3o/ZamW+1rjjgL0LTIHkKJx6jbkMYn60MXpoWc4BgY7HVwmqX7 W+TTdOHqLIOBN2s5tl2mRWsGLetHtZ1V93LXEiLem2Y4e5+IqEOKhUVZ+BSStva3bBEFnTWQ4Nl 95AMQOvYdg== X-Received: by 2002:a05:6214:1306:b0:6d9:3016:d10d with SMTP id 6a1803df08f44-6df9b2ce096mr534239966d6.42.1737022431530; Thu, 16 Jan 2025 02:13:51 -0800 (PST) X-Google-Smtp-Source: AGHT+IHhhXYTmTakpaxR0BQz1UBFRWUwtSmRcQE5iCUP9fkcX9HqCNccXjgbt/4rbaNxqd8EhUiGKQ== X-Received: by 2002:a05:6214:1306:b0:6d9:3016:d10d with SMTP id 6a1803df08f44-6df9b2ce096mr534239606d6.42.1737022431170; Thu, 16 Jan 2025 02:13:51 -0800 (PST) Received: from jlelli-thinkpadt14gen4.remote.csb ([151.29.92.51]) by smtp.gmail.com with ESMTPSA id 6a1803df08f44-6dfad85f64dsm74375596d6.12.2025.01.16.02.13.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 16 Jan 2025 02:13:50 -0800 (PST) Date: Thu, 16 Jan 2025 11:13:45 +0100 From: Juri Lelli To: Doug Berger Cc: Ingo Molnar , Peter Zijlstra , Vincent Guittot , Dietmar Eggemann , Steven Rostedt , Ben Segall , Mel Gorman , Valentin Schneider , Daniel Bristot de Oliveira , Florian Fainelli , linux-kernel@vger.kernel.org Subject: Re: [RFC PATCH] sched/topology: clear freecpu bit on detach Message-ID: References: <20250114230404.661569-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: <20250114230404.661569-1-opendmb@gmail.com> Hi Doug, On 14/01/25 15:04, 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. The details can be found here: > https://lore.kernel.org/lkml/20250110233010.2339521-1-opendmb@gmail.com > > The free_cpus mask is expected to be cleared by set_rq_offline(); > however, the hazard occurs before the root domain is made online > during CPU hotplug so that function is not invoked for the CPU > that is being made active. > > This commit works around the issue by ensuring the free_cpus bit > for a CPU is always cleared when the CPU is removed from a > root_domain. This likely makes the call of cpudl_clear_freecpu() > in rq_offline_dl() fully redundant, but I have not removed it > here because I am not certain of all flows. Hummm, I am not sure we are covering all the cases this way. Would you mind trying out the following maybe? It's based around your idea on the first patch you proposed. Thanks! Juri --- kernel/sched/cpudeadline.c | 23 ++++++----------------- kernel/sched/cpudeadline.h | 3 +-- kernel/sched/deadline.c | 11 ++++++----- 3 files changed, 13 insertions(+), 24 deletions(-) diff --git a/kernel/sched/cpudeadline.c b/kernel/sched/cpudeadline.c index 95baa12a1029..30419fa92c1e 100644 --- a/kernel/sched/cpudeadline.c +++ b/kernel/sched/cpudeadline.c @@ -170,7 +170,7 @@ int cpudl_find(struct cpudl *cp, struct task_struct *p, * * Returns: (void) */ -void cpudl_clear(struct cpudl *cp, int cpu) +void cpudl_clear(struct cpudl *cp, int cpu, bool offline) { int old_idx, new_cpu; unsigned long flags; @@ -181,11 +181,8 @@ void cpudl_clear(struct cpudl *cp, int cpu) old_idx = cp->elements[cpu].idx; if (old_idx == IDX_INVALID) { - /* - * Nothing to remove if old_idx was invalid. - * This could happen if a rq_offline_dl is - * called for a CPU without -dl tasks running. - */ + if (offline) + cpumask_clear_cpu(cpu, cp->free_cpus); } else { new_cpu = cp->elements[cp->size - 1].cpu; cp->elements[old_idx].dl = cp->elements[cp->size - 1].dl; @@ -195,7 +192,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 (!offline) + cpumask_set_cpu(cpu, cp->free_cpus); } raw_spin_unlock_irqrestore(&cp->lock, flags); } @@ -243,19 +241,10 @@ void cpudl_set(struct cpudl *cp, int cpu, u64 dl) */ void cpudl_set_freecpu(struct cpudl *cp, int cpu) { + guard(raw_spinlock)(&cp->lock); cpumask_set_cpu(cpu, cp->free_cpus); } -/* - * cpudl_clear_freecpu - Clear the cpudl.free_cpus - * @cp: the cpudl max-heap context - * @cpu: rd attached CPU - */ -void cpudl_clear_freecpu(struct cpudl *cp, int cpu) -{ - cpumask_clear_cpu(cpu, cp->free_cpus); -} - /* * cpudl_init - initialize the cpudl structure * @cp: the cpudl max-heap context diff --git a/kernel/sched/cpudeadline.h b/kernel/sched/cpudeadline.h index 0adeda93b5fb..cb2be7a205dd 100644 --- a/kernel/sched/cpudeadline.h +++ b/kernel/sched/cpudeadline.h @@ -18,9 +18,8 @@ struct cpudl { #ifdef CONFIG_SMP int cpudl_find(struct cpudl *cp, struct task_struct *p, struct cpumask *later_mask); void cpudl_set(struct cpudl *cp, int cpu, u64 dl); -void cpudl_clear(struct cpudl *cp, int cpu); +void cpudl_clear(struct cpudl *cp, int cpu, bool offline); int cpudl_init(struct cpudl *cp); void cpudl_set_freecpu(struct cpudl *cp, int cpu); -void cpudl_clear_freecpu(struct cpudl *cp, int cpu); void cpudl_cleanup(struct cpudl *cp); #endif /* CONFIG_SMP */ diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c index 643d101cb96a..b52234ebb991 100644 --- a/kernel/sched/deadline.c +++ b/kernel/sched/deadline.c @@ -1870,7 +1870,7 @@ static void dec_dl_deadline(struct dl_rq *dl_rq, u64 deadline) if (!dl_rq->dl_nr_running) { dl_rq->earliest_dl.curr = 0; dl_rq->earliest_dl.next = 0; - cpudl_clear(&rq->rd->cpudl, rq->cpu); + cpudl_clear(&rq->rd->cpudl, rq->cpu, false); cpupri_set(&rq->rd->cpupri, rq->cpu, rq->rt.highest_prio.curr); } else { struct rb_node *leftmost = rb_first_cached(&dl_rq->root); @@ -2921,9 +2921,11 @@ static void rq_online_dl(struct rq *rq) if (rq->dl.overloaded) dl_set_overload(rq); - cpudl_set_freecpu(&rq->rd->cpudl, rq->cpu); - if (rq->dl.dl_nr_running > 0) + if (rq->dl.dl_nr_running > 0) { cpudl_set(&rq->rd->cpudl, rq->cpu, rq->dl.earliest_dl.curr); + } else { + cpudl_set_freecpu(&rq->rd->cpudl, rq->cpu); + } } /* Assumes rq->lock is held */ @@ -2932,8 +2934,7 @@ static void rq_offline_dl(struct rq *rq) if (rq->dl.overloaded) dl_clear_overload(rq); - cpudl_clear(&rq->rd->cpudl, rq->cpu); - cpudl_clear_freecpu(&rq->rd->cpudl, rq->cpu); + cpudl_clear(&rq->rd->cpudl, rq->cpu, true); } void __init init_sched_dl_class(void) -- 2.47.1