Messages in this thread | | | Date | Wed, 2 Nov 2022 17:59:22 +0100 | From | Michal Koutný <> | Subject | Re: [PATCH v2] sched: async unthrottling for cfs bandwidth |
| |
Hello.
On Wed, Oct 26, 2022 at 03:44:49PM -0700, Josh Don <joshdon@google.com> wrote: > To fix this, we can instead unthrottle cfs_rq's asynchronously via a > CSD. Each cpu is responsible for unthrottling itself, thus sharding the > total work more fairly across the system, and avoiding hard lockups.
FIFO behavior of the cfs_b->throttled_cfs_rq is quite important to ensure fairness of throttling (historically when it FIFO wasn't honored, it caused some cfs_rq starving issues).
Despite its name, distribute_cfs_runtime() doesn't distribute the runtime, the time is pulled inside assign_cfs_rq_runtime() (but that's already on target cpu). Currently, it's all synchronized under cfs_b->lock but with your change, throttled cfs_rq would be dissolved among cpus that'd run concurrently (assign_cfs_rq_runtime() still takes cfs_b->lock but it won't be necessarily in the unthrottling order).
Have you observed any such fairness issues? [1][2]
> +static inline void __unthrottle_cfs_rq_async(struct cfs_rq *cfs_rq) > [...] > + if (rq == this_rq()) { > + unthrottle_cfs_rq(cfs_rq); > + return; > + }
It was pointed out to me that generic_exec_single() does something similar. Wouldn't the flow bandwidth control code be simpler relying on that?
Also, can a particular cfs_rq be on both cfs_b->throttled_csd_list and cfs_b->throttled_cfs_rq lists at any moment? I wonder if having a single list_head node in cfs_rq would be feasible (and hence enforcing this constraint in data).
Regards, Michal
[1] I'm not familiar with IPIs, just to illustrate the concurrency: the fairness could be skewed towards CPUs that are on same "NUMA" node as the timer callback if closer CPUs received them sooner.
[2] Currently, I don't think it's a prohibitive issue because with my reasoning even the current code relies on cfs_b->lock being a queued spinlock to ensure the FIFO of cfs_b->throttled_cfs_rq. [unhandled content-type:application/pgp-signature] | |