On Wed, Nov 07, 2018 at 02:48:09PM +0000, Patrick Bellasi wrote: > On 07-Nov 14:35, Peter Zijlstra wrote: > You mean se_count overflow ?
Yah.. > > And I'm not really a fan of hiding that error in a define like you keep > > doing. > > The #define is not there to mask an overflow, it's there to catch the +#define UCLAMP_MAPERR "clamp value [%u] mapping to clamp group failed\n" Is what I was talking about. > > What's wrong with something like: > > > > if (SCHED_WARN(free_group_id == UCLAMP_GROUPS)) > > return; > > Right, the flow should be: > > 1. try to find a valid clamp group > 2. if you don't find one, the data structures are corrupted > warn once for data corruption > do not map this scheduling entity and return > 3. map the scheduling entity > > Is that ok ? That's what the proposed does. > > and > > > > > + uc_map_new.se_count = uc_map_old.se_count + 1; > > > > if (SCHED_WARN(!new.se_count)) > > new.se_count = -1; > > Mmm... not sure we can recover from a corrupted refcount or an > overflow. > > What should we do on these cases, disable uclamp completely ? You can teach put to never decrement -1 (aka. all 1s). But its all SCHED_DEBUG stuff anyway, so who really cares.
