From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM12-MW2-obe.outbound.protection.outlook.com (mail-mw2nam12on2084.outbound.protection.outlook.com [40.107.244.84]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B58D870831 for ; Fri, 27 Dec 2024 04:55:00 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=fail smtp.client-ip=40.107.244.84 ARC-Seal:i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1735275302; cv=fail; b=BAz343jPrITC2Xd68SNAEfkd2qm6RAtiv5sXvuEanbq/48u15SgXUF2T8NoGfXZR+RARiLjvgyIp74ysKiKcU9vBCH3snKN0xLC/yg0qZBIBpSeO4mfdFZrGuBcRQa+e2ScifiBfceKVYSTygJmbotlawgYSMdI9ssaCwJ2CiHA= ARC-Message-Signature:i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1735275302; c=relaxed/simple; bh=PHeBSg1gW7tuXLNhKA3gCwvoPFrzYrJn+YZtHSUrsxo=; h=Message-ID:Date:MIME-Version:Subject:To:CC:References:From: In-Reply-To:Content-Type; b=p4u1NECA4wCfDNjRDxF3fB5De1U5lijYJEuWwLz2adll34Xns0I5AR5CXt8swAvaB2ZpEfG7TQ8tAo4Vlo5MlwyEwqvnjdDphlsOvb/D+a0ezATRZGCVbDSCmRQBjMjHLFPODnNmXQ6HQMKQGhL9KJCuC7LL8yNnsjAdiI3IMNE= ARC-Authentication-Results:i=2; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=amd.com; spf=fail smtp.mailfrom=amd.com; dkim=pass (1024-bit key) header.d=amd.com header.i=@amd.com header.b=jbEv6F+8; arc=fail smtp.client-ip=40.107.244.84 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=amd.com Authentication-Results: smtp.subspace.kernel.org; spf=fail smtp.mailfrom=amd.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=amd.com header.i=@amd.com header.b="jbEv6F+8" ARC-Seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=kahIIqU9MyPFjp6zFaNO6dKGxFZWBYDT4AhE0P6CKJIXiNE15fwZCeEfMAK2KpCi7v+lgPlhsFkeBl+z9SbHXSpNaNuestN0Gl+99qXo6jmaZAlHfHsx5NoI9SnKQxovABVZebPPfwk+6MEkBeGfdhnnlS66nBvX3uLPpZychGVqhiEPiWBHkDxptP5gfM/o166e/b8u99jq3zChiU/TZtIsNP9ZhTpFuRiyr/DaZHGL1Ee4I0mZJCwwDunay935Z18JJdALEX7/oLlIZWAdbeJ+PY9Imgk123BD3aipm7pUJb/qkpr5BWZjNSh1bIrljPK3/hP41+a6a7SRYa3MwQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=+ilD33Oz96of9/uYNWxKCtBvotcYPOP4LyiqhVb1PvE=; b=qVP3QyupOhMQJHoRO/kFvbr8xeSKcngBEzFuFDPk56YMkGajeJQLmPbTxmrWZni+NgExrKsZ2wZ0JWyYjFmSDNwMhpqWxygPkLFkNOgFxbnB/hMpOfkx2lusoC8HJpx05l+fB2BWSUe2WPIa3SRlUmaDyygNn+sYtgb9XyX8byeyeJOrlh/4J7mSLTL1Fp8Nqg03mW1lsTDes2p/qFtQcV8WYd8ejurYyP8sHQo6hMFNqjeu4Z8/IOAIz1R1kL4IvaYjxGi/pZoIw8M68C7wkW19iRA59HayOuj9GkrOapn42K9RRn2jqP6Kdra56zx4HTTKiV9O3GD4d364YqJiJA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 165.204.84.17) smtp.rcpttodomain=linux.dev smtp.mailfrom=amd.com; dmarc=pass (p=quarantine sp=quarantine pct=100) action=none header.from=amd.com; dkim=none (message not signed); arc=none (0) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=+ilD33Oz96of9/uYNWxKCtBvotcYPOP4LyiqhVb1PvE=; b=jbEv6F+8/K7HbFDnSuzaN1SPKEB6PUSuxTMZO02oXfdMIjhlg97ucG6N047PvkcAttuG56PQDmbUgMBEtcqF2CFk9TJkgJcYpARP0LA1Sz0gd1jhbuBPv67q3lv7holY01AULvlwr0uDMQW2mtNb06nHqbcZN5BSIlV/MTSstd4= Received: from SN7PR04CA0216.namprd04.prod.outlook.com (2603:10b6:806:127::11) by DM4PR12MB6136.namprd12.prod.outlook.com (2603:10b6:8:a9::14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.8293.14; Fri, 27 Dec 2024 04:54:52 +0000 Received: from SA2PEPF00003F67.namprd04.prod.outlook.com (2603:10b6:806:127:cafe::72) by SN7PR04CA0216.outlook.office365.com (2603:10b6:806:127::11) with Microsoft SMTP Server (version=TLS1_3, cipher=TLS_AES_256_GCM_SHA384) id 15.20.8293.16 via Frontend Transport; Fri, 27 Dec 2024 04:54:52 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 165.204.84.17) smtp.mailfrom=amd.com; dkim=none (message not signed) header.d=none;dmarc=pass action=none header.from=amd.com; Received-SPF: Pass (protection.outlook.com: domain of amd.com designates 165.204.84.17 as permitted sender) receiver=protection.outlook.com; client-ip=165.204.84.17; helo=SATLEXMB04.amd.com; pr=C Received: from SATLEXMB04.amd.com (165.204.84.17) by SA2PEPF00003F67.mail.protection.outlook.com (10.167.248.42) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.20.8293.12 via Frontend Transport; Fri, 27 Dec 2024 04:54:52 +0000 Received: from [10.252.205.52] (10.180.168.240) by SATLEXMB04.amd.com (10.181.40.145) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.39; Thu, 26 Dec 2024 22:54:34 -0600 Message-ID: Date: Fri, 27 Dec 2024 10:24:26 +0530 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] psi: Fix race when task wakes up before psi_sched_switch() adjusts flags To: Chengming Zhou , Johannes Weiner , Suren Baghdasaryan , Ingo Molnar , Peter Zijlstra , Juri Lelli , Vincent Guittot , CC: Dietmar Eggemann , Steven Rostedt , Ben Segall , Mel Gorman , Valentin Schneider , Chengming Zhou , Muchun Song , "Gautham R. Shenoy" , Chuyi Zhou References: <20241226053441.1110-1-kprateek.nayak@amd.com> <20df37b9-c653-49d6-83e7-da4f21d5b848@linux.dev> <6bb3fd31-6b26-4bbf-8833-e4842b1dc463@amd.com> <103e4236-c01e-4286-9152-007d9a249a65@linux.dev> <409b4a72-483e-467b-8d00-9a8dae48bdc9@linux.dev> <4e6e7308-1d39-427d-af47-2957025f501b@amd.com> Content-Language: en-US From: K Prateek Nayak In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-ClientProxiedBy: SATLEXMB03.amd.com (10.181.40.144) To SATLEXMB04.amd.com (10.181.40.145) X-EOPAttributedMessage: 0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: SA2PEPF00003F67:EE_|DM4PR12MB6136:EE_ X-MS-Office365-Filtering-Correlation-Id: c3c387e7-c16c-4629-5ee6-08dd26329990 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0;ARA:13230040|376014|82310400026|1800799024|7416014|36860700013|7053199007; X-Microsoft-Antispam-Message-Info: =?utf-8?B?K0JCQkRXVHdhN0h0UTkwUTBORkNnNmsyMGVFUVRwZmtzZnprZkkxL2QweXRS?= =?utf-8?B?K0JVcFBLcnFSUEZxWWZrNU9YUEcrbTM2Z1lPWEIzMm1lc0pUSTdnQURIZjZM?= =?utf-8?B?cHdtbnpzeS9PUzlSa0U4WUxzNWFKUXhONHo0QW5iQmttemM3MnQvMTVwY1Q0?= =?utf-8?B?Sk02N1VmbWMrZEsrd1hxL2FvUTR1Q0dLVWpScm0wWWppQ3ZDcHlQUnp4REZr?= =?utf-8?B?RDl1UEFsSCsxRTVBQklNRWRRaE9lU0VBSlFlSDROVHRCblh1bDVmNUdueTl3?= =?utf-8?B?aW0wbTNiRlNQbFBIb1BPTGxhZkZVNkJGMGlJbGlSOU5ZRTVyM3QrUGhEeWxP?= =?utf-8?B?eXhocnFYL2hwNzIxOGdSTWlyaW4zbWJzUHJnT1Q0WXJxcDBEV3RtUWZnT2Yy?= =?utf-8?B?U3ZhY01ZV001VmcvTlVUVGpRNndWdTh2R3ZrTGVkaHArbVhaYUdHejJJVXVn?= =?utf-8?B?RVV4dkZoRVpIdTR3djZXN3VLU3VOREc3ZktqejRjdGh4aGh2b1hQVERoR1ZO?= =?utf-8?B?N2d0TmJtcEtYN3AyZEtadFRlSkJ4dUxJTzlwT2ZjQUJ1eDBUWlVYb2RLS1pz?= =?utf-8?B?MlcrdnZXOUo3U2wvVitsZHNta2pydlFpeEwraDRIeTFjeW50ak5VUXNKVkcy?= =?utf-8?B?T0JxVG5vOUpuVityNmxxTVVuL2JkejllWEs2QkkzVm1XVVNuWlo5QnVOQWVp?= =?utf-8?B?QklCM2d6M1JneW0velZTdFNDTEpZSjlIbUhPN29IZlZJZWRoSmNNcDNMeDkv?= =?utf-8?B?ejR3TENkaVl4MjhKSEJBdWlHb3gvd0E4Wnhnb0hpVkRuUko2dGdhMmtORzJQ?= =?utf-8?B?YWNhNHhzZ25SaXZ2MlFGUjY4ZFkwaHFCVU1GZ2FTamRUZmlMcTZrdUt5a2sw?= =?utf-8?B?OGdacFhnaWZFMmZHNEFCSEh0ZzVHTXk4R3NqNHlxbmtwRGxiWnBSZWhOWENm?= =?utf-8?B?a1JDRThWMVZCZlUvcW41YlBidFBhOHlLNE44bDhuVk1qR05BbmNYVzVudVNK?= =?utf-8?B?WElVTk5KSG5POWhnbkorbTRHSCt5NzU3VlJzTjdaNWErb1RvQzcwcFJkVXU1?= =?utf-8?B?UUZJZU16YUJYbk93UHQ1UDBjZDFpLzJDUUxkVDZuQXl2MUxhdk53T1AycVZO?= =?utf-8?B?QUNkWVN0S0k0cjYxbGU2K2hVZkxtZGFpZXJkakp4RU10NE42eko1R3V3cHNo?= =?utf-8?B?MDVhc25HdVRRVk5TTmxTWHd0aGZ0M2hBL241ZDh0UGdQMytxV2hPRmhCUXdN?= =?utf-8?B?anJvWU5jMGFKekMrU2NrcEY0K091TmFJenZ0Qk9DMVlVWjNVckVSRHJSaWYr?= =?utf-8?B?V2UyRVNyYldxcUhTYW5yV0txWHR3MWZ0TzI3ZWJvOHp3RC9vRVovS254eEFl?= =?utf-8?B?dDBEb0ZGWFltWWo5MzFuWitBbXU4STlYR0srTjFNdkF0N05KYm00bjBkY2N0?= =?utf-8?B?VW8rVU52akNMSkMxaDNEcENpa1AvTWNBZWgzYzYxZDRQdWhKYUo0OUZ4Q1U3?= =?utf-8?B?S1N6U0xOWkh1Z0FWV055QmFnVW1DbEdiYXNDUGZ5RjNUS1IyQjRRZWpzWlRS?= =?utf-8?B?bzNocWQwbm5uKzNBMDM4eWRTR0hDS2NlYlRON2FxN1AwOFZsOEF0V2l2Umc1?= =?utf-8?B?d1pUM201VzRHNTR5S3JCK2tNLzBCekVDSUhXTFNVZHkrNndDSFFpODBtT2Nn?= =?utf-8?B?aEhMOEpkRWN4OHJDaWxuc0k0am13TmJBamhrSlcxeSswYWh5NVZoTE9Jb2Qy?= =?utf-8?B?TUJiZUhYRXozeEVpdlZrRG44L1BybjBmcWxGWW84VSthbFZzTngzcHpxS3BS?= =?utf-8?B?NlYrUitKVTFRT0R3WHdWeWFTWkxNaUZFUU1mYXlZL3FyM0lmQVRQOVZ6WDhs?= =?utf-8?B?bUdwNUVFRGVKRG9id2N0UTltMDk1SkFZMVdpNVNEQ1d5WDdvT3I2dG93RW5z?= =?utf-8?B?d2hLbmpONmNQUVFjRHhhTVBFcEwvaEdkVDRIOTM3cFJOeDFCcEJEdzZBTmd4?= =?utf-8?Q?1X235CjbkOxuXIdNv6iZN564+oYbNk=3D?= X-Forefront-Antispam-Report: CIP:165.204.84.17;CTRY:US;LANG:en;SCL:1;SRV:;IPV:CAL;SFV:NSPM;H:SATLEXMB04.amd.com;PTR:InfoDomainNonexistent;CAT:NONE;SFS:(13230040)(376014)(82310400026)(1800799024)(7416014)(36860700013)(7053199007);DIR:OUT;SFP:1101; X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Dec 2024 04:54:52.2136 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: c3c387e7-c16c-4629-5ee6-08dd26329990 X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=3dd8961f-e488-4e60-8e11-a82d994e183d;Ip=[165.204.84.17];Helo=[SATLEXMB04.amd.com] X-MS-Exchange-CrossTenant-AuthSource: SA2PEPF00003F67.namprd04.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR12MB6136 On 12/27/2024 10:10 AM, Chengming Zhou wrote: > On 2024/12/27 12:10, K Prateek Nayak wrote: >> Hello there, >> > [...] >>> >>> Just made a quick fix and tested passed using your script. >> >> Thank you! The diff seems to be malformed as a result of whitespaces but >> I was able to test if by recreating the diff. Feel free to add: >> >> Reported-by: K Prateek Nayak >> Closes: https://lore.kernel.org/lkml/20241226053441.1110-1- kprateek.nayak@amd.com/ >> Tested-by: K Prateek Nayak >> >> If you can give your sign off, I could add a commit message and send it on >> your behalf too. > > Great, thanks for your time! > > Signed-off-by: Chengming Zhou Thank you! > >> >>> >>> diff --git a/kernel/sched/core.c b/kernel/sched/core.c >>> index 3e5a6bf587f9..065ac76c47f9 100644 >>> --- a/kernel/sched/core.c >>> +++ b/kernel/sched/core.c >>> @@ -6641,7 +6641,6 @@ static void __sched notrace __schedule(int sched_mode) >>>           * as a preemption by schedule_debug() and RCU. >>>           */ >>>          bool preempt = sched_mode > SM_NONE; >>> -       bool block = false; >>>          unsigned long *switch_count; >>>          unsigned long prev_state; >>>          struct rq_flags rf; >>> @@ -6702,7 +6701,7 @@ static void __sched notrace __schedule(int sched_mode) >>>                          goto picked; >>>                  } >>>          } else if (!preempt && prev_state) { >>> -               block = try_to_block_task(rq, prev, prev_state); >>> +               try_to_block_task(rq, prev, prev_state); >>>                  switch_count = &prev->nvcsw; >>>          } >>> >>> @@ -6748,7 +6747,8 @@ static void __sched notrace __schedule(int sched_mode) >>> >>>                  migrate_disable_switch(rq, prev); >>>                  psi_account_irqtime(rq, prev, next); >>> -               psi_sched_switch(prev, next, block); >>> +               psi_sched_switch(prev, next, !task_on_rq_queued(prev) || >>> +                                               prev->se.sched_delayed); >>> >>>                  trace_sched_switch(preempt, prev, next, prev_state); >>> >>> diff --git a/kernel/sched/stats.h b/kernel/sched/stats.h >>> index 8ee0add5a48a..65efe45fcc77 100644 >>> --- a/kernel/sched/stats.h >>> +++ b/kernel/sched/stats.h >>> @@ -150,7 +150,7 @@ static inline void psi_enqueue(struct task_struct *p, int flags) >>>                  set = TSK_RUNNING; >>>                  if (p->in_memstall) >>>                          set |= TSK_MEMSTALL | TSK_MEMSTALL_RUNNING; >>> -       } else { >>> +       } else if (!task_on_cpu(task_rq(p), p)) { >> >> One small nit. here >> >> If the task is on CPU at this point, both set and clear are 0 but >> psi_task_change() is still called and I don't see it bailing out if it >> doesn't have to adjust any flags. > > Yes. > >> >> Can we instead just do an early return if task_on_cpu(task_rq(p), p) >> returns true? I've tested that version too and I haven't seen any >> splats. > > I thought it's good to preserve the current flow that: > > if (restore) >     return; > > if (migrate) >     ... > else if (wakeup) >     ... > > As for early return when `task_on_cpu()`, it looks right to me. Thank you for your feedback. I agree the current approach fits the flow well but the call to psi_task_change() which does a whole lot although the flags remain same (like seq_count updates, etc.) seems unnecessary. > Anyway, it's not a migrate or wakeup from PSI POV. > > Thanks! > >> >>>                  /* Wakeup of new or sleeping task */ >>>                  if (p->in_iowait) >>>                          clear |= TSK_IOWAIT; >> -- Thanks and Regards, Prateek