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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 449B3C04A6A for ; Wed, 9 Aug 2023 04:17:17 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 5982A6B0071; Wed, 9 Aug 2023 00:17:16 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 548438D0002; Wed, 9 Aug 2023 00:17:16 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3E8898D0001; Wed, 9 Aug 2023 00:17:16 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 2C3ED6B0071 for ; Wed, 9 Aug 2023 00:17:16 -0400 (EDT) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 01AD7A0886 for ; Wed, 9 Aug 2023 04:17:15 +0000 (UTC) X-FDA: 81103256472.18.98EB4E3 Received: from NAM10-BN7-obe.outbound.protection.outlook.com (mail-bn7nam10on2062.outbound.protection.outlook.com [40.107.92.62]) by imf08.hostedemail.com (Postfix) with ESMTP id 239DB160009 for ; Wed, 9 Aug 2023 04:17:12 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=Nvidia.com header.s=selector2 header.b=Vq7W8RrT; dmarc=pass (policy=reject) header.from=nvidia.com; spf=pass (imf08.hostedemail.com: domain of apopple@nvidia.com designates 40.107.92.62 as permitted sender) smtp.mailfrom=apopple@nvidia.com; arc=pass ("microsoft.com:s=arcselector9901:i=1") ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1691554633; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=IettJjZx3saKceZDiVNaL8X4cHeZbE15Mrti01UMEzA=; b=caA+aMsvh7LnoDbPdoG9rl8jmRWQPfj28XBOhJUflF7Z0K7cqvGhgkC3r8WcSY27vXnZNl 3Q0zHaPJJAc8iwFHRsW868qncvVoEfL/zzpCIPxprSnDrS4V6BzzLuppshtIu65EAMfY+Y r2BQTcBs1ZF/pnkE7DG7LKCsxaUhNYs= ARC-Authentication-Results: i=2; imf08.hostedemail.com; dkim=pass header.d=Nvidia.com header.s=selector2 header.b=Vq7W8RrT; dmarc=pass (policy=reject) header.from=nvidia.com; spf=pass (imf08.hostedemail.com: domain of apopple@nvidia.com designates 40.107.92.62 as permitted sender) smtp.mailfrom=apopple@nvidia.com; arc=pass ("microsoft.com:s=arcselector9901:i=1") ARC-Seal: i=2; s=arc-20220608; d=hostedemail.com; t=1691554633; a=rsa-sha256; cv=pass; b=KDyzIyd2NFBYgZo9X5S68LHqrAAq9ItAyTUxUJWf7Yv8UmiJ2LdESN/HWATzEi7Ngkhl/C 5+8nSl4rLNS8j5j1BBs3L+Nqz06STaGI/SFfouZtD21FmooNp9l0eS8zLq+U06TIumxnuL Li0j1/gqvH4ynfY7QFK4mgFz+jUFKUM= ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Em0Q0dfMC5xLq5hOLgNFaIjPZRw6xHzJGawLDSZVdkq4BcPNm3t7kR6cQwC6vNMuQUQUqJhOXcM3dJSl9BzgRL4Mzp6Q2Jiv9uzUkj8qQiNilNuxX147QKysU2z2098fSGRDlXH5lP7IKL8yukXdlTtu1ixN6axSlQDzQeM1qTM2H4bGZ1ntmlKMt+QzGHCZJaZAGP31NFUG5gZDjCOzYOfHU2lh3QCW8X3CXjzZZxXcWgC5t3eS21totD91citp1os/My30G8SKsovk2y5Jx4e5SFleFl9JZFWoxrHrvul9Jyu9PmVCoDtCeqr+cO9oVXiXqfp0kFRL/jk7CgFpzQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; 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=IettJjZx3saKceZDiVNaL8X4cHeZbE15Mrti01UMEzA=; b=QdbfCYz7dhKWcDXT0Sajqf7yO6JZlR2xZfiNV+l73gHj2plWM/XFWWg8yAQ28wDMwLESeejTnFIdt5LwMi04CZReLdCrGyKhUr3mOKC6VnL5qKwAGPUvfbkEcQTVcQt/EkDmLpSlCD9XgVh6kdjkc8H4pJrbtHH1QQxtLtfvTVhFy50UnmSDmJ+kXXl6ANHWfGe6c3rNUHyLwMttb0wB681ibKVbQSjLEH5ktVS0xEMGXHVVakbOmG7YueTfbS+CiiDmsPh7ne+V5+UjMQhMJ75Hnq+uQ8hLsSo4dlMd0DsqTEqyWgMNLTs+FaWA8Z3jcRfJpxCdJA3oMmE75ZAP9g== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=nvidia.com; dmarc=pass action=none header.from=nvidia.com; dkim=pass header.d=nvidia.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Nvidia.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=IettJjZx3saKceZDiVNaL8X4cHeZbE15Mrti01UMEzA=; b=Vq7W8RrTzwgUKkRLOQwnTwytWSDrEZFN+L7XNQuRerygNuEoRXz3fK21ytKPUNROjGvNuG/20oZOVCF/SE8spFuMBn/vTk0hKYHo/I2kC7exAe4MpTG/MmraVnv99YPXy+6JMQpqcMBBLqdjE0Gq26o8MWNXplLyGLKjC2aRq/8zLE/22CZJjd7fUHjc1rs6gNNl0EhqBAkhnzOVtANEyxV7nWXSDN90BSLletoaOTnmPtMAmOQKHOYKApG6k7G2P4ZhT8baAqxJEItatfYmeGy31P1nQ2TE73RXu7GXef6DByNeMaRdGYi+qC3UOPRG/9EY651RiXVoRuMiBikN/g== Received: from BYAPR12MB3176.namprd12.prod.outlook.com (2603:10b6:a03:134::26) by BN9PR12MB5356.namprd12.prod.outlook.com (2603:10b6:408:105::23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6652.28; Wed, 9 Aug 2023 04:17:10 +0000 Received: from BYAPR12MB3176.namprd12.prod.outlook.com ([fe80::b2b1:755a:a175:c160]) by BYAPR12MB3176.namprd12.prod.outlook.com ([fe80::b2b1:755a:a175:c160%4]) with mapi id 15.20.6652.026; Wed, 9 Aug 2023 04:17:10 +0000 References: <20230807063945.911582-1-apopple@nvidia.com> <87pm3z2fer.fsf@nvdebian.thelocal> User-agent: mu4e 1.8.13; emacs 28.2 From: Alistair Popple To: Michal Hocko Cc: Andrew Morton , linux-mm@kvack.org, jhubbard@nvidia.com, ying.huang@intel.com, osalvador@suse.de, baolin.wang@linux.alibaba.com, ziy@nvidia.com, shy828301@gmail.com, ryan.roberts@arm.com Subject: Re: [PATCH 1/2] mm/migrate.c: Fix return code when migration fails Date: Wed, 09 Aug 2023 14:10:08 +1000 In-reply-to: Message-ID: <87jzu4alz3.fsf@nvdebian.thelocal> Content-Type: text/plain X-ClientProxiedBy: SY5PR01CA0103.ausprd01.prod.outlook.com (2603:10c6:10:207::17) To BYAPR12MB3176.namprd12.prod.outlook.com (2603:10b6:a03:134::26) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: BYAPR12MB3176:EE_|BN9PR12MB5356:EE_ X-MS-Office365-Filtering-Correlation-Id: 8268fb20-d96f-4740-7725-08db988f8006 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: +Kff3WLgxME+W8sCttf/vkM4t4SvJCJche0YazGTIegjHOerS+7dnTwscweYmtCJT6h0ruc6tPPs/C+e2v1jJUHybvn2odj9hAhRBUIQBXngYIXSfgnVeXwj3zlsBM+ZBl/K5e/gPjdldMeqRJGY7t9bAuejahgnCbmT9ZdKaFPVPtKfOarRd1ODZu9B7cmo+u7S4wlp6FQZzyN8yMTi1waq897r0ujNKtjwg7hTO5iP6xBWAd+AyXTfILuvpRLZfZL02/t7b8OV94hoOl9PQomWzeK1CscvOE8Gz0VT7V6QMZt6+q0peg9uimjPN/qt2tdc+jGHeTQAfxQzix1EBEsa1ZJXvp/+60+m11DXKhd75RtMC4RcM0GR3hj1sRntJ28jW1mRO0ETIhJlKxsFyBmr68GlxhyPVj9PLjMkEmmcLH1SxF5gyLmX6S6zpSU8+jJ6YHut85XkX07OIOf5h+4dCkMTUsscEqTyRWOE9X7ra64wzpaaGl9O6Chvuaa9GZxght01vgOizQSHYI0IQXfxqB1rInytQI+hSuPbtjkQCN4VU0Kni2+QMi+g5FCO X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:BYAPR12MB3176.namprd12.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230028)(4636009)(396003)(376002)(136003)(346002)(39860400002)(366004)(451199021)(186006)(1800799006)(6506007)(26005)(38100700002)(83380400001)(2906002)(5660300002)(478600001)(86362001)(9686003)(6512007)(6486002)(41300700001)(316002)(8936002)(8676002)(4326008)(66946007)(66556008)(66476007)(6916009);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?qLariZur3w5qIIaabPeJBbuvuh5hucya+50GVa67CSkvyXAGsWmNddbHlEBL?= =?us-ascii?Q?fsjC0A/40I4XOibYCPIkoeMH9FoTlEou27/0n3XActKm0nNPfIlPXPGuIKMa?= =?us-ascii?Q?oRpSOLQ4uPDnKXkpPYTlschRSb69o8dKjP3BegDCJ75cXKdSbNYuzU/P0MIE?= =?us-ascii?Q?EhNNtFovvDuydkr6RdkvsW5fd5NSflpHDUPuB457BHOfNK63Px0MjnfG8k5M?= =?us-ascii?Q?IpCJ1kZd0uV22dE77rUydQhfWOxMQ4dqysPvMOQ9CFErAJagc3xDwObeCCBs?= =?us-ascii?Q?RVMW7zou6K+KvPg/klGaQrmzq/XPcelRCC7IHh2LAp6KSPw7nNeB/0eNDLXe?= =?us-ascii?Q?tgIENs0WHAeW33TQPybgRkHf4DU/wa+yaIhz/snbb5a+3SNr37xi8WSjdvf0?= =?us-ascii?Q?ekI0O7tkMGZu7sIfJtNkgXT61AUM+Gh2K+YARtzRdwHPzENqoyLJTdFZ/irL?= =?us-ascii?Q?4wm+EUNR30HxZJdh+xVkUDUKxk+joM0Egjjr2jjn0KEixTL4Q7qprlQspIKq?= =?us-ascii?Q?bDYAB60Winvvg0I+FIHd2mObZ7WoXgR+TdZ0Z6pAGAkemE7AYsXENr2hUUkv?= =?us-ascii?Q?AjlJmFcM7vYwQKOQj36Zw6vRUO+PJ3wfUI96XjOFF9PCja1BWY8foz/FVUEJ?= =?us-ascii?Q?chBBx1t4nIyT7j9BdCk+LTzVI47igRG5VuqHk+1T/4U2SqksLjDt6s4+npMi?= =?us-ascii?Q?an1j/vHvvt+lh6f1C2hTu5OdJ/bXN+VyP2jP8K2FE4oWUuUCzuCZXVFNlDvi?= =?us-ascii?Q?OrnDcg17spVn7WgOuKSW98h55DTDuj3EA+UwUW6T+PTOjUrjT105uDyYpG55?= =?us-ascii?Q?qxTYfSYS3NdsJVANph+BZeYUoyz5xIKFqgtHSGHOmYRWg6gai9Rom3oFce5N?= =?us-ascii?Q?GQllu1ALdILV3wayMYJntRKou1FGMp28MMNaJCpRo+DbQqik7Fc4t+CJXKrQ?= =?us-ascii?Q?mRv4lHVxgXQ+QSfLMQxzic+QPhF5jhGRE6LfAQoT2bH91mAbvYdfyL/0/C2B?= =?us-ascii?Q?lV6Fh/1xyhzUoeQt+Q4bRkOoEZADyjFu3mYCkq8/TMqCfByWQJT030PHRJJL?= =?us-ascii?Q?RSNWyVjhl3yec95j2qAjxv2PZxm6FRcCZ0wF+j2BY6/hCgw0g7RqBU9tllLK?= =?us-ascii?Q?JpFLacKN5Y61rEf7XlUZLzi32dJCenBoZqfXO/mYscu3DP7UP/Hpv6scFL9L?= =?us-ascii?Q?ROW2BiJAT7owVnDj+pdpAoG2DtFSofV4PCH3J5GPECcU0lJu3EOpKbiPr3uM?= =?us-ascii?Q?IvyPM4r5jfsLWCTFzjYSyIiVs0RQR1UB0nFheFWZFr5cYI3zfhKrrWhoRN2t?= =?us-ascii?Q?qjtgkllSGLjE/ocFupajpsnPUpaTr9KLwbxmSiOJuCQVpmybS0frliWHFXeZ?= =?us-ascii?Q?uINMz2YjiDpCunzGZZssXtpOgeyaYKhMQzMVjd7YFXXJDDWVLer3WtwJmJB/?= =?us-ascii?Q?MOoEL5Op1wPT0mvS0N0yv6tbmw5WImWsc0pEjtWFD/MlMHLdtuUQ7KQmfXHD?= =?us-ascii?Q?IiTteSf2VrEh11V/MiWMbnT6EugU+8OmjuQXj5O1zIjd+IV4uMSUOHmnHk1h?= =?us-ascii?Q?E4PSCmJoq3a/mQzWXu4QpsAGAwMlA3On8kbhbxFm?= X-OriginatorOrg: Nvidia.com X-MS-Exchange-CrossTenant-Network-Message-Id: 8268fb20-d96f-4740-7725-08db988f8006 X-MS-Exchange-CrossTenant-AuthSource: BYAPR12MB3176.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Aug 2023 04:17:09.9871 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: a/ULaDN5m2MMY0Ik2kDJwY91hKCJw3qBWg1gkON3fLG0wtwxdTvSHqKyoB/AmOfvlhVTRiPYjo/UXg1CR4qh4g== X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN9PR12MB5356 X-Rspamd-Queue-Id: 239DB160009 X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: 6ppa585na4kxzbqrto5t5prof8cptrxf X-HE-Tag: 1691554632-635827 X-HE-Meta: U2FsdGVkX1/x/Wt2YFcJZAvZj5qDy3sqzGk/ASiafj8hwvW4GNdqbIEKVEa5JRksyRZSPQRE3VJMX2jvduobvG6tG/RQL79S9GSCfHGPyqrv6xA4MolKKG3e8eCvug4d35KmWm6ZXo/DO6WSTYLxJq4S98XkLuJJDOW1UQnPlhbabA/kOTV+2PiT8AHK5/KDTI2Pc/kd2FmRFZVvbX9dn22q6w/VMa5eb/t6ZRySTa66gbk0N3UIl4ar7sCahfyUH5WQ2poocmPuzg6obMnFBnqgIxZqYFUZ88AESCeXsoUS2AbcKk/qxn7hkbuhGSOhye715pOUEuUSiU2/7DA8RKucTYxMZ2EMeHZJdGrjOsIODj+Y9cI5mkdWerxvEL1YGj28CI18aJbTNPxJ28yep1URufm+Xdb8+dWeIyMdmlGgRzijabLpBh30HdgDlee+MBpXlNcTUE/ARkeLs72rlGVOgks0qJvQOvstbH9ihHyHoh5aaQORmOoiBe1fjClqpLJXk4wfyF2InZGLixdm17xhYBHvZJwH+r3+msHMCRWimUklQDovhGaRTGxCeveR5uFONWV1GwgZE6higscWTMnTiwGX1PMgLW5ozNSsvatJte7ZlJrm4Zg1mP5qdqBOQLBKVhoUJ+hwhAfNLAQsIxojNAv+xCVtH8NATnf6VHE5WOSAatnRjxSz70rajP6soFIsQBOgnrTm1ljpt/QQ0X+wQU/94UKOEtcX/azSNcbU88VjU7AU2K0tYiHnrxFbBcASxhea96dgNHiDNU2mRnVPvJ7toLJZvpYeW7iyfGEx02GQ4iZfO9GzP9059GLVMTjzCanZ8PTB94obXt/jn2VYbQfBW0Pi9PpZCvOrbgFzf6EajWTLAJetdeSZyQDo82XGWQKnds9VXB5vzBZdzHmatkszGqbVW9/9NMr/53GSIqg6Tx4PbsMjS2mjrgQFz9xynJGUlyBQgxjPJi1 xOEBP8ww CkWuRGFRbwVovBLtzMVlYSSQPe+0kG/46Vz+yDnW2iXrO7KtrH7z3hvT6bq1wyGbwL+t7uXRnRPTlynA+48fq+j/Bovm9reaHZ0c+FxjBcvu+HBN+QZ5sLlXHV8IqDZOxLPLWaDscWvmFPRx2frlEy8NkV6oxQ8ol7B0dTQzPacS+1sH1Ur70aZa7A1/oREnwbjS0wsLUUkt7RC5qd+27HkWacw== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: Michal Hocko writes: > On Mon 07-08-23 22:31:52, Alistair Popple wrote: >> >> Michal Hocko writes: >> >> > On Mon 07-08-23 16:39:44, Alistair Popple wrote: >> >> When a page fails to migrate move_pages() returns the error code in a >> >> per-page array of status values. The function call itself is also >> >> supposed to return a summary error code indicating that a failure >> >> occurred. >> >> >> >> This doesn't always happen. Instead success can be returned even >> >> though some pages failed to migrate. This is due to incorrectly >> >> returning the error code from store_status() rather than the code from >> >> add_page_for_migration. Fix this by only returning an error from >> >> store_status() if the store actually failed. >> > >> > Error reporting by this syscall has been really far from >> > straightforward. Please read through a49bd4d71637 and the section "On a >> > side note". >> > Is there any specific reason you are trying to address this now or is >> > this motivated by the code inspection? >> >> Thanks Michal. There was no specific reason to address this now other >> than I came across this behaviour when updating the migration selftest >> to inspect the status array and thought it was odd. I was seeing pages >> had failed to migrate according to the status argument even though >> move_pages() had returned 0 (ie. success) rather than a number of >> non-migrated pages. > > It is good to mention such a motivation in the changelog to make it > clear. Also do we have a specific test case which trigger this case? Not explicitly/reliably although I could write one. >> If I'm interpreting the side note correctly the behaviour you were >> concerned about was the opposite - returning a fail return code from >> move_pages() but not indicating failure in the status array. >> >> That said I'm happy to leave the behaviour as is, although in that case >> an update to the man page is in order to clarify a return value of 0 >> from move_pages() doesn't actually mean all pages were successfully >> migrated. > > While I would say that it is better to let old dogs sleep I do not mind > changing the behavior and see whether anything breaks. I suspect nobody > except for couple of test cases hardcoded to the original behavior will > notice. > >> >> Signed-off-by: Alistair Popple >> >> Fixes: a49bd4d71637 ("mm, numa: rework do_pages_move") > > The patch itself looks good. I am not sure the fixes tag is accurate. > Has the reporting been correct before this change? I didn't have time to > re-read the original code which was quite different. I dug deeper into the history and the fixes tag is wrong. The behaviour was actually introduced way back in commit e78bbfa82624 ("mm: stop returning -ENOENT from sys_move_pages() if nothing got migrated"). As you may guess from the title it was intentional, so suspect it is better to update documentation. > Anyway > Acked-by: Michal Hocko Thanks for looking, but I will drop this and see if I can get the man page updated. > Anyway rewriting this function to clarify the error handling would be a > nice exercise if somebody is interested. Yeah, everytime I look at this function I want to do that but haven't yet found the time. >> >> --- >> >> mm/migrate.c | 4 +++- >> >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> >> >> diff --git a/mm/migrate.c b/mm/migrate.c >> >> index 24baad2571e3..bb3a37245e13 100644 >> >> --- a/mm/migrate.c >> >> +++ b/mm/migrate.c >> >> @@ -2222,7 +2222,9 @@ static int do_pages_move(struct mm_struct *mm, nodemask_t task_nodes, >> >> * If the page is already on the target node (!err), store the >> >> * node, otherwise, store the err. >> >> */ >> >> - err = store_status(status, i, err ? : current_node, 1); >> >> + err1 = store_status(status, i, err ? : current_node, 1); >> >> + if (err1) >> >> + err = err1; >> >> if (err) >> >> goto out_flush; >> >> >> >> -- >> >> 2.39.2