[LTP] [PATCH v3] munlockall: add test case that verifies memory has been unlocked

[LTP] [PATCH v3] munlockall: add test case that verifies memory has been unlocked

[Dennis Brendel]

Changes compared to v2:
- replacing munlockall01.c as suggested
- respecting line length of 100 chars
- use tst_brk(TBROK | TERRNO, ...) to print symbolic errno value
- remove braces in the final error/success conditional
- use the suggested success message

---
 .../kernel/syscalls/munlockall/munlockall01.c | 148 ++++--------------
 1 file changed, 31 insertions(+), 117 deletions(-)

diff --git a/testcases/kernel/syscalls/munlockall/munlockall01.c b/testcases/kernel/syscalls/munlockall/munlockall01.c
index 51f731b65..2745a1aee 100644
--- a/testcases/kernel/syscalls/munlockall/munlockall01.c
+++ b/testcases/kernel/syscalls/munlockall/munlockall01.c
@@ -1,134 +1,48 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
- * Copyright (c) Wipro Technologies Ltd, 2002.  All Rights Reserved.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of version 2 of the GNU General Public License as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it would be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, write the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
- *
+ * Copyright Red Hat
+ * Author: Dennis Brendel <dbrendel@redhat.com>
  */
-/**************************************************************************
- *
- *    TEST IDENTIFIER	: munlockall01
- *
- *    EXECUTED BY	: root / superuser
- *
- *    TEST TITLE	: Basic test for munlockall(2)
- *
- *    TEST CASE TOTAL	: 1
- *
- *    AUTHOR		: sowmya adiga<sowmya.adiga@wipro.com>
- *
- *    SIGNALS
- * 	Uses SIGUSR1 to pause before test if option set.
- * 	(See the parse_opts(3) man page).
- *
- *    DESCRIPTION
- *	This is a phase I test for the munlockall(2) system call.
- *	It is intended to provide a limited exposure of the system call.
- *
- * 	Setup:
- *	  Setup signal handling.
- *	  Pause for SIGUSR1 if option specified.
- *
- * 	Test:
- *        Execute system call
- *	  Check return code, if system call failed (return=-1)
- *	  Log the errno and Issue a FAIL message.
- *	  Otherwise, Issue a PASS message.
- *
- * 	Cleanup:
- * 	  Print errno log and/or timing stats if options given
- *
- * USAGE:  <for command-line>
- *  munlockall01 [-c n] [-e] [-i n] [-I x] [-p x] [-t]
- *		where,		-c n : Run n copies concurrently
- *	               		-e   : Turn on errno logging.
- *				-h   : Show this help screen
- *				-i n : Execute test n times.
- *				-I x : Execute test for x seconds.
- *				-p   : Pause for SIGUSR1 before starting
- *                      	-P x : Pause for x seconds between iterations.
- *                       	 t   : Turn on syscall timing.
- *
- * RESTRICTIONS
- * Must be root/superuser to run it.
- *****************************************************************************/
-#include <errno.h>
-#include <sys/mman.h>
-#include "test.h"
 
-void setup();
-void cleanup();
+/*\
+ * [Description]
+ *
+ * Verify that munlockall(2) unlocks all previously locked memory
+ */
 
-char *TCID = "munlockall01";
-int TST_TOTAL = 1;
+#include <sys/mman.h>
 
-#if !defined(UCLINUX)
+#include "tst_test.h"
 
-int main(int ac, char **av)
+static void verify_munlockall(void)
 {
-	int lc;
-
-	tst_parse_opts(ac, av, NULL, NULL);
+	unsigned long size = 0;
 
-	setup();
+	SAFE_FILE_LINES_SCANF("/proc/self/status", "VmLck: %ld", &size);
 
-	/* check looping state */
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
+	if (size != 0UL)
+		tst_brk(TBROK, "Locked memory after init should be 0 but is %ld", size);
 
-		tst_count = 0;
+	if (mlockall(MCL_CURRENT | MCL_FUTURE) != 0)
+		tst_brk(TBROK | TERRNO, "Could not lock memory using mlockall()");
 
-		TEST(munlockall());
+	SAFE_FILE_LINES_SCANF("/proc/self/status", "VmLck: %ld", &size);
 
-		/* check return code */
-		if (TEST_RETURN == -1) {
-			tst_resm(TFAIL | TTERRNO, "munlockall() Failed with"
-				 " return=%ld", TEST_RETURN);
-		} else {
-			tst_resm(TPASS, "munlockall() passed with"
-				 " return=%ld ", TEST_RETURN);
+	if (size == 0UL)
+		tst_brk(TBROK, "Locked memory after mlockall() should be greater than 0, "
+			       "but is %ld", size);
 
-		}
-	}
+	if (munlockall() != 0)
+		tst_brk(TBROK | TERRNO, "Could not unlock memory using munlockall()");
 
-	/* cleanup and exit */
-	cleanup();
-	tst_exit();
+	SAFE_FILE_LINES_SCANF("/proc/self/status", "VmLck: %ld", &size);
 
+	if (size != 0UL)
+		tst_res(TFAIL, "Locked memory after munlockall() should be 0 but is %ld", size);
+	else
+		tst_res(TPASS, "Memory successfully locked and unlocked");
 }
 
-#else
-
-int main(void)
-{
-	tst_resm(TINFO, "test is not available on uClinux");
-	tst_exit();
-}
-
-#endif /* if !defined(UCLINUX) */
-
-/* setup() - performs all ONE TIME setup for this test. */
-void setup(void)
-{
-	tst_require_root();
-
-	tst_sig(NOFORK, DEF_HANDLER, cleanup);
-
-	TEST_PAUSE;
-}
-
-/*
- * cleanup() - performs all ONE TIME cleanup for this test at
- *		completion or premature exit.
- */
-void cleanup(void)
-{
-}
+static struct tst_test test = {
+	.test_all = verify_munlockall,
+};
-- 
2.44.0


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

Re: [LTP] [PATCH v3] munlockall: add test case that verifies memory has been unlocked

[Petr Vorel]

Hi Dennis,

Reviewed-by: Petr Vorel <pvorel@suse.cz>

LGTM, thanks!

I would just explicitly note the test was rewritten from scratch
(can be added before merge).

Kind regards,
Petr

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

Re: [LTP] [PATCH v3] munlockall: add test case that verifies memory has been unlocked

[Li Wang]

Hi Dennis,

Good to see your patch soon, nice work!

Next time remember to commit the patch with Signed-off-by.
  `git commit -sm  xxxx`

Reviewed-by: Li Wang <liwang@redhat.com>


-- 
Regards,
Li Wang


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

Re: [LTP] [PATCH v3] munlockall: add test case that verifies memory has been unlocked

[Petr Vorel]

Hi Dennis, Li,

> Hi Dennis,

> Good to see your patch soon, nice work!

> Next time remember to commit the patch with Signed-off-by.
>   `git commit -sm  xxxx`

+1 (planning to add that before merge).

Also these comments "Changes compared to v2:" should be below "---",
that way we can read them but they will not endup in the git commit message when
we apply the patch.

Kind regards,
Petr

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

Re: [LTP] [PATCH v3] munlockall: add test case that verifies memory has been unlocked

[Petr Vorel]

Hi Dennis,

> Hi Petr and Li,

> Thank you very much for your valuable feedback!

> What's left to do for me? Re-submit as v4 with sign-off and
> proper commit message?

v3 is ready to be merged, I'll add your SBT and cleanup the commit message,
while I'll be adding my and Li's RBT.

I'm just waiting a little bit because Cyril looked into this patchset,
he might have look (I merge tonight).

Kind regards,
Petr

> Thanks,
> Dennis

> On 3/6/24 11:41, Petr Vorel wrote:
> > Hi Dennis, Li,

> >> Hi Dennis,

> >> Good to see your patch soon, nice work!

> >> Next time remember to commit the patch with Signed-off-by.
> >>   `git commit -sm  xxxx`

> > +1 (planning to add that before merge).

> > Also these comments "Changes compared to v2:" should be below "---",
> > that way we can read them but they will not endup in the git commit message when
> > we apply the patch.

> > Kind regards,
> > Petr







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

Re: [LTP] [PATCH v3] munlockall: add test case that verifies memory has been unlocked

[Cyril Hrubis]

Hi!
> +/*\
> + * [Description]
> + *
> + * Verify that munlockall(2) unlocks all previously locked memory
> + */
>  
> -char *TCID = "munlockall01";
> -int TST_TOTAL = 1;
> +#include <sys/mman.h>
>  
> -#if !defined(UCLINUX)
> +#include "tst_test.h"
>  
> -int main(int ac, char **av)
> +static void verify_munlockall(void)
>  {
> -	int lc;
> -
> -	tst_parse_opts(ac, av, NULL, NULL);
> +	unsigned long size = 0;
>  
> -	setup();
> +	SAFE_FILE_LINES_SCANF("/proc/self/status", "VmLck: %ld", &size);
>  
> -	/* check looping state */
> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
> +	if (size != 0UL)
> +		tst_brk(TBROK, "Locked memory after init should be 0 but is %ld", size);
>  
> -		tst_count = 0;
> +	if (mlockall(MCL_CURRENT | MCL_FUTURE) != 0)
> +		tst_brk(TBROK | TERRNO, "Could not lock memory using mlockall()");
>  
> -		TEST(munlockall());
> +	SAFE_FILE_LINES_SCANF("/proc/self/status", "VmLck: %ld", &size);
>  
> -		/* check return code */
> -		if (TEST_RETURN == -1) {
> -			tst_resm(TFAIL | TTERRNO, "munlockall() Failed with"
> -				 " return=%ld", TEST_RETURN);
> -		} else {
> -			tst_resm(TPASS, "munlockall() passed with"
> -				 " return=%ld ", TEST_RETURN);
> +	if (size == 0UL)
> +		tst_brk(TBROK, "Locked memory after mlockall() should be greater than 0, "
> +			       "but is %ld", size);

This line can be shorter:

	tst_brk(TBROK, "After mlockall() locked memory should be >0");

We already checked that size is 0 so no need to print it.

> -		}
> -	}
> +	if (munlockall() != 0)
> +		tst_brk(TBROK | TERRNO, "Could not unlock memory using munlockall()");

We are testing the munlockall() syscall here so it would be better to
use the TST_EXP_PASS() macro.

> -	/* cleanup and exit */
> -	cleanup();
> -	tst_exit();
> +	SAFE_FILE_LINES_SCANF("/proc/self/status", "VmLck: %ld", &size);
>  
> +	if (size != 0UL)
> +		tst_res(TFAIL, "Locked memory after munlockall() should be 0 but is %ld", size);
> +
> +	else
> +		tst_res(TPASS, "Memory successfully locked and unlocked");
>  }
>  
> -#else
> -
> -int main(void)
> -{
> -	tst_resm(TINFO, "test is not available on uClinux");
> -	tst_exit();
> -}
> -
> -#endif /* if !defined(UCLINUX) */
> -
> -/* setup() - performs all ONE TIME setup for this test. */
> -void setup(void)
> -{
> -	tst_require_root();
> -
> -	tst_sig(NOFORK, DEF_HANDLER, cleanup);
> -
> -	TEST_PAUSE;
> -}
> -
> -/*
> - * cleanup() - performs all ONE TIME cleanup for this test at
> - *		completion or premature exit.
> - */
> -void cleanup(void)
> -{
> -}
> +static struct tst_test test = {
> +	.test_all = verify_munlockall,
> +};
> -- 
> 2.44.0
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v3] munlockall: add test case that verifies memory has been unlocked

[Li Wang]

Hi Petr and Dennis,

As this is the first patch Dennis(thanks!!) writes down for LTP.

I would suggest sending a patch V4 (including SBT, Cyril's new reviews)
to make you familiar with the workflow of the community:).


On Wed, Mar 6, 2024 at 8:44 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi Dennis,
>
> > Hi Petr and Li,
>
> > Thank you very much for your valuable feedback!
>
> > What's left to do for me? Re-submit as v4 with sign-off and
> > proper commit message?
>
> v3 is ready to be merged, I'll add your SBT and cleanup the commit message,
> while I'll be adding my and Li's RBT.
>
> I'm just waiting a little bit because Cyril looked into this patchset,
> he might have look (I merge tonight).
>
> Kind regards,
> Petr
>
> > Thanks,
> > Dennis
>
> > On 3/6/24 11:41, Petr Vorel wrote:
> > > Hi Dennis, Li,
>
> > >> Hi Dennis,
>
> > >> Good to see your patch soon, nice work!
>
> > >> Next time remember to commit the patch with Signed-off-by.
> > >>   `git commit -sm  xxxx`
>
> > > +1 (planning to add that before merge).
>
> > > Also these comments "Changes compared to v2:" should be below "---",
> > > that way we can read them but they will not endup in the git commit message when
> > > we apply the patch.
>
> > > Kind regards,
> > > Petr
>
>
>
>
>
>


-- 
Regards,
Li Wang


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

Re: [LTP] [PATCH v3] munlockall: add test case that verifies memory has been unlocked

[Dennis Brendel]

Hi Cyril,

On 3/6/24 16:26, Cyril Hrubis wrote:
> Hi!
>> +/*\
>> + * [Description]
>> + *
>> + * Verify that munlockall(2) unlocks all previously locked memory
>> + */
>>  
>> -char *TCID = "munlockall01";
>> -int TST_TOTAL = 1;
>> +#include <sys/mman.h>
>>  
>> -#if !defined(UCLINUX)
>> +#include "tst_test.h"
>>  
>> -int main(int ac, char **av)
>> +static void verify_munlockall(void)
>>  {
>> -	int lc;
>> -
>> -	tst_parse_opts(ac, av, NULL, NULL);
>> +	unsigned long size = 0;
>>  
>> -	setup();
>> +	SAFE_FILE_LINES_SCANF("/proc/self/status", "VmLck: %ld", &size);
>>  
>> -	/* check looping state */
>> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
>> +	if (size != 0UL)
>> +		tst_brk(TBROK, "Locked memory after init should be 0 but is %ld", size);
>>  
>> -		tst_count = 0;
>> +	if (mlockall(MCL_CURRENT | MCL_FUTURE) != 0)
>> +		tst_brk(TBROK | TERRNO, "Could not lock memory using mlockall()");
>>  
>> -		TEST(munlockall());
>> +	SAFE_FILE_LINES_SCANF("/proc/self/status", "VmLck: %ld", &size);
>>  
>> -		/* check return code */
>> -		if (TEST_RETURN == -1) {
>> -			tst_resm(TFAIL | TTERRNO, "munlockall() Failed with"
>> -				 " return=%ld", TEST_RETURN);
>> -		} else {
>> -			tst_resm(TPASS, "munlockall() passed with"
>> -				 " return=%ld ", TEST_RETURN);
>> +	if (size == 0UL)
>> +		tst_brk(TBROK, "Locked memory after mlockall() should be greater than 0, "
>> +			       "but is %ld", size);
> 
> This line can be shorter:
> 
> 	tst_brk(TBROK, "After mlockall() locked memory should be >0");
> 
> We already checked that size is 0 so no need to print it.

Agreed :-)

>> -		}
>> -	}
>> +	if (munlockall() != 0)
>> +		tst_brk(TBROK | TERRNO, "Could not unlock memory using munlockall()");
> 
> We are testing the munlockall() syscall here so it would be better to
> use the TST_EXP_PASS() macro.

The actual purpose of the test was not checking the return value of the syscall
(wrapper), but the behavior as reported by the kernel through /proc - but fair
enough, it does not do any harm :-)

>> -	/* cleanup and exit */
>> -	cleanup();
>> -	tst_exit();
>> +	SAFE_FILE_LINES_SCANF("/proc/self/status", "VmLck: %ld", &size);
>>  
>> +	if (size != 0UL)
>> +		tst_res(TFAIL, "Locked memory after munlockall() should be 0 but is %ld", size);
>> +
>> +	else
>> +		tst_res(TPASS, "Memory successfully locked and unlocked");
>>  }
>>  
>> -#else
>> -
>> -int main(void)
>> -{
>> -	tst_resm(TINFO, "test is not available on uClinux");
>> -	tst_exit();
>> -}
>> -
>> -#endif /* if !defined(UCLINUX) */
>> -
>> -/* setup() - performs all ONE TIME setup for this test. */
>> -void setup(void)
>> -{
>> -	tst_require_root();
>> -
>> -	tst_sig(NOFORK, DEF_HANDLER, cleanup);
>> -
>> -	TEST_PAUSE;
>> -}
>> -
>> -/*
>> - * cleanup() - performs all ONE TIME cleanup for this test at
>> - *		completion or premature exit.
>> - */
>> -void cleanup(void)
>> -{
>> -}
>> +static struct tst_test test = {
>> +	.test_all = verify_munlockall,
>> +};
>> -- 
>> 2.44.0
>>
>>
>> -- 
>> Mailing list info: https://lists.linux.it/listinfo/ltp
> 


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