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
