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;
References: <fca6bb0bd48a0292a0ace2fadd0f44579a060cbb.1666335603.git.baolin.wang@linux.alibaba.com>
User-agent: mu4e 1.8.10; emacs 28.2
From:   Alistair Popple <apopple@nvidia.com>
To:     Baolin Wang <baolin.wang@linux.alibaba.com>
Cc:     akpm@linux-foundation.org, david@redhat.com, ying.huang@intel.com,
        ziy@nvidia.com, shy828301@gmail.com, jingshan@linux.alibaba.com,
        linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/2] mm: migrate: Fix return value if all subpages of
 THPs are migrated successfully
Date:   Mon, 24 Oct 2022 13:36:20 +1100
In-reply-to: <fca6bb0bd48a0292a0ace2fadd0f44579a060cbb.1666335603.git.baolin.wang@linux.alibaba.com>
Message-ID: <87pmeiq6qc.fsf@nvidia.com>
Content-Type: text/plain
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?FnPiTjYxjpHDgUtwieteHmjAFgr0ZUjjmir2ryJY42DZyqHBhNrBnh/Khk8Z?=
 =?us-ascii?Q?GFnJbNXiuk6FwC/Js+PG1QiSC5Kx5yfTEgfs5eArPr6j81N7dzIbOewovf8b?=
 =?us-ascii?Q?d0G2kkpm8IXqWuE6TP6xiLsRUgpCybvxvOzOcYKCc0QoPeSI+Cx0UOvr99y/?=
 =?us-ascii?Q?FbZmgBPAHx6JgafzSnVuYc/w5uFPd5YYVJH9oQG8b0rI365X4ND2nWXQDoK6?=
 =?us-ascii?Q?ipgLkmEh92i1nWCLNwnYNRHOkdmNQDntz2pA41Sprx90mT5ZYu4Nb6P3WjyM?=
 =?us-ascii?Q?O4OOPpmhf1raQl8t3FtqsohY8ytGDN8Fg14DCO6ryApCmG+Zj2UC71gMFoGY?=
 =?us-ascii?Q?RUe0zGI32uqTaQfDsYzKs4Dei2jEpSKWu6T8H0z0ghRldunOt+jRSLkdMMK9?=
 =?us-ascii?Q?taoypd37e+A50Hp+pbGCRALEyC+3xtCyZK+AhogZ7oQyZzbeDAbzokp0X/4w?=
 =?us-ascii?Q?8+ORolJmyBZVpBd+OLBY10FA2OvTikBxefoihJq4ToGaL995bqWqEVvwxPJh?=
 =?us-ascii?Q?zyPnvQ77e67r9UBf1KRiENhnTt8mWLFatq6eeDmDt12nbpVU1akWooqlhik7?=
 =?us-ascii?Q?gWTGKD2Ib8SJFSapiALlbTkI6/EE71Xye97YEn0mdhgSPX7ANS20D84nYW8O?=
 =?us-ascii?Q?GpxggJSocbEa/2TtYpD1JEiLpfcRc8cELPZ/S4RvyTRJAOMLYVqxUQ/t6wKW?=
 =?us-ascii?Q?PvdyX7rwNbaTBF5a3E3UTrhedDIeEFUYcja/xB1YVxuPyhzfxx2xPfZ0WwQM?=
 =?us-ascii?Q?AeGFdwQQBR1iqxqHpf/ISIYp+kEKRoO9CY+McD5qk5lbK/Brm1b88AuL/CPX?=
 =?us-ascii?Q?ZUG/aVpWT+bo05kb2jvXmZUHgcUkdPu489K/ubsOrTYwBNH4QC6BaAhWtmYV?=
 =?us-ascii?Q?visQRNd+HGDKiGNEcR7H68JmXYykaA3bf3le6R4oDvTRw3Dcr9EWqQFGr7tN?=
 =?us-ascii?Q?MEW/RTicm/LScHHeAmUYg5v3lUPX7wYuQZ7swCuAVDNw/oel2y/FfXv5GFd+?=
 =?us-ascii?Q?fKvy7s7HWx94fLZneHi9pK+NkPlX0HWcF9T6lDODdWiks7cz+7q9FeJmqxD0?=
 =?us-ascii?Q?HdUktcbhPqLhV0eevgjIQKruuwWTP39wDeNvJxyQMX71peWr6JhcjcCz5yMD?=
 =?us-ascii?Q?JTEk8bDG1TWQi+3Owe42bxyCtPH1I2TwEOK2giDwcM81qMsTYco1yNqg4z5I?=
 =?us-ascii?Q?rf1Zb+7RKLn9e3Fuj5Su1V8bJDDqZieCfRlHwtpOtQD1wSRu9YZ+KzjRZgPu?=
 =?us-ascii?Q?CfHFzHxzj7KM2zOBJumUkDl5mrhPn1FkB3TpjRYqRAjrbGuNKGIHFhJoD8aS?=
 =?us-ascii?Q?s53wavnTaOH6uqd/SLCe7QeFEzuYnhKzXnIZd8kyJ4X7Xc6ipQTVceTHeDkT?=
 =?us-ascii?Q?sjllb9EPpNODjPpakNx96BV7/kMiSzFOt51uMKTexW9q853zEmR7q/OIJxpZ?=
 =?us-ascii?Q?v1TwzHSR8psivx+yk4zB9C97KaCRTTxIeNZUcpr0LPMQpZB4TCXTUjdBJ88V?=
 =?us-ascii?Q?soSP5I8CiZefvMvfFoiCNAIeLzsissI7InXub19WQYs7U0EipqKsAVvO38aq?=
 =?us-ascii?Q?vg3Dwatuz54nqEbZnkjYNGMY9EtKM2+BYiPnKSl6?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 5883800e-933c-4ecb-74b7-08dab569e741
X-MS-Exchange-CrossTenant-AuthSource: BYAPR12MB3176.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 Oct 2022 02:46:08.4397
 (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: bulmRrtIGKXTRDbPuJ3F38vJe4jdwIscqOqhknTCE12zOLcjS9hq/KvjAMsmSd7QEmYsL+4Ujggm9K3RuYYFMw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR12MB5347
X-Spam-Status: No, score=-2.3 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE
        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


Baolin Wang <baolin.wang@linux.alibaba.com> writes:

> When THP migration, if THPs are split and all subpages are migrated successfully
> , the migrate_pages() will still return the number of THP that were not migrated.
> That will confuse the callers of migrate_pages(), for example, which will make
> the longterm pinning failed though all pages are migrated successfully.
>
> Thus we should return 0 to indicate all pages are migrated in this case.
>
> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> ---
> Changes from v1:
> - Fix the return value of migrate_pages() instead of fixing the
>   callers' validation.
> ---
>  mm/migrate.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/mm/migrate.c b/mm/migrate.c
> index 8e5eb6e..1da0dbc 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -1582,6 +1582,13 @@ int migrate_pages(struct list_head *from, new_page_t get_new_page,
>  	 */
>  	list_splice(&ret_pages, from);
>
> +	/*
> +	 * Return 0 in case all subpages of fail-to-migrate THPs are
> +	 * migrated successfully.
> +	 */
> +	if (nr_thp_split && list_empty(from))
> +		rc = 0;

Why do you need to check nr_thp_split? Wouldn't list_empty(from) == True
imply success? And if it doesn't imply success wouldn't it be possible
to end up with nr_thp_split && list_empty(from) whilst still having
pages that failed to migrate?

The list management and return code logic from unmap_and_move() has
gotten pretty difficult to follow and could do with some rework IMHO.

>  	count_vm_events(PGMIGRATE_SUCCESS, nr_succeeded);
>  	count_vm_events(PGMIGRATE_FAIL, nr_failed_pages);
>  	count_vm_events(THP_MIGRATION_SUCCESS, nr_thp_succeeded);