<div dir="ltr">Hi,<br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Dec 21, 2015 at 3:20 PM, Alexey Kodanev <span dir="ltr"><<a href="mailto:alexey.kodanev@oracle.com" target="_blank">alexey.kodanev@oracle.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi,<div><div class="h5"><br>
On 12/16/2015 05:59 AM, Li Wang wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
shmget()/shmat() fails to allocate huge pages shared memory segment<br>
with EINVAL if its size is not in the range [ N*HUGE_PAGE_SIZE - 4095,<br>
N*HUGE_PAGE_SIZE ]. This is a problem in the memory segment size round<br>
up algorithm. The requested size is rounded up to PAGE_SIZE (4096), but<br>
if this roundup does not match HUGE_PAGE_SIZE (2Mb) boundary - the<br>
allocation fails.<br>
<br>
This bug is present in all RHEL6 versions, but not in RHEL7. It looks<br>
like this was fixed in mainstream kernel > v3.3 by the following patches:<br>
<br>
091d0d5 shm: fix null pointer deref when userspace specifies invalid hugepage size<br>
af73e4d hugetlbfs: fix mmap failure in unaligned size request<br>
42d7395 mm: support more pagesizes for MAP_HUGETLB/SHM_HUGETLB<br>
40716e2 hugetlbfs: fix alignment of huge page requests<br>
<br>
Signed-off-by: Li Wang <<a href="mailto:liwang@redhat.com" target="_blank">liwang@redhat.com</a>><br>
---<br>
<br>
Notes:<br>
     v1 --> v2<br>
     a.remove the key value from shmget()<br>
     b.improve the brk messages<br>
     c.just use one loop in the main function<br>
<br>
  runtest/hugetlb                                    |   1 +<br>
  testcases/kernel/mem/.gitignore                    |   1 +<br>
  .../kernel/mem/hugetlb/hugeshmat/hugeshmat05.c     | 136 +++++++++++++++++++++<br>
  3 files changed, 138 insertions(+)<br>
  create mode 100644 testcases/kernel/mem/hugetlb/hugeshmat/hugeshmat05.c<br>
<br>
diff --git a/runtest/hugetlb b/runtest/hugetlb<br>
index 2e9f215..75e6426 100644<br>
--- a/runtest/hugetlb<br>
+++ b/runtest/hugetlb<br>
@@ -10,6 +10,7 @@ hugeshmat01 hugeshmat01 -i 5<br>
  hugeshmat02 hugeshmat02 -i 5<br>
  hugeshmat03 hugeshmat03 -i 5<br>
  hugeshmat04 hugeshmat04 -i 5<br>
+hugeshmat05 hugeshmat05 -i 5<br>
</blockquote>
<br></div></div>
Did you forget to increase iterations here or it is not needed anymore?<br>
In the first patch, if I am not mistaken, was 15 (because of two loops).<span class=""><br></span></blockquote><div><br></div><div>it is no needed, 5 loops is enough i think. :)<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="">
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
    hugeshmctl01 hugeshmctl01 -i 5<br>
  hugeshmctl02 hugeshmctl02 -i 5<br>
</blockquote></span>
...<span class=""><br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+<br>
+int main(int ac, char **av)<br>
+{<br>
+       int lc;<br>
+<br>
+       tst_parse_opts(ac, av, NULL, NULL);<br>
+<br>
+       setup();<br>
+<br>
+       for (lc = 0; TEST_LOOPING(lc); lc++) {<br>
+               tst_count = 0;<br>
+<br>
+               /* N*hpage_size - page_size FAIL */<br>
+               shm_test(N * hpage_size - page_size);<br>
+<br>
+               /* N*hpage_size - page_size - 1 SUCCESS*/<br>
+               shm_test(N * hpage_size - page_size - 1);<br>
+<br>
+               /* N*hpage_size  SUCCESS */<br>
+               shm_test(N * hpage_size);<br>
+<br>
+               /* N*hpage_size + 1 FAIL */<br>
</blockquote>
<br></span>
Could you please add more informative comments (may be explaining why it is "FAIL"/"SUCCESS")<br>
or remove them completely, they just repeat what is passed to the function.<br>
<br>
We could also set sizes in array, defining it before the "for":<br></blockquote><div><br></div><div>ok, I tend to accept the second suggestion. because it looks like more neatly than before.<br> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
const int tst_sizes[] = { N * hpage_size - page_size,<br>
                                            N * hpage_size, ... };<br>
<br>
for (lc ...) {<br>
    tst_count = 0;<br>
    for(i = 0; i < ARRAY_SIZE(tst_sizes); ++i)<br>
        shm_test(tst_sizes[i]);<br>
}<br>
<br>
<br>
Best regards,<br>
Alexey<div class=""><div class="h5"><br>
</div></div></blockquote></div><br></div><div class="gmail_extra">thanks a lot! If there is no other comments, I'd like to sent patch v3 this evening.<br></div><div class="gmail_extra"><br clear="all"><br>-- <br><div class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div>Regards,<br></div>Li Wang<br></div><div>Email: <a href="mailto:liwang@redhat.com" target="_blank">liwang@redhat.com</a><br></div></div></div></div></div></div>
</div></div>