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 X-Spam-Level: X-Spam-Status: No, score=-13.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_IN_DEF_DKIM_WL autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5EEDAC433ED for ; Fri, 9 Apr 2021 05:43:45 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id EC0D261179 for ; Fri, 9 Apr 2021 05:43:44 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org EC0D261179 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 411826B006C; Fri, 9 Apr 2021 01:43:44 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 399CB6B006E; Fri, 9 Apr 2021 01:43:44 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1EC8D6B0070; Fri, 9 Apr 2021 01:43:44 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0083.hostedemail.com [216.40.44.83]) by kanga.kvack.org (Postfix) with ESMTP id F32AF6B006C for ; Fri, 9 Apr 2021 01:43:43 -0400 (EDT) Received: from smtpin18.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 94003180327E8 for ; Fri, 9 Apr 2021 05:43:43 +0000 (UTC) X-FDA: 78011736726.18.0CB4E0F Received: from mail-il1-f178.google.com (mail-il1-f178.google.com [209.85.166.178]) by imf17.hostedemail.com (Postfix) with ESMTP id 662F440002C3 for ; Fri, 9 Apr 2021 05:43:41 +0000 (UTC) Received: by mail-il1-f178.google.com with SMTP id x12so932577ilm.2 for ; Thu, 08 Apr 2021 22:43:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=QdyZIH69QZLX+qD184zaI/95rRxErKOr+ptyeL501Dg=; b=IFJpyGI29VNx2ZZT/zab37cZtLh7k2KFjyFBZ5nh9pV4MAsPibOAKyTfUPEZlriKdL VI5krZLxUhCox6dw1qR1p5Q8QKLhl562H8UiAb05GzVeC4UwHhixz8F20z1uqTduV8W3 +woVCatxn6E+YIhmgt1O5Jzy7b0DHlo3YDmTBRlCP5DkwDSQhZPXtHDPmQmXYjlTzyMV O+l0MtYLM0zbKnhGXq1NSOOUOTIbSA1kY7/UsDMNSL/hxH6bAgnsxJwmGjdmCJ+JCY5l C+0zRd0FCX3jRBOZDytVWzbUyqqnNEsX8f6vZr/CRP0PgXaAv2rpehSqT7hJMrhG3tNQ AIDA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=QdyZIH69QZLX+qD184zaI/95rRxErKOr+ptyeL501Dg=; b=V0QQ0/CbIjJA1Sm+2/3pSeCbOuI3ueIobSsnxmxROS+Q7zyRwKTMYgwL+vtH3OwwAG XhteDzIyp6FvJfXJWeTWJxeMfUGsMNaRhfHkNLgEIN+tXtxvF3X4b4ua8W34vV/iZWQW +UKSVRnROj3i2Da2EdBbLjDXD+yve7JGj9U+WNIrtWT0V4tXZlpgG005jcIBXFxQtiSy TJJAt0H5Fh46Gc1EbUDod+SF8Mtk5KB8GbyOVISU/jVeVaKBEp7G3IvQLDB23fvPugGk ty/vrN3DaT9LVT9dh4yHFGYe14JKJB8L7Nu9+JY20qKyVkDyVaRVrPtCGkxFy0wblTH5 XWfw== X-Gm-Message-State: AOAM531uWAW6VppWs9zBpHsAv9ERM/oSZ3ta2HkvHNlomlbRHOEPntvM AmXg/G688Vs7ezzkFxnAe5jPJrIO16RW2dzKVTSgDg== X-Google-Smtp-Source: ABdhPJwBLNzQvO+jGFN3ALoPBxIkWt9qei6kpyabY/dUNKkbBbeBR/YLrbhcC2j5a34lmYBlAKiZE2VL3kI1LJWpJXY= X-Received: by 2002:a05:6e02:1a43:: with SMTP id u3mr10086739ilv.292.1617947022330; Thu, 08 Apr 2021 22:43:42 -0700 (PDT) MIME-Version: 1.0 References: <20210401183216.443C4443@viggo.jf.intel.com> <20210401183223.80F1E291@viggo.jf.intel.com> In-Reply-To: From: Wei Xu Date: Thu, 8 Apr 2021 22:43:31 -0700 Message-ID: Subject: Re: [PATCH 04/10] mm/migrate: make migrate_pages() return nr_succeeded To: Oscar Salvador Cc: Yang Shi , Dave Hansen , Linux MM , Linux Kernel Mailing List , Yang Shi , Huang Ying , Dan Williams , David Hildenbrand Content-Type: text/plain; charset="UTF-8" X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: 662F440002C3 X-Stat-Signature: oxfits4m1ffh76twnarjzwjdfqypkyzw Received-SPF: none (google.com>: No applicable sender policy available) receiver=imf17; identity=mailfrom; envelope-from=""; helo=mail-il1-f178.google.com; client-ip=209.85.166.178 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1617947021-190356 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: I agree that it is a good further improvement to make nr_succeeded an optional output argument of migrate_pages() given that most callers don't need it. IMHO, the most important thing in this matter is to ensure that nr_succeeded only returns (when its return value is needed) the successfully migrated pages in this round and doesn't accumulate. This is addressed by both proposals. On Thu, Apr 8, 2021 at 10:06 PM Oscar Salvador wrote: > > On Thu, Apr 08, 2021 at 01:40:33PM -0700, Yang Shi wrote: > > Thanks a lot for the example code. You didn't miss anything. At first > > glance, I thought your suggestion seemed neater. Actually I > > misunderstood what Dave said about "That could really have caused some > > interesting problems." with multiple calls to migrate_pages(). I was > > thinking about: > > > > unsigned long foo() > > { > > unsigned long *ret_succeeded; > > > > migrate_pages(..., ret_succeeded); > > > > migrate_pages(..., ret_succeeded); > > > > return *ret_succeeded; > > } > > But that would not be a problem as well. I mean I am not sure what is > foo() supposed to do. > I assume is supposed to return the *total* number of pages that were > migrated? > > Then could do something like: > > unsigned long foo() > { > unsigned long ret_succeeded; > unsigned long total_succeeded = 0; > > migrate_pages(..., &ret_succeeded); > total_succeeded += ret_succeeded; > > migrate_pages(..., &ret_succeeded); > total_succeeded += ret_succeeded; > > return *total_succeeded; > } > > But AFAICS, you would have to do that with Wei Xu's version and with > mine, no difference there. > > IIUC, Dave's concern was that nr_succeeded was only set to 0 at the beginning > of the function, and never reset back, which means, we would carry the > sum of previous nr_succeeded instead of the nr_succeeded in that round. > That would be misleading for e.g: reclaim in case we were to call > migrate_pages() several times, as instead of a delta value, nr_succeeded > would accumulate. > > But that won't happen neither with Wei Xu's version nor with mine. > > -- > Oscar Salvador > SUSE L3