Thread (16 messages) 16 messages, 3 authors, 2011-06-13

Re: [PATCH] [BUGFIX] mm: hugepages can cause negative commitlimit

From: Rafael Aquini <hidden>
Date: 2011-05-19 13:37:17

Howdy Russ,

On Thu, May 19, 2011 at 1:56 AM, Russ Anderson [off-list ref] wrote:
On Wed, May 18, 2011 at 09:51:03PM -0300, Rafael Aquini wrote:
quoted
Howdy,

On Wed, May 18, 2011 at 12:34 PM, Russ Anderson [off-list ref] wrote:
quoted
If the total size of hugepages allocated on a system is
over half of the total memory size, commitlimit becomes
a negative number.

What happens in fs/proc/meminfo.c is this calculation:

       allowed = ((totalram_pages - hugetlb_total_pages())
               * sysctl_overcommit_ratio / 100) + total_swap_pages;

The problem is that hugetlb_total_pages() is larger than
totalram_pages resulting in a negative number.  Since
allowed is an unsigned long the negative shows up as a
big number.

A similar calculation occurs in __vm_enough_memory() in mm/mmap.c.

A symptom of this problem is that /proc/meminfo prints a
very large CommitLimit number.

CommitLimit:    737869762947802600 kB

To reproduce the problem reserve over half of memory as hugepages.
For example "default_hugepagesz=1G hugepagesz=1G hugepages=64
Then look at /proc/meminfo "CommitLimit:" to see if it is too big.

The fix is to not subtract hugetlb_total_pages().  When hugepages
are allocated totalram_pages is decremented so there is no need to
subtract out hugetlb_total_pages() a second time.

Reported-by: Russ Anderson <redacted>
Signed-off-by: Russ Anderson <redacted>

---

Example of "CommitLimit:" being too big.

uv1-sys:~ # cat /proc/meminfo
MemTotal:       32395508 kB
MemFree:        32029276 kB
Buffers:            8656 kB
Cached:            89548 kB
SwapCached:            0 kB
Active:            55336 kB
Inactive:          73916 kB
Active(anon):      31220 kB
Inactive(anon):       36 kB
Active(file):      24116 kB
Inactive(file):    73880 kB
Unevictable:           0 kB
Mlocked:               0 kB
SwapTotal:             0 kB
SwapFree:              0 kB
Dirty:              1692 kB
Writeback:             0 kB
AnonPages:         31132 kB
Mapped:            15668 kB
Shmem:               152 kB
Slab:              70256 kB
SReclaimable:      17148 kB
SUnreclaim:        53108 kB
KernelStack:        6536 kB
PageTables:         3704 kB
NFS_Unstable:          0 kB
Bounce:                0 kB
WritebackTmp:          0 kB
CommitLimit:    737869762947802600 kB
Committed_AS:     394044 kB
VmallocTotal:   34359738367 kB
VmallocUsed:      713960 kB
VmallocChunk:   34325764204 kB
HardwareCorrupted:     0 kB
HugePages_Total:      32
HugePages_Free:       32
HugePages_Rsvd:        0
HugePages_Surp:        0
Hugepagesize:    1048576 kB
DirectMap4k:       16384 kB
DirectMap2M:     2064384 kB
DirectMap1G:    65011712 kB

 fs/proc/meminfo.c |    2 +-
 mm/mmap.c         |    3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

Index: linux/fs/proc/meminfo.c
===================================================================
--- linux.orig/fs/proc/meminfo.c        2011-05-17 16:03:50.935658801
-0500
quoted
quoted
+++ linux/fs/proc/meminfo.c     2011-05-18 08:53:00.568784147 -0500
@@ -36,7 +36,7 @@ static int meminfo_proc_show(struct seq_
       si_meminfo(&i);
       si_swapinfo(&i);
       committed = percpu_counter_read_positive(&vm_committed_as);
-       allowed = ((totalram_pages - hugetlb_total_pages())
+       allowed = (totalram_pages
               * sysctl_overcommit_ratio / 100) + total_swap_pages;

       cached = global_page_state(NR_FILE_PAGES) -
Index: linux/mm/mmap.c
===================================================================
--- linux.orig/mm/mmap.c        2011-05-17 16:03:51.727658828 -0500
+++ linux/mm/mmap.c     2011-05-18 08:54:34.912222405 -0500
@@ -167,8 +167,7 @@ int __vm_enough_memory(struct mm_struct
               goto error;
       }

-       allowed = (totalram_pages - hugetlb_total_pages())
-               * sysctl_overcommit_ratio / 100;
+       allowed = totalram_pages * sysctl_overcommit_ratio / 100;
       /*
        * Leave the last 3% for root
        */
--
Russ Anderson, OS RAS/Partitioning Project Lead
SGI - Silicon Graphics Inc          rja@sgi.com

I'm afraid this will introduce a bug on how accurate kernel will account
memory for overcommitment limits.

totalram_pages is not decremented as hugepages are allocated. Since
Are you running on x86?  It decrements totalram_pages on a x86_64
test system.  Perhaps different architectures allocate hugepages
differently.

The way it was verified was putting a printk in to print totalram_pages
and hugetlb_total_pages.  First the system was booted without any huge
pages.  The next boot one huge page was allocated.  The next boot more
hugepages allocated.  Each time totalram_pages was reduced by the nuber
of huge pages allocated, with totalram_pages + hugetlb_total_pages
equaling the original number of pages.

That behavior is also consistent with allocating over half of memory
resulting in CommitLimit going negative (as is shown in the above
output).

Here is some data.  Each represents a boot using 1G hugepages.
  0 hugepages : totalram_pages 16519867 hugetlb_total_pages       0
  1 hugepages : totalram_pages 16257723 hugetlb_total_pages  262144
  2 hugepages : totalram_pages 15995578 hugetlb_total_pages  524288
 31 hugepages : totalram_pages  8393403 hugetlb_total_pages 8126464
 32 hugepages : totalram_pages  8131258 hugetlb_total_pages 8388608

quoted
hugepages are reserved, hugetlb_total_pages() has to be accounted and
subtracted from totalram_pages in order to render an accurate number of
remaining pages available to the general memory workload commitment.

I've tried to reproduce your findings on my boxes,  without
success, unfortunately.
Put a printk in meminfo_proc_show() to print totalram_pages and
hugetlb_total_pages().  Add "default_hugepagesz=1G hugepagesz=1G
hugepages=64"
to the boot line (varying the number of hugepages).
quoted
I'll keep chasing to hit this behaviour, though.

Cheers!
--aquini
--
Russ Anderson, OS RAS/Partitioning Project Lead
SGI - Silicon Graphics Inc          rja@sgi.com

I got what I was doing different, and you are partially right.
Checking mm/hugetlb.c:
1811 static int __init hugetlb_nrpages_setup(char *s)
1812 {
....
1834         /*
1835          * Global state is always initialized later in hugetlb_init.
1836          * But we need to allocate >= MAX_ORDER hstates here early to
still
1837          * use the bootmem allocator.
1838          */
1839         if (max_hstate && parsed_hstate->order >= MAX_ORDER)
1840                 hugetlb_hstate_alloc_pages(parsed_hstate);
1841
1842         last_mhp = mhp;
1843
1844         return 1;
1845 }
1846 __setup("hugepages=", hugetlb_nrpages_setup);

I realize this issue you've reported only happens when you're using
oversized hugepages. As their order are always >= MAX_ORDER, they got pages
early allocated from bootmem allocator. So, these pages are not accounted
for totalram_pages.

Although your patch covers a fix for the proposed case, it only works for
scenarios where oversized hugepages are allocated on boot. I think it will,
unfortunately, cause a bug for the remaining scenarios.

Cheers!
--aquini
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help