From: Mina Almasry <firstname.lastname@example.org> To: Mike Kravetz <email@example.com> Cc: shuah <firstname.lastname@example.org>, open list <email@example.com>, firstname.lastname@example.org, email@example.com, firstname.lastname@example.org, Aneesh Kumar <email@example.com>, Hillf Danton <firstname.lastname@example.org>, Andrew Morton <email@example.com> Subject: Re: [PATCH v8 1/9] hugetlb_cgroup: Add hugetlb_cgroup reservation counter Date: Fri, 8 Nov 2019 15:35:55 -0800 [thread overview] Message-ID: <CAHS8izPp9aW=uxA2z6BLmRXgaiDVyKYvAbnrH0rcirN4wTD55g@mail.gmail.com> (raw) In-Reply-To: <firstname.lastname@example.org> On Thu, Nov 7, 2019 at 3:42 PM Mike Kravetz <email@example.com> wrote: > > Cc: Andrew > This series is getting closer to consideration for the mm tree. > Mina, > Be sure to cc Andrew with next version of series. > Absolutely! > On 10/29/19 6:36 PM, Mina Almasry wrote: > > These counters will track hugetlb reservations rather than hugetlb > > memory faulted in. This patch only adds the counter, following patches > > add the charging and uncharging of the counter. > > I honestly am not sure the preferred method for including the overall > design in a commit message. Certainly it should be in the first patch. > Perhaps, say this is patch 1 of a 9 patch series here. > Will do. I read somewhere I can't find right now it's better this way so that the useful information becomes part of the git log. If anyone has strong opinions on this I'll just go back to putting it into a cover letter. > > Problem: > > Currently tasks attempting to allocate more hugetlb memory than is available get > > a failure at mmap/shmget time. This is thanks to Hugetlbfs Reservations . > > However, if a task attempts to allocate hugetlb memory only more than its > > hugetlb_cgroup limit allows, the kernel will allow the mmap/shmget call, > > but will SIGBUS the task when it attempts to fault the memory in. > > > > We have developers interested in using hugetlb_cgroups, and they have expressed > > dissatisfaction regarding this behavior. We'd like to improve this > > behavior such that tasks violating the hugetlb_cgroup limits get an error on > > mmap/shmget time, rather than getting SIGBUS'd when they try to fault > > the excess memory in. > > > > The underlying problem is that today's hugetlb_cgroup accounting happens > > at hugetlb memory *fault* time, rather than at *reservation* time. > > Thus, enforcing the hugetlb_cgroup limit only happens at fault time, and > > the offending task gets SIGBUS'd. > > > > Proposed Solution: > > A new page counter named hugetlb.xMB.reservation_[limit|usage]_in_bytes. This > > counter has slightly different semantics than > > hugetlb.xMB.[limit|usage]_in_bytes: > > > > - While usage_in_bytes tracks all *faulted* hugetlb memory, > > reservation_usage_in_bytes tracks all *reserved* hugetlb memory and > > hugetlb memory faulted in without a prior reservation. > > > > - If a task attempts to reserve more memory than limit_in_bytes allows, > > the kernel will allow it to do so. But if a task attempts to reserve > > more memory than reservation_limit_in_bytes, the kernel will fail this > > reservation. > > > > This proposal is implemented in this patch series, with tests to verify > > functionality and show the usage. We also added cgroup-v2 support to > > hugetlb_cgroup so that the new use cases can be extended to v2. > > > > Alternatives considered: > > 1. A new cgroup, instead of only a new page_counter attached to > > the existing hugetlb_cgroup. Adding a new cgroup seemed like a lot of code > > duplication with hugetlb_cgroup. Keeping hugetlb related page counters under > > hugetlb_cgroup seemed cleaner as well. > > > > 2. Instead of adding a new counter, we considered adding a sysctl that modifies > > the behavior of hugetlb.xMB.[limit|usage]_in_bytes, to do accounting at > > reservation time rather than fault time. Adding a new page_counter seems > > better as userspace could, if it wants, choose to enforce different cgroups > > differently: one via limit_in_bytes, and another via > > reservation_limit_in_bytes. This could be very useful if you're > > transitioning how hugetlb memory is partitioned on your system one > > cgroup at a time, for example. Also, someone may find usage for both > > limit_in_bytes and reservation_limit_in_bytes concurrently, and this > > approach gives them the option to do so. > > > > Testing: > > I think that simply mentioning the use of hugetlbfs for regression testing > would be sufficient here. > Will do. > > - Added tests passing. > > - libhugetlbfs tests mostly passing, but some tests have trouble with and > > without this patch series. Seems environment issue rather than code: > > - Overall results: > > ********** TEST SUMMARY > > * 2M > > * 32-bit 64-bit > > * Total testcases: 84 0 > > * Skipped: 0 0 > > * PASS: 66 0 > > * FAIL: 14 0 > > * Killed by signal: 0 0 > > * Bad configuration: 4 0 > > * Expected FAIL: 0 0 > > * Unexpected PASS: 0 0 > > * Test not present: 0 0 > > * Strange test result: 0 0 > > ********** > > It is curious that you only ran the tests for 32 bit applications. Certainly > the more common case today is 64 bit. I don't think there are any surprises > for you as I also have been running hugetlbfs on this series. I did run them, with similar results. I'll add them. > -- > Mike Kravetz > > > - Failing tests: > > - elflink_rw_and_share_test("linkhuge_rw") segfaults with and without this > > patch series. > > - LD_PRELOAD=libhugetlbfs.so HUGETLB_MORECORE=yes malloc (2M: 32): > > FAIL Address is not hugepage > > - LD_PRELOAD=libhugetlbfs.so HUGETLB_RESTRICT_EXE=unknown:malloc > > HUGETLB_MORECORE=yes malloc (2M: 32): > > FAIL Address is not hugepage > > - LD_PRELOAD=libhugetlbfs.so HUGETLB_MORECORE=yes malloc_manysmall (2M: 32): > > FAIL Address is not hugepage > > - GLIBC_TUNABLES=glibc.malloc.tcache_count=0 LD_PRELOAD=libhugetlbfs.so > > HUGETLB_MORECORE=yes heapshrink (2M: 32): > > FAIL Heap not on hugepages > > - GLIBC_TUNABLES=glibc.malloc.tcache_count=0 LD_PRELOAD=libhugetlbfs.so > > libheapshrink.so HUGETLB_MORECORE=yes heapshrink (2M: 32): > > FAIL Heap not on hugepages > > - HUGETLB_ELFMAP=RW linkhuge_rw (2M: 32): FAIL small_data is not hugepage > > - HUGETLB_ELFMAP=RW HUGETLB_MINIMAL_COPY=no linkhuge_rw (2M: 32): > > FAIL small_data is not hugepage > > - alloc-instantiate-race shared (2M: 32): > > Bad configuration: sched_setaffinity(cpu1): Invalid argument - > > FAIL Child 1 killed by signal Killed > > - shmoverride_linked (2M: 32): > > FAIL shmget failed size 2097152 from line 176: Invalid argument > > - HUGETLB_SHM=yes shmoverride_linked (2M: 32): > > FAIL shmget failed size 2097152 from line 176: Invalid argument > > - shmoverride_linked_static (2M: 32): > > FAIL shmget failed size 2097152 from line 176: Invalid argument > > - HUGETLB_SHM=yes shmoverride_linked_static (2M: 32): > > FAIL shmget failed size 2097152 from line 176: Invalid argument > > - LD_PRELOAD=libhugetlbfs.so shmoverride_unlinked (2M: 32): > > FAIL shmget failed size 2097152 from line 176: Invalid argument > > - LD_PRELOAD=libhugetlbfs.so HUGETLB_SHM=yes shmoverride_unlinked (2M: 32): > > FAIL shmget failed size 2097152 from line 176: Invalid argument > > > > : https://www.kernel.org/doc/html/latest/vm/hugetlbfs_reserv.html > > > > Signed-off-by: Mina Almasry <firstname.lastname@example.org> > > Acked-by: Hillf Danton <email@example.com>
prev parent reply other threads:[~2019-11-08 23:36 UTC|newest] Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-10-30 1:36 Mina Almasry 2019-10-30 1:36 ` [PATCH v8 2/9] hugetlb_cgroup: add interface for charge/uncharge hugetlb reservations Mina Almasry 2019-11-08 0:57 ` Mike Kravetz 2019-11-08 23:48 ` Mina Almasry 2019-11-09 0:01 ` Mike Kravetz 2019-11-09 0:40 ` Mina Almasry 2019-11-09 0:46 ` Mike Kravetz 2019-11-25 20:26 ` Mina Almasry 2019-11-26 0:05 ` Mike Kravetz 2019-10-30 1:36 ` [PATCH v8 3/9] hugetlb_cgroup: add cgroup-v2 support Mina Almasry 2019-10-30 1:36 ` [PATCH v8 4/9] hugetlb_cgroup: add reservation accounting for private mappings Mina Almasry 2019-10-30 1:36 ` [PATCH v8 5/9] hugetlb: disable region_add file_region coalescing Mina Almasry 2019-11-01 23:23 ` Mike Kravetz 2019-11-04 21:04 ` Mina Almasry 2019-11-04 21:15 ` Mike Kravetz 2019-11-04 21:19 ` Mina Almasry 2019-11-17 11:03 ` Wenkuan Wang 2019-11-18 19:41 ` Mina Almasry 2019-10-30 1:36 ` [PATCH v8 6/9] hugetlb_cgroup: add accounting for shared mappings Mina Almasry 2019-10-30 1:36 ` [PATCH v8 7/9] hugetlb_cgroup: support noreserve mappings Mina Almasry 2019-10-30 1:37 ` [PATCH v8 8/9] hugetlb_cgroup: Add hugetlb_cgroup reservation tests Mina Almasry 2019-10-30 1:37 ` [PATCH v8 9/9] hugetlb_cgroup: Add hugetlb_cgroup reservation docs Mina Almasry 2019-11-07 23:42 ` [PATCH v8 1/9] hugetlb_cgroup: Add hugetlb_cgroup reservation counter Mike Kravetz 2019-11-08 23:35 ` Mina Almasry [this message]
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAHS8izPp9aW=uxA2z6BLmRXgaiDVyKYvAbnrH0rcirN4wTD55g@mail.gmail.com' \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --subject='Re: [PATCH v8 1/9] hugetlb_cgroup: Add hugetlb_cgroup reservation counter' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).