RE: [PATCH] mm/hugetlb: Fix incorrect error return from hugetlb_reserve_pages()

[Shameer Kolothum]

> -----Original Message-----
> From: Oscar Salvador <osalvador@xxxxxxx>
> Sent: 25 November 2025 09:34
> To: Shameer Kolothum <skolothumtho@xxxxxxxxxx>
> Cc: linux-mm@xxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> muchun.song@xxxxxxxxx; vivek.kasireddy@xxxxxxxxx; Jason Gunthorpe
> <jgg@xxxxxxxxxx>; Nicolin Chen <nicolinc@xxxxxxxxxx>; Nathan Chen
> <nathanc@xxxxxxxxxx>; Matt Ochs <mochs@xxxxxxxxxx>
> Subject: Re: [PATCH] mm/hugetlb: Fix incorrect error return from
> hugetlb_reserve_pages()
> 
> External email: Use caution opening links or attachments
> 
> 
> On Wed, Oct 22, 2025 at 11:29:56AM +0100, Shameer Kolothum wrote:
> > The function hugetlb_reserve_pages() returns the number of pages added
> > to the reservation map on success and a negative error code on failure
> > (e.g. -EINVAL, -ENOMEM). However, in some error paths, it may return
> > -1 directly.
> >
> > For example, a failure at:
> >
> >     if (hugetlb_acct_memory(h, gbl_reserve) < 0)
> >         goto out_put_pages;
> >
> > results in returning -1 (since add = -1), which may be misinterpreted
> > in userspace as -EPERM.
> >
> > Fix this by explicitly capturing and propagating the return values
> > from helper functions, and using -EINVAL for all other failure cases.
> >
> > Fixes: 986f5f2b4be3 ("mm/hugetlb: make hugetlb_reserve_pages() return
> > nr of entries updated")
> > Signed-off-by: Shameer Kolothum <skolothumtho@xxxxxxxxxx>
> > ---
> >  mm/hugetlb.c | 25 ++++++++++++++++++-------
> >  1 file changed, 18 insertions(+), 7 deletions(-)
> >
> > diff --git a/mm/hugetlb.c b/mm/hugetlb.c index
> > 795ee393eac0..1767f7599f91 100644
> > --- a/mm/hugetlb.c
> > +++ b/mm/hugetlb.c
> > @@ -7269,6 +7269,7 @@ long hugetlb_reserve_pages(struct inode *inode,
> >       struct resv_map *resv_map;
> >       struct hugetlb_cgroup *h_cg = NULL;
> >       long gbl_reserve, regions_needed = 0;
> > +     int ret;
> >
> >       /* This should never happen */
> >       if (from > to) {
> > @@ -7308,8 +7309,10 @@ long hugetlb_reserve_pages(struct inode
> *inode,
> >       } else {
> >               /* Private mapping. */
> >               resv_map = resv_map_alloc();
> > -             if (!resv_map)
> > +             if (!resv_map) {
> > +                     ret = -EINVAL;
> 
> Why is this one EINVAL? Should not this be ENOMEM?

Yes, looking at it again it should be ENOMEM. I will change that and
send out a v3 soon.

Btw, there is a v2 of the patch here,
https://lore.kernel.org/linux-mm/20251024094240.337630-1-skolothumtho@xxxxxxxxxx/

Please take a look if you haven't already.

Thanks,
Shameer