Re: [PATCH v4 2/2] mm,hwpoison: make get_hwpoison_page call get_any_page()

[HORIGUCHI NAOYA(堀口 直也)]

On Mon, May 17, 2021 at 02:59:03PM +0200, Oscar Salvador wrote:
> On Mon, May 17, 2021 at 01:54:01PM +0900, Naoya Horiguchi wrote:
> >  static int __get_hwpoison_page(struct page *page)
> >  {
> >  	struct page *head = compound_head(page);
> > @@ -1098,7 +1091,9 @@ static int __get_hwpoison_page(struct page *page)
> >  
> >  #ifdef CONFIG_HUGETLB_PAGE
> >  	spin_lock(&hugetlb_lock);
> > -	if (PageHuge(head) && (HPageFreed(head) || HPageMigratable(head)))
> > +	if (!PageHuge(head))
> > +		ret = -EBUSY;
> 
> Unless I'm missing something, we will be returning -EBUSY for any non-hugetlb
> page, which is not right.

(sigh, hard to say why I wrote this, really sorry)

> I think what you want is to return -EBUSY in case the page is a hugetlb page
> but we cannot grab a refcount because the page is in a floating state, meaning
> !HPageFreed && !HPageMigratable.
> 
> Right?

Totally right, so the below should what's wanted.

        spin_lock(&hugetlb_lock);
        if (PageHuge(head))
                if (!HPageFreed(head) && !HPageMigratable(head))
                        ret = -EBUSY;
                else
                        ret = get_page_unless_zero(head);
        spin_unlock(&hugetlb_lock);

- Naoya