Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: PRdfZAkSqCte8tPA3DTEL3g5Y2VbXdQzeh7pvRieNTyDgbEv4glOnVNMi8lfv517Xa2/iuusUe
 vt36/MF2I6gA==
IronPort-SDR: Fcrmoel/737e3wECpopU3F4AZnWUrECkqiiJYi97M+pcQYYLbWFzMtnvsu/JwY9NzeE8ykUzFA
 vUmIWQJZNC3Q==
From:   "Huang\, Ying" <ying.huang@intel.com>
To:     Dave Hansen <dave.hansen@linux.intel.com>
Cc:     <linux-kernel@vger.kernel.org>, <yang.shi@linux.alibaba.com>,
        <rientjes@google.com>, <dan.j.williams@intel.com>
Subject: Re: [RFC][PATCH 4/9] mm/migrate: make migrate_pages() return nr_succeeded
References: <20200818184122.29C415DF@viggo.jf.intel.com>
        <20200818184129.14758DAD@viggo.jf.intel.com>
Date:   Thu, 17 Sep 2020 09:25:27 +0800
In-Reply-To: <20200818184129.14758DAD@viggo.jf.intel.com> (Dave Hansen's
        message of "Tue, 18 Aug 2020 11:41:29 -0700")
Message-ID: <87lfh9ry7s.fsf@yhuang-dev.intel.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain; charset=ascii
Precedence: bulk

Dave Hansen <dave.hansen@linux.intel.com> writes:

> diff -puN mm/migrate.c~migrate_pages-add-success-return mm/migrate.c
> --- a/mm/migrate.c~migrate_pages-add-success-return	2020-08-18 11:36:51.284583183 -0700
> +++ b/mm/migrate.c	2020-08-18 11:36:51.295583183 -0700
> @@ -1432,6 +1432,7 @@ out:
>   * @mode:		The migration mode that specifies the constraints for
>   *			page migration, if any.
>   * @reason:		The reason for page migration.
> + * @nr_succeeded:	The number of pages migrated successfully.
>   *
>   * The function returns after 10 attempts or if no pages are movable any more
>   * because the list has become empty or no retryable pages exist any more.
> @@ -1442,11 +1443,10 @@ out:
>   */
>  int migrate_pages(struct list_head *from, new_page_t get_new_page,
>  		free_page_t put_new_page, unsigned long private,
> -		enum migrate_mode mode, int reason)
> +		enum migrate_mode mode, int reason, unsigned int *nr_succeeded)
>  {
>  	int retry = 1;
>  	int nr_failed = 0;
> -	int nr_succeeded = 0;
>  	int pass = 0;
>  	struct page *page;
>  	struct page *page2;
> @@ -1500,7 +1500,7 @@ retry:
>  				retry++;
>  				break;
>  			case MIGRATEPAGE_SUCCESS:
> -				nr_succeeded++;
> +				(*nr_succeeded)++;

I think now we should consider THP in counting now.  Because later
nr_succeeded will be used to counting the number of reclaimed pages,
and THP is respected for that.

Best Regards,
Huang, Ying

>  				break;
>  			default:
>  				/*
> @@ -1517,11 +1517,11 @@ retry:
>  	nr_failed += retry;
>  	rc = nr_failed;
>  out:
> -	if (nr_succeeded)
> -		count_vm_events(PGMIGRATE_SUCCESS, nr_succeeded);
> +	if (*nr_succeeded)
> +		count_vm_events(PGMIGRATE_SUCCESS, *nr_succeeded);
>  	if (nr_failed)
>  		count_vm_events(PGMIGRATE_FAIL, nr_failed);
> -	trace_mm_migrate_pages(nr_succeeded, nr_failed, mode, reason);
> +	trace_mm_migrate_pages(*nr_succeeded, nr_failed, mode, reason);
>  
>  	if (!swapwrite)
>  		current->flags &= ~PF_SWAPWRITE;