RE: [PATCH v2 1/2] mm/memfd: reserve hugetlb folios before allocation

["Kasireddy, Vivek" <vivek.kasireddy@intel.com>]

Hi Andrew,

> Subject: Re: [PATCH v2 1/2] mm/memfd: reserve hugetlb folios before
> allocation
> 
> 
> > There are cases when we try to pin a folio but discover that it has
> > not been faulted-in. So, we try to allocate it in memfd_alloc_folio()
> > but there is a chance that we might encounter a crash/failure
> > (VM_BUG_ON(!h->resv_huge_pages)) if there are no active reservations
> > at that instant. This issue was reported by syzbot:
> >
> > kernel BUG at mm/hugetlb.c:2403!
> >
> > ...
> >
> > Therefore, to avoid this situation and fix this issue, we just need
> > to make a reservation (by calling hugetlb_reserve_pages()) before
> > we try to allocate the folio. This will ensure that we are properly
> > doing region/subpool accounting associated with our allocation.
> >
> > While at it, move subpool_inode() into hugetlb header and also
> > replace the VM_BUG_ON() with WARN_ON_ONCE() as there is no need to
> > crash the system in this scenario and instead we could just warn
> > and fail the allocation.
> >
> > ...
> >
> > @@ -2397,12 +2392,15 @@ struct folio *alloc_hugetlb_folio_reserve(struct
> hstate *h, int preferred_nid,
> >  	struct folio *folio;
> >
> >  	spin_lock_irq(&hugetlb_lock);
> > +	if (WARN_ON_ONCE(!h->resv_huge_pages)) {
> > +		spin_unlock_irq(&hugetlb_lock);
> > +		return NULL;
> > +	}
> > +
> 
> What is is that we're warning of here?
The warning serves two purposes:
1) To flag a situation that is unexpected at that instant
2) To alert the callers (mostly future) that they need to somehow reserve
     their hugetlb folios before calling this function

> Is there any action which
> either kernel developers or the user can take to prevent this warning
> from being issued?
Yeah, the callers of this function need to make a reservation and ensure that
hugetlb_reserve_pages() gets called (probably via hugetlbfs_file_mmap() or
other possible means) before they get their folios allocated via this function.

> 
> IOW, maybe the WARN shouldn't be present?
Instead of silently failing, warning the caller about the failure mode seems
like the right thing to do in this situation. However, I am OK if this warning is
not present (given that this not a common use-case as of now) but do you
see any concern if it stays?

Thanks,
Vivek