Re: [LTP] [PATCH 1/1] mremap06: Add mremap() reproducer for 7e7757876f25

[Vlastimil Babka <vbabka@suse.cz>]

On 7/4/23 13:46, Petr Vorel wrote:
> Hi Cyril,
> 
>> > +#define PAGE_SIZE 4096
> 
>> This wouldn't work on 64k page size.
> 
>> > +#define MMAP_SIZE ((ARRAY_SIZE(tcases)+1)*PAGE_SIZE)
>> > +#define MREMAP_SIZE (ARRAY_SIZE(tcases)*PAGE_SIZE)
> 
>> These have to be variables initialized on the fly with getpagesize()
>> instead of PAGE_SIZE.
> 
> Yep, I thought about getpagesize().

Agreed.

Also IMHO it's awkward to have the number of pages and the iterations
expressed using ARRAY_SIZE(tcases). I'd say it's just an accident that the
number of pages involved in the merging and the number of testcases is both
3. I'd rather separate those.

> ...
>> > +static int check_pages(void)
>> > +{
>> > +	int fail = 0, i;
>> > +	char val;
>> > +
>> > +	for (i = 0; i < (int)ARRAY_SIZE(tcases); i++) {
>> > +		val = buf[i * PAGE_SIZE];
>> > +		if (val != 0x30 + i) {
>> > +			tst_res(TFAIL, "page %d wrong value %d (0x%x)", i, val - 0x30, val);
> 
>> Woudn't this generate too many FAILURE messages? Maybe we should just
>> break the for cycle here.
> 
> It could be. I wasn't sure if it's important to know which pages were wrong.

There's just 3 pages to print so it won't be that many messages, I'd just
print them all.

> Kind regards,
> Petr


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