Messages in this thread | | | From | Valentin Schneider <> | Subject | Re: [RFC PATCH v2 0/5] sched/fair: Defer CFS throttle to user entry | Date | Wed, 07 Feb 2024 14:34:35 +0100 |
| |
On 06/02/24 13:55, Benjamin Segall wrote: > Valentin Schneider <vschneid@redhat.com> writes: > > >> Proposed approach >> ================= >> >> Peter mentioned [1] that there have been discussions on changing /when/ the >> throttling happens: rather than have it be done immediately upon updating >> the runtime statistics and realizing the cfs_rq has depleted its quota, we wait >> for the task to be about to return to userspace: if it's in userspace, it can't >> hold any in-kernel lock. >> >> I submitted an initial jab at this [2] and Ben Segall added his own version to >> the conversation [3]. This series contains Ben's patch plus my additions. The >> main change here is updating the .h_nr_running counts throughout the cfs_rq >> hierachies to improve the picture given to load_balance(). >> >> The main thing that remains doing for this series is making the second cfs_rq >> tree an actual RB tree (it's just a plain list ATM). >> >> This also doesn't touch rq.nr_running yet, I'm not entirely sure whether we want >> to expose this outside of CFS, but it is another field that's used by load balance. > > Then there's also all the load values as well; I don't know the load > balance code well, but it looks like the main thing would be > runnable_avg and that it isn't doing anything that would particularly > care about h_nr_running and runnable_avg being out of sync. >
Yes, all of the runnable, load and util averages are still going to be an issue unfortunately. AFAICT tackling this would imply pretty much dequeuing the throttle_pending user tasks, which was my earlier attempt.
> Maybe pulling a pending-throttle user task and then not seeing the > update in h_nr_running could be a bit of trouble?
That too is something I hadn't considered. Given the h_nr_running count is updated accordingly, we could change can_migrate_task() to only allow kernel tasks to be pulled if the hierarchy is ->throttle_pending. That would probably require implementing a throttle_pending_count (as you suggested in the other email) so we don't waste too much time checking up the hierarchy for every task.
| |