Re: [LTP] [PATCH 06/13] Hugetlb: Migrating libhugetlbfs mremap-fixed-normal-near-huge

[Tarun Sahu <tsahu@linux.ibm.com>]

Hi,

Li Wang <liwang@redhat.com> writes:

> On Fri, Dec 30, 2022 at 3:06 AM Tarun Sahu <tsahu@linux.ibm.com> wrote:
>
>> Hi,
>>
>>
>> Tarun Sahu <tsahu@linux.ibm.com> writes:
>>
>> > Hi Li,
>> > Thanks for reviewing the patch.
>> > I will update it in next revision.
>> >
>> > --skip
>> >>> +static int do_readback(void *p, size_t size, const char *stage)
>> >>> +{
>> >>> +       unsigned int *q = p;
>> >>> +       size_t i;
>> >>> +
>> >>> +       tst_res(TINFO, "%s(%p, 0x%lx, \"%s\")", __func__, p,
>> >>> +              (unsigned long)size, stage);
>> >>> +
>> >>> +       for (i = 0; i < (size / sizeof(*q)); i++)
>> >>> +               q[i] = RANDOM_CONSTANT ^ i;
>> >>> +
>> >>> +       for (i = 0; i < (size / sizeof(*q)); i++) {
>> >>> +               if (q[i] != (RANDOM_CONSTANT ^ i)) {
>> >>> +                       tst_res(TFAIL, "Stage \"%s\": Mismatch at
>> offset
>> >>> 0x%lx: 0x%x "
>> >>> +                                       "instead of 0x%lx", stage, i,
>> >>> q[i], RANDOM_CONSTANT ^ i);
>> >>> +                       return -1;
>> >>> +               }
>> >>> +       }
>> >>> +       return 0;
>> >>> +}
>> >>> +
>> >>> +static int do_remap(void *target)
>> >>> +{
>> >>> +       void *a, *b;
>> >>> +       int ret;
>> >>> +
>> >>> +       a = SAFE_MMAP(NULL, page_size, PROT_READ|PROT_WRITE,
>> >>> +                 MAP_SHARED|MAP_ANONYMOUS, -1, 0);
>> >>> +
>> >>> +       ret = do_readback(a, page_size, "base normal");
>> >>> +       if (ret)
>> >>> +               goto cleanup;
>> >>> +       b = mremap(a, page_size, page_size, MREMAP_MAYMOVE |
>> MREMAP_FIXED,
>> >>> +                  target);
>> >>> +
>> >>> +       if (b != MAP_FAILED) {
>> >>> +               do_readback(b, page_size, "remapped");
>> >>> +               a = b;
>> >>> +       } else
>> >>> +               tst_res(TINFO|TERRNO, "mremap(MAYMOVE|FIXED)
>> disallowed");
>> >>> +
>> >>> +cleanup:
>> >>> +       SAFE_MUNMAP(a, page_size);
>> >>> +       return ret;
>> >>> +}
>> >>>
>> >>
>> >> Those two functions do_readback() and do_remap() are
>> >> copy&past from hugemmap2[4|5].c, what about extracting
>> >> them into a common header file(tst_hugetlb.h or mem.h) for
>> >> easy reusing?
>> >>
>> > ok, I think, hugetlb.h/.c will be better place to keep them.
>> >
>> These two functions are very specific to tests (specially the do_remap).
>> Also, they use values that are defined only inside the test.
>> Though do_readback is more general which can be put in hugetlb.h/c.
>>
>> WDYT?
>>
>
> Ah yes, I didn't realize the difference.
> Or can we pass the specific arguments based on the variable part?
> e.g.
>
>     static int do_remap(int fd, size_t page_size; int prot, void *target);
>
This can be done this way, but What concerned me is mremap function
itself. it is defined in sys/mmap under _GNU_SOURCE. I am not sure
whether it will be ok to use _GNU_SOURCE specific function in
lib/hugetlb.c/h file.

please confirm if it is ok to define _GNU_SOURCE in hugetlb.c/h.
>
> -- 
> Regards,
> Li Wang
>
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp