Received-SPF: pass (google.com: domain of linux-kernel+bounces-160670-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <d19e1033-285f-474b-af1f-ae4c32e32e21@oracle.com>
Date: Fri, 26 Apr 2024 13:33:27 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] mm/memory-failure: remove shake_page()
From: Jane Chu <jane.chu@oracle.com>
To: Matthew Wilcox <willy@infradead.org>,
        Sidhartha Kumar <sidhartha.kumar@oracle.com>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        akpm@linux-foundation.org, linmiaohe@huawei.com,
        nao.horiguchi@gmail.com, osalvador@suse.de
References: <20240426171511.122887-1-sidhartha.kumar@oracle.com>
 <ZivlrMAwRI6xJhc-@casper.infradead.org>
 <d05cbf22-f3e8-414a-a2e3-03e0b857eaca@oracle.com>
 <ZivyC3vqa2BIBoMj@casper.infradead.org>
 <c40cfd0b-f045-4887-a955-fee7e0392cf1@oracle.com>
 <Ziv67sGBi02YCYZ5@casper.infradead.org>
 <47614f5d-0942-4b2c-a51a-451f6bc9c802@oracle.com>
Content-Language: en-US
In-Reply-To: <47614f5d-0942-4b2c-a51a-451f6bc9c802@oracle.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?utf-8?B?dmoxTVd2dnc0dG5VbjM1RnVyOXZnMzJndnVDRWM2bjlRNzgwR0poMzY1RE5u?=
 =?utf-8?B?N2tHN0lnSGJyNDBRT005Ymd3cjB2TDlEQ2M0WSt1U3d2VU43OWYyRVloc3pz?=
 =?utf-8?B?dEZ6VmYwQjA2QkIzaHdTbnFaRmtDQjhuYmQ5QnhTZ0l6dktSQWQ1N3A3TUwz?=
 =?utf-8?B?SnVxZjI5N1BCUG85MFdzWlJKUjNrUnd2U1E5VDVhSUtoTkJtTExWc2l4SSty?=
 =?utf-8?B?SURqajBKOGNPbkh4dXZseTBGR1NUNHJ5UnQ1UWJDckZGWklxM3JpdkcwbitS?=
 =?utf-8?B?OVNYT2pwZ1Nvd3hyYlB6bUZiMTZMZm5FdFFyRWxQanRkRWZjMGhnVHN1bnZE?=
 =?utf-8?B?UVhLRFlLdTB2YTVlOGxzZTF6SGtEODJRbGJjdytOM0J4Q3dYdHllZkVyU1hr?=
 =?utf-8?B?cTVPYVVpQXZnaEk3TWtsdFhnejBQWXZubmIvb1pNK2ZsUmNvckh2NXY4aEVm?=
 =?utf-8?B?eHdHeStCZW9BTGxWVUlra2lvbTBBaGxNbkZGNExQem1NM05ZVE16WWpHSjRB?=
 =?utf-8?B?bEliREpybTUrQzhUU2RORGVLc1dtTm9UbFprbHM1dlFkWDNSM01oSlNTNlMr?=
 =?utf-8?B?Y05sdVlVK1BmVU1TRlVvTVhVUE1ld212THJRMCtiQmVWZTZZelBDZmNtVktR?=
 =?utf-8?B?c1ZoUkd1TVB1RjJ2R2NsMW40TUt6Y2ZiWFhtTzFWQ0VmcWdMMkJGRGJsbzNN?=
 =?utf-8?B?UWpoWU5nOVhHRkdTUWN0WmQ1T1psUlVUaElSRXN1SkVDVFRTMUJjWStkUS9I?=
 =?utf-8?B?RG5LeHQ5SmNaZE5uUC91eHlPV1YzTGM1WVFVaWZnZk1MN0EyNEhnOG93ZXZW?=
 =?utf-8?B?NGh5YjdoNXZhSTlhNXV3SURITEJPTG42TnEyV01paFhTTVhIYkMyNkVFQmpw?=
 =?utf-8?B?UzBma2UrMDlZR1VZcVhwRDl6N0V0UzlLNkxpbjRZenFmSXdMQ3k4SjNkVi92?=
 =?utf-8?B?clM0RG55VG11RmxHVnR1aDQ1T0JXeHoyWkhNMjdISWw1cG90UzZhS3lmN1Fa?=
 =?utf-8?B?cDYvS2dOOHJVa0hyaUJsUmc1OFdGSFNTcVRUbWJONGJuVHE3WmluMjFsckZE?=
 =?utf-8?B?WGYrZS9KVG5ERmlnb0RKQ1FMYUZQallBdVYzdDgzRFVKWUM3bDVMVUIxc1JX?=
 =?utf-8?B?TWVuOHdUR0kwM0tGMnR6cHVrNXhnRzkzbE9sU1lsK2t6SzNQZnJ6MmhGc1Rx?=
 =?utf-8?B?N3IzNVhzRmF6NGRJanZ1aHI3RkNFQy9hd1V2RTdXY2NvZmpoZkZkTHpnbVJ2?=
 =?utf-8?B?bEs0Y3hSdWN0R1NrZTJDVGNlRFF0ZW5OOFZjK1hTU0duaHZmNEh1emJEVzll?=
 =?utf-8?B?aDlPTElPZVNRd1ZrbGZZbUdmeDZJN2FWWTVJVzBQRlgvRHJtaEhmL3FoSlRT?=
 =?utf-8?B?L2ROU2YyaDV3LzBzMjlscmJCRCtTcjhXSUY3TTFYRCswenBVTWdEVy8rSys4?=
 =?utf-8?B?ZlZacG4rMWhiYnZjK2pOUFpvVnk4OHc5czFpU3dhREJhWlUxRGxMTm1IYzRn?=
 =?utf-8?B?c2JnU25lbFppWkdTVXZUbmFwWDZwUjZ4bDgrWDE4Y2srTzhhQktTaWZIUk9l?=
 =?utf-8?B?alNDUzdQcytCeldTczhNRzQ5Z0duK28wczhaNFIzNmR4KytOSnVZSy90NlN1?=
 =?utf-8?B?ZDBJeFJBaGpiaGk2L21WbHNRUEh5TDFWV3Y0eHlKZDFQRFY1YWhOOHJvZHJw?=
 =?utf-8?B?cWc3OEIxeS9KR1lsMHdsc2JQKzBnSUJEZFBjQ1dEOHY3RktaMVQrQlFMWU9V?=
 =?utf-8?B?MzhtaHJrczFvM1Ryam1LVDFRMjVVbmhhUnJ3cnZXSjN2MTlFM2lhdkJsbHkx?=
 =?utf-8?B?R2gxMFgzdElZNlVqdW85MUdLelcxMk90eExLZ0dJUUZSUjhRdU9HK2F3NllP?=
 =?utf-8?B?K0VPL3lidVFLd2R2K2V1M3puNk9ieFY1YVp1Y0Q3YTh5a3lOZFJWb2Y2dnlh?=
 =?utf-8?B?aXFUUWMwcmZSQ1p3MzJsQ0VEVnEwTUxyenY4QXU1Y2Z6Q2ZvRGJGTmcwT1lZ?=
 =?utf-8?B?Q1piL1J0Y1VzanI3dWpnMXJBOHBBb0w3QVZqQWJHeUN1N2tzRjJPVmlsV240?=
 =?utf-8?B?bHljSjlGbkVNSVlPZWRpcHJiOHhqd2lwTVVjRDJHZXVXRG1pUlp3NkRiRDZx?=
 =?utf-8?Q?8AL5USX8+nXYq8D6BUtPmy3WZ?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	nW0cDJvNbhUAisdWWVA6A5Da5RdDnhRwLu2wWQUnuVVkGtr16v2JAPyIKx2JnONi721mZTyN4QVmk3s+ROxLtS3wqZftlNq5BZDfXf5qGpSQdwWOcJN2lmrscOF8PHQRihtkFQdBnGzsivX6w+9Gp+5wJ2Q2GoJ//THsG4/Z/aGN9UxtjNK6Bi8nouHRbhWImdSPaKusTludFhvj8QMFGdFKITbxQSCoQrwuo3Ng1nr66F+u3xqDBned77pfLrJd7i/ozIhNny9Ifw5/fEWnDCelStzanxc4AI2P/nYXElU2e8iaMrqYCbCeilY4mDg3ciblvBi/2AijRLGHY8804hAlTWD0BgwZh7FKBu7L44NfTCzE82ylz3RdFJED6dmeggfJOjcJQxc15ZHy/OVYcl+LG0/XhslxuJIp2JmEwoDJ6D8vhlT2M8UwW8F7fiFaJAnTfGRPpES/0B44mLzPFvJgbvmmaj+FaN4BHJiZDunMWPqyao3Ns6OeUl97Vee+iU5YqPdzaNj/eAhzTG57qPUJqN2lf2a2ol3qmCLaBeOnHmjVQniXdfav6WWX1be/jRuyOTC/gTlnT/giaNE2XjI81cSCHh5caIBlz/AIFQ0=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 6db439ce-b1f1-4635-54aa-08dc663022e0
X-MS-Exchange-CrossTenant-AuthSource: SJ0PR10MB4429.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Apr 2024 20:33:31.0715
 (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: ETuzw2TXThVnDt6WSx81dTZK3nlAC53ekHtX1+q42qSLoWrnGBlTwy5grcptxMcx6vOhiPmY3/PjPCXlXe5fvA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR10MB6898
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1011,Hydra:6.0.650,FMLib:17.11.176.26
 definitions=2024-04-26_17,2024-04-26_02,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 mlxlogscore=999
 bulkscore=0 mlxscore=0 suspectscore=0 adultscore=0 malwarescore=0
 spamscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2404010000 definitions=main-2404260142
X-Proofpoint-ORIG-GUID: S2LA-1jh9vvmn1u_CUwX3_IQDrHziwEk
X-Proofpoint-GUID: S2LA-1jh9vvmn1u_CUwX3_IQDrHziwEk

My apology for the gobbled message earlier.

On 4/26/2024 12:52 PM, Jane Chu wrote:
> On 4/26/2024 12:05 PM, Matthew Wilcox wrote:
> [..]
>> That would be unsafe, the safe way would be if we moved page_folio() 
>> after
>>> the call to __get_hw_poison() in get_any_page() and there would 
>>> still be one
>>> remaining user of shake_page() that we can't convert. A safe version 
>>> of this
>>> patch would result in a removal of one use of PageHuge() and two 
>>> uses of
>>> put_page(), would that be worth submitting?
>>>
>>> get_any_page()
>>>     if(__get_hwpoison_page())
>>>         folio = page_folio() /* folio_try_get() returned 1, safe */
>> I think we should convert __get_hwpoison_page() to return either the 
>> folio
>> or an ERR_PTR or NULL.  Also, I think we should delete the "cannot catch
>> tail" part and just loop in __get_hwpoison_page() until we do catch it.
>> See try_get_folio() in mm/gup.c for inspiration (although you can't use
>> it exactly because that code knows that the page is mapped into a page
>> table, so has a refcount).
>>
>> But that's just an immediate assessment; you might find a reason that
>> doesn't work.
>
Besides, in a possible hugetlb demote scenario, it seems to me that we 
should retry
get_hwpoison_hugetlb_folio() instead of falling thru to folio_try_get().

static int __get_hwpoison_page(struct page *page, unsigned long flags)
{
         struct folio *folio = page_folio(page);
         int ret = 0;
         bool hugetlb = false;

         ret = get_hwpoison_hugetlb_folio(folio, &hugetlb, false);
         if (hugetlb) {
                 /* Make sure hugetlb demotion did not happen from under 
us. */
                 if (folio == page_folio(page))
                         return ret;
                 if (ret > 0) {      <===== still hugetlb, don't fall 
thru, retry
                         folio_put(folio);
                         folio = page_folio(page);
                 }

         }

thanks!
-jane