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;
Message-ID: <90bb07c3-6365-8dcf-7d0e-ef2c0163614f@redhat.com>
Date:   Mon, 8 May 2023 08:57:57 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Subject: Re: [PATCH] mm/migrate_device: Try to handle swapcache pages.
Content-Language: en-US
To:     "Huang, Ying" <ying.huang@intel.com>
Cc:     linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        apopple@nvidia.com, jhubbard@nvidia.com, rcampbell@nvidia.com
References: <20230507061726.13422-1-mpenttil@redhat.com>
 <87wn1ja793.fsf@yhuang6-desk2.ccr.corp.intel.com>
 <f93c14f6-024d-4abc-7598-fa82cc3ea1a5@redhat.com>
 <87sfc79zg8.fsf@yhuang6-desk2.ccr.corp.intel.com>
From:   =?UTF-8?Q?Mika_Penttil=c3=a4?= <mpenttil@redhat.com>
In-Reply-To: <87sfc79zg8.fsf@yhuang6-desk2.ccr.corp.intel.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hi,


On 8.5.2023 8.14, Huang, Ying wrote:
> Mika Penttilä <mpenttil@redhat.com> writes:
> 
>> H,
>>
>>
>> On 8.5.2023 5.26, Huang, Ying wrote:
>>> mpenttil@redhat.com writes:
>>>
>>>> From: Mika Penttil <mpenttil@redhat.com>
>>>>
>>>> Migrating file pages and swapcache pages into device memory is not supported.
>>>> The decision is done based on page_mapping(). For now, swapcache pages are not migrated.
>>>>
>>>> Things can however be improved, for swapcache pages. Try to get rid of the swap cache,
>>>> and if successful, go ahead as with other anonymous pages.
>>>>
>>>> As a detail, do not mess with shmem pages, as they handle swap internally.
>>>>
>>>> Cc: Alistair Popple <apopple@nvidia.com>
>>>> Cc: John Hubbard <jhubbard@nvidia.com>
>>>> Cc: Ralph Campbell <rcampbell@nvidia.com>
>>>> Signed-off-by: Mika Penttil <mpenttil@redhat.com>
>>>> ---
>>>>    mm/migrate_device.c | 12 ++++++++++--
>>>>    1 file changed, 10 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/mm/migrate_device.c b/mm/migrate_device.c
>>>> index d30c9de60b0d..e8169c58c341 100644
>>>> --- a/mm/migrate_device.c
>>>> +++ b/mm/migrate_device.c
>>>> @@ -12,6 +12,7 @@
>>>>    #include <linux/mmu_notifier.h>
>>>>    #include <linux/oom.h>
>>>>    #include <linux/pagewalk.h>
>>>> +#include <linux/shmem_fs.h>
>>>>    #include <linux/rmap.h>
>>>>    #include <linux/swapops.h>
>>>>    #include <asm/tlbflush.h>
>>>> @@ -750,10 +751,17 @@ static void __migrate_device_pages(unsigned long *src_pfns,
>>>>    			/*
>>>>    			 * For now only support anonymous memory migrating to
>>>>    			 * device private or coherent memory.
>>>> +			 *
>>>> +			 * Try to get rid of swap cache if possible.
>>>> +			 * Leave shmem pages alone, they handle swap internally
>>>> +			 *
>>>>    			 */
>>>>    			if (mapping) {
>>>> -				src_pfns[i] &= ~MIGRATE_PFN_MIGRATE;
>>>> -				continue;
>>>> +				if (shmem_mapping(mapping) ||
>>>> +				    !folio_free_swap(page_folio(page))) {
>>> Should we filter out file pages firstly?
>>
>> folio_free_swap() checks first and returns false if not swapcache page
>> (ie including normal file pages) so think that is enough,
> 
> You are right.  folio_free_swap() will call folio_test_swapcache(), and
> which will check folio_test_swapbacked().
> 
> And, IIUC, we don't need to check shmem too.  Because folio_free_swap()
> will not return true for shmem pages.

Ah yes true, for not written out shmem page folio_free_swap() returns 
false (because !swapcache). And written out (but in swap cache) returns 
false because folio_swapped() == true

> 
>> but maybe for clarity and not depending on it, could be good
>> explicitly check for swapcache before calling folio_free_swap().
> 
> Yes.  That will be more clear.  I suggest to use folio_test_anon(), that
> will filter out shmem pages too.

Agreed.

> 
>>>
>>>> +					src_pfns[i] &= ~MIGRATE_PFN_MIGRATE;
>>>> +					continue;
>>>> +				}
>>>>    			}
>>>>    		} else if (is_zone_device_page(newpage)) {
>>>>    			/*
> 
> Best Regards,
> Huang, Ying
> 

Thanks,
Mika