Re: [PATCH] Do not account for the address space used by hugetlbfsusing VM_ACCOUNT V2 (Was Linus 2.6.29-rc4)

[Mel Gorman]

On Wed, Feb 11, 2009 at 12:03:17PM +0000, Andy Whitcroft wrote:
> > <SNIP>
> >
> > Yes, and this was a mistake. For noreserve mappings, we may now be taking
> > twice the amount of quota and probably leaking it. This is wrong and I need
> > to move the check for quota below the check for VM_NORESERVE. Good spot.
> 
> Thanks.
> 

How about this?

=====
[PATCH] Do not account for hugetlbfs quota at mmap() time if mapping *_NORESERVE

Commit 5a6fe125950676015f5108fb71b2a67441755003 brought hugetlbfs more in line
with the core VM by obeying VM_NORESERVE and not reserving hugepages for both
shared and private mappings when [SHM|MAP]_NORESERVE are specified. However,
it is still taking filesystem quota unconditionally and this leads to
double-accounting.

At fault time, if there are no reserves and attempt is made to allocate the
page and account for filesystem quota. If either fail, the fault fails. This
patch prevents quota being taken when [SHM|MAP]_NORESERVE is specified.
To help prevent this mistake happening again, it improves the documentation
of hugetlb_reserve_pages().

Reported-by: Andy Whitcroft <apw@xxxxxxxxxxxxx>
Signed-off-by: Mel Gorman <mel@xxxxxxxxx>
--- 
 hugetlb.c |   29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 2074642..b0b63cd 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -2289,24 +2289,41 @@ int hugetlb_reserve_pages(struct inode *inode,
 	if (chg < 0)
 		return chg;
 
-	if (hugetlb_get_quota(inode->i_mapping, chg))
-		return -ENOSPC;
-
 	/*
-	 * Only apply hugepage reservation if asked. We still have to
-	 * take the filesystem quota because it is an upper limit
-	 * defined for the mount and not necessarily memory as a whole
+	 * Only apply hugepage reservation if asked. At fault time, an
+	 * attempt will be made for VM_NORESERVE to allocate a page
+	 * and filesystem quota without using reserves
 	 */
 	if (acctflag & VM_NORESERVE) {
 		reset_vma_resv_huge_pages(vma);
 		return 0;
 	}
 
+	/* There must be enough filesystem quota for the mapping */
+	if (hugetlb_get_quota(inode->i_mapping, chg))
+		return -ENOSPC;
+
+	/*
+	 * Check enough hugepages are available for the reservation.
+	 * Hand back the quota if there are not
+	 */
 	ret = hugetlb_acct_memory(h, chg);
 	if (ret < 0) {
 		hugetlb_put_quota(inode->i_mapping, chg);
 		return ret;
 	}
+
+	/*
+	 * Account for the reservations made. Shared mappings record regions
+	 * that have reservations as they are shared by multiple VMAs.
+	 * When the last VMA disappears, the region map says how much
+	 * the reservation was and the page cache tells how much of
+	 * the reservation was consumed. Private mappings are per-VMA and
+	 * only the consumed reservations are tracked. When the VMA
+	 * disappears, the original reservation is the VMA size and the
+	 * consumed reservations are stored in the map. Here, we just need
+	 * to allocate the region map.
+	 */
 	if (!vma || vma->vm_flags & VM_SHARED)
 		region_add(&inode->i_mapping->private_list, from, to);
 	else {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/