On 2023/7/8 4:19, Jiaqi Yan wrote:
> Traversal on llist (e.g. llist_for_each_safe) is only safe AFTER entries
> are deleted from the llist. Correct the way __folio_free_raw_hwp deletes
> and frees raw_hwp_page entries in raw_hwp_list: first llist_del_all, then
> kfree within llist_for_each_safe.
>
> As of today, concurrent adding, deleting, and traversal on raw_hwp_list
> from hugetlb.c and/or memory-failure.c are fine with each other. Note
I think there's a race on freeing the raw_hwp_list between unpoison_memory and __update_and_free_hugetlb_folio:
unpoison_memory __update_and_free_hugetlb_folio
if (folio_test_hwpoison)
folio_clear_hugetlb_hwpoison
folio_free_raw_hwp folio_free_raw_hwp
folio_test_clear_hwpoison
unpoison_memory and __update_and_free_hugetlb_folio can traverse and free the raw_hwp_list
at the same time. And I believe your patch will fix the problem. Thanks.
> this is guaranteed partly by the lock-free nature of llist, and partly
> by holding hugetlb_lock and/or mf_mutex. For example, as llist_del_all
> is lock-free with itself, folio_clear_hugetlb_hwpoison()s from
> __update_and_free_hugetlb_folio and memory_failure won't need explicit
> locking when freeing the raw_hwp_list. New code that manipulates
> raw_hwp_list must be careful to ensure the concurrency correctness.
>
> Acked-by: Mike Kravetz <mike.kravetz@xxxxxxxxxx>
> Acked-by: Naoya Horiguchi <naoya.horiguchi@xxxxxxx>
> Signed-off-by: Jiaqi Yan <jiaqiyan@xxxxxxxxxx>
Anyway, this patch looks good to me.
Reviewed-by: Miaohe Lin <linmiaohe@xxxxxxxxxx>
Thanks.
