Ack -- gotcha. Too much stuff going on... I'll revert some of the changes and fix things up a bit. Thanks, -Garrett
On Mon, Jan 3, 2011 at 8:48 PM, CAI Qian <[email protected]> wrote: > >> TESTDIR isn't publicly visible anymore because people really shouldn't >> be mucking around with that value, or querying it if they haven't >> called tst_tmpdir (which is actually what you did in this test!). > I am sorry about that. That is why I had sent a follow-up patch to address > it, > http://marc.info/?l=ltp-list&m=129379291515012&w=2 > >> > Again, what issues are your facing with this code? >> >> + char *tmp_tstdir; >> >> + >> >> + tmp_tstdir = get_tst_tmpdir(); >> >> + if (tmp_tstdir == NULL) >> >> + tst_brkm(TBROK|TERRNO, cleanup, "get_tmp_tstdir failed"); >> >> + snprintf(s, BUFSIZ, "%s/hugemmap05/file", tmp_tstdir); >> >> + free(tmp_tstdir); >> >> + >> >> + fd = open(s, O_CREAT|O_RDWR, 0755); >> >> if (fd == -1) >> >> tst_brkm(TBROK|TERRNO, cleanup, "open"); >> >> addr = mmap(ADDR, (long)(length * MB), PROTECTION, FLAGS, fd, >> >> @@ -195,19 +203,14 @@ static void overcommit(void) >> >> return; >> >> fclose(fp); >> >> } >> >> - if (shmid != -1) { >> >> - tst_resm(TINFO, "shmid: 0x%x", shmid); >> >> - shmaddr = shmat(shmid, ADDR, SHMAT_FLAGS); >> >> - if (shmaddr == (void *)-1) >> >> - tst_brkm(TBROK|TERRNO, cleanup, "shmat"); >> >> - write_bytes(shmaddr); >> >> - read_bytes(shmaddr); >> >> - } else { >> >> - write_bytes(addr); >> >> - read_bytes(addr); >> >> - } >> > As you can read from the help(), the test can allocate hugepages >> > either >> > from shmget() or mmap(). In case of shmget(), a global variable >> > shmid with >> > the default value -1 will be change to 1 via command-line, >> >> Ok, I must have missed that part. Sorry... >> >> > static option_t options[] = { >> > { "s", &opt_sysfs, NULL}, >> > { "m", &shmid, NULL}, >> > { "a:", &opt_alloc, &opt_allocstr}, >> > { NULL, NULL, NULL} >> > }; >> > >> > Therefore, there is a need to call shmat(). That is why it only call >> > it >> > when shmid != -1. In case of mmap(), there is no need to call >> > shmat(). >> >> + tst_resm(TINFO, "shmid: 0x%x", shmid); >> >> + shmaddr = shmat(shmid, ADDR, SHMAT_FLAGS); >> >> + if (shmaddr == (void *)-1) >> >> + tst_brkm(TBROK|TERRNO, cleanup, "shmat"); >> >> + write_bytes(shmaddr); >> >> + read_bytes(shmaddr); >> >> if (opt_sysfs) { >> >> - tst_resm(TINFO, "check sysfs."); >> >> + tst_resm(TINFO, "checking sysfs."); >> >> if (checksys(_PATH_SYS_2M_HUGE, "HugePages_Total", >> >> length / 2) != 0) >> >> return; >> >> @@ -221,7 +224,7 @@ static void overcommit(void) >> >> != 0) >> >> return; >> >> } else { >> >> - tst_resm(TINFO, "check /proc/meminfo."); >> >> + tst_resm(TINFO, "checking /proc/meminfo."); >> > If you are going to change the syntax here, there are many other >> > places >> > also need to changed to be consistent like, >> > "check sysfs." >> > "check /proc/meminfo before allocation." >> > etc >> >> Agreed. It was just an example, and the point was to clearly state >> with proper verb tenses so that people could get a clear idea of what >> the test was doing instead of having to read the code to decypher the >> intent of the test writer. Granted, I've gotten lazy and stopped >> fixing some of these items in later commits because of all of the LOCs >> I've made, but it needs to be fixed later because the grammar and >> spelling used by test writers is less than desirable, which makes it >> hard for folks trying to understand what the intention is behind these >> tests. > OK. > > Thanks. > > CAI Qian > ------------------------------------------------------------------------------ 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
