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:   Mon, 22 May 2023 12:10:57 +0800
From:   Chen Yu <yu.c.chen@intel.com>
To:     K Prateek Nayak <kprateek.nayak@amd.com>
CC:     Mike Galbraith <efault@gmx.de>,
        Peter Zijlstra <peterz@infradead.org>,
        Vincent Guittot <vincent.guittot@linaro.org>,
        Ingo Molnar <mingo@redhat.com>,
        Juri Lelli <juri.lelli@redhat.com>,
        Mel Gorman <mgorman@techsingularity.net>,
        Tim Chen <tim.c.chen@intel.com>,
        Dietmar Eggemann <dietmar.eggemann@arm.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        Abel Wu <wuyun.abel@bytedance.com>,
        Yicong Yang <yangyicong@hisilicon.com>,
        "Gautham R . Shenoy" <gautham.shenoy@amd.com>,
        Len Brown <len.brown@intel.com>,
        Chen Yu <yu.chen.surf@gmail.com>,
        Arjan Van De Ven <arjan.van.de.ven@intel.com>,
        Aaron Lu <aaron.lu@intel.com>, Barry Song <baohua@kernel.org>,
        <linux-kernel@vger.kernel.org>
Subject: Re: [RFC PATCH] sched/fair: Introduce SIS_PAIR to wakeup task on
 local idle core first
Message-ID: <ZGrrUVZBY6qqeS0K@chenyu5-mobl1>
References: <20230516011159.4552-1-yu.c.chen@intel.com>
 <19664c68f77f5b23a86e5636a17ad2cbfa073f78.camel@gmx.de>
 <ZGNBt7vWJ3fDs5Sc@chenyu5-mobl1>
 <795a6d9475ecb444d219a9d36dc93b48a69e960e.camel@gmx.de>
 <ZGUHa+Si4dJbdsZN@chenyu5-mobl1>
 <0ac968e3-cd80-6339-970d-37005876b145@amd.com>
 <ZGWmveL2YTiXp2XR@chenyu5-mobl1>
 <12061140-4f09-b83f-843c-2fb8ff9f6e81@amd.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <12061140-4f09-b83f-843c-2fb8ff9f6e81@amd.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?V7qjebyzTTf4ersh/U4P3vu0iwjkhgGDqYbit0Is1u3QBH2k6ptnr5fXHebn?=
 =?us-ascii?Q?zpdUOSOZ44xzCCsCw+YdY5XoXU3dvf6YkQGYbuk+74TR4N3KgsfPIo6/eJrh?=
 =?us-ascii?Q?9kAUuSDo3aaisyaOYMIhCrO67HCuwZIQBeMfLtuBiMVw5PE58XOwV+sDKcd7?=
 =?us-ascii?Q?hau45zdqYdSAA0tzTxQmdwXEGEtXmU1oh8hBk6KJ19d7R52uFXzUWyD4v6JU?=
 =?us-ascii?Q?rzTo4zZO56Uc3MAF3b0FGEwq/cNojShyMbXUsF97wF3ZvMWmvNpF/cgpAymn?=
 =?us-ascii?Q?0PhKS9O4jo/6ND0ZrSgfvUE25kjm6LLYJB4bgtRFU3YmTBj3CcZeZS+nUt1P?=
 =?us-ascii?Q?nUfyUmFZ7pka4v6qDPmfabCYjbl0jtB3Gj6RL9lKNzeYDncLUOzsO1+xeaB4?=
 =?us-ascii?Q?nx5kLUEqwjhdjOevU9jaULiyQHrNDv7In3s4Ei5+vDf/GQ17fH+EMJ9UBVwz?=
 =?us-ascii?Q?a8dgCFcSFqnIugLyrrqmU4Zcsp4l6LcEhe1K19LbIhIZ3CLT+E+Jf8wihhk8?=
 =?us-ascii?Q?UwosnJcv9zSAhstXT72HYt1Og/Cm3tFmyPpBcZog21S5rZOZy9dy+ZQcKJPv?=
 =?us-ascii?Q?FX64Pd8eO3kV7y/4JM2quCqWxT3uunpAkL3bKwTe6bRwpB2/YeIMhkkXWh3Y?=
 =?us-ascii?Q?rn7Cjr2oIrGeVXqLspj1HFW5hOLWHNK4z+BzY6ckkXERBXQPMxBJMoINv3O1?=
 =?us-ascii?Q?sg8XEPfaiTln2ZIxPZRnUQE8pvCZvCMBX9L2FBWeWb5WEr4JyBOyZyjUf7jr?=
 =?us-ascii?Q?ze6YCes9lanQkbGz6fB65yRZQvD/72d7pX00aEDiN1WqAtb/M4FVUrLSCJHO?=
 =?us-ascii?Q?mic0NVI8JGkUdihQTjxW5LIFIGoOpZSmdaV4zjeavKbnSMn9R6Huh/orYr5B?=
 =?us-ascii?Q?oQXKGQJ11eOh6KP8jd+dfW0fxR7+upfOg3UdoimbcTvQL25E0ekU9ADdkp5b?=
 =?us-ascii?Q?pKQTpDKLevmQ4N6LcGNv0OIxwmMdMINALjZVQp7vPgwKvyfc4IxeoKjdoVg9?=
 =?us-ascii?Q?aPwZkySCb6wnGO0oEaKOan48U1HWg/Psc2C0DC2BmFqiPC1fxdp4zL6lD6R3?=
 =?us-ascii?Q?5lyp5qJ0YyYpFDe4waW4NcpCsZ7o3vPVohsEVdfALijhy9qeMGWALRWix5HR?=
 =?us-ascii?Q?4N9idgR60mipcLzj0C9CJv6I7xwdDtJqKkkkh3Mb40EA34+coXf5nZzGVVCu?=
 =?us-ascii?Q?6ZuTW4QO05k5FI5YGEaxEBYdFBItGfjNol9o2Jvv0gNjMgfJZsKzVm8twcmy?=
 =?us-ascii?Q?zT1oCorNXggivgT4+tU5kWAM6iVMOtB/QSfW50dpey1UerLo3q/oJ6EsHtEA?=
 =?us-ascii?Q?6JIrT429VhGJh07h+6931VFtkHrMNdcYpxC9ol0EImupJisdY82lP1hqjAbO?=
 =?us-ascii?Q?csKhHRCxcpmiGf67V+VQK/BawEL4bqf1ZR6dhpdQF44jQLbL8XlxJqC+2HIt?=
 =?us-ascii?Q?ydS/M4yAnejGfcIGngFv0KGTJ++H7h29xuGZpEzGSjezFBXL0XN6VFkJFe3V?=
 =?us-ascii?Q?NwXOaNUdIpCfxPMnfL/AWkfiwoKtwEqdWQx01Zdzk/eekTwxWIQNUVXszq7P?=
 =?us-ascii?Q?cm2LwxkGGJK7sT0YSUGnadAB0arPPPt3r7drR1HV?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 0863e868-bb43-4164-c532-08db5a7a982a
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6206.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 May 2023 04:11:19.3006
 (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: 9D3hMpzJaf4UDfQZ22BcjQSRdg0QjTNSz4lx5XatpMSOaOvtDoCgbXjAPZ2ZgnAU/+W8d1nmQMrRGL9pCBI9UQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN9PR11MB5530
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 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,T_SCC_BODY_TEXT_LINE 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 2023-05-18 at 15:56:12 +0530, K Prateek Nayak wrote:
[snip]
> >>
> >> Also wondering if asym_fits_cpu() check is needed in some way here.
> >> Consider a case where waker is on a weaker capacity CPU but wakee
> >> previously ran on a stronger capacity CPU. It might be worthwhile
> >> to wake the wakee on previous CPU if the current CPU does not fit
> >> the task's utilization and move the pair to the CPU with larger
> >> capacity during the next wakeup. wake_affine_weight() would select
> >> a target based on load and capacity consideration but here we
> >> switch the wakeup target to a thread on the current core.
> >>
> >> Wondering if the capacity details already considered in the path?
> >>
> > Good point, I guess what you mean is that, target could be other CPU rather than
> > the current one, there should be a check if the target equals to current CPU.
> 
> Yup. That should handle the asymmetric capacity condition too but
> wondering if it makes the case too narrow to see the same benefit.
> 
> Can you perhaps try "cpus_share_cache(target, smp_processor_id())"
> instead of a "target == smp_processor_id()"? Since we use similar
> logic to test if p->recent_used_cpu is a good target or not?
> 
> This will be equivalent to your current implementation for a single
> socket with one LLC and as for dual socket or multiple LLC case,
> we can be sure "has_idle_core" is indicates the status of MC which
> is shared by both target and current cpu.
>
Right, in this way we can avoid the issue that target and current CPU
are in difference LLCs and has_idle_core does not reflect that.
And asym_fits_cpu() might also be needed to check if the task can fit in.
> > Let me refine the patch and have a test.
> > 
> 
> I'll hold off queuing a full test run until then.
>
Thank you. I'm also thinking of removing the check of last_wakee,
so there is no much heuristic involved. I'll do some investigation.

Meanwhile, I looked back at Yicong's proposal on waking up task
on local cluster first. It did show some improvement on Jacobsville,
I guess that could also be a chance to reduce C2C latency.

thanks,
Chenyu
> > thanks,
> > Chenyu
> >
> > [..snip..]
> --
> Thanks and Regards,
> Prateek