Messages in this thread | | | Date | Mon, 2 Feb 2009 11:23:33 -0800 (PST) | From | Linus Torvalds <> | Subject | Re: [PATCH] Fix OOPS in mmap_region() when merging adjacent VM_LOCKED file segments |
| |
On Mon, 2 Feb 2009, Mel Gorman wrote: > > Do not account for address space usage when making hugetlbfs mappings RW > > hugetlbfs accounts for its address space usage separate from the VM > core. VM_ACCOUNT should not be set for its mappings but it is possible it gets > set if a user creates a RO hugetlbfs mapping MAP_NORESERVE and then calls > mprotect(). This patch stops VM_ACCOUNT being set for hugetlbfs mappings > during mprotect(). > > Credit goes to Kosaki Motohiro for spotting this. > > Signed-off-by: Mel Gorman <mel@csn.ul.ie> > > diff --git a/mm/mprotect.c b/mm/mprotect.c > index abe2694..31ddc6a 100644 > --- a/mm/mprotect.c > +++ b/mm/mprotect.c > @@ -153,7 +153,7 @@ mprotect_fixup(struct vm_area_struct *vma, struct vm_area_struct **pprev, > * but (without finer accounting) cannot reduce our commit if we > * make it unwritable again. > */ > - if (newflags & VM_WRITE) { > + if (newflags & VM_WRITE && !(vma->vm_flags & VM_HUGETLB)) {
Wouldn't it be _much_ nicer to just depend on that whole VM_NORESERVE thing?
Those hugetlb mappings _should_ have VM_NORESERVE on them, so the following test:
> if (!(oldflags & (VM_ACCOUNT|VM_WRITE| > VM_SHARED|VM_NORESERVE))) { > charged = nrpages;
should do it all correctly.
Why make up some ad-hoc testing, when we already have a flag for _exactly_ this issue. That's what VM_NORESERVE means: don't apply VM_ACCOUNT.
IOW, I don't see the point of this patch at all.
And if there is some hugetlb path that doesn't set VM_NORESERVE, then fix _that_ instead.
Linus
| |