Re: [LTP] [PATCH 1/3] mmap001: Convert to new API

Ricardo B. Marliere via ltp
2025-01-29T17:19:41Z

Hi Andrea,

thanks for reviewing!

On Wed Jan 15, 2025 at 10:14 AM -03, Andrea Cervesato wrote:
> Hi!
>
> On 1/14/25 23:26, Ricardo B. Marliere via ltp wrote:
>> From: Ricardo B. Marliere <rbm@suse.com>
>>
>> Signed-off-by: Ricardo B. Marliere <rbm@suse.com>
>> ---
>> testcases/kernel/syscalls/mmap/mmap001.c | 206 ++++++++-----------------------
>> 1 file changed, 49 insertions(+), 157 deletions(-)
>>
>> diff --git a/testcases/kernel/syscalls/mmap/mmap001.c b/testcases/kernel/syscalls/mmap/mmap001.c
>> index dabb7d1e4998b1097e179abe23555926f5841117..bc9b4155e8b53f942ef694fdf3187c0e544a97cd 100644
>> --- a/testcases/kernel/syscalls/mmap/mmap001.c
>> +++ b/testcases/kernel/syscalls/mmap/mmap001.c
>> @@ -1,183 +1,75 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> /*
>> * Copyright (C) 2000 Juan Quintela <quintela@fi.udc.es>
>> * Aaron Laffin <alaffin@sgi.com>
>> + * Copyright (c) 2025 Linux Test Project
>> + */
>> +
>> +/*\
>> + * [Description]
>> *
>> - * This program is free software; you can redistribute it and/or
>> - * modify it under the terms of the GNU General Public License
>> - * as published by the Free Software Foundation; either version 2
>> - * of the License, or (at your option) any later version.
>> - *
>> - * This program is distributed in the hope that it will be useful,
>> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> - * GNU General Public License for more details.
>> - *
>> - * You should have received a copy of the GNU General Public License
>> - * along with this program; if not, write to the Free Software
>> - * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
>> - *
>> - * mmap001.c - Tests mmapping a big file and writing it once
>> + * Tests mmapping a big file and writing it once
> Description is a bit short and it needs a bit more information. For
> example, what we expect to see and what could happen during test (SEGV
> for example).

Ack.

>> - if (TEST_RETURN == -1) {
>> - tst_resm(TFAIL | TTERRNO,
>> - "munmapping %s failed", filename);
>> - } else {
>> - tst_resm(TPASS, "munmapping %s successful", filename);
>> - }
>> + /*
>> + * Seems that if the map area was bad, we'd get SEGV,
>> + * hence we can indicate a PASS.
>> + */
> If this is true, we need to find a way to test that specific scenario.
> This can e done by forking a process where test is running and to check
> if SIGSEGV killed it after calling SAFE_WAITPID()

Good idea, I kept the original comment and TPASS string but it could
surely be expanded. What do you think of the following? I'll be sending
as v2 when the series receive some more reviewing.

diff --git a/testcases/kernel/syscalls/mmap/mmap001.c b/testcases/kernel/syscalls/mmap/mmap001.c
index bc9b4155e8b5..10ce7a48e7c2 100644
--- a/testcases/kernel/syscalls/mmap/mmap001.c
+++ b/testcases/kernel/syscalls/mmap/mmap001.c
@@ -8,7 +8,8 @@
/*\
* [Description]
*
- * Tests mmapping a big file and writing it once
+ * This test will map a big file to memory and write to it once,
+ * making sure nothing bad happened in between such as a SIGSEGV.
*/

#include "tst_test.h"
@@ -25,6 +26,8 @@ static void setup(void)

static void run(void)
{
+ pid_t pid;
+ int status;
char *array;
unsigned int i;
unsigned int pages, memsize;
@@ -40,20 +43,23 @@ static void run(void)
SAFE_LSEEK(fd, memsize, SEEK_SET);
SAFE_WRITE(SAFE_WRITE_ALL, fd, "\0", 1);

- array = SAFE_MMAP(NULL, memsize, PROT_WRITE, MAP_SHARED, fd, 0);
-
- tst_res(TINFO, "touching mmaped memory");
- for (i = 0; i < memsize; i++)
- array[i] = (char)i;
-
- /*
- * Seems that if the map area was bad, we'd get SEGV,
- * hence we can indicate a PASS.
- */
- tst_res(TPASS, "we're still here, mmaped area must be good");
-
- SAFE_MSYNC(array, memsize, MS_SYNC);
- SAFE_MUNMAP(array, memsize);
+ pid = SAFE_FORK();
+ if (pid == 0) {
+ array = SAFE_MMAP(NULL, memsize, PROT_WRITE, MAP_SHARED, fd, 0);
+ tst_res(TINFO, "touching mmapped memory");
+ for (i = 0; i < memsize; i++)
+ array[i] = (char)i;
+ SAFE_MSYNC(array, memsize, MS_SYNC);
+ SAFE_MUNMAP(array, memsize);
+ exit(0);
+ } else {
+ SAFE_WAITPID(pid, &status, 0);
+ if (WIFSIGNALED(status) && WEXITSTATUS(status) == SIGSEGV)
+ tst_res(TFAIL, "test was killed by SIGSEGV");
+ else
+ tst_res(TPASS,
+ "memory was mapped and written to successfully");
+ }
}

static void cleanup(void)
@@ -66,6 +72,7 @@ static struct tst_test test = {
.setup = setup,
.test_all = run,
.cleanup = cleanup,
+ .forks_child = 1,
.options =
(struct tst_option[]){
{ "m:", &m_copt,

> Kind regards,
> Andrea

Thank you,
- Ricardo.