From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id EBDFDC47258 for ; Thu, 11 Jan 2024 18:30:41 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 5D9AD8D0003; Thu, 11 Jan 2024 13:30:41 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 589BD8D0001; Thu, 11 Jan 2024 13:30:41 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4512E8D0003; Thu, 11 Jan 2024 13:30:41 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 36FE08D0001 for ; Thu, 11 Jan 2024 13:30:41 -0500 (EST) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 0A127404EE for ; Thu, 11 Jan 2024 18:30:41 +0000 (UTC) X-FDA: 81667871082.05.5C7DCB9 Received: from mail-yb1-f172.google.com (mail-yb1-f172.google.com [209.85.219.172]) by imf19.hostedemail.com (Postfix) with ESMTP id 33CF61A0022 for ; Thu, 11 Jan 2024 18:30:38 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=f4K3WWAT; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf19.hostedemail.com: domain of jiaqiyan@google.com designates 209.85.219.172 as permitted sender) smtp.mailfrom=jiaqiyan@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1704997839; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=8fY44QehQLsPtLxgWQHF6aw3Iv2rADsazU3iCR26tI4=; b=r4YIpViOyNwZDuXMCA5jL+6RtZBC4yDZqcwmQd7o1fFxDjnIiFaCbvQS7l8qI86WprrXj/ hWYBZJsbo8i0rCqiAKd+aBbXjCgiyDASF66k6uPtsOU10Pul5+JZstweOql/6cx2auuTWo Mml5s7WYPHmnO1iIpx8491aHVxmJjM0= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=f4K3WWAT; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf19.hostedemail.com: domain of jiaqiyan@google.com designates 209.85.219.172 as permitted sender) smtp.mailfrom=jiaqiyan@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1704997839; a=rsa-sha256; cv=none; b=eAKtb8PjxrTiCBihybVQUorIE1NgkHWmx2hy94ruP6/UyhvPCUhKZjsu7Bdn2VxKTnPP/W Rv4oqv45Dta2BXadfSLPCDPrll1Tj0+7uKIPwh+JvIctlkI3ys4LgaE1SQ6BHgTKVmaFAf ODu2PtWBDkSlsIJ8j9OevJqLc5GUakc= Received: by mail-yb1-f172.google.com with SMTP id 3f1490d57ef6-dbed5d1ffb0so5089124276.1 for ; Thu, 11 Jan 2024 10:30:38 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1704997838; x=1705602638; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=8fY44QehQLsPtLxgWQHF6aw3Iv2rADsazU3iCR26tI4=; b=f4K3WWATPD8qRj64mGL6VPiFUZpB4xkMYq0UYWcPQ6z3jlutgze/nqMD08ufwQOBVu oymRTpp6tcC1mMNFWTBFU9p2ppaVBtFLuZPuOydGyTLKKvofYBxoAgtgCwVYqsVCeSOl EEmNGd1KdYhJk6XhYwxOcVHWJeTLS584k6FLKAbpw2oIxIyl8fQpv3+J46RbwbuYsa7J S/0+UIxZwz+3S2E5qGu8Yrhs7Zlw9im8Ll9+72jbRSsFs8tLpoT0nnzj3ppw/g9dpwlK 3DATtSqYnKhD46MN9E6G49PloNJyB7qME9iz4N3I2ml7cUD6qWsHxRIKvUC28ovzbMC+ Cucg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1704997838; x=1705602638; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=8fY44QehQLsPtLxgWQHF6aw3Iv2rADsazU3iCR26tI4=; b=TajVal2sV0ZLs75PR9OC3p9Jrt4bwHdkiEzrQBxZraHgP7N9gKQAmUxZeakEohdKiX 3H4NetDrn6k4rnmXBHIAZC/R8Utx0+7BctwhxLpECxPEILObMXUGvLNPcoMpifE4Xy2k hmgtviNX26t8LJ9cZgOYCGZzuzF+bytHT1e6Ss05AEvpCAPNJxu8M/pgaod0Lf21qo+H mZWcc3rc0f03wD1Qh3FaoRa/ArVOBmwzj/3QK3zrMy+3hjP2TNK5xtvKBvHWpJSNAzqY v3QR3f372HWrXV1mpnHr3VBBKZYTKqJKNu4T7eNPuZpwXAOXUxdoNlAczKDatFjUy5Gs dgkw== X-Gm-Message-State: AOJu0YyWmt1pnR/Om/CqCF2OR5cNT/dEb+2ggarwbWHeir5ef7nEqxju bS8zpELyR2y69Dun1jMAti4S9rgGHeNBwV68HTs/f85G6OEi X-Google-Smtp-Source: AGHT+IGu8xM1mI1IuVg0Xh1NHzyABL7jO33XxIfjwVz0zOvSd0/h6PSkZLOyDnDcac1Pu/0tOy/PQ5be2Aq836WKPSw= X-Received: by 2002:a5b:8:0:b0:dbf:23cd:c05c with SMTP id a8-20020a5b0008000000b00dbf23cdc05cmr66460ybp.13.1704997838063; Thu, 11 Jan 2024 10:30:38 -0800 (PST) MIME-Version: 1.0 References: <20230713001833.3778937-1-jiaqiyan@google.com> <20230713001833.3778937-5-jiaqiyan@google.com> <079335ab-190f-41f7-b832-6ffe7528fd8b@collabora.com> <6bacbd7c-88cb-1399-8bd0-db98c93a1adf@oracle.com> <39b90dce-fe0f-e1d8-3094-75cabbfa38a3@oracle.com> In-Reply-To: <39b90dce-fe0f-e1d8-3094-75cabbfa38a3@oracle.com> From: Jiaqi Yan Date: Thu, 11 Jan 2024 10:30:25 -0800 Message-ID: Subject: Re: [PATCH v4 4/4] selftests/mm: add tests for HWPOISON hugetlbfs read To: Sidhartha Kumar Cc: Matthew Wilcox , Muhammad Usama Anjum , linmiaohe@huawei.com, mike.kravetz@oracle.com, naoya.horiguchi@nec.com, akpm@linux-foundation.org, songmuchun@bytedance.com, shy828301@gmail.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org, jthoughton@google.com, "kernel@collabora.com" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 33CF61A0022 X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: 3jd564873d4x3i7odikn3rakic1rk41e X-HE-Tag: 1704997838-856563 X-HE-Meta: U2FsdGVkX19ZN7fnHy7p9ZIrL3uOjR6VCVUO0loI2aKfddk3eeRb5EPKdO9ZM86dPHX3gx07lRCXfSHdgBo+L8BXr9PFptwNEUzZwvgUv+ZRwPtAnlF4PycTn/RFkgFNOV+tqHoN+Fo7FTlmweaabhJ2OwESW4zZNRaUHFbx9Ank1DUVw4I8+t4go6X4Rbbp7fEWyuoiR8X+hCx4bfxczbE0wU5Me6uaRYRFN1cpcP6H9Ju/OiS/8hGhVXEMKcvRvBOOjhzsPAjDKMDoo4K0XvPzQA34BvhLj2GybLWOKd13I4xOt/rOXfnaWRyvuGF8YSe3kFrc7w0luYGld90PzelKwrzCe1/EYTvJDeUc9IMspMnNrUuQcplSb3WVxRCv1JVCcRyNlUK9wwVXmT+Zn9lxfOfGIbpr7x/5Um7oAZKXGQ5C9QeCy7uwEqAJi1YDG2b/x754svCFXSEKp5qdPD71+d6PFHB52fiAl/guJhMLOTGHBqZuAQoZpe/gZuDEZWqmbgdf2SUmzO9KV4aN7+f+RX9rWHpqHcK67JVxn7rZhlHOdDAmVxhUQ95NmlmMroDpflPuB06t0uadYodsfTni59kZeUOM8stcNkFTJ/JGH2PmL2QPeSG5O3VYEQXC7XzcplXE1Vt5S2K/mKtVd52viNqPNJfeuqEWvZlXDh+iHvsYW6b0WOsxz6+zSJ80+iJs8LKCtnCWJKO2p6wNrZtdHvRAdILizvjbssFvf3LbgNNh3UTzWTYnzF3V9lzDeOMTJxaPh6DcFG5CDj2sG+GGYahRDrE5sKApTlv8k9kuEVLpm5jqeCnYAm+8uikL+O0U2W1u92fht+cS/B88crZ11uh1rJkCgXNqMFLaU81urd66M1382hT9f87kdNn/1bE3IpLLs2rhOSwnhcPN+wV+ZZ5X8Zj09CpMQ/dzNHP5Lb+0yCTEQAS3iPQmr5bCbsLQwkIiwDPRtepzxmW bSU9Cadq DgPK1pDSY2B71Owm/IvrCQqwjWk9hMMqILqlTtzsIybNdRtqACEtL/oWIP3uxY8/6t+RQob+m2tLf9LnhLqVVVEsICRhepcqwyY3m0+w6wJN0Z49+8O34JuXSbFASXiznqKY2X86a+fQTLcuioTOJQQoou2wg5z6ogahOEIz/6/xz/MLZ6XRDbklmg7+6Xx3welkY+f3KEDI3mZP9DGDbNEWu1stOcPoAsy7cx6SfugRc+XBzen+OP+V7LGX/OH+OZryAmFdd4XYrJ5W0oci5LE++Fn4VTmOky9s2 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Thu, Jan 11, 2024 at 10:11=E2=80=AFAM Sidhartha Kumar wrote: > > On 1/11/24 10:03 AM, Matthew Wilcox wrote: > > On Thu, Jan 11, 2024 at 09:51:47AM -0800, Sidhartha Kumar wrote: > >> On 1/11/24 9:34 AM, Jiaqi Yan wrote: > >>>> - if (!folio_test_has_hwpoisoned(folio)) > >>>> + if (!folio_test_hwpoison(folio)) > >>> > >>> Sidhartha, just curious why this change is needed? Does > >>> PageHasHWPoisoned change after commit > >>> "a08c7193e4f18dc8508f2d07d0de2c5b94cb39a3"? > >> > >> No its not an issue PageHasHWPoisoned(), the original code is testing = for > >> the wrong flag and I realized that has_hwpoison and hwpoison are two > >> different flags. The memory-failure code calls folio_test_set_hwpoison= () to > >> set the hwpoison flag and does not set the has_hwpoison flag. When > >> debugging, I realized this if statement was never true despite the cod= e > >> hitting folio_test_set_hwpoison(). Now we are testing the correct flag= . > >> > >> From page-flags.h > >> > >> #ifdef CONFIG_MEMORY_FAILURE > >> PG_hwpoison, /* hardware poisoned page. Don't touch */ > >> #endif > >> > >> folio_test_hwpoison() checks this flag ^^^ > >> > >> /* At least one page in this folio has the hwpoison flag set */ > >> PG_has_hwpoisoned =3D PG_error, > >> > >> while folio_test_has_hwpoisoned() checks this flag ^^^ > > > > So what you're saying is that hugetlb behaves differently from THP > > with how memory-failure sets the flags? > > I think so, in memory_failure() THP goes through this path: > > hpage =3D compound_head(p); > if (PageTransHuge(hpage)) { > /* > * The flag must be set after the refcount is bumped > * otherwise it may race with THP split. > * And the flag can't be set in get_hwpoison_page() since > * it is called by soft offline too and it is just called > * for !MF_COUNT_INCREASED. So here seems to be the best > * place. > * > * Don't need care about the above error handling paths f= or > * get_hwpoison_page() since they handle either free page > * or unhandlable page. The refcount is bumped iff the > * page is a valid handlable page. > */ > SetPageHasHWPoisoned(hpage); > > which sets has_hwpoisoned flag while hugetlb goes through > folio_set_hugetlb_hwpoison() which calls folio_test_set_hwpoison(). Yes, hugetlb sets HWPoison flag as the whole hugepage is poisoned once a raw page is poisoned. It can't split to make other supages still available as THP. This "Improve hugetlbfs read on HWPOISON hugepages" patchset only improves fs case as splitting is not needed. I found commit a08c7193e4f18 ("mm/filemap: remove hugetlb special casing in filemap.c") has the following changes in inode.c: --- a/fs/hugetlbfs/inode.c +++ b/fs/hugetlbfs/inode.c @@ -334,7 +334,7 @@ static ssize_t hugetlbfs_read_iter(struct kiocb *iocb, struct iov_iter *to) ssize_t retval =3D 0; while (iov_iter_count(to)) { - struct page *page; + struct folio *folio; size_t nr, copied, want; /* nr is the maximum number of bytes to copy from this page= */ @@ -352,18 +352,18 @@ static ssize_t hugetlbfs_read_iter(struct kiocb *iocb, struct iov_iter *to) } nr =3D nr - offset; /* nr is the maximum number of bytes to copy from this page= */ @@ -352,18 +352,18 @@ static ssize_t hugetlbfs_read_iter(struct kiocb *iocb, struct iov_iter *to) } nr =3D nr - offset; - /* Find the page */ - page =3D find_lock_page(mapping, index); - if (unlikely(page =3D=3D NULL)) { + /* Find the folio */ + folio =3D filemap_lock_hugetlb_folio(h, mapping, index); + if (IS_ERR(folio)) { /* * We have a HOLE, zero out the user-buffer for the * length of the hole or request. */ copied =3D iov_iter_zero(nr, to); } else { - unlock_page(page); + folio_unlock(folio); - if (!PageHWPoison(page)) + if (!folio_test_has_hwpoisoned(folio)) want =3D nr; So I guess this "PageHWPoison =3D> folio_test_has_hwpoisoned" change is another regression aside from the refcount thing?