From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E5E99C433EF for ; Tue, 9 Nov 2021 12:29:41 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C54A16108D for ; Tue, 9 Nov 2021 12:29:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242431AbhKIMc0 (ORCPT ); Tue, 9 Nov 2021 07:32:26 -0500 Received: from foss.arm.com ([217.140.110.172]:33146 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231250AbhKIMcZ (ORCPT ); Tue, 9 Nov 2021 07:32:25 -0500 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 188BDED1; Tue, 9 Nov 2021 04:29:39 -0800 (PST) Received: from [192.168.178.6] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 26B0F3F7F5; Tue, 9 Nov 2021 04:29:37 -0800 (PST) Subject: Re: [Resend PATCH] psi : calc cfs task memstall time more precisely To: Zhaoyang Huang , Xuewen Yan Cc: Johannes Weiner , Andrew Morton , Michal Hocko , Vladimir Davydov , Zhaoyang Huang , "open list:MEMORY MANAGEMENT" , LKML , Peter Zijlstra , Vincent Guittot , xuewen.yan@unisoc.com, Ke Wang References: <1634278612-17055-1-git-send-email-huangzhaoyang@gmail.com> <78b3f72b-3fe7-f2e0-0e6b-32f28b8ce777@arm.com> <85c81ab7-49ed-aba5-6221-ea6a8f37f8ad@arm.com> From: Dietmar Eggemann Message-ID: <25828616-1976-6e24-cd7d-a95b789f1bc0@arm.com> Date: Tue, 9 Nov 2021 13:29:35 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 08/11/2021 10:20, Zhaoyang Huang wrote: > On Mon, Nov 8, 2021 at 4:49 PM Xuewen Yan wrote: >> >> Hi Dietmar >> >> On Sat, Nov 6, 2021 at 1:20 AM Dietmar Eggemann >> wrote: >>> >>> On 05/11/2021 06:58, Zhaoyang Huang wrote: [...] >>>>> This will let the idle task (swapper) pass. Is this indented? Or do you >>>>> want to only let CFS tasks (including SCHED_IDLE) pass? >>>> idle tasks will NOT call psi_memstall_xxx. We just want CFS tasks to >>>> scale the STALL time. >>> >>> Not sure I get this. >>> >>> __schedule() -> psi_sched_switch() -> psi_task_change() -> >>> psi_group_change() -> record_times() -> psi_memtime_fixup() >>> >>> is something else than calling psi_memstall_enter() or _leave()? >>> >>> IMHO, at least record_times() can be called with current equal >>> swapper/X. Or is it that PSI_MEM_SOME is never set for the idle task in >>> this callstack? I don't know the PSI internals. > According to my understanding, PSI_MEM_SOME represents the CORE's > state within which there is at least one task trapped in memstall > path(only counted in by calling PSI_MEMSTALL_ENTER). record_times is > responsible for collecting the delta time of the CORE since it start. > What we are doing is to make the delta time more precise. So idle task > is irrelevant for these. Coming back to the original snippet of the patch. static unsigned long psi_memtime_fixup(u32 growth) { if (!(current->policy == SCHED_NORMAL || current->policy == SCHED_BATCH)) return growth_fixed; With this condition: (1) you're not bailing when current is the idle task. It has policy equal 0 (SCHED_NORMAL) (2) But you're bailing for a SCHED_IDLE (CFS) task. I'm not sure that this is indented here? Since you want to do the scaling later based on whats left for CFS tasks from the CPU capacity my hunch is that you want to rather do: if (current->sched_class != &fair_sched_class) return growth_fixed; What's the possible sched classes of current in psi_memtime_fixup?