Messages in this thread | | | From | Xuewen Yan <> | Date | Tue, 23 Apr 2024 11:05:20 +0800 | Subject | Re: [PATCH v2] sched/eevdf: Prevent vlag from going out of bounds when reweight_eevdf |
| |
On Mon, Apr 22, 2024 at 11:59 PM Peter Zijlstra <peterz@infradead.org> wrote: > > On Mon, Apr 22, 2024 at 09:12:12PM +0800, Xuewen Yan wrote: > > > By adding a log to observe weight changes in reweight_entity, I found > > that calc_group_shares() often causes new_weight to become very small: > > Yes, cgroups do that. But over-all that should not matter no? > > Specifically, the whole re-weight thing turns into a series like: > > w_0 w_1 w_n-1 w_0 > S = --- * --- * ... * ----- = --- > w_1 w_2 w_n w_n > > Where S is our ultimate scale factor. > > So even if w_m (0 < m < n) is 2, it completely disappears. But yes, it > will create a big term, which is why the initial vlag should be limited.
Okay, I understand what you mean. Even if the weight during dequeue is very small, the weight will be eliminated during enqueue. In this case, the necessity of the !on_rq case does not seem to be very important.
On the other hand, the following case: place_entity() { .. 5244 load = cfs_rq->avg_load; 5245 if (curr && curr->on_rq) 5246 load += scale_load_down(curr->load.weight); 5247 5248 lag *= load + scale_load_down(se->load.weight); 5249 if (WARN_ON_ONCE(!load)) 5250 load = 1; 5251 lag = div_s64(lag, load);<<<< .. } reweight_eevdf() { .. if (avruntime != se->vruntime) { 3770 vlag = entity_lag(avruntime, se); 3771 vlag = div_s64(vlag * old_weight, weight); <<<< 3772 se->vruntime = avruntime - vlag; 3773 } .... }
There is no need to clamp the above two positions because these two calculations will not theoretically cause s64 overflow?
Thanks!
> > Notably, nice should not exceed 88761*1024 / 2, but I'm not sure I > remember the limits (if there are any on the cgrou pmuck). > > But if roughly 27 bits go to weight, then vlag should not exceed 36, > which should be well within the slice limit iirc. > > Also, as said before, due to integer division being truncating, the > actual S should be smaller than the expected S due to error > accumulation. > > Anyway, the things to verify are: > > - the S series is complete -- missing terms will mess things up right > quick; > > - the limits on both the weight and vlag part, their sum exceeding > 63bit (plut 1 for sign) will also mess things up.
| |