On Mon, Oct 05, 2020 at 11:11:00AM +0200, Juri Lelli wrote:
...
> > /*
> > * Here we want to check the bandwidth not being set to some
> > * value smaller than the currently allocated bandwidth in
> > * any of the root_domains.
> > - *
> > - * FIXME: Cycling on all the CPUs is overdoing, but simpler than
> > - * cycling on root_domains... Discussion on different/better
> > - * solutions is welcome!
>
> So, this patch changes 2 things: it actually fixes the problem and it
> optimizes scanning of root domains. Even though the changes are limited,
> I'd be more comfortable if we split them in two (fix + optimization).
>
> Would you be up for doing it?
OK, I will split them.
>
> > struct dl_bandwidth {
> > raw_spinlock_t dl_runtime_lock;
> > @@ -801,6 +795,8 @@ struct root_domain {
> > struct dl_bw dl_bw;
> > struct cpudl cpudl;
> >
> > + u64 visit_gen;
> > +
>
> I think this deserves a comment explaining what it is and how it's used.
>
> Also, do we really need an u64? Maybe an smaller type would be OK as
> well and could fit in the 4 bytes hole that seems available after
> dlo_count, I'm thinking.
>
> Thanks,
> Juri
>
u32 is also fine, after all consecutively 2^32 - 1 times validation
failure followed by a success in sched_dl_bandwidth_validate() is still
a very very unlikely case.
On a 64-bit machine, dlo_count should be aligned on a 8 bytes boundary,
there would be a 4 bytes hole between dlo_mask and dl_bw, put the new
member there seems reasonable. I'm OK with this.
