Hi,
I have followed most of the comments, and resent a new patch.
Any comment for the new one?
Thanks.

On 04/08/2011 03:20 PM, tangchen wrote:
> 
> 
> On 04/07/2011 02:47 PM, Garrett Cooper wrote:
>>> @@ -121,6 +125,7 @@ main(int ac, char **av)
>>>         tst_exit();
>>>     }
>>>
>>> +    mount_point = (char *)malloc(sizeof(char) * 50);
>>
>> Casting not required. Also, why are you assuming the mountpoint is 50
>> chars long?
> Have already changed :)
>>
>>> +
>>> +    int ret = mkdir(mount_point, 0755);
>>> +    mkdir_err = errno;
>>
>> You don't need to capture the errno unless it fails.
> Here, mkdir_err is used to find out if the directory /huge has already 
> existed. If so, we will not remove it in the end. 
>>
>>
>>> +    nr_hugepages_file = fopen("/proc/sys/vm/nr_hugepages", "w+");
>>> +    if (nr_hugepages_file == NULL) {
>>> +        tst_brkm(TBROK, NULL, "fopen() Failed on /proc/sys/vm/nr_hugepages,
>>> "
>>> +            "errno=%d : %s", errno, strerror(errno));
>>
>> See above about TERRNO. Also, please avoid using the stdio functions
>> unless you're trying to be 100% portable.
> By "stdio functions", do you mean fopen() or strerror()? These two functions 
> are used in many other testcases.
>>
> All other comments are accepted, thanks.
> The following is a new patch.
>  
> 
> 
> 
> hugemmap test could fail because I didn't mount hugetlbfs by myself. And I 
> need to umount it after test.
> In my opinion, firstly, mounting hugetlbfs on /tmp is not a good idea, 
> because lots of programs could use /tmp for other purpose. This could cause 
> other tests fail; secondly, mounting hugetlbfs could be done automatically.
> This patch creates a directory /huge, automatically mounts hugetlbfs on /huge 
> before test starts, and umounts it when the test is over.
> 
> Signed-off-by: tangchen <[email protected]>
> ---
>  runtest/hugetlb                                    |    2 +-
>  testcases/kernel/mem/hugetlb/hugemmap/hugemmap01.c |   32 
> +++++++++++++++++++-
>  2 files changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/runtest/hugetlb b/runtest/hugetlb
> index 1347f32..1490bda 100644
> --- a/runtest/hugetlb
> +++ b/runtest/hugetlb
> @@ -1,4 +1,4 @@
> -hugemmap01 hugemmap01 -H/tmp
> +hugemmap01 hugemmap01 -H/huge
>  hugemmap02 hugemmap02 -H/tmp -c10
>  hugemmap03 hugemmap03 -H/tmp -I2 -c10
>  hugemmap04 hugemmap04 -H/tmp
> diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap01.c 
> b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap01.c
> index a9b8a36..ad5c59e 100644
> --- a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap01.c
> +++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap01.c
> @@ -69,6 +69,7 @@
>  #include <sys/stat.h>
>  #include <sys/mman.h>
>  #include <sys/types.h>
> +#include <sys/mount.h>
>  
>  #include "test.h"
>  #include "usctest.h"
> @@ -85,6 +86,9 @@ char *Hopt;                     /* location of hugetlbfs */
>  int beforetest=0;            /* Amount of free huge pages before testing */
>  int aftertest=0;             /* Amount of free huge pages after testing */
>  int hugepagesmapped=0;               /* Amount of huge pages mapped after 
> testing */
> +FILE *nr_hugepages_file = NULL; /* Used for allocate enough hugepages for 
> test. */
> +char *mount_point = NULL;       /* The location where we mount hugetlbfs. */
> +int mkdir_err = 0;              /* Used to judge if we need to remove the 
> mount point. */
>  
>  void setup();                        /* Main setup function of test */
>  int getfreehugepages();              /* Reads free huge pages */
> @@ -194,6 +198,28 @@ void
>  setup()
>  {
>       char mypid[40];
> +     mount_point = (char *)malloc(sizeof(char) * strlen(Hopt));
> +     mount_point = strcpy(mount_point, Hopt);
> +
> +     tst_require_root(tst_exit);
> +
> +     int ret = mkdir(mount_point, 0755);
> +     mkdir_err = errno;
> +     if (ret < 0 && mkdir_err != EEXIST) {
> +             tst_brkm(TBROK|TERRNO, NULL, "mkdir failed on %s", mount_point);
> +     }
> +     if (mount("none", mount_point, "hugetlbfs", 0, NULL) < 0) {
> +             tst_brkm(TBROK|TERRNO, NULL, "mount failed on %s", mount_point);
> +     }
> +
> +     nr_hugepages_file = fopen("/proc/sys/vm/nr_hugepages", "w+");
> +     if (nr_hugepages_file == NULL) {
> +             tst_brkm(TBROK|TERRNO, NULL, "fopen failed on 
> /proc/sys/vm/nr_hugepages");
> +     }
> +     if (fprintf(nr_hugepages_file, "%d", 1024) < 0) {
> +             tst_brkm(TBROK|TERRNO, NULL, "fprintf failed on 
> /proc/sys/vm/nr_hugepages");
> +     }
> +     fclose(nr_hugepages_file);
>  
>       sprintf(mypid,"/%d",getpid());
>       TEMPFILE=strcat(mypid,TEMPFILE);
> @@ -275,5 +301,9 @@ cleanup()
>       TEST_CLEANUP;
>  
>       unlink(TEMPFILE);
> +     umount(mount_point);
>  
> -}
> \ No newline at end of file
> +     if (mkdir_err != EEXIST) {
> +             remove(mount_point);
> +     }
> +}

-- 
Best Regards,
Tang chen
--------------------------------------------------
Tang Chen
Development Dept.I
Nanjing Fujitsu Nanda Software Tech. Co., Ltd.(FNST)
No.6 Wenzhu Road, Nanjing, 210012, China 
TEL: +86+25-86630566-8513
FUJITSU INTERNAL: 7998-8513
FAX: +86+25-83317685
EMail: [email protected]
--------------------------------------------------
This communication is for use by the intended recipient(s) only and may contain 
information that is privileged, confidential and exempt from disclosure under 
applicable law. If you are not an intended recipient of this communication, you 
are hereby notified that any dissemination, distribution or copying hereof is 
strictly prohibited.  If you have received this communication in error, please 
notify me by reply e-mail, permanently delete this communication from your 
system, and destroy any hard copies you may have printed


------------------------------------------------------------------------------
Benefiting from Server Virtualization: Beyond Initial Workload 
Consolidation -- Increasing the use of server virtualization is a top
priority.Virtualization can reduce costs, simplify management, and improve 
application availability and disaster protection. Learn more about boosting 
the value of server virtualization. http://p.sf.net/sfu/vmware-sfdev2dev
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

Reply via email to