Received-SPF: pass (google.com: domain of linux-kernel+bounces-24004-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <b43ed3bd-ddf6-6587-7eb2-3e081ae1d928@oracle.com>
Date: Thu, 11 Jan 2024 11:21:12 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.15.1
Subject: Re: [PATCH] fix hugetlbfs hwpoison handling
Content-Language: en-US
To: linux-kernel@vger.kernel.org, linux-mm@kvack.org
Cc: akpm@linux-foundation.org, usama.anjum@collabora.com,
        muchun.song@linux.dev, jiaqiyan@google.com, willy@infradead.org,
        linmiaohe@huawei.com, naoya.horiguchi@nec.com, shy828301@gmail.com,
        jthoughton@google.com, jane.chu@oracle.com, stable@vger.kernel.org
References: <20240111191655.295530-1-sidhartha.kumar@oracle.com>
From: Sidhartha Kumar <sidhartha.kumar@oracle.com>
In-Reply-To: <20240111191655.295530-1-sidhartha.kumar@oracle.com>
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?OGIzLzh4Zkh0VUVXSXpNOWozQ05BOEE3UGpFbTF0SmtiOUdpcEdkRWQ3M0NO?=
 =?utf-8?B?ejd6b3QwRkxTV3dCdlIrQjFkS1B2TjFrZ3NCSlFtSGcydzdyNDJacjRIZ2g1?=
 =?utf-8?B?TDd0V3UxVUtIclJ2djlmTmZid1NOSDBNTXhXQ2JWZk12N2lOSkRKTlRhRTJP?=
 =?utf-8?B?eDNnV2FEV1J5eTVVQlR5aGxRd1QzT0FDbG5RelMraDEyT0EvcUZRQlNFMHJC?=
 =?utf-8?B?Y2ZtOURINnAzdEVoVG5QWXdscHRILzBJNGcvMHordUVvcGY0cTdMWWRRZisv?=
 =?utf-8?B?RnROcTZXdHovSUhLQ1kvc1RKdklVekxBMkRPQTRzUkdxVis1NzJaZ2kzQXlN?=
 =?utf-8?B?UklRV0I2WTlJNGk1emJYZTBiOVVtNzdKTUR1WVltd0t2aGRUQ2lFOTNtQXU4?=
 =?utf-8?B?SzQ5MlVLd2RoUnhJRDZDeTVzVFNBNUZxNGJKSHlvSFNUU3IzaFBUcHE2WDMy?=
 =?utf-8?B?T205NE4rYTRFemxuMldvMVloNW1seFlUNVRvRWdkUFVqK24xSGczMEI4NHk5?=
 =?utf-8?B?RXZVdFB1Z2pqeGlJS2RyVkZ4N2NIKzZrdjJxbWQxQndnQUtQS1BQYnFJeHlm?=
 =?utf-8?B?K0xKTHJoRi9PQlZIZUlhL3pWT0tsRVpyZkhUWUpHUTBoZDk1Z2puVXFJN1N5?=
 =?utf-8?B?WnovZlJjazFUcjA0cndvUjNLcmEyelc5R0R2cDA0SExhNXJQdDJvdHNod09W?=
 =?utf-8?B?ZDhNWFpmRWJMUXBldDlGYTBrdEMxYnRnVmY3cFpOZU5rakQyWW0ralBBbGxW?=
 =?utf-8?B?cXpWN3ljTzBtVFhPN3ZRbnJkaDU1RjVMUGVyQVhsT0JiQkplR1dub1dTNHpE?=
 =?utf-8?B?ZUdpMHFrSnBrODYwUjRrUjNVZ0lwSUhybkN3MGZ3d0tMTDRieFRVUlg4bzFv?=
 =?utf-8?B?WHk1N2pNVGZCbjRZYjd2RS92eTI5S3FnQjZPcmRoVHJobnhRb2hEdUdVWXNj?=
 =?utf-8?B?aHQzdit2Yyt1djNJTmdKMGxNdmttZE1UTnVUTTY5M0pBSU5BdG5vTmJSRE9L?=
 =?utf-8?B?MUhTWHU5QUs0SDUvTG5NSFNWa2FiTTF2TEt1b0J0VndCWHE3UTRVcmRpZk1C?=
 =?utf-8?B?enhtYWtWSU4zRTBxK1NyL2d3UlgrUjlwcTNYcHovb3BwNlZHekJweTRGTHBz?=
 =?utf-8?B?MHNJQnpnMHZrbDFDeUM5bC9LZFh6SmJtenJyTFExaWtxdnBsQnBHRlBhTW1T?=
 =?utf-8?B?dzFHU3psaWw5Uk5JdUxWZldjUXNVMlR1b0FuQXNwcElWVU5vOVZsbGZVei9D?=
 =?utf-8?B?bWF3VTFYL2NFSFZrdWtIZEh5bW11ZWh0Vy92UzFCeWtGb3hzekpyZXlwQ21K?=
 =?utf-8?B?cmpxNWx1MEhHWWsrMlVrRS9aMXhPUWhPeGNicm9uZG1xbGpjRHFtenlmMUMy?=
 =?utf-8?B?ZUY2b1FrZXBXNWN0OHFuRzNHcWk1VnFIdzNOSndjRHl1ZTdZOUhlRUFqeWRo?=
 =?utf-8?B?bVIyVDJpdGx2ZkJFdmZMczd6cktSckxEYnVSVTNyUHp2VFROYk5qRDNLKzRo?=
 =?utf-8?B?NFlmWGFQMllvQU45WUozNzJudEF3aFFuMEI5bXpUb0hPS2ppbjAyeWZQWlN0?=
 =?utf-8?B?MVFlbmJDeUFpRjFkaXRiZ0RhUzhiVENQSGJ6My95bjM4WG9ZNUZzK3M5WDhz?=
 =?utf-8?B?T2pHQ3lHTFVOa1pwNzU1M0kxNmNhMmxsVUJNSGQzVjNHazRjZWh1ajcyK1c4?=
 =?utf-8?B?aEtLbEFQYXhudThCd01PQzNDc0I3cjQ2L2xhaU5xWmVBUzdJdytNZm0xenhT?=
 =?utf-8?B?QXNYRmxmUG1KNGhSdDJtaUZiR0hXNVVOWEJidDBDTGhIYURZSVBFNWVyY2xM?=
 =?utf-8?B?M08vRW83NW9lTXBHUUV2LzQ2QkxtQ1FoK0ZlZXNGeldzUGx6anBJeWM5TVB5?=
 =?utf-8?B?VGdmcER4T0x0UzE3YmZMcjIxRk5uYTNQWWkzYWROOHptWVZ5eGlsajg1Zlhy?=
 =?utf-8?B?bm5KOTNqSWJyS2V0VUorVUJLMjJTenRrSTZreTlkaEVsWlNFYTJXVU9wc3dK?=
 =?utf-8?B?NVRqV1FZU3JTY1lSMnVMSWVWQk1lWW1UU0F1QXEyZE8xc1R0M2VrL3lSNEl1?=
 =?utf-8?B?MEticlB3WGllQm0xRExBcERLN0NwbFRvbDdTb1NkbFZ1YkhIcHRMU0NLSXls?=
 =?utf-8?B?NHhLMVVudTVMeTE4OFkvaU52dG9PbGJYUkdWRU5nSnpJSGM5K2VRVW1sVnZX?=
 =?utf-8?Q?W6YIEfLFeqmLMOQvpw1O/Q4=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	IeC6Cv8TYbF6e5Eaq6zNIUI7YOdK9yVEdviiPlDThTHy1K8L+rg/XfyAUXmfg9OlLWL67zWUZ3Zc5j8Tr3reA3/Dl14uYVF1UF4JpF58ZL9u9HxW22Ct5knzmoGIHxRiXFgxenymm/BL0xMc7iTKZixpfwaBRTAbfVFpQPOROrDsaGu0IBLgElF9exnIxSrBxi+4KcL71pUY9+u6m0aiC6xVemkFVpQ8T218zjBBTkTtFUMqutt5NIPGer3rFZIhli4T/F4TsYfCPqzLEGFly5HbmEWEg7b6mmy5eczwN4wul/fWTpPriGVD10R8J6RlbTnRq4HSuqNYaeOfdfrgZ2L0gPlRX+oSu1aKge4ju7hi2zucgOrsSLIbgxcUS9SGKqI9T68T15QStyrTQJCwSkJH18p66sl5EnOSkHOdJoDAujYCu4ulw9XEwmTmSfz8FyYhpXUwNO50gntsqxNx7UaNK0b8bRFXDLUsrT8iz9YpgmZqGsDYxoKfWRLqSebx3ir+RMluY1pRfTcwqMyLBZHsaqgek0oNsn+ghwmRKIWcrbpY+Wg3kRyyOGO+Za9PpHzogKnATCNn9bLyduvHKLcInD/GhPCH1xu2LOyIO0U=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 193c8177-41ec-4c63-4daa-08dc12da7ad6
X-MS-Exchange-CrossTenant-AuthSource: CH0PR10MB5113.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Jan 2024 19:21:15.5048
 (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: DdVgo/Trw7SCjKlNtaCSk/LB4yzlBned/lKRNn6x3tene61qvqQP4LnpCqhGGq0G2xgV2jJ0F5GEtEKe86bOrd4zUJ35MHQwat38uDYB8aI=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR10MB7200
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.997,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2024-01-11_11,2024-01-11_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 bulkscore=0 adultscore=0
 phishscore=0 malwarescore=0 mlxlogscore=999 suspectscore=0 mlxscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2311290000
 definitions=main-2401110151
X-Proofpoint-GUID: yLO6KSO2927Dit6g_TsUFbImwOCHfkXB
X-Proofpoint-ORIG-GUID: yLO6KSO2927Dit6g_TsUFbImwOCHfkXB

On 1/11/24 11:16 AM, Sidhartha Kumar wrote:
> has_extra_refcount() makes the assumption that a ref count of 1 means
> the page is not referenced by other users. Commit a08c7193e4f1
> (mm/filemap: remove hugetlb special casing in filemap.c) modifies
> __filemap_add_folio() by calling folio_ref_add(folio, nr); for all cases
> (including hugtetlb) where nr is the number of pages in the folio. We
> should check if the page is not referenced by other users by checking
> the page count against the number of pages rather than 1.
> 
> In hugetlbfs_read_iter(), folio_test_has_hwpoisoned() is testing the wrong
> flag as, in the hugetlb case, memory-failure code calls
> folio_test_set_hwpoison() to indicate poison. folio_test_hwpoison() is the
> correct function to test for that flag.
> 
> After these fixes, the hugetlb hwpoison read selftest passes all cases.
> 
> Fixes: a08c7193e4f1 ("mm/filemap: remove hugetlb special casing in filemap.c")
> Closes: https://lore.kernel.org/linux-mm/20230713001833.3778937-1-jiaqiyan@google.com/T/#m8e1469119e5b831bbd05d495f96b842e4a1c5519
> Cc: <stable@vger.kernel.org> # 6.7+
> Signed-off-by: Sidhartha Kumar <sidhartha.kumar@oracle.com>
> Reported-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> Tested-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
>   fs/hugetlbfs/inode.c | 2 +-
>   mm/memory-failure.c  | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> index 36132c9125f9..3a248e4f7e93 100644
> --- a/fs/hugetlbfs/inode.c
> +++ b/fs/hugetlbfs/inode.c
> @@ -340,7 +340,7 @@ static ssize_t hugetlbfs_read_iter(struct kiocb *iocb, struct iov_iter *to)
>   		} else {
>   			folio_unlock(folio);
>   
> -			if (!folio_test_has_hwpoisoned(folio))
> +			if (!folio_test_hwpoison(folio))
>   				want = nr;
>   			else {
>   				/*
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index d8c853b35dbb..87f6bf7d8bc1 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -973,7 +973,7 @@ struct page_state {
>   static bool has_extra_refcount(struct page_state *ps, struct page *p,
>   			       bool extra_pins)
>   {
> -	int count = page_count(p) - 1;
> +	int count = page_count(p) - folio_nr_pages(page_folio(p));
>   
>   	if (extra_pins)
>   		count -= 1;

the patch title is missing a fs/hugetlbfs: prefix, could this be added Andrew?

Thanks,
Sidhartha