Received-SPF: pass (google.com: domain of linux-kernel+bounces-66102-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <df59f5e3-6c16-e371-f987-7776f2d24f15@oracle.com>
Date: Wed, 14 Feb 2024 15:10:20 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.15.1
Subject: Re: [PATCH 2/2] mm/migrate_device: further convert
 migrate_device_finalize() to folios
To: Alistair Popple <apopple@nvidia.com>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        akpm@linux-foundation.org, willy@infradead.org
References: <20240214202055.77776-1-sidhartha.kumar@oracle.com>
 <20240214202055.77776-2-sidhartha.kumar@oracle.com>
 <8734tu7jbe.fsf@nvdebian.thelocal>
Content-Language: en-US
From: Sidhartha Kumar <sidhartha.kumar@oracle.com>
In-Reply-To: <8734tu7jbe.fsf@nvdebian.thelocal>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?utf-8?B?QllwK0E4RXdHa0pZejAwN25haXBMb0ZvaHRmUnk2ak5IVVpXUlBwVDRBR3VH?=
 =?utf-8?B?VmtFdWZidE9QYUJTdjdMNWFZbjV5ZU96emZBWVhmRWZCMUlkeVc2Q0d4NEpX?=
 =?utf-8?B?T0FLcFh6T2FkTlc4dGxlZmJLVFRQdEZMRVlNK0FqWWYyWGpxdVh5T0g5cTNL?=
 =?utf-8?B?NzJiNm5SUFdGRVFCbDFSdkNLKzlodlhxdm5JQ0NmZTMxSWMzYzU2SVRGZ0sr?=
 =?utf-8?B?eW1NRHFtbTVmRHZvVGhkRi95MTFYeFU0anVwNUNiU3pVS2lCSGNBdUh0L0o5?=
 =?utf-8?B?em8wZnRZK1E3R210a0lSSVlMdTlyemVEYWxTRHlCUnluM2pHaDc3MmV4dElM?=
 =?utf-8?B?REFiQUR3M1NUb0hBa2RGSERNTHd4Qk5iejJjOWJHRlhpYzhTY0ZWQnJYeTZ4?=
 =?utf-8?B?QnZheDNuRER1RkpqM2VkR1dIK0NWWC9NeCtvUVlMRTFMZ0sybG1sWEFCMmg4?=
 =?utf-8?B?Q09xemZnU0d3aTdaRHlYbnNzdSt1MFJuaU8rakcwam9yMDZQOFI5bFpIY2Ry?=
 =?utf-8?B?TzZIVG5rK0NFSFVEZnI2ZVpEeENjVHp3SmNFZDYybC9rR01ZbTNPYWJZcU9j?=
 =?utf-8?B?c0dhRVFyY2FVbGJrOTM4OHRrTkN2UUkrK3hsVUxSMlZudkZDNDFwUHU3Mlgx?=
 =?utf-8?B?aDlmZWZneTFIMDFUTmhJV0Q3L01uTjIweXpJNkh4dFNNU2Q3ZFh2NGljTEZ6?=
 =?utf-8?B?U21lcCtBd2N3akpmRytJWVpyZ2dINUMvRFJFc1JyZHNGNWoxWGNlYkpRVWg2?=
 =?utf-8?B?d1FvOU1CR2VUaXFJbFBsK0hkcUpsZGFiVmREaHMxTVNYbEkxZ00wUFZrMVBF?=
 =?utf-8?B?NGZHRUh5YkY0NEpNS3VCcG9aMTBTOUYrK2JTdGd0R0hiNDhhWWNlQnZ6QU9S?=
 =?utf-8?B?cTRXR2RBR1JERzdwVkF6YmV3eDhvZzl0NVJ2SW5mUGM4VEJsb0lncFFKa0Zx?=
 =?utf-8?B?WUlrNjU4MlhaSThiR0E5N0gwS1J3SVhudzJLWjIzWnFhMWlqdEpwREpibnhw?=
 =?utf-8?B?M0ZGTkFEWTZEaUluZVdXUDNBY2RJQ1JzUXpkaXJYWDNSY1ltZzJJY3dsS05X?=
 =?utf-8?B?UER1Skhid09OdjFJRVlZTW85SlNBYkRZMmhINkhGaFE5Y24veVpyRGtWRFJl?=
 =?utf-8?B?MWVONlZ6R21mZHFWUWpncG5ybFBCTlNFNWJzN1F0b3JMVHlNQnNLcFh6UFp6?=
 =?utf-8?B?dStmM3FvQ1VnZnFPT1ZFbytvNE5hRFA0U3QvdGNPcklBTEc5dTJsVWtiaXhV?=
 =?utf-8?B?cldyc21hQ0dLK0ViYTYxT2UvZTNGbDJrV0JnT2hSL1ZyYVY1UVArNkJJWHdz?=
 =?utf-8?B?UGptNndHL21SVTNHRVBBYkhnUGVSNVRvWGJLT0JPMEpjRlkrWExiN2d1T0ho?=
 =?utf-8?B?UXc1Ymp6eVNYMVZjclNsdnpVT2JuKzdWNjZjTVpwOFE4b2s2dkxHblQ5NEU4?=
 =?utf-8?B?QmNOaVRjUjR5cmpwN21yemlnMlVNVm94YXVQNmIzSjBpcG1PZEpmSlNJZ2RS?=
 =?utf-8?B?OVFEb2lZYTRKK1JCZXZxeTlFeDc4Tk85V09CU3ZlelY0U1RvaGZQVlBxT2lJ?=
 =?utf-8?B?MGpPc0Frcm9OaE1CS2lmN3cvTzdkMFpOWnNmSy9iYWZaRzJtM2Q4VzRzQ1lz?=
 =?utf-8?B?cGVwWTZIY3hFeGZOT01CQVFmeTBnWHJlb0dlQWQ3V28wYXB3MjhVVXVadTND?=
 =?utf-8?B?Qkt1MGx5WUY2QjY2emRVUUNrMWdsVHJpMG1vY2ZwUnZ6Q3ZFTk04ajFhUGV2?=
 =?utf-8?B?eXdJZnNWcWtpOHFHRDZyVHlieHRuNUVKWjlNY2gvRzY0eG1nVGNnSmpDakh5?=
 =?utf-8?B?OWV1WkNCYzk1MnpEYzJVYjQ4TzZPZWRqbGpjdnhyQzVJSGlUR3ZicSs0blRv?=
 =?utf-8?B?d1JndUg5TzNNNk0wemQwcHRWcnBHZkcxems5UUliK2NWREErYk5JVzVaWWx4?=
 =?utf-8?B?VkpCRHpUVWJOQkxiWlVnWFBVUzlWU2psNzhQOWdUQ2pVVnRyTXc0WHJXQ25B?=
 =?utf-8?B?ZEhFN1hCeWRtclcwakZTUnJ2ZW9kM2VENzlSM1p5RlZnQVF6MzQzQVQxSkZk?=
 =?utf-8?B?bjNWbWM0U2ZDSVNCNkxyQUdjTXJybUJvRjZJU2JmYzg0d2xPTXZxc0lLdXEy?=
 =?utf-8?B?R28xMjNQUmUyQ1dnZTJiZGpTWWhvanVjZ0lCdmQyRkxLbFZiSkw5aUJEUEtr?=
 =?utf-8?Q?imv1+N3Qo8gJ1CsSt/YqNPU=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	sGvxietifGFYdYwe4hVBckWJfhrWmtSXKb3jsNpwU6a4zE/UFMobt6BNR/3KS3tj+573/qLGt0QkYoFn0hraQlMhS01EzElAmOeCu1BjEi8xFTm/Kzqbn6WIPY2XEdE9iICz3dl3IW9464HJeZX9IvAYkSeUEQ7XlJjKEoVexmiAPdRFtF3uVXy5DKkaVqnrxsdi5E/6o9lR/X89m7YhEaOieNJ5RzROP+ww1l6nkbCRzWoIcrfKDB0iXPtp4H5ukCH9cGlyV0tCp/dpG9P8oqhOApVfWKxQSf0q7BkW597zjqc4znD2xHIFjAVsZtrPlFXo72kzou77i9GJF+Ijh81mmNVkd1VJ3wYXI0yf2UfaJFR/hELTXOXe/EJugzF0LvHKIksXX/HGzqQ2isXviDZOJIgP07Xij59oSUiIINFAGhBmH+Ga+UAU4CBKLPvTEXPxpwN21850NTjnlk4QCUCtbXgr79NGkezrRnmnXjJm7cRYj5NR6BhF7GK2rHj560SScHBQk/leWF9GwyOgJrkDBrzwS+30xPLTU8KarHF21REUeDqOpFV1tulJkmsXmN0X4YhYIKy097BSs5/raZ4sIKVTXq937JdXcixAHQg=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 702349b3-dc3c-45dc-3263-08dc2db21ec7
X-MS-Exchange-CrossTenant-AuthSource: CH0PR10MB5113.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Feb 2024 23:10:22.6953
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: P66Tf9AGSuUnS7NIINkiVmDU1e/EWM2GK5U/iRY/CjFq9VbDJ7bt86/6+k5umakXG89o0J3dIRhsUoIwm3Z8XyJFl42um0PhmAJPrWI/ick=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH8PR10MB6339
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2024-02-14_15,2024-02-14_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 spamscore=0
 mlxlogscore=999 bulkscore=0 phishscore=0 mlxscore=0 suspectscore=0
 malwarescore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2311290000 definitions=main-2402140175
X-Proofpoint-GUID: waEIMJAbLRmI_33QICuMwvK-_zWEsUOp
X-Proofpoint-ORIG-GUID: waEIMJAbLRmI_33QICuMwvK-_zWEsUOp

On 2/14/24 2:45 PM, Alistair Popple wrote:
> 
> Sidhartha Kumar <sidhartha.kumar@oracle.com> writes:
> 
>> Use folio api functions from the already defined src and dst folio
>> variables.
>>
>> Signed-off-by: Sidhartha Kumar <sidhartha.kumar@oracle.com>
>> ---
>>   mm/migrate_device.c | 14 +++++++-------
>>   1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/mm/migrate_device.c b/mm/migrate_device.c
>> index 9152a329b0a68..a48d5cdb28553 100644
>> --- a/mm/migrate_device.c
>> +++ b/mm/migrate_device.c
>> @@ -843,17 +843,17 @@ void migrate_device_finalize(unsigned long *src_pfns,
>>   		remove_migration_ptes(src, dst, false);
>>   		folio_unlock(src);
>>   
>> -		if (is_zone_device_page(page))
>> -			put_page(page);
>> +		if (folio_is_zone_device(src))
>> +			folio_put(src);
>>   		else
>> -			putback_lru_page(page);
>> +			folio_putback_lru(src);
>>   
>>   		if (newpage != page) {
>> -			unlock_page(newpage);
>> -			if (is_zone_device_page(newpage))
>> -				put_page(newpage);
> 
> Defining migrate_pfn_to_folio() would also allow the removal of the
> newpage and page variables entirely which I think would make this
> clearer.
> 
> As an aside is there any motivation for making these changes other than
> as a general cleanup? I ask only because I have been looking at allowing
> device pages with order > 0 so have some of these clean-ups in a local
> tree as they're a pre-requisite for that.
> 
>   - Alistair
> 

Hello,

The motivation is just general cleanup. In folio-compat.c I saw that 
putback_lru_page() does not have much users left so I could convert them and 
then just get rid of putback_lru_page(). Should I still continue with a v2 that 
will include defining a migrate_pfn_to_folio() or wait for your clean-ups?

Thanks,
Sid

>> +			folio_unlock(dst);
>> +			if (folio_is_zone_device(dst))
>> +				folio_put(dst);
>>   			else
>> -				putback_lru_page(newpage);
>> +				folio_putback_lru(dst);
>>   		}
>>   	}
>>   }
>