[LTP] [PATCH] Fix mmap01 testcase on hppa architecture

[LTP] [PATCH] Fix mmap01 testcase on hppa architecture

[Helge Deller]

The mmap() syscall to map a file at any given page-aligned address (via the
MAP_FIXED flag) can fail on the hppa architecture due to cache colouring
requirements of the architecture.

This patch drops the MAP_FIXED flag and thus maps the file instead at
some convenient address based on the hint as given by the addr
parameter. The tests covered by mmap01 check the *end* of the mmaped
file for consistency, so mapping the beginning of the file at any
address is okay without breaking the main intention of the mmap01
testcase.

Signed-off-by: Helge Deller <deller@gmx.de>

diff --git a/testcases/kernel/syscalls/mmap/mmap01.c b/testcases/kernel/syscalls/mmap/mmap01.c
index 41183d109..a508edb10 100644
--- a/testcases/kernel/syscalls/mmap/mmap01.c
+++ b/testcases/kernel/syscalls/mmap/mmap01.c
@@ -81,7 +81,7 @@ int main(int ac, char **av)
 		 */
 		errno = 0;
 		addr = mmap(addr, page_sz, PROT_READ | PROT_WRITE,
-			    MAP_FILE | MAP_SHARED | MAP_FIXED, fildes, 0);
+			    MAP_FILE | MAP_SHARED, fildes, 0);
 
 		/* Check for the return value of mmap() */
 		if (addr == MAP_FAILED) {

[LTP] [PATCH] Fix mmap01 testcase on hppa architecture

[Jan Stancek]

----- Original Message -----
> The mmap() syscall to map a file at any given page-aligned address (via the
> MAP_FIXED flag) can fail on the hppa architecture due to cache colouring
> requirements of the architecture.
> 
> This patch drops the MAP_FIXED flag and thus maps the file instead at
> some convenient address based on the hint as given by the addr
> parameter.

It looks like we don't need addr at all. Test is remapping part
of heap with MAP_FIXED, which looks risky. I'd drop that page
initialisation inside heap too. Now, without MAP_FIXED, it's
just completely unrelated page. What do you think?

Regards,
Jan

[LTP] [PATCH] Fix mmap01 testcase on hppa architecture

[Helge Deller]

On 13.06.2017 13:18, Jan Stancek wrote:
>> The mmap() syscall to map a file at any given page-aligned address (via the
>> MAP_FIXED flag) can fail on the hppa architecture due to cache colouring
>> requirements of the architecture.
>>
>> This patch drops the MAP_FIXED flag and thus maps the file instead at
>> some convenient address based on the hint as given by the addr
>> parameter.
> 
> It looks like we don't need addr at all. Test is remapping part
> of heap with MAP_FIXED, which looks risky. I'd drop that page
> initialisation inside heap too. Now, without MAP_FIXED, it's
> just completely unrelated page. What do you think?

Yes, that's probably the best choice.
The original code tries to initialize the area where the file is going
to be mapped with non-zero values to be able to check if after the mapping
the memory area beyond the EOF was zero-initialized by the kernel.
This part of the check was already changed by my initial patch, with
your suggestion we would now drop the initialization completely.
I think your suggestion is fair.

New patch is attached.

Helge

-------------- next part --------------
A non-text attachment was scrubbed...
Name: mmap01.patch
Type: text/x-diff
Size: 2087 bytes
Desc: not available
URL: <http://lists.linux.it/pipermail/ltp/attachments/20170616/201330d6/attachment.patch>

[LTP] [PATCH] Fix mmap01 testcase on hppa architecture

[Jan Stancek]

----- Original Message -----
> On 13.06.2017 13:18, Jan Stancek wrote:
> >> The mmap() syscall to map a file at any given page-aligned address (via
> >> the
> >> MAP_FIXED flag) can fail on the hppa architecture due to cache colouring
> >> requirements of the architecture.
> >>
> >> This patch drops the MAP_FIXED flag and thus maps the file instead at
> >> some convenient address based on the hint as given by the addr
> >> parameter.
> > 
> > It looks like we don't need addr at all. Test is remapping part
> > of heap with MAP_FIXED, which looks risky. I'd drop that page
> > initialisation inside heap too. Now, without MAP_FIXED, it's
> > just completely unrelated page. What do you think?
> 
> Yes, that's probably the best choice.
> The original code tries to initialize the area where the file is going
> to be mapped with non-zero values to be able to check if after the mapping
> the memory area beyond the EOF was zero-initialized by the kernel.
> This part of the check was already changed by my initial patch, with
> your suggestion we would now drop the initialization completely.
> I think your suggestion is fair.
> 
> New patch is attached.

Pushed.

Thanks,
Jan