Re: [PATCH] Fix OOPS in mmap_region() when merging adjacent VM_LOCKED file segments

Mel Gorman
2009-02-02T22:35:38Z

On Mon, Feb 02, 2009 at 02:12:30PM -0800, Linus Torvalds wrote:
>
>
> On Mon, 2 Feb 2009, Mel Gorman wrote:
> >
> > Lets say someone does the following
> >
> > 1. mmap(PROT_READ, MAP_PRIVATE) on a hugetlbfs file
> > VM_ACCOUNT is not set for hugetlbfs
> > VM_NORESERVE is not set because MAP_NORESERVE was not there
>
> But isn't this exactly the thing that we have that odd "accountable" flag
> for, and we do the whole
>
> if (!accountable)
> vm_flags |= VM_NORESERVE;
>
> in mmap_region() for?
>
> So VM_NORESERVE _will_ be set.
>

Then it's getting unconditionally set which breaks the hugetlb accounting
for reserving hugepages. See mm/hugetlb.c#decrement_hugepage_resv_vma() and
mm/hugetlb.c#hugetlb_reserve_pages() which depend on VM_NORESERVE being set
or not set depending on MAP_NORESERVE, not whether the core VM is accounting
it or not.

This is why I tried replacing

if (!accountable)
vm_flags |= VM_NORESERVE;

with

if ((flags & MAP_NORESERVE) && should_overcommit(file))
vm_flags |= VM_NORESERVE;

and

static inline int should_overcommit(struct file *file)
{
/* Check if the sysctl allows overcommit */
if (sysctl_overcommit_memory != OVERCOMMIT_NEVER)
return 1;

/* hugetlbfs does its own accounting */
if (file && is_file_hugepages(file))
return 1;

return 0;
}

in the first patch I mailed out.

--
Mel Gorman
Part-time Phd Student Linux Technology Center
University of Limerick IBM Dublin Software Lab