Message-ID: <3a3220f7-e35b-ee77-649c-8c75dcb26b6c@bytedance.com>
Date:   Fri, 17 Feb 2023 10:44:51 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.7.2
Subject: Re: [PATCH v5 2/2] sched/fair: Introduce SIS_SHORT to wake up short
 task on current CPU
Content-Language: en-US
To:     Chen Yu <yu.c.chen@intel.com>
Cc:     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>,
        Ben Segall <bsegall@google.com>,
        K Prateek Nayak <kprateek.nayak@amd.com>,
        Yicong Yang <yangyicong@hisilicon.com>,
        "Gautham R . Shenoy" <gautham.shenoy@amd.com>,
        Honglei Wang <wanghonglei@didichuxing.com>,
        Len Brown <len.brown@intel.com>,
        Chen Yu <yu.chen.surf@gmail.com>,
        Tianchen Ding <dtcccc@linux.alibaba.com>,
        Joel Fernandes <joel@joelfernandes.org>,
        Josh Don <joshdon@google.com>, Hillf Danton <hdanton@sina.com>,
        linux-kernel@vger.kernel.org,
        kernel test robot <yujie.liu@intel.com>
References: <cover.1675361144.git.yu.c.chen@intel.com>
 <1b8af8d99da99a20449288ab4fbba64dc05057ce.1675361144.git.yu.c.chen@intel.com>
 <36ba3b68-5b73-9db0-2247-061627b0d95a@bytedance.com>
 <Y+5Kwxnrj16xRklf@chenyu5-mobl1>
From:   Abel Wu <wuyun.abel@bytedance.com>
In-Reply-To: <Y+5Kwxnrj16xRklf@chenyu5-mobl1>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 2/16/23 11:24 PM, Chen Yu wrote:
>> The following change greatly reduced the p99lat of Redis service
>> from 150ms to 0.9ms, at exactly the same throughput (QPS).
>>
>> @@ -5763,6 +5787,9 @@ wake_affine_weight(struct sched_domain *sd, struct
>> task_struct *p,
>> 	s64 this_eff_load, prev_eff_load;
>> 	unsigned long task_load;
>>
>> +	if (is_short_task(p))
>> +		return nr_cpumask_bits;
>> +
> So above change wants to wake up the short task on its previous
> CPU if I understand correctly.

Yes.

>> 	this_eff_load = cpu_load(cpu_rq(this_cpu));
>>
>> 	if (sync) {
>>
>> I know that 'short' tasks are not necessarily 'small' tasks, e.g.
>> sleeping duration is small or have large weights, but this works
>> really well for this case. This is partly because delivering data
>> is memory bandwidth intensive hence prefer cache hot cpus. And I
>> think this is also applicable to the general purposes: do NOT let
>> the short running tasks suffering from cache misses caused by
>> migration.
>>
> I see. My original thought was to mitigate short task migration
> as much as possible. Either waking up the task on current CPU or previous
> CPU should both achieve the goal in theory. Could you please describe
> a little more about how Redis proxy server was tested? Was it tested
> locally or using multiple machines? I asked this because for network
> benchmarks, it might be better to wake the task close to the waker(maybe
> the NIC interrupt) due to hot network buffer. Anyway I will test
> your change slightly changed to see the impact, and also Redis. But it
> would be even better if you could provide some simple test steps I can
> try locally : )

Sorry for missing the info. The test was done in production environment,
and what I have done is only updating the kernel in several machines
which are highly loaded, that is over 85% cpu util observed by mpstat.
Please let me know if you want any specific info.

Best,
	Abel