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 20FAC12E1C9; Wed, 27 Mar 2024 13:35:05 +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=1711546507; cv=none; b=SXCcqpovZAsilJF0mTm3FW5MIxKpqKKgyN0+9DCxvY+8oGJdPHdbyBG+2px8dACcWpvmDsmMBqj+sUIp1a5zOy+unt0VRoXDXFOHXBLUsL5zh5LZj6pbRWc1XNJMOkodAsD5ytjriUIwfSjN5Q6vNMEneEhHHirgWAlnGpv9q4k= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1711546507; c=relaxed/simple; bh=ooKZ6O9tR9+QN4moCAzd2mBAdIEUT5709FoWcHHdLnM=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=ZIphwFNRtYRBK6mNSaBz5HdFc0/muB19hym7+tcaY0b1Fz1w/bdpwRpHY8MTuFM5iL1oo5CTZSWnik57sWdqusz84NMOcVSftTK2UHAfSecG9hNWroBN91M+/Y7dUTsltYkL2/mCkYU++/NacAk2cQclRizWWXhiejxJVCsnV9I= 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 792FE2F4; Wed, 27 Mar 2024 06:35:39 -0700 (PDT) Received: from [10.57.53.95] (unknown [10.57.53.95]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id D365E3F7BD; Wed, 27 Mar 2024 06:35:02 -0700 (PDT) Message-ID: Date: Wed, 27 Mar 2024 13:35:01 +0000 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: [RFC PATCH] sched: Consolidate cpufreq updates To: Qais Yousef , "Rafael J. Wysocki" , Viresh Kumar , Ingo Molnar , Peter Zijlstra , Vincent Guittot , Juri Lelli Cc: Steven Rostedt , Dietmar Eggemann , Ben Segall , Mel Gorman , Daniel Bristot de Oliveira , Valentin Schneider , Christian Loehle , linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org References: <20240324020139.1032473-1-qyousef@layalina.io> Content-Language: en-US From: Hongyan Xia In-Reply-To: <20240324020139.1032473-1-qyousef@layalina.io> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 24/03/2024 02:01, Qais Yousef wrote: > Improve the interaction with cpufreq governors by making the > cpufreq_update_util() calls more intentional. > > At the moment we send them when load is updated for CFS, bandwidth for > DL and at enqueue/dequeue for RT. But this can lead to too many updates > sent in a short period of time and potentially be ignored at a critical > moment due to the rate_limit_us in schedutil. > > For example, simultaneous task enqueue on the CPU where 2nd task is > bigger and requires higher freq. The trigger to cpufreq_update_util() by > the first task will lead to dropping the 2nd request until tick. Or > another CPU in the same policy triggers a freq update shortly after. > > Updates at enqueue for RT are not strictly required. Though they do help > to reduce the delay for switching the frequency and the potential > observation of lower frequency during this delay. But current logic > doesn't intentionally (at least to my understanding) try to speed up the > request. > > To help reduce the amount of cpufreq updates and make them more > purposeful, consolidate them into these locations: > > 1. context_switch() > 2. task_tick_fair() > 3. {attach, detach}_entity_load_avg() > 4. update_blocked_averages() > > The update at context switch should help guarantee that DL and RT get > the right frequency straightaway when they're RUNNING. As mentioned > though the update will happen slightly after enqueue_task(); though in > an ideal world these tasks should be RUNNING ASAP and this additional > delay should be negligible. For fair tasks we need to make sure we send > a single update for every decay for the root cfs_rq. Any changes to the > rq will be deferred until the next task is ready to run, or we hit TICK. > But we are guaranteed the task is running at a level that meets its > requirements after enqueue. > > To guarantee RT and DL tasks updates are never missed, we add a new > SCHED_CPUFREQ_FORCE_UPDATE to ignore the rate_limit_us. If we are > already running at the right freq, the governor will end up doing > nothing, but we eliminate the risk of the task ending up accidentally > running at the wrong freq due to rate_limit_us. There may be two things in this patch: 1. Have well-defined, centralized places where we update CPU frequency. 2. The FORCE_UPDATE flag. I agree that at the moment, frequency updates inside the scheduler are scattered around in many places, and they can be called consecutively in a short period of time. Defining those places explicitly instead of triggering frequency updates here and there sounds like a good idea, so I definitely support 1. Not sure about 2. I think rate limit is there for a reason, although I don't have that many platforms to test on to know whether forcing the update is a problem. > > [...]