On 02/24/23 02:51, HORIGUCHI NAOYA(堀口 直也) wrote:
> On Thu, Feb 23, 2023 at 05:28:10PM +0000, Matthew Wilcox wrote:
> > On Thu, Feb 23, 2023 at 04:31:56PM +0800, Yin Fengwei wrote:
> > > +
> > > + if (folio_test_hwpoison(folio) && !(flags & TTU_IGNORE_HWPOISON)) {
> > > + pteval = swp_entry_to_pte(make_hwpoison_entry(&folio->page));
> > > + set_huge_pte_at(mm, address, pvmw.pte, pteval);
> > > + }
> > > +
> > > + /*** try_to_unmap_one() called dec_mm_counter for
> > > + * (folio_test_hwpoison(folio) && !(flags & TTU_IGNORE_HWPOISON)) not
> > > + * true case, looks incorrect. Change it to hugetlb_count_sub() here.
> > > + */
> > > + hugetlb_count_sub(folio_nr_pages(folio), mm);
>
> I have no objection to this change (moving hugetlb_count_sub() outside the
> if), but I have a question related to this.
>
> Generally TTU_IGNORE_HWPOISON is used to control the pte-conversion based
> on page dirtiness. But actually what it depends on is whether data lost
> happens when the page is forcibly dropped. And for hugetlb pages, that's
> true regardless of PageDirty flag on it.
> So I think we can assume that try_to_unmap_one_hugetlb() is called with
> TTU_IGNORE_HWPOISON clear. So maybe we don't need the if-check?
Thanks for looking at this Naoya!
In another reply I asked about the possibility of that if statement ever
being false for hugetlb pages. Looks like that can never happen. I went
back and looked at the memory failure/poison code just to be sure.
Yin,
Since we NEVER took went down the (folio_test_hwpoison(folio) &&
!(flags & TTU_IGNORE_HWPOISON)) not true path in the current code,
perhaps we not need the comment possibly calling dec_mm_counter.
--
Mike Kravetz
