From: Geetika M <geetika@linux.ibm.com>
To: Petr Vorel <pvorel@suse.cz>
Cc: ltp@lists.linux.it
Subject: Re: [LTP] [PATCH] [PATCH] Migrating the libhugetlbfs/testcases/straddle_4GB.c
Date: Wed, 27 Mar 2024 15:34:32 +0530 [thread overview]
Message-ID: <8cdc9eba-d567-4f57-8731-bcf1497c3d1a@linux.ibm.com> (raw)
In-Reply-To: <20231128115041.GB369141@pevik>
Hi Petr,
On 28/11/23 5:20 pm, Petr Vorel wrote:
> Hi Geetika,
>
> Please have look at my comments at your previous patch [1] [2], these changes
> apply a lot for your patches as well.
>
> [1]https://lore.kernel.org/ltp/20231128111024.GA364870@pevik/
> [2]https://lore.kernel.org/ltp/20231128112254.GA367506@pevik/
>
Noted.
I will apply whatever changes are applicable to this testcase in the
next version.
...
>> +
>> +/*\
>> + * [Description]
>> + * This test tries to allocate hugepages to cover a memory range
>> + * that straddles the 4GB boundary.
>> + * Scenario 1 : mmap without MAP_FIXED
>> + * Scenario 2 : mmap with MAP_FIXED
> This will be badly formatted (it will not be list, but inline).
>
> How about:
>
> /*\
> * [Description]
> *
> * Test tries to allocate hugepages to cover a memory range that straddles the
> * 4GB boundary, using mmap(2) with and without MAP_FIXED.
> */
>> + */
>> +
>> +#define MAPS_BUF_SZ 4096
>> +#define FOURGB (1UL << 32)
>> +#define MNTPOINT "hugetlbfs/"
>> +#define HUGETLBFS_MAGIC 0x958458f6
> Could you please add this magic definition to include/tst_fs.h
> (as a separate patch), we store all magic there.
>
>> +#define _LARGEFILE64_SOURCE /* Need this for statfs64 */
> We would probably define it in Makefile
>
> hugemmap40: CFLAGS += -D_LARGEFILE64_SOURCE
Noted.
I will apply above changes with the next version.
...
> +
> +static void run_test(void)
> +{
> + void *p;
> +
> + /* We first try to get the mapping without MAP_FIXED */
> + tst_res(TINFO, "Mapping without MAP_FIXED at %lx...", straddle_addr);
> + p = mmap((void *)straddle_addr, 2*hpage_size, PROT_READ|PROT_WRITE,
> + MAP_SHARED, fd, 0);
> + if (p == (void *)straddle_addr) {
> + /* These tests irrelevant if we didn't get the straddle address*/
> + if (test_addr_huge(p) != 1) {
> + tst_brk(TFAIL, "1st Mapped address is not hugepage");
> + goto windup;
> + }
> + if (test_addr_huge(p + hpage_size) != 1) {
> + tst_brk(TFAIL, "2nd Mapped address is not hugepage");
> + goto windup;
> + }
> + memset(p, 0, hpage_size);
> + memset(p + hpage_size, 0, hpage_size);
> + tst_res(TPASS, "Mapping without MAP_FIXED at %lx... completed", straddle_addr);
> + } else {
> + tst_res(TINFO, "Got %p instead, never mind, let's move to mapping with MAP_FIXED\n", p);
> + munmap(p, 2*hpage_size);
> + }
> + tst_res(TINFO, "Mapping with MAP_FIXED at %lx...", straddle_addr);
> Maybe use .tcnt = 2, in struct tst_test and separate these 2 cases into it's
> wonf functions?
>
> You would either use:
> static void run_test(unsigned int n)
>
> With that you would reduce code duplicity and make test function smaller.
>
> Also, sometimes we use test struct (pointer to the function and description, see
> e.g. testcases/kernel/mem/hugetlb/hugeshmctl/hugeshmctl02.c).
>
> ...
Can you suggest how we can handle the test exit upon failure inside one
of test cases then?
Currently I am handling it by using label windup placed at the end of
function.
In one of the other tests there was feedback to not use exit(0); then
how can we exit
easily upon failure from one of the testcase?
Thanks & Regards,
Geetika
--
Mailing list info: https://lists.linux.it/listinfo/ltp
prev parent reply other threads:[~2024-03-27 10:05 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-03 13:37 [LTP] [PATCH] [PATCH] Migrating the libhugetlbfs/testcases/straddle_4GB.c Geetika
2023-11-28 11:50 ` Petr Vorel
2024-03-27 10:04 ` Geetika M [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8cdc9eba-d567-4f57-8731-bcf1497c3d1a@linux.ibm.com \
--to=geetika@linux.ibm.com \
--cc=ltp@lists.linux.it \
--cc=pvorel@suse.cz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).