On Thu, Jul 15, 2021 at 1:39 AM Mike Kravetz <mike.kravetz@xxxxxxxxxx> wrote: > > On 7/14/21 3:57 AM, Muchun Song wrote: > > On Sat, Jul 10, 2021 at 8:25 AM Mike Kravetz <mike.kravetz@xxxxxxxxxx> wrote: > >> + /* > >> + * Very subtle > >> + * > >> + * For non-gigantic pages set the destructor to the normal compound > >> + * page dtor. This is needed in case someone takes an additional > >> + * temporary ref to the page, and freeing is delayed until they drop > >> + * their reference. > >> + * > >> + * For gigantic pages set the destructor to the null dtor. This > >> + * destructor will never be called. Before freeing the gigantic > >> + * page destroy_compound_gigantic_page will turn the compound page > >> + * into a simple group of pages. After this the destructor does not > >> + * apply. > >> + * > >> + * This handles the case where more than one ref is held when and > >> + * after update_and_free_page is called. > >> + */ > >> set_page_refcounted(page); > >> - set_compound_page_dtor(page, NULL_COMPOUND_DTOR); > >> + if (hstate_is_gigantic(h)) > >> + set_compound_page_dtor(page, NULL_COMPOUND_DTOR); > >> + else > >> + set_compound_page_dtor(page, COMPOUND_PAGE_DTOR); > > > > Hi Mike, > > > > The race is really subtle. But we also should remove the WARN from > > free_contig_range, right? Because the refcount of the head page of > > the gigantic page can be greater than one, but free_contig_range has > > the following warning. > > > > WARN(count != 0, "%lu pages are still in use!\n", count); > > > > I did hit that warning in my testing and thought about removing it. > However, I decided to keep it because non-hugetlb code also makes use of > alloc_contig_range/free_contig_range and it might be useful in those > cases. > > My 'guess' is that the warning was added not because of temporary ref > count increases but rather to point out any code that forgot to drop a > reference. Got it. At least this patch looks good to me. So Reviewed-by: Muchun Song <songmuchun@xxxxxxxxxxxxx> > > BTW - It is not just the 'head' page which could trigger this warning, but > any 'tail' page as well. That is because we do not call free_contig_range > with a compound page, but rather a group of pages all with ref count of > at least one. Right. > > I'm happy to remove the warning if people do not think it is generally > useful. For me, I suggest removing it. If someone has any ideas, please let us know. > -- > Mike Kravetz
