From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-dl1-f48.google.com (mail-dl1-f48.google.com [74.125.82.48]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B7233175A95 for ; Thu, 28 May 2026 21:46:19 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=74.125.82.48 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780004781; cv=none; b=oR5ln2CJkAo0iK/WYnSIDVq3BttJ1qpkjkCwIpZxG5h3t8tt1bMRNuq28NwOtygnMd9kvcSN5SVGxXb8LGeQQpe2pebK5ouO9cYh5hShhfvxjjuqHxciah2YRQn8kJPwCtAJ9mRhcczMaUkwtFAnxHMAqiJUsHuvkJcnO4r0sB8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780004781; c=relaxed/simple; bh=WkA50iNQWWUuJvewP+GsKAX86/gpW6sgADAeWB/csYg=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=gB/L2Pq7r6j2cgjTW56wNYUAkVS2nSZgZ8TMnjpTsaRKdV6r7F5XQJDDn2rwVac2GYgfUhbl/psqeF4jrE/Icw/zFezsgxg1KdRdJ74M0exXbdTEG8bUxWDcGQPqQzU7Nt0Rc6Prx56xh1gh2l1fBcOPHffZifF3AfFivHZdec0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=lBSCof2W; arc=none smtp.client-ip=74.125.82.48 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="lBSCof2W" Received: by mail-dl1-f48.google.com with SMTP id a92af1059eb24-1353c2f35cfso5545217c88.1 for ; Thu, 28 May 2026 14:46:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20251104; t=1780004779; x=1780609579; darn=vger.kernel.org; h=mime-version:user-agent:message-id:date:references:in-reply-to :subject:cc:to:from:from:to:cc:subject:date:message-id:reply-to; bh=kzU2t6RJeeiyaRfWVYllMBG0zRW9pCrR34lmvTb1NHc=; b=lBSCof2WXsi/xv70kKZF3HwsacBq/50g7260vy0iVJ/Qp1Z2JUnxXznsIePChtgxMi cw5uj8OVaGVz3MUT2DJpDjte7P93k8kuzjgOQYJsz1dvLqSVXizilP+MO/HPd5MWn1hc rPrRqJfJ+i+Wb1MISG1orJNHpeRwoepyxdzFDwfmRh9rDvTlrFLg1pgu219owNRe89G7 mLSwPLQGGUXAeyQ5kuD+hgBlf/cqS5lv2NZ4QXcEMOOYxf3MXL7Bs+9EEMmusBekHEbs ahUj/BTD9c1hnzNCelCJQsN6cD5tt/NCHMEZNuOn+s8/pRC6eFqrf8yA+YcnZCl1M7Fu rb8Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1780004779; x=1780609579; h=mime-version:user-agent:message-id:date:references:in-reply-to :subject:cc:to:from:x-gm-gg:x-gm-message-state:from:to:cc:subject :date:message-id:reply-to; bh=kzU2t6RJeeiyaRfWVYllMBG0zRW9pCrR34lmvTb1NHc=; b=NX2evNLKO5JqQNj3b+GIqd2EhiSOnj7QzhaCOObyioPbSbqOmO47qwxxH/06m9ElgR oS0s1bTd8fQju01umSPfB7Z/L9Q/QwtqGpm8ObmDUBbag/ezrEyjzly3Ppx6OLkos8+F wMhGimNK67Il8RLXpbWcIS9PFcMZ+nIEprqvDjn51k+wdN2tMJvx3sFoyV5lYftWVzGu 0rIrYToHQ37DElARm34M46I4unwq9/6yA0somvAIqSytFdE0imBsx66M5NHZVBPudJpb hApNYE5KENbNiMFMI1tLKFZ/eiH3jVC/vpL6etI/O88GNnh2icu+BZrWyOlqKsU3lSs0 vjsw== X-Forwarded-Encrypted: i=1; AFNElJ9CyC0UrSSWW27DgndFw1rPnAP1sEPkGK7JBz39gS9wGJZa3ONFJgJm8kDJ5zYJNolxwyEfWSEoVN+vnek=@vger.kernel.org X-Gm-Message-State: AOJu0YycQZqsjuDivM/0hQOeZM0KYLp7mgpHnT0eN9KixIEl/HbDkQ71 eVBS0SDQdi7p3nOJ+sEg83hhsj68ofHUM6uly2N7XS2BBrZa7lLMXdHJilHm0SOzY3PGLLFYanV S2s/uTTQTgIQ= X-Gm-Gg: Acq92OHZHBP4Ej5BEMY8ISVkXLsSNfHYhTRUkUuBpMEpA9yopa+n+QRO43XZDztEO8G mRnipU0DWbnpKxhstyjdBt5I1SWQFo5Hl5Hr0svKh9dSBtyNq9N+UO0N6740U0MhfqlhmOO4Oqm iWOB3UlcAcPMt1OM6BK1l1SLVqs3qkkGra3fufkzUR0VWvTvq+1neeW/yO/Nc2Nqnvw04b5Y2l7 TqXMr7TwRNlQFzjCfJpeB3wSYh4Mz04XL/Ef5pTROXKBjpXmjEIrnhR00Fp0DwqrEbW7U5NQOrj fLF1JWHt0SjOBrpbtDQtyAnjByd2iQ8KG5IxdAf7zrSBkGB2e1l76BOWSo4wueapNGkb7e8u1Hb wyewOFS7z064uDjrdk7ggn4EhuUFpmttwc1QGIaNmOYdmz/ArDqnEnYpVvcWsNSZ8gtM2uk9U71 zHUIwpnRpMLhOtebivV7p/EFpvOZwXbdCFZyS7J7Dms9AUa3z8ml4L X-Received: by 2002:a05:7022:ba7:b0:132:2486:acb with SMTP id a92af1059eb24-137af2232f6mr80342c88.12.1780004777987; Thu, 28 May 2026 14:46:17 -0700 (PDT) Received: from bsegall27.localhost ([98.45.141.147]) by smtp.gmail.com with ESMTPSA id a92af1059eb24-137b03fd9b8sm15628c88.3.2026.05.28.14.46.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 28 May 2026 14:46:17 -0700 (PDT) From: Benjamin Segall To: K Prateek Nayak Cc: Ingo Molnar , Peter Zijlstra , Juri Lelli , Vincent Guittot , Dietmar Eggemann , Steven Rostedt , Mel Gorman , Valentin Schneider , Aaron Lu , Josh Don , Subject: Re: [PATCH 1/5] sched/fair: Convert cfs bandwidth throttling to use guards In-Reply-To: <20260528094830.13291-2-kprateek.nayak@amd.com> (K. Prateek Nayak's message of "Thu, 28 May 2026 09:48:26 +0000") References: <20260528094830.13291-1-kprateek.nayak@amd.com> <20260528094830.13291-2-kprateek.nayak@amd.com> Date: Thu, 28 May 2026 14:46:15 -0700 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain K Prateek Nayak writes: > Routine conversion of rcu_read_lock(), spin_lock*, and rq_lock usage > within the cfs bandwidth controller to use class guards. > > Only notable changes are: > > - Conversion of do_sched_cfs_slack_timer() to return a bool and moving > the call to distribute_cfs_runtime() into the caller for a cleaner > guard usage. > > - Reordering of list_del_rcu() against throttled_clock indicator update > in unthrottle_cfs_rq(). Both are done with "cfs_b->lock" held after > the "cfs_rq->throttled" is cleared which make the reordering safe > against concurrent list modifications. > > No functional changes intended. Some minor style nitpicks that I don't care /that/ much about, so: Reviewed-By: Ben Segall > @@ -7011,40 +7006,33 @@ static bool distribute_cfs_runtime(struct cfs_bandwidth *cfs_b) > if (cfs_rq->runtime_remaining > 0) { > if (cpu_of(rq) != this_cpu) { > unthrottle_cfs_rq_async(cfs_rq); > - } else { > - /* > - * We currently only expect to be unthrottling > - * a single cfs_rq locally. > - */ > - WARN_ON_ONCE(!list_empty(&local_unthrottle)); > - list_add_tail(&cfs_rq->throttled_csd_list, > - &local_unthrottle); > + continue; > } > - } else { > - throttled = true; > + > + /* > + * We currently only expect to be unthrottling > + * a single cfs_rq locally. > + */ > + WARN_ON_ONCE(!list_empty(&local_unthrottle)); > + list_add_tail(&cfs_rq->throttled_csd_list, &local_unthrottle); > + continue; > } > > -next: > - rq_unlock_irqrestore(rq, &rf); > + throttled = true; > } I don't think the extra continues wind up being clearer here than the original if/else code and just removing the next/unlock. > @@ -7197,29 +7185,25 @@ static __always_inline void return_cfs_rq_runtime(struct cfs_rq *cfs_rq) > * This is done with a timer (instead of inline with bandwidth return) since > * it's necessary to juggle rq->locks to unthrottle their respective cfs_rqs. > */ > -static void do_sched_cfs_slack_timer(struct cfs_bandwidth *cfs_b) > +static bool do_sched_cfs_slack_timer(struct cfs_bandwidth *cfs_b) > { > u64 runtime = 0, slice = sched_cfs_bandwidth_slice(); > - unsigned long flags; > > /* confirm we're still not at a refresh boundary */ > - raw_spin_lock_irqsave(&cfs_b->lock, flags); > + guard(raw_spinlock_irqsave)(&cfs_b->lock); > + > cfs_b->slack_started = false; > > - if (runtime_refresh_within(cfs_b, min_bandwidth_expiration)) { > - raw_spin_unlock_irqrestore(&cfs_b->lock, flags); > - return; > - } > + if (runtime_refresh_within(cfs_b, min_bandwidth_expiration)) > + return false; > > if (cfs_b->quota != RUNTIME_INF && cfs_b->runtime > slice) > runtime = cfs_b->runtime; > > - raw_spin_unlock_irqrestore(&cfs_b->lock, flags); > - > if (!runtime) > - return; > + return false; > > - distribute_cfs_runtime(cfs_b); > + return true; > } > > /* Here I think it's also nicer to just use a scoped_guard rather than move part of it out and have part of the work done outside of the do_ helper.