Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 8 Oct 2020 21:58:07 +0800
From:   Peng Liu <iwtbavbm@gmail.com>
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     mingo@redhat.com, juri.lelli@redhat.com,
        vincent.guittot@linaro.org, dietmar.eggemann@arm.com,
        rostedt@goodmis.org, bsegall@google.com, mgorman@suse.de,
        bristot@redhat.com, valentin.schneider@arm.com, raistlin@linux.it,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 1/2] sched/deadline: optimize
 sched_dl_global_validate()
Message-ID: <20201008135807.GA29010@iZj6chx1xj0e0buvshuecpZ>
References: <cover.1601993091.git.iwtbavbm@gmail.com>
 <9ad8eff54a3dd6f7f0da50e827047e4d3c4bb00a.1601993091.git.iwtbavbm@gmail.com>
 <20201007165533.GL2628@hirez.programming.kicks-ass.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20201007165533.GL2628@hirez.programming.kicks-ass.net>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk

On Wed, Oct 07, 2020 at 06:55:33PM +0200, Peter Zijlstra wrote:
> On Wed, Oct 07, 2020 at 11:12:29PM +0800, Peng Liu wrote:
> > +/* Used for dl_bw check and update. */
> > +static u32 dl_generation;
> 
> > diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> > index 28709f6b0975..53477e8b26b0 100644
> > --- a/kernel/sched/sched.h
> > +++ b/kernel/sched/sched.h
> > @@ -798,6 +798,13 @@ struct root_domain {
> >  	 */
> >  	cpumask_var_t		dlo_mask;
> >  	atomic_t		dlo_count;
> > +
> > +	/*
> > +	 * Indicate whether a root_domain's dl_bw has been checked or
> > +	 * updated. It's monotonously increasing, then wrap around.
> > +	 */
> > +	u32 visit_gen;
> > +
> >  	struct dl_bw		dl_bw;
> >  	struct cpudl		cpudl;
> >  
> > diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> > index dd7770226086..90f3e5558fa2 100644
> > --- a/kernel/sched/topology.c
> > +++ b/kernel/sched/topology.c
> > @@ -516,6 +516,7 @@ static int init_rootdomain(struct root_domain *rd)
> >  	init_irq_work(&rd->rto_push_work, rto_push_irq_work_func);
> >  #endif
> >  
> > +	rd->visit_gen = 0;
> >  	init_dl_bw(&rd->dl_bw);
> >  	if (cpudl_init(&rd->cpudl) != 0)
> >  		goto free_rto_mask;
> 
> I'm fairly sure I made the generation a u64, the above is susceptible to
> a false positive due to wrap-around.
> 
> Increase the generation to -1, create a new root domain, then the next
> generation is 0 and we'll skip the new domain, even though it should be
> updated.

Ah... at first, I also thought that u32 is "big enough" given that
no one would frequently change the settings, 'wrap-around' shouldn't
be a concern.

So...OK, I will revert it back to u64. What a big circle! :)

Thanks for your time!