Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 21 Oct 2022 12:34:08 +0800
From:   Chen Yu <yu.c.chen@intel.com>
To:     Abel Wu <wuyun.abel@bytedance.com>
CC:     Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@kernel.org>, Mel Gorman <mgorman@suse.de>,
        Vincent Guittot <vincent.guittot@linaro.org>,
        Dietmar Eggemann <dietmar.eggemann@arm.com>,
        Valentin Schneider <valentin.schneider@arm.com>,
        Josh Don <joshdon@google.com>,
        Tim Chen <tim.c.chen@linux.intel.com>,
        K Prateek Nayak <kprateek.nayak@amd.com>,
        "Gautham R . Shenoy" <gautham.shenoy@amd.com>,
        Aubrey Li <aubrey.li@intel.com>,
        Qais Yousef <qais.yousef@arm.com>,
        Juri Lelli <juri.lelli@redhat.com>,
        Rik van Riel <riel@surriel.com>,
        Yicong Yang <yangyicong@huawei.com>,
        Barry Song <21cnbao@gmail.com>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v6 3/4] sched/fair: Introduce SIS_CORE
Message-ID: <Y1IhQPWMX3TX+sYo@chenyu5-mobl1>
References: <20221019122859.18399-1-wuyun.abel@bytedance.com>
 <20221019122859.18399-4-wuyun.abel@bytedance.com>
 <Y1IZ/DZ1SWMn0QDs@chenyu5-mobl1>
 <9ec8a474-d923-953c-0b73-02ba2fd6ea82@bytedance.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <9ec8a474-d923-953c-0b73-02ba2fd6ea82@bytedance.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?9qYAt6sPZ5D2duK5ktLT9kyEXOpRRHxqRhODgBsci+7/Ke3wyISruyxDA2NM?=
 =?us-ascii?Q?sbpTHj/nSWA3vtH6YyMEyz+xHHfB0fsIRqvUI267Gmh+tcVED7CCEXxowWAQ?=
 =?us-ascii?Q?kFs6dxHoTmtyS0+WTTfuVa9Qy5hatXJmuSBU5vOnr1Tm8QY2hTg4suUO6Wgs?=
 =?us-ascii?Q?ImARW5aEtfIeJmwxixqhoucbi8keXMsPOIHD0LtaoTtqTI/QKqqoSwhICokV?=
 =?us-ascii?Q?nUojQzFYM3g4HC040U1JlNA43fwvbmU0OyIu9jWdDokTLF6TjeFVgTP83vUj?=
 =?us-ascii?Q?c2g2z0Lig5gAGaJIc8FkYUQK9pKiHRrJSpy/p9PD2lDSWN241su1shXUhdy+?=
 =?us-ascii?Q?pBNd4JRfVhkYVtmcaF14mraMkciYvzMxRiLZbMwSxjNMI8aTLlgRwqZvu72Q?=
 =?us-ascii?Q?nUv50ljQVtu0ARgas2unrMlAUrbhSXeDhybaK+iHBd/gjF6HP+N/tzVit8/9?=
 =?us-ascii?Q?PJwFbBliY/3c/ZIQqd5aaZC9vdEI2fzwAPfgAvyFUNmpQ2wAYYq+0IejFnxU?=
 =?us-ascii?Q?q/TA/3pmgqzOOdz1/kVM623kqNZ9c2l6ZnG7eik5hsOy0Yo7K+Sbw2iNOeW+?=
 =?us-ascii?Q?Rk58jFXSHbEY7h9doWQkFgxjQQy3QCQdM7jvhdT8FBSFCJjLnCCUCWOJ3ZhO?=
 =?us-ascii?Q?FDRzZNMYS35/V9e9G4ZJv2ql1z4jCW+08aAfCSfGcnMpYqxj9GjK8cIiUmyh?=
 =?us-ascii?Q?M9Djyri14UqlRL7LbPIzF+x3nVeX4uF5hACr7OKXI9WJKGhgURadcPtborFg?=
 =?us-ascii?Q?zeGK7mV32IrhvWhRpc6eJqVsnx707PUJfaHveM1lbdwwwt1Ezch2rwRgaWoI?=
 =?us-ascii?Q?5vsg4NEgLSCc3Uh+qRKSat3dQ4UH4V2OU2Y1BKvjaD+KRiRog8Kp9rXDxJu9?=
 =?us-ascii?Q?yBjkHo0E0kF828GaOhRE85w1SbU0ao1tI1c3k8pAiRLKT6iXD9V6nwzfex5d?=
 =?us-ascii?Q?/oCW/W+ozFq8o2NWWMwgXzI5CJAzfyZbuwg1Uqp6fOsAgnSQFNnBdwkEoOZk?=
 =?us-ascii?Q?eDrKaS3rhtJtXg3D2oG5VtNbpdVVeTFeShQv/heFPsMABmZ2fxs2F3vpbpjQ?=
 =?us-ascii?Q?ZnzqnO5DviWxnE1F3rTe7qLkcowl80o1sp6VLubCeCZzd8vYQ+jTsoFhJeya?=
 =?us-ascii?Q?W8dxAzTWp66A4pzJYS7FY/u277NIU6FyF4Zo4K6yfYZJEZYUozzxbjtVMgyL?=
 =?us-ascii?Q?bDk+1Xq9u/Euf9CGMVZp9th+ud5G53VTcPFRSFCbRCI/Yvv7oVVHYCqNq9ID?=
 =?us-ascii?Q?Groae8SEAlQS9tjIxXN5jsIIdsV7vJQYMxEstrEWOsQTdRu5P4ih2sIxEx3f?=
 =?us-ascii?Q?9M9Y16Wp80aL5CpyGzPEIXLPOrWYSZ08NUmhWxS3YnDgKa3RMnj7KWwippX/?=
 =?us-ascii?Q?hDvSLFtHXDt9qplDYzty90DujpdNUlyFPDyoJML5Il00e3MuGj0wsuFFfbq7?=
 =?us-ascii?Q?OfEALseckiyvT6xVQOFyzJ54lpTd/N65R1u7MHfFW1rqUMUQGyhF8W83Zqyc?=
 =?us-ascii?Q?xlkFEDgAAW2CtnxTVmCVovwRlw5Zky+/kSAZ+BDuyD9sIvswNevmi47aMg9V?=
 =?us-ascii?Q?DpdK2wvRnoc0HAFu6yEnLW6RTAC7ursnpi7pjP8F?=
X-MS-Exchange-CrossTenant-Network-Message-Id: e18c753d-a583-4414-c369-08dab31d88db
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6206.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Oct 2022 04:34:25.9435
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Vfw9vuO+c+I5bi2vxvMTTIL/lVXvgbsYATlvhKG33VE5ZBXFhX21GsH3SIIQdSifkKlK/ltYV8VdnaOjC06pkg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR11MB4879
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE,URIBL_BLOCKED autolearn=ham autolearn_force=no
        version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 2022-10-21 at 12:30:56 +0800, Abel Wu wrote:
> Hi Chen, thanks for your reviewing!
> 
> On 10/21/22 12:03 PM, Chen Yu wrote:
> > On 2022-10-19 at 20:28:58 +0800, Abel Wu wrote:
> > [cut]
> > > A major concern is the accuracy of the idle cpumask. A cpu present
> > > in the mask might not be idle any more, which is called the false
> > > positive cpu. Such cpus will negate lots of benefit this feature
> > > brings. The strategy against the false positives will be introduced
> > > in next patch.
> > > 
> > I was thinking that, if patch[3/4] needs [4/4] to fix the false positives,
> > maybe SIS_CORE could be disabled by default in 3/4 but enabled
> > in 4/4? So this might facilicate git bisect in case of any regression
> > check?
> 
> Agreed. Will fix in next version.
> 
> > [cut]
> > > + * To honor the rule of CORE granule update, set this cpu to the LLC idle
> > > + * cpumask only if there is no cpu of this core showed up in the cpumask.
> > > + */
> > > +static void update_idle_cpu(int cpu)
> > > +{
> > > +	struct sched_domain_shared *sds;
> > > +
> > > +	if (!sched_feat(SIS_CORE))
> > > +		return;
> > > +
> > > +	sds = rcu_dereference(per_cpu(sd_llc_shared, cpu));
> > > +	if (sds) {
> > > +		struct cpumask *icpus = to_cpumask(sds->icpus);
> > > +
> > > +		/*
> > > +		 * This is racy against clearing in select_idle_cpu(),
> > > +		 * and can lead to idle cpus miss the chance to be set to
> > > +		 * the idle cpumask, thus the idle cpus are temporarily
> > > +		 * out of reach in SIS domain scan. But it should be rare
> > > +		 * and we still have ILB to kick them working.
> > > +		 */
> > > +		if (!cpumask_intersects(cpu_smt_mask(cpu), icpus))
> > > +			cpumask_set_cpu(cpu, icpus);
> > Maybe I miss something, here we only set one CPU in the icpus, but
> > when we reach update_idle_cpu(), all SMT siblings of 'cpu' are idle,
> > is this intended for 'CORE granule update'?
> 
> The __update_idle_core() is called by all the cpus that need to go idle
> to update has_idle_core if necessary, and update_idle_cpu() is called
> before that check.
>
I see.

Since __update_idle_core() has checked all SMT siblings of 'cpu' if
they are idle, can that information also be updated to icpus?

thanks,
Chenyu 
> Thanks,
> Abel
>