Messages in this thread | | | Subject | Re: [PATCH v3 2/4] mm/hwpoison: check if a subpage of a hugetlb folio is raw HWPOISON | From | Miaohe Lin <> | Date | Sat, 8 Jul 2023 10:57:00 +0800 |
| |
On 2023/7/8 4:19, Jiaqi Yan wrote: > Add the functionality, is_raw_hwp_subpage, to tell if a subpage of a > hugetlb folio is a raw HWPOISON page. This functionality relies on > RawHwpUnreliable to be not set; otherwise hugepage's raw HWPOISON list > becomes meaningless. > > is_raw_hwp_subpage needs to hold hugetlb_lock in order to synchronize > with __get_huge_page_for_hwpoison, who iterates and inserts an entry to > raw_hwp_list. llist itself doesn't ensure insertion is synchornized with > the iterating used by __is_raw_hwp_list. Caller can minimize the > overhead of lock cycles by first checking if folio / head page's > HWPOISON flag is set. > > Exports this functionality to be immediately used in the read operation > for hugetlbfs. > > Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com> > Reviewed-by: Naoya Horiguchi <naoya.horiguchi@nec.com> > Signed-off-by: Jiaqi Yan <jiaqiyan@google.com> > --- > include/linux/hugetlb.h | 19 +++++++++++++++++++ > include/linux/mm.h | 7 +++++++ > mm/hugetlb.c | 10 ++++++++++ > mm/memory-failure.c | 34 ++++++++++++++++++++++++---------- > 4 files changed, 60 insertions(+), 10 deletions(-) > ... > -static inline struct llist_head *raw_hwp_list_head(struct folio *folio) > +bool __is_raw_hwp_subpage(struct folio *folio, struct page *subpage) > { > - return (struct llist_head *)&folio->_hugetlb_hwpoison; > + struct llist_head *raw_hwp_head; > + struct raw_hwp_page *p, *tmp; > + bool ret = false; > + > + if (!folio_test_hwpoison(folio)) > + return false; > + > + /* > + * When RawHwpUnreliable is set, kernel lost track of which subpages > + * are HWPOISON. So return as if ALL subpages are HWPOISONed. > + */ > + if (folio_test_hugetlb_raw_hwp_unreliable(folio)) > + return true; > + > + raw_hwp_head = raw_hwp_list_head(folio); > + llist_for_each_entry_safe(p, tmp, raw_hwp_head->first, node) {
Since we don't free the raw_hwp_list, does llist_for_each_entry works same as llist_for_each_entry_safe?
> + if (subpage == p->page) { > + ret = true; > + break; > + } > + } > + > + return ret; > }
It seems there's a race between __is_raw_hwp_subpage and unpoison_memory: unpoison_memory __is_raw_hwp_subpage if (!folio_test_hwpoison(folio)) -- hwpoison is set folio_free_raw_hwp llist_for_each_entry_safe raw_hwp_list llist_del_all .. folio_test_clear_hwpoison
But __is_raw_hwp_subpage is used in hugetlbfs, unpoison_memory couldn't reach here because there's a folio_mapping == NULL check before folio_free_raw_hwp.
Anyway, this patch looks good to me.
Reviewed-by: Miaohe Lin <linmiaohe@huawei.com> Thanks.
| |