<div dir="ltr">Hi,<br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Nov 26, 2015 at 5:22 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"><span>On 11/26/2015 06:02 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">
v1 ---> v2<br>
1. remove the useless include file<br>
2. add huge pages checking in setup()<br>
3. mmap huge pages in reverse order<br>
</blockquote>
<br></span>
A paragraph about new version changes must be after:<span><br>
<br>
Signed-off-by: Li Wang <<a href="mailto:liwang@redhat.com" target="_blank">liwang@redhat.com</a>><br>
---<br>
<br></span>
as it shouldn't be in a commit message.</blockquote><div><br></div><div>ok, thanks for reminding.<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"><div><div><br>
<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
The fix are all these below commits:<br>
commit f522c3ac00a49128115f99a5fcb95a447601c1c3<br>
Author:Joonsoo Kim < iamjoonsoo.kim @ <a href="http://lge.com" rel="noreferrer" target="_blank">lge.com</a> ><br>
Date:Wed Sep 11 14:21:53 2013 - 0700<br>
</blockquote>
<br></div></div>
This is a more appropriate way of listing commits:<br>
<br>
f522c3ac00a4 ("mm, hugetlb: change variable name reservations to resv")<span><br></span></blockquote><div><br>yeah, that's pretty good.<br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ * Copyright (c) Author: Herton R. Krzesinski <<a href="mailto:herton@redhat.com" target="_blank">herton@redhat.com</a>><br>
+ * Modify: Li Wang <<a href="mailto:liwang@redhat.com" target="_blank">liwang@redhat.com</a>><br>
+ *<br>
+ * This program is free software; you can redistribute it and/or modify<br>
+ * it under the terms of the GNU General Public License as published by<br>
+ * the Free Software Foundation; either version 2 of the License, or<br>
+ * (at your option) any later version.<br>
+ *<br>
+ * This program is distributed in the hope that it will be useful,<br>
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of<br>
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See<br>
+ * the GNU General Public License for more details.<br>
+ *<br>
+ * You should have received a copy of the GNU General Public License<br>
+ * along with this program; if not, write to the Free Software<br>
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA<br>
+ * 02110-1301 USA<br>
+ */<br>
</blockquote></div>
Please change it to<span><br>
<br>
You should have received a copy of the GNU General Public License<br></span>
along with this program. If not, see <<a href="http://www.gnu.org/licenses/" rel="noreferrer" target="_blank">http://www.gnu.org/licenses/</a>>.<div><div><br></div></div></blockquote><div><br></div><div>ok, let me read it again. ;-)<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"><div><div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ tst_brkm(TBROK | TERRNO, cleanup, "Cannot allocate hugepage");<br>
+ }<br>
+<br>
+ for (i = ARSZ - 1; i > 0; i--) {<br>
</blockquote>
<br></div></div>
Why this is done in reverse order?<span><br></span></blockquote><div><br></div><div>well, as Jan pointed in the previous comments:<br><br>"How about allocating the largest area first and then mmaping smaller ones<br>
on top of it?<br>
<br>
That could prevent situation where first smallest area mmaps a gap between<br>
existing libraries/heap/etc. and then larger ones with same start overlap<br>
with those - since we write to those areas, bad things could happen."<br><br></div><div>I think that is very clear to describe the potential issue. since I run the program on some s390x for many times, it is easily to get segment fault. after done in reverse order. the error gone. <br><br></div><div>here I print the detailed mapings of the program, it show something that.<br>-----<br>Program received signal SIGSEGV, Segmentation fault.<br>0x0000004cf80a68b0 in mmap64 () from /lib64/libc.so.6<br>(gdb) bt<br>#0 0x0000004cf80a68b0 in mmap64 () from /lib64/libc.so.6<br>Cannot access memory at address 0x70<br><br><br> Start Addr End Addr Size Offset objfile<br> 0x80000000 0x80001000 0x1000 0 /root/a.out<br> 0x80001000 0x80002000 0x1000 0x1000 /root/a.out<br> 0xb7a9b000 0xb7abc000 0x21000 0 [heap]<br> 0x4cf7f73000 0x4cf7f94000 0x21000 0 /lib64/<a href="http://ld-2.12.so">ld-2.12.so</a><br> 0x4cf7f94000 0x4cf7f95000 0x1000 0x20000 /lib64/<a href="http://ld-2.12.so">ld-2.12.so</a><br> 0x4cf7f95000 0x4cf7f96000 0x1000 0x21000 /lib64/<a href="http://ld-2.12.so">ld-2.12.so</a><br> 0x4cf7f96000 0x4cf7f97000 0x1000 0 <br> 0x4cf7f9d000 0x4cf814d000 0x1b0000 0 /lib64/<a href="http://libc-2.12.so">libc-2.12.so</a><br> 0x4cf814d000 0x4cf8151000 0x4000 0x1af000 /lib64/<a href="http://libc-2.12.so">libc-2.12.so</a><br> 0x4cf8151000 0x4cf8152000 0x1000 0x1b3000 /lib64/<a href="http://libc-2.12.so">libc-2.12.so</a><br> 0x4cf8152000 0x4cf8157000 0x5000 0 <br> 0x4cf8159000 0x4cf8176000 0x1d000 0 /lib64/<a href="http://libpthread-2.12.so">libpthread-2.12.so</a><br> 0x4cf8176000 0x4cf8177000 0x1000 0x1c000 /lib64/<a href="http://libpthread-2.12.so">libpthread-2.12.so</a><br> 0x4cf8177000 0x4cf8178000 0x1000 0x1d000 /lib64/<a href="http://libpthread-2.12.so">libpthread-2.12.so</a><br><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>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ mmap_sz[i].sz = sz;<br>
+ mmap_sz[i].addr = addr;<br>
+<br>
+ TEST(pthread_create(tid + i, NULL, thr, &mmap_sz[i]));<br>
</blockquote>
<br></span>
This is not right, use tid[i] ( = tid + sizeof(pthread_t) * i), not tid + i.<span><br></span></blockquote><div><br>sorry, probably you want say this:<br><br> (pthread_t *)((char *)tid + sizeof(pthread_t) * i)<br><br>but I think the original way is right.<br></div><div>-----------<br># cat a.c <br>#include <stdio.h><br>int main()<br>{<br> int a[2];<br><br> *a = 'a';<br> *(a + 1) = 'b';<br><br> printf("a[1] = %d\n", a[1]);<br><br> printf("&a[0] = %p\n", a);<br> printf("&a[1] = %p\n", a + 1);<br>}<br><br>--------<br># ./a.out <br>a[1] = 98<br>&a[0] = 0x3ffffe25f18<br>&a[1] = 0x3ffffe25f1c<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"><span>
<br>
</span><br><span><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ }<br>
+<br>
+ for (++i; i < ARSZ; i++) {<br>
</blockquote>
<br></span>
Could you initialize "i" here explicitly?</blockquote><div> </div><div>ok, no problem.<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"><div><div><br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ cleanup();<br>
+ tst_exit();<br>
</blockquote>
<br></div></div>
Isn't tst_exit() calling cleanup?<br></blockquote><div><br></div><div>hmm, I checked the /lib/tst_res.c and got the function:<br><br>----<br>void tst_exit(void)<br>{<br> pthread_mutex_lock(&tmutex);<br><br>#if DEBUG<br> printf("IN tst_exit\n");<br> fflush(stdout);<br> fflush(stdout);<br>#endif<br><br> /* Call tst_flush() flush any output in the buffer. */<br> tst_flush();<br><br> /* Mask out TINFO result from the exit status. */<br> exit(T_exitval & ~TINFO);<br>}<br><br></div><div>I didn't saw there anyplace call the cleanup() function. :(<br></div><div> <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>
Best regards,<br>
Alexey<br>
</blockquote></div><br></div><div class="gmail_extra">thanks!<br clear="all"></div><div class="gmail_extra"><br>-- <br><div><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>