In addition to what Hannu mentioned... 1. It seems silly that the test is split in to s390 / 32-bit / 64-bit when all of this stuff is limit driven AFAICS. Would be nice if this was all folded in one body of test code with a minimal amount of setup code. 2. More comments inline.
On Dec 22, 2010, at 11:02 PM, Caspar Zhang <[email protected]> wrote: > ping... any comments? > > On 12/21/2010 06:12 PM, Caspar Zhang wrote: >> We get segfaults during testing mtest01 on a 5TB memory machine, the >> problem was traced to the array pdi_list[] went to overflow and >> corrupted memory. This fix makes pid_list[] dynamically sized with >> correct memory size to avoid overflow. >> >> Signed-off-by: Caspar Zhang<[email protected]> >> --- >> testcases/kernel/mem/mtest01/mtest01.c | 50 >> +++++++++++++++++++------------ >> 1 files changed, 31 insertions(+), 19 deletions(-) >> >> diff --git a/testcases/kernel/mem/mtest01/mtest01.c >> b/testcases/kernel/mem/mtest01/mtest01.c >> index e4e9a56..1362e08 100644 >> --- a/testcases/kernel/mem/mtest01/mtest01.c >> +++ b/testcases/kernel/mem/mtest01/mtest01.c >> @@ -44,6 +44,10 @@ >> >> #include "test.h" >> >> +#define MAX_31BIT_SIZE (unsigned long)(500*1024*1024) >> +#define MAX_32BIT_SIZE (unsigned long)(1024*1024*1024) >> +#define MAX_64BIT_SIZE (unsigned long long)(3*1024*1024*1024) >> + >> char *TCID = "mtest01"; >> int TST_TOTAL = 1; >> >> @@ -62,11 +66,12 @@ int main(int argc, char* argv[]) { >> unsigned long bytecount, alloc_bytes; >> unsigned long long original_maxbytes,maxbytes=0; >> unsigned long long pre_mem, post_mem; >> + unsigned long long total_ram, total_free, D, C; >> extern char* optarg; >> int chunksize = 1024*1024; /* one meg at a time by default */ >> struct sysinfo sstats; >> int i,pid_cntr; >> - pid_t pid,pid_list[1000]; >> + pid_t pid,*pid_list; >> struct sigaction act; >> >> act.sa_handler = handler; >> @@ -74,8 +79,18 @@ int main(int argc, char* argv[]) { >> sigemptyset(&act.sa_mask); >> sigaction(SIGRTMIN,&act, 0); >> >> - for (i=0;i<1000;i++) >> - pid_list[i]=(pid_t)0; >> + sysinfo(&sstats); >> + total_ram=sstats.totalram; >> + total_ram=total_ram+sstats.totalswap; >> +#if defined (_s390_) >> + pidlist_sz = total_ram / MAX_31BIT_SIZE + 1; >> +#elif __WORDSIZE==32 >> + pidlist_sz = total_ram / MAX_32BIT_SIZE + 1; >> +#elif __WORDSIZE==64 >> + pidlist_sz = total_ram / MAX_64BIT_SIZE + 1; >> +#endif >> + pid_list = (pid_t *)malloc(sizeof(pid_t) * pidlist_sz); You aren't checking the return from malloc and the cast is unnecessary. >> + memset(pid_list, 0, sizeof(pid_t) * pidlist_sz); >> >> while ((c=getopt(argc, argv, "c:b:p:wvh")) != EOF) { >> switch((char)c) { >> @@ -115,12 +130,8 @@ int main(int argc, char* argv[]) { >> >> sysinfo(&sstats); >> if (maxpercent) { >> - unsigned long long total_ram, total_free, D, C; >> percent=(float)maxpercent/100.00; >> >> - total_ram=sstats.totalram; >> - total_ram=total_ram+sstats.totalswap; >> - >> total_free=sstats.freeram; >> total_free=total_free+sstats.freeswap; >> >> @@ -157,47 +168,47 @@ int main(int argc, char* argv[]) { >> pid_list[i]=pid; >> >> #if defined (_s390_) /* s390's 31bit addressing requires smaller chunks */ >> - while ( (pid!=0)&& (maxbytes> 500*1024*1024) ) >> + while ((pid!=0)&& (maxbytes> MAX_31BIT_SIZE)) If you're going to trim the whitespace, please remove the unnecessary parentheses and add whitespace around the logic operators please. >> { >> i++; >> - maxbytes=maxbytes-(500*1024*1024); >> + maxbytes -= MAX_31BIT_SIZE; >> pid=fork(); >> if (pid != 0) >> pid_cntr++; >> pid_list[i]=pid; >> } >> - if ( maxbytes> 500*1024*1024 ) >> - alloc_bytes=500*1024*1024; >> + if (maxbytes> MAX_31BIT_SIZE) >> + alloc_bytes = MAX_31BIT_SIZE; >> else >> alloc_bytes=(unsigned long)maxbytes; >> >> #elif __WORDSIZE==32 >> - while ( (pid!=0)&& (maxbytes> 1024*1024*1024) ) >> + while ((pid!=0)&& (maxbytes> MAX_32BIT_SIZE)) >> { >> i++; >> - maxbytes=maxbytes-(1024*1024*1024); >> + maxbytes -= MAX_32BIT_SIZE; >> pid=fork(); >> if (pid != 0) >> pid_cntr++; >> pid_list[i]=pid; >> } >> - if ( maxbytes> 1024*1024*1024 ) >> - alloc_bytes=1024*1024*1024; >> + if (maxbytes> MAX_32BIT_SIZE) >> + alloc_bytes = MAX_32BIT_SIZE; >> else >> alloc_bytes=(unsigned long)maxbytes; >> >> #elif __WORDSIZE==64 >> - while ( (pid!=0)&& (maxbytes> (unsigned long long)3*1024*1024*1024) ) >> + while ((pid!=0)&& (maxbytes> MAX_64BIT_SIZE)) >> { >> i++; >> - maxbytes=maxbytes-(unsigned long long)3*1024*1024*1024; >> + maxbytes - MAX_64BIT_SIZE; >> pid=fork(); >> if (pid != 0) >> pid_cntr++; >> pid_list[i]=pid; >> } >> - if ( maxbytes> (unsigned long long)3*1024*1024*1024 ) >> - alloc_bytes=(unsigned long long)3*1024*1024*1024; >> + if (maxbytes> MAX_64BIT_SIZE) >> + alloc_bytes = MAX_64BIT_SIZE; >> else >> alloc_bytes=(unsigned long)maxbytes; >> #endif >> @@ -258,5 +269,6 @@ int main(int argc, char* argv[]) { >> else >> tst_resm(TPASS, "%llu kbytes allocated only.", >> original_maxbytes/1024); >> } >> + free(pid_list); >> exit(0); >> } ------------------------------------------------------------------------------ Learn how Oracle Real Application Clusters (RAC) One Node allows customers to consolidate database storage, standardize their database environment, and, should the need arise, upgrade to a full multi-node Oracle RAC database without downtime or disruption http://p.sf.net/sfu/oracle-sfdevnl _______________________________________________ Ltp-list mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/ltp-list
