From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id BB734226D1B for ; Mon, 10 Mar 2025 12:24:35 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741609477; cv=none; b=cATrErUFm1XOX9eTr75wSuTsUi3PstUUZgCzTQMG3YhN3JvlimopZGP+dg9Y15UQxjSiSJorgO4ba9SkToazVWkkOw7Ys6Rb2AL5UDRE3Q1l7mQg834SwOggI797up5f/sk2tvRiIZS0zhTB5llgoIFLGcdbpF30JGjywMIxLcI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741609477; c=relaxed/simple; bh=be7iYBhWCt5fNoFpxb56QSV9NrXBBlZi37xKCzsw8eI=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=UCj8TdDGyTQEOyB1o1Mq6CWcHT3gZJewM7D702oJp68bGybxKS3L6RxyNuGLNLpwAxrD7Tm5uPlKk8d2HnE45CNK14GIlNVUKE3criTpXLc9b9arISooncQCRCf242eWKuqRZc6Da+Pl0ItDbgDffuwqtogAXm8RwpMP3flokBM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 6F66D152B; Mon, 10 Mar 2025 05:24:46 -0700 (PDT) Received: from [192.168.178.6] (usa-sjc-mx-foss1.foss.arm.com [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 9B4C03F5A1; Mon, 10 Mar 2025 05:24:32 -0700 (PDT) Message-ID: Date: Mon, 10 Mar 2025 13:24:31 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] sched/uclamp: Let each sched_class handle uclamp To: Hongyan Xia , Xuewen Yan Cc: Ingo Molnar , Peter Zijlstra , Juri Lelli , Vincent Guittot , Steven Rostedt , Ben Segall , Mel Gorman , Valentin Schneider , Tejun Heo , David Vernet , Andrea Righi , Changwoo Min , linux-kernel@vger.kernel.org, Xuewen Yan References: <84441660bef0a5e67fd09dc3787178d0276dad31.1740664400.git.hongyan.xia2@arm.com> <4394f2a7-b4e8-419a-a299-f1afa560c944@arm.com> <80c2c9f4-eb84-4a43-9c48-8f776615b45a@arm.com> <65365ec7-6a16-4e66-8005-e78788cbedfa@arm.com> <84a8f306-e2f6-4c9e-a150-72ee3c187b64@arm.com> From: Dietmar Eggemann Content-Language: en-US In-Reply-To: <84a8f306-e2f6-4c9e-a150-72ee3c187b64@arm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On 10/03/2025 12:56, Hongyan Xia wrote: > On 10/03/2025 11:22, Dietmar Eggemann wrote: >> On 10/03/2025 12:03, Xuewen Yan wrote: >>> Hi Dietmar, >>> >>> On Mon, Mar 10, 2025 at 6:53 PM Dietmar Eggemann >>> wrote: >>>> >>>> On 10/03/2025 03:41, Xuewen Yan wrote: >>>>> On Sat, Mar 8, 2025 at 2:32 AM Dietmar Eggemann >>>>> wrote: >>>>>> >>>>>> On 06/03/2025 13:01, Xuewen Yan wrote: >>>>>>> On Thu, Mar 6, 2025 at 2:24 AM Dietmar Eggemann >>>>>>> wrote: >>>>>>>> >>>>>>>> On 27/02/2025 14:54, Hongyan Xia wrote: [...] >>> I submitted a patch similar to yours before: >>> >>> https://lore.kernel.org/all/CAB8ipk_AvaOWp9QhmnFDdbFSWcKLhCH151=no6kRO2z+pSJfyQ@mail.gmail.com/ >>> >>> And Hongyan fears that as more complexity goes into each sched_class >>> like delayed dequeue, >>> so it's better to just let the sched_class handle how uclamp is >>> enqueued and dequeued within itself rather than leaking into core.c. >> >> Ah, OK. Your patch didn't have 'sched' in the subject so I didn't see it >> immediately. >> >> I would prefer that uclamp stays in core.c. ENQUEUE_DELAYED among all >> the other flags is already used there (ttwu_runnable()). >> >> task_struct contains  sched_{,rt_,dl_}entity}. We just have to be >> careful when switching policies. > > I lean towards letting each class handle uclamp. We've seen the trouble > with delayed dequeue. Just like the if condition we have for util_est, > if uclamp is in each class then we can re-use the condition easily, > otherwise we need to carefully synchronize the enqueue/dequeue between > core.c and the sub class. > > Also I think so far we are assuming delayed dequeue is the only trouble > maker. If RT and sched_ext have their own corner cases (I think maybe > sched_ext is likely because it may eventually want the ext scheduler to > be able to decide on uclamp itself) then the uclamp inc/dec in core.c > need to cater for that as well. Once a task is in a class, the variables > in another class may be in an undefined state, so checking corner cases > for all the sub-classes in a centralized place like core.c may not even > be easy to get right. I do understand your concern with sched_ext but I still prefer the less invasive change to get uclamp & util_est aligned for fair.c aligned. AFAICS, related to policy changes, we have a SCHED_WARN_ON(p->se.sched_delayed) in switched_to_fair() so far.