Re: [PATCH 1/2] sched/uclamp: Fix initialization of strut uclamp_rq

[Qais Yousef]

On 06/19/20 20:13, Peter Zijlstra wrote:
> On Fri, Jun 19, 2020 at 06:39:44PM +0100, Qais Yousef wrote:
> > On 06/19/20 19:30, Peter Zijlstra wrote:
> > > On Thu, Jun 18, 2020 at 08:55:24PM +0100, Qais Yousef wrote:
> > > 
> > > > +	for_each_clamp_id(clamp_id) {
> > > > +		memset(uc_rq[clamp_id].bucket,
> > > > +		       0,
> > > > +		       sizeof(struct uclamp_bucket)*UCLAMP_BUCKETS);
> > > > +
> > > > +		uc_rq[clamp_id].value = uclamp_none(clamp_id);
> > > 
> > > I think you can replace all that with:
> > > 
> > > 		*uc_rq = (struct uclamp_rq){
> > > 			.value = uclamp_none(clamp_id),
> > > 		};
> > > 
> > > it's shorter and is free or weird line-breaks :-)
> > 
> > Sure. I just sent v2 so that people will be encouraged to run tests hopefully.
> > But will fix in v3.
> > 
> > Do we actually need to 0 out anything here? Shouldn't the runqueues all be in
> > BSS which gets initialized to 0 by default at boot?
> > 
> > Maybe better stay explicit..
> 
> C99 named initializer (as used here) explicitly zero initializes all
> unnamed members. Is that explicit enough? ;-)

Hehe yes, but what I meant is that unless

	DEFINE_PER_CPU_SHARED_ALIGNED(struct rq, runqueues);

has some special rules, it should be in BSS and already zeroed out when we do
sched_init(). So do we really need to explicitly zero out anything? It seems
redundant, but again maybe being explicit is more readable,
so maybe better keep it the way it is (named initializer of struct).

Thanks

--
Qais Yousef