[LTP] [PATCH v2 1/2] shmget03: don't depend on existed shared resources

[LTP] [PATCH v2 1/2] shmget03: don't depend on existed shared resources

[Alexey Kodanev]

It's unlikely, but still possible that some of them could be
created/released during the test as well, so the patch only
checks errno.

Signed-off-by: Alexey Kodanev <aleksei.kodanev@bell-sw.com>
---
v2: * Move the loop to the test run function and try to get
      ENOSPC errno there.
    * Rename queues* to segments*

 .../kernel/syscalls/ipc/shmget/shmget03.c     | 42 ++++++++++---------
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/testcases/kernel/syscalls/ipc/shmget/shmget03.c b/testcases/kernel/syscalls/ipc/shmget/shmget03.c
index efbc465e1..5dc5d55fd 100644
--- a/testcases/kernel/syscalls/ipc/shmget/shmget03.c
+++ b/testcases/kernel/syscalls/ipc/shmget/shmget03.c
@@ -21,47 +21,49 @@
 #include "tst_safe_sysv_ipc.h"
 #include "libnewipc.h"
 
-static int *queues;
-static int maxshms, queue_cnt;
+static int *segments;
+static int maxshms, segments_cnt;
 static key_t shmkey;
 
 static void verify_shmget(void)
 {
-	TST_EXP_FAIL2(shmget(shmkey + maxshms, SHM_SIZE, IPC_CREAT | IPC_EXCL | SHM_RW), ENOSPC,
-		"shmget(%i, %i, %i)", shmkey + maxshms, SHM_SIZE, IPC_CREAT | IPC_EXCL | SHM_RW);
+	int res = 0, num;
+
+	errno = 0;
+	for (num = 0; num <= maxshms; ++num) {
+		res = shmget(shmkey + num, SHM_SIZE, IPC_CREAT | IPC_EXCL | SHM_RW);
+		if (res == -1)
+			break;
+		segments[segments_cnt++] = res;
+	}
+
+	if (res != -1 || errno != ENOSPC)
+		tst_brk(TFAIL | TERRNO, "Failed to trigger ENOSPC error");
+
+	tst_res(TPASS, "Maximum number of segments reached (%d), used by test %d",
+		maxshms, segments_cnt);
 }
 
 static void setup(void)
 {
-	int res, num;
-
 	shmkey = GETIPCKEY();
 
 	SAFE_FILE_SCANF("/proc/sys/kernel/shmmni", "%i", &maxshms);
 
-	queues = SAFE_MALLOC(maxshms * sizeof(int));
-	for (num = 0; num < maxshms; num++) {
-		res = shmget(shmkey + num, SHM_SIZE, IPC_CREAT | IPC_EXCL | SHM_RW);
-		if (res == -1)
-			tst_brk(TBROK | TERRNO, "shmget failed unexpectedly");
-
-		queues[queue_cnt++] = res;
-	}
-	tst_res(TINFO, "The maximum number of memory segments (%d) has been reached",
-		maxshms);
+	segments = SAFE_MALLOC((maxshms + 1) * sizeof(int));
 }
 
 static void cleanup(void)
 {
 	int num;
 
-	if (!queues)
+	if (!segments)
 		return;
 
-	for (num = 0; num < queue_cnt; num++)
-		SAFE_SHMCTL(queues[num], IPC_RMID, NULL);
+	for (num = 0; num < segments_cnt; num++)
+		SAFE_SHMCTL(segments[num], IPC_RMID, NULL);
 
-	free(queues);
+	free(segments);
 }
 
 static struct tst_test test = {
-- 
2.25.1

[LTP] [PATCH v2 2/2] msgget03: don't depend on existed shared resources

[Alexey Kodanev]

It's unlikely, but still possible that some of them could be
created/released during the test as well, so the patch only
checks errno.

Signed-off-by: Alexey Kodanev <aleksei.kodanev@bell-sw.com>
---
v2: * Move the loop to the test run function and try to get
      ENOSPC errno there.

 .../kernel/syscalls/ipc/msgget/msgget03.c     | 31 ++++++++++---------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/testcases/kernel/syscalls/ipc/msgget/msgget03.c b/testcases/kernel/syscalls/ipc/msgget/msgget03.c
index 76cf82cd3..1ade8f942 100644
--- a/testcases/kernel/syscalls/ipc/msgget/msgget03.c
+++ b/testcases/kernel/syscalls/ipc/msgget/msgget03.c
@@ -26,29 +26,30 @@ static key_t msgkey;
 
 static void verify_msgget(void)
 {
-	TST_EXP_FAIL2(msgget(msgkey + maxmsgs, IPC_CREAT | IPC_EXCL), ENOSPC,
-		"msgget(%i, %i)", msgkey + maxmsgs, IPC_CREAT | IPC_EXCL);
+	int res = 0, num;
+
+	errno = 0;
+	for (num = 0; num <= maxmsgs; ++num) {
+		res = msgget(msgkey + num, IPC_CREAT | IPC_EXCL);
+		if (res == -1)
+			break;
+		queues[queue_cnt++] = res;
+	}
+
+	if (res != -1 || errno != ENOSPC)
+		tst_brk(TFAIL | TERRNO, "Failed to trigger ENOSPC error");
+
+	tst_res(TPASS, "Maximum number of queues reached (%d), used by test %d",
+		maxmsgs, queue_cnt);
 }
 
 static void setup(void)
 {
-	int res, num;
-
 	msgkey = GETIPCKEY();
 
 	SAFE_FILE_SCANF("/proc/sys/kernel/msgmni", "%i", &maxmsgs);
 
-	queues = SAFE_MALLOC(maxmsgs * sizeof(int));
-
-	for (num = 0; num < maxmsgs; num++) {
-		res = msgget(msgkey + num, IPC_CREAT | IPC_EXCL);
-		if (res == -1)
-			tst_brk(TBROK | TERRNO, "msgget failed unexpectedly");
-		queues[queue_cnt++] = res;
-	}
-
-	tst_res(TINFO, "The maximum number of message queues (%d) reached",
-		maxmsgs);
+	queues = SAFE_MALLOC((maxmsgs + 1) * sizeof(int));
 }
 
 static void cleanup(void)
-- 
2.25.1

[LTP] [PATCH v2 2/2] msgget03: don't depend on existed shared resources

[Petr Vorel]

Hi Alexey, Li,

> It's unlikely, but still possible that some of them could be
> created/released during the test as well, so the patch only
> checks errno.

> Signed-off-by: Alexey Kodanev <aleksei.kodanev@bell-sw.com>
> ---
> v2: * Move the loop to the test run function and try to get
>       ENOSPC errno there.

>  .../kernel/syscalls/ipc/msgget/msgget03.c     | 31 ++++++++++---------
>  1 file changed, 16 insertions(+), 15 deletions(-)

> diff --git a/testcases/kernel/syscalls/ipc/msgget/msgget03.c b/testcases/kernel/syscalls/ipc/msgget/msgget03.c
> index 76cf82cd3..1ade8f942 100644
> --- a/testcases/kernel/syscalls/ipc/msgget/msgget03.c
> +++ b/testcases/kernel/syscalls/ipc/msgget/msgget03.c
> @@ -26,29 +26,30 @@ static key_t msgkey;

>  static void verify_msgget(void)
>  {
> -	TST_EXP_FAIL2(msgget(msgkey + maxmsgs, IPC_CREAT | IPC_EXCL), ENOSPC,
> -		"msgget(%i, %i)", msgkey + maxmsgs, IPC_CREAT | IPC_EXCL);
> +	int res = 0, num;
> +
> +	errno = 0;
> +	for (num = 0; num <= maxmsgs; ++num) {
In different patch [1] (I forget you already send patches to fix this) I counted
items in /proc/sysvipc/shm. Not sure what is safer: <= looks a bit drastic
(how about bug which reports ENOSPC much earlier than it should be?), but
obviously new mapping from other program created in the middle of testing.

Kind regards,
Petr

[1] https://patchwork.ozlabs.org/project/ltp/patch/20210722073523.5099-1-pvorel@suse.cz/
> +		res = msgget(msgkey + num, IPC_CREAT | IPC_EXCL);
> +		if (res == -1)
> +			break;
> +		queues[queue_cnt++] = res;
> +	}
> +
> +	if (res != -1 || errno != ENOSPC)
> +		tst_brk(TFAIL | TERRNO, "Failed to trigger ENOSPC error");
> +
> +	tst_res(TPASS, "Maximum number of queues reached (%d), used by test %d",
> +		maxmsgs, queue_cnt);
>  }
...

[LTP] [PATCH v2 2/2] msgget03: don't depend on existed shared resources

[Cyril Hrubis]

Hi!
> In different patch [1] (I forget you already send patches to fix this) I counted
> items in /proc/sysvipc/shm. Not sure what is safer: <= looks a bit drastic
> (how about bug which reports ENOSPC much earlier than it should be?), but
> obviously new mapping from other program created in the middle of testing.

I think that we allready discussed this in another thread:

https://lists.linux.it/pipermail/ltp/2021-July/023831.html

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 2/2] msgget03: don't depend on existed shared resources

[Petr Vorel]

> Hi!
> > In different patch [1] (I forget you already send patches to fix this) I counted
> > items in /proc/sysvipc/shm. Not sure what is safer: <= looks a bit drastic
> > (how about bug which reports ENOSPC much earlier than it should be?), but
> > obviously new mapping from other program created in the middle of testing.

> I think that we allready discussed this in another thread:

> https://lists.linux.it/pipermail/ltp/2021-July/023831.html

Thanks, I forgot this. In that case my approach (not using <=, but count
segments in /proc/sysvipc/shm before testing) might be more precise.
But no strong feeling about that, both solutions fix the test, let's chose one
and merge.

Kind regards,
Petr

[LTP] [PATCH v2 2/2] msgget03: don't depend on existed shared resources

[Cyril Hrubis]

Hi!
> > I think that we allready discussed this in another thread:
> 
> > https://lists.linux.it/pipermail/ltp/2021-July/023831.html
> 
> Thanks, I forgot this. In that case my approach (not using <=, but count
> segments in /proc/sysvipc/shm before testing) might be more precise.
> But no strong feeling about that, both solutions fix the test, let's chose one
> and merge.

As I said previously, there are many SysV IPC tests that do expect that
nobody will add/remove IPC shm/queue/semaphores during the testrun and
some of the testcases cannot even be implemented without this
expectation.

Hence I wouldn't complicate the test here and just count how many
segments are there at the start and be done with it.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 2/2] msgget03: don't depend on existed shared resources

[xuyang2018.jy]

Hi Cyril, Petr
> Hi!
>>> I think that we allready discussed this in another thread:
>>
>>> https://lists.linux.it/pipermail/ltp/2021-July/023831.html
>>
>> Thanks, I forgot this. In that case my approach (not using<=, but count
>> segments in /proc/sysvipc/shm before testing) might be more precise.
>> But no strong feeling about that, both solutions fix the test, let's chose one
>> and merge.
>
> As I said previously, there are many SysV IPC tests that do expect that
> nobody will add/remove IPC shm/queue/semaphores during the testrun and
> some of the testcases cannot even be implemented without this
> expectation.
>
> Hence I wouldn't complicate the test here and just count how many
> segments are there at the start and be done with it.
Agree.

A possible solution(alter get_used_queues api in new ipc lib and add 
file parametrers, so we can use this api for msgget03) I have mentioned 
in the previous email, the url as below:
https://lists.linux.it/pipermail/ltp/2021-July/023653.html
>

[LTP] [PATCH v2 2/2] msgget03: don't depend on existed shared resources

[Petr Vorel]

Hi all,

> Hi Cyril, Petr
> > Hi!
> >>> I think that we allready discussed this in another thread:

> >>> https://lists.linux.it/pipermail/ltp/2021-July/023831.html

> >> Thanks, I forgot this. In that case my approach (not using<=, but count
> >> segments in /proc/sysvipc/shm before testing) might be more precise.
> >> But no strong feeling about that, both solutions fix the test, let's chose one
> >> and merge.

> > As I said previously, there are many SysV IPC tests that do expect that
> > nobody will add/remove IPC shm/queue/semaphores during the testrun and
> > some of the testcases cannot even be implemented without this
> > expectation.

> > Hence I wouldn't complicate the test here and just count how many
> > segments are there at the start and be done with it.
> Agree.

> A possible solution(alter get_used_queues api in new ipc lib and add 
> file parametrers, so we can use this api for msgget03) I have mentioned 
> in the previous email, the url as below:
> https://lists.linux.it/pipermail/ltp/2021-July/023653.html
LGTM. Or use /proc/sysvipc/shm instead of /proc/sysvipc/msg in get_used_queues()
as you noted get_used_queues() has not been used yet.

BTW searching where get_used_queues() appeared, I see [LTP] [PATCH v3 1/4]
syscalls/ipc: add newipc library for new API [1], but if I'm not mistaken
get_used_queues() was not used even there, maybe it was in some previous
versions.

Kind regards,
Petr

[1] https://lists.linux.it/pipermail/ltp/2016-December/003239.html

[LTP] [PATCH v2 2/2] msgget03: don't depend on existed shared resources

[xuyang2018.jy]

Hi Petr
> Hi all,
>
>> Hi Cyril, Petr
>>> Hi!
>>>>> I think that we allready discussed this in another thread:
>
>>>>> https://lists.linux.it/pipermail/ltp/2021-July/023831.html
>
>>>> Thanks, I forgot this. In that case my approach (not using<=, but count
>>>> segments in /proc/sysvipc/shm before testing) might be more precise.
>>>> But no strong feeling about that, both solutions fix the test, let's chose one
>>>> and merge.
>
>>> As I said previously, there are many SysV IPC tests that do expect that
>>> nobody will add/remove IPC shm/queue/semaphores during the testrun and
>>> some of the testcases cannot even be implemented without this
>>> expectation.
>
>>> Hence I wouldn't complicate the test here and just count how many
>>> segments are there at the start and be done with it.
>> Agree.
>
>> A possible solution(alter get_used_queues api in new ipc lib and add
>> file parametrers, so we can use this api for msgget03) I have mentioned
>> in the previous email, the url as below:
>> https://lists.linux.it/pipermail/ltp/2021-July/023653.html
> LGTM. Or use /proc/sysvipc/shm instead of /proc/sysvipc/msg in get_used_queues()
> as you noted get_used_queues() has not been used yet.
I rename get_used_queues to get_used_sysvipc_cnt. see attached patch.
>
> BTW searching where get_used_queues() appeared, I see [LTP] [PATCH v3 1/4]
> syscalls/ipc: add newipc library for new API [1], but if I'm not mistaken
> get_used_queues() was not used even there, maybe it was in some previous
> versions.
Yes, no new api case use GET_USED_QUEUES api.
>
> Kind regards,
> Petr
>
> [1] https://lists.linux.it/pipermail/ltp/2016-December/003239.html

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0001-libs-libnewipc-Rename-get_used_queues-as-get_used_sy.patch
URL: <http://lists.linux.it/pipermail/ltp/attachments/20210727/401bfffc/attachment.ksh>

[LTP] [PATCH v2 2/2] msgget03: don't depend on existed shared resources

[xuyang2018.jy]

Hi Cyril, Petr
> Hi Petr
>> Hi all,
>>
>>> Hi Cyril, Petr
>>>> Hi!
>>>>>> I think that we allready discussed this in another thread:
>>
>>>>>> https://lists.linux.it/pipermail/ltp/2021-July/023831.html
>>
>>>>> Thanks, I forgot this. In that case my approach (not using<=, but
>>>>> count
>>>>> segments in /proc/sysvipc/shm before testing) might be more precise.
>>>>> But no strong feeling about that, both solutions fix the test,
>>>>> let's chose one
>>>>> and merge.
>>
>>>> As I said previously, there are many SysV IPC tests that do expect that
>>>> nobody will add/remove IPC shm/queue/semaphores during the testrun and
>>>> some of the testcases cannot even be implemented without this
>>>> expectation.
>>
>>>> Hence I wouldn't complicate the test here and just count how many
>>>> segments are there at the start and be done with it.
>>> Agree.
>>
>>> A possible solution(alter get_used_queues api in new ipc lib and add
>>> file parametrers, so we can use this api for msgget03) I have mentioned
>>> in the previous email, the url as below:
>>> https://lists.linux.it/pipermail/ltp/2021-July/023653.html
>> LGTM. Or use /proc/sysvipc/shm instead of /proc/sysvipc/msg in
>> get_used_queues()
>> as you noted get_used_queues() has not been used yet.
> I rename get_used_queues to get_used_sysvipc_cnt. see attached patch.
Any idea about the attached patch(it was in previous eamil)?

ps:more and more people sent patch to fix this problme, I don't want to 
see many patches for this problem. Let's choose a solution to fix this 
problem.

Best Regards
Yang Xu
>>
>> BTW searching where get_used_queues() appeared, I see [LTP] [PATCH v3
>> 1/4]
>> syscalls/ipc: add newipc library for new API [1], but if I'm not mistaken
>> get_used_queues() was not used even there, maybe it was in some previous
>> versions.
> Yes, no new api case use GET_USED_QUEUES api.
>>
>> Kind regards,
>> Petr
>>
>> [1] https://lists.linux.it/pipermail/ltp/2016-December/003239.html
>

[LTP] [PATCH v2 2/2] msgget03: don't depend on existed shared resources

[Cyril Hrubis]

Hi!
> From 2772f8f0bbc1526389cb2090895dded41e2c43dc Mon Sep 17 00:00:00 2001
> From: Yang Xu <xuyang2018.jy@fujitsu.com>
> Date: Tue, 27 Jul 2021 16:22:42 +0800
> Subject: [PATCH] libs/libnewipc:Rename get_used_queues as get_used_sysvipc_cnt
> 
> Rename get_used_queues as get_used_sysvipc_cnt, so we can use GET_USED_QUEQUES()
> and GET_USED_SEGMENTS() to get the corresponding used sysvipc resource total.
> 
> Then we can use them in shmget03/msgget03, so we can trigger the ENOSPC error correctly
> even current environment has consume some sysvipc resource.
> 
> I don't use this api in verify function since we don't support run cases in parallel and
> we should assume this situation that this case is the only case to use(free or alloc) sysv
> ipc resource at that time.
> 
> Fixes: #842
> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
> ---
>  include/libnewipc.h                             |  6 ++++--
>  libs/libltpnewipc/libnewipc.c                   | 16 ++++++++--------
>  testcases/kernel/syscalls/ipc/msgget/msgget03.c | 10 +++++++---
>  testcases/kernel/syscalls/ipc/shmget/shmget03.c | 10 ++++++----
>  4 files changed, 25 insertions(+), 17 deletions(-)
> 
> diff --git a/include/libnewipc.h b/include/libnewipc.h
> index 075364f85..b0448841a 100644
> --- a/include/libnewipc.h
> +++ b/include/libnewipc.h
> @@ -49,9 +49,11 @@ key_t getipckey(const char *file, const int lineno);
>  #define GETIPCKEY() \
>  	getipckey(__FILE__, __LINE__)
>  
> -int get_used_queues(const char *file, const int lineno);
> +int get_used_sysvipc_cnt(const char *file, const int lineno, const char *sysvipc_file);
>  #define GET_USED_QUEUES() \
> -	get_used_queues(__FILE__, __LINE__)
> +	get_used_sysvipc_cnt(__FILE__, __LINE__, "/proc/sysvipc/msg")
> +#define GET_USED_SEGMENTS() \
> +	get_used_sysvipc_cnt(__FILE__, __LINE__, "/proc/sysvipc/shm")

I would just call it get_used_sysvipc()

>  void *probe_free_addr(const char *file, const int lineno);
>  #define PROBE_FREE_ADDR() \
> diff --git a/libs/libltpnewipc/libnewipc.c b/libs/libltpnewipc/libnewipc.c
> index d0974bbe0..687a907e7 100644
> --- a/libs/libltpnewipc/libnewipc.c
> +++ b/libs/libltpnewipc/libnewipc.c
> @@ -48,25 +48,25 @@ key_t getipckey(const char *file, const int lineno)
>  	return key;
>  }
>  
> -int get_used_queues(const char *file, const int lineno)
> +int get_used_sysvipc_cnt(const char *file, const int lineno, const char *sysvipc_file)
>  {
>  	FILE *fp;
> -	int used_queues = -1;
> +	int used_cnt = -1;

And here as well the _cnt is not adding any value over I would say.

>  	char buf[BUFSIZE];
>  
> -	fp = safe_fopen(file, lineno, NULL, "/proc/sysvipc/msg", "r");
> +	fp = safe_fopen(file, lineno, NULL, sysvipc_file, "r");
>  
>  	while (fgets(buf, BUFSIZE, fp) != NULL)
> -		used_queues++;
> +		used_cnt++;
>  
>  	fclose(fp);
>  
> -	if (used_queues < 0) {
> -		tst_brk(TBROK, "can't read /proc/sysvipc/msg to get "
> -			"used message queues at %s:%d", file, lineno);
> +	if (used_cnt < 0) {
> +		tst_brk(TBROK, "can't read %s to get used message queues "
> +			"at %s:%d", sysvipc_file, file, lineno);
>  	}
>  
> -	return used_queues;
> +	return used_cnt;
>  }
>  
>  void *probe_free_addr(const char *file, const int lineno)
> diff --git a/testcases/kernel/syscalls/ipc/msgget/msgget03.c b/testcases/kernel/syscalls/ipc/msgget/msgget03.c
> index ab5714cdc..8ccffc547 100644
> --- a/testcases/kernel/syscalls/ipc/msgget/msgget03.c
> +++ b/testcases/kernel/syscalls/ipc/msgget/msgget03.c
> @@ -21,7 +21,7 @@
>  #include "tst_safe_sysv_ipc.h"
>  #include "libnewipc.h"
>  
> -static int maxmsgs, queue_cnt;
> +static int maxmsgs, queue_cnt, existed_cnt;
                                  ^
				  Why not 'used_cnt' ?
>  static int *queues;
>  static key_t msgkey;
>  
> @@ -37,11 +37,15 @@ static void setup(void)
>  
>  	msgkey = GETIPCKEY();
>  
> +	existed_cnt = GET_USED_QUEUES();
> +	tst_res(TINFO, "Current environment %d message queues are already in use",
> +		existed_cnt);
> +
>  	SAFE_FILE_SCANF("/proc/sys/kernel/msgmni", "%i", &maxmsgs);
>  
> -	queues = SAFE_MALLOC(maxmsgs * sizeof(int));
> +	queues = SAFE_MALLOC((maxmsgs - existed_cnt) * sizeof(int));
>  
> -	for (num = 0; num < maxmsgs; num++) {
> +	for (num = 0; num < maxmsgs - existed_cnt; num++) {
>  		res = msgget(msgkey + num, IPC_CREAT | IPC_EXCL);
>  		if (res == -1)
>  			tst_brk(TBROK | TERRNO, "msgget failed unexpectedly");
> diff --git a/testcases/kernel/syscalls/ipc/shmget/shmget03.c b/testcases/kernel/syscalls/ipc/shmget/shmget03.c
> index efbc465e1..acd352796 100644
> --- a/testcases/kernel/syscalls/ipc/shmget/shmget03.c
> +++ b/testcases/kernel/syscalls/ipc/shmget/shmget03.c
> @@ -22,7 +22,7 @@
>  #include "libnewipc.h"
>  
>  static int *queues;
> -static int maxshms, queue_cnt;
> +static int maxshms, queue_cnt, existed_cnt;
                                   ^
				   Here as well.
>  static key_t shmkey;
>  
>  static void verify_shmget(void)
> @@ -36,11 +36,13 @@ static void setup(void)
>  	int res, num;
>  
>  	shmkey = GETIPCKEY();
> -
> +	existed_cnt = GET_USED_SEGMENTS();
> +	tst_res(TINFO, "Current environment %d shared memory segments are already in use",
> +		existed_cnt);
>  	SAFE_FILE_SCANF("/proc/sys/kernel/shmmni", "%i", &maxshms);
>  
> -	queues = SAFE_MALLOC(maxshms * sizeof(int));
> -	for (num = 0; num < maxshms; num++) {
> +	queues = SAFE_MALLOC((maxshms - existed_cnt) * sizeof(int));
> +	for (num = 0; num < maxshms - existed_cnt; num++) {
>  		res = shmget(shmkey + num, SHM_SIZE, IPC_CREAT | IPC_EXCL | SHM_RW);
>  		if (res == -1)
>  			tst_brk(TBROK | TERRNO, "shmget failed unexpectedly");

Other than the very minor differencies I would do in naming the
variables and function this looks good to me.

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>


-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 2/2] msgget03: don't depend on existed shared resources

[Petr Vorel]

Hi Xu, Cyril,

> Other than the very minor differencies I would do in naming the
> variables and function this looks good to me.

Agree with all Cyril's suggestions for rename.
Reviewed-by: Petr Vorel <pvorel@suse.cz>

I suppose you just merge it fixed version, right?
Because now we discuss under Alexey's "[v2,2/2] msgget03: don't depend on
existed shared resources" patch.

Kind regards,
Petr

[LTP] [PATCH v2 2/2] msgget03: don't depend on existed shared resources

[xuyang2018.jy]

Hi Cyril, Petr
> Hi!
>>  From 2772f8f0bbc1526389cb2090895dded41e2c43dc Mon Sep 17 00:00:00 2001
>> From: Yang Xu<xuyang2018.jy@fujitsu.com>
>> Date: Tue, 27 Jul 2021 16:22:42 +0800
>> Subject: [PATCH] libs/libnewipc:Rename get_used_queues as get_used_sysvipc_cnt
>>
>> Rename get_used_queues as get_used_sysvipc_cnt, so we can use GET_USED_QUEQUES()
>> and GET_USED_SEGMENTS() to get the corresponding used sysvipc resource total.
>>
>> Then we can use them in shmget03/msgget03, so we can trigger the ENOSPC error correctly
>> even current environment has consume some sysvipc resource.
>>
>> I don't use this api in verify function since we don't support run cases in parallel and
>> we should assume this situation that this case is the only case to use(free or alloc) sysv
>> ipc resource at that time.
>>
>> Fixes: #842
>> Signed-off-by: Yang Xu<xuyang2018.jy@fujitsu.com>
>> ---
>>   include/libnewipc.h                             |  6 ++++--
>>   libs/libltpnewipc/libnewipc.c                   | 16 ++++++++--------
>>   testcases/kernel/syscalls/ipc/msgget/msgget03.c | 10 +++++++---
>>   testcases/kernel/syscalls/ipc/shmget/shmget03.c | 10 ++++++----
>>   4 files changed, 25 insertions(+), 17 deletions(-)
>>
>> diff --git a/include/libnewipc.h b/include/libnewipc.h
>> index 075364f85..b0448841a 100644
>> --- a/include/libnewipc.h
>> +++ b/include/libnewipc.h
>> @@ -49,9 +49,11 @@ key_t getipckey(const char *file, const int lineno);
>>   #define GETIPCKEY() \
>>   	getipckey(__FILE__, __LINE__)
>>
>> -int get_used_queues(const char *file, const int lineno);
>> +int get_used_sysvipc_cnt(const char *file, const int lineno, const char *sysvipc_file);
>>   #define GET_USED_QUEUES() \
>> -	get_used_queues(__FILE__, __LINE__)
>> +	get_used_sysvipc_cnt(__FILE__, __LINE__, "/proc/sysvipc/msg")
>> +#define GET_USED_SEGMENTS() \
>> +	get_used_sysvipc_cnt(__FILE__, __LINE__, "/proc/sysvipc/shm")
>
> I would just call it get_used_sysvipc()
OK.
>
>>   void *probe_free_addr(const char *file, const int lineno);
>>   #define PROBE_FREE_ADDR() \
>> diff --git a/libs/libltpnewipc/libnewipc.c b/libs/libltpnewipc/libnewipc.c
>> index d0974bbe0..687a907e7 100644
>> --- a/libs/libltpnewipc/libnewipc.c
>> +++ b/libs/libltpnewipc/libnewipc.c
>> @@ -48,25 +48,25 @@ key_t getipckey(const char *file, const int lineno)
>>   	return key;
>>   }
>>
>> -int get_used_queues(const char *file, const int lineno)
>> +int get_used_sysvipc_cnt(const char *file, const int lineno, const char *sysvipc_file)
>>   {
>>   	FILE *fp;
>> -	int used_queues = -1;
>> +	int used_cnt = -1;
>
> And here as well the _cnt is not adding any value over I would say.
OK.
>
>>   	char buf[BUFSIZE];
>>
>> -	fp = safe_fopen(file, lineno, NULL, "/proc/sysvipc/msg", "r");
>> +	fp = safe_fopen(file, lineno, NULL, sysvipc_file, "r");
>>
>>   	while (fgets(buf, BUFSIZE, fp) != NULL)
>> -		used_queues++;
>> +		used_cnt++;
>>
>>   	fclose(fp);
>>
>> -	if (used_queues<  0) {
>> -		tst_brk(TBROK, "can't read /proc/sysvipc/msg to get "
>> -			"used message queues at %s:%d", file, lineno);
>> +	if (used_cnt<  0) {
>> +		tst_brk(TBROK, "can't read %s to get used message queues "
>> +			"at %s:%d", sysvipc_file, file, lineno);
>>   	}
I also modify this info.
message queues => sysvipc resource total
>>
>> -	return used_queues;
>> +	return used_cnt;
>>   }
>>
>>   void *probe_free_addr(const char *file, const int lineno)
>> diff --git a/testcases/kernel/syscalls/ipc/msgget/msgget03.c b/testcases/kernel/syscalls/ipc/msgget/msgget03.c
>> index ab5714cdc..8ccffc547 100644
>> --- a/testcases/kernel/syscalls/ipc/msgget/msgget03.c
>> +++ b/testcases/kernel/syscalls/ipc/msgget/msgget03.c
>> @@ -21,7 +21,7 @@
>>   #include "tst_safe_sysv_ipc.h"
>>   #include "libnewipc.h"
>>
>> -static int maxmsgs, queue_cnt;
>> +static int maxmsgs, queue_cnt, existed_cnt;
>                                    ^
> 				  Why not 'used_cnt' ?
Yes.
>>   static int *queues;
>>   static key_t msgkey;
>>
>> @@ -37,11 +37,15 @@ static void setup(void)
>>
>>   	msgkey = GETIPCKEY();
>>
>> +	existed_cnt = GET_USED_QUEUES();
>> +	tst_res(TINFO, "Current environment %d message queues are already in use",
>> +		existed_cnt);
>> +
>>   	SAFE_FILE_SCANF("/proc/sys/kernel/msgmni", "%i",&maxmsgs);
>>
>> -	queues = SAFE_MALLOC(maxmsgs * sizeof(int));
>> +	queues = SAFE_MALLOC((maxmsgs - existed_cnt) * sizeof(int));
>>
>> -	for (num = 0; num<  maxmsgs; num++) {
>> +	for (num = 0; num<  maxmsgs - existed_cnt; num++) {
>>   		res = msgget(msgkey + num, IPC_CREAT | IPC_EXCL);
>>   		if (res == -1)
>>   			tst_brk(TBROK | TERRNO, "msgget failed unexpectedly");
>> diff --git a/testcases/kernel/syscalls/ipc/shmget/shmget03.c b/testcases/kernel/syscalls/ipc/shmget/shmget03.c
>> index efbc465e1..acd352796 100644
>> --- a/testcases/kernel/syscalls/ipc/shmget/shmget03.c
>> +++ b/testcases/kernel/syscalls/ipc/shmget/shmget03.c
>> @@ -22,7 +22,7 @@
>>   #include "libnewipc.h"
>>
>>   static int *queues;
>> -static int maxshms, queue_cnt;
>> +static int maxshms, queue_cnt, existed_cnt;
>                                     ^
> 				   Here as well.
OK.
>>   static key_t shmkey;
>>
>>   static void verify_shmget(void)
>> @@ -36,11 +36,13 @@ static void setup(void)
>>   	int res, num;
>>
>>   	shmkey = GETIPCKEY();
>> -
>> +	existed_cnt = GET_USED_SEGMENTS();
>> +	tst_res(TINFO, "Current environment %d shared memory segments are already in use",
>> +		existed_cnt);
>>   	SAFE_FILE_SCANF("/proc/sys/kernel/shmmni", "%i",&maxshms);
>>
>> -	queues = SAFE_MALLOC(maxshms * sizeof(int));
>> -	for (num = 0; num<  maxshms; num++) {
>> +	queues = SAFE_MALLOC((maxshms - existed_cnt) * sizeof(int));
>> +	for (num = 0; num<  maxshms - existed_cnt; num++) {
>>   		res = shmget(shmkey + num, SHM_SIZE, IPC_CREAT | IPC_EXCL | SHM_RW);
>>   		if (res == -1)
>>   			tst_brk(TBROK | TERRNO, "shmget failed unexpectedly");
>
> Other than the very minor differencies I would do in naming the
> variables and function this looks good to me.
>
> Reviewed-by: Cyril Hrubis<chrubis@suse.cz>
Thanks for your review. I spilt this patch into a patchset because it is 
more friendly for user or tester.

Best Regards
Yang Xu
>
>

[LTP] [PATCH v2 2/2] msgget03: don't depend on existed shared resources

[Petr Vorel]

Hi Xu,

> >> -	if (used_queues<  0) {
> >> -		tst_brk(TBROK, "can't read /proc/sysvipc/msg to get "
> >> -			"used message queues at %s:%d", file, lineno);
> >> +	if (used_cnt<  0) {
> >> +		tst_brk(TBROK, "can't read %s to get used message queues "
> >> +			"at %s:%d", sysvipc_file, file, lineno);
> >>   	}
> I also modify this info.
> message queues => sysvipc resource total
+1. nit: I'd also move "at" at the previous line (better for grep).

Kind regards,
Petr

[LTP] [PATCH v2 2/2] msgget03: don't depend on existed shared resources

[xuyang2018.jy]

Hi Petr
> Hi Xu,
>
>>>> -	if (used_queues<   0) {
>>>> -		tst_brk(TBROK, "can't read /proc/sysvipc/msg to get "
>>>> -			"used message queues at %s:%d", file, lineno);
>>>> +	if (used_cnt<   0) {
>>>> +		tst_brk(TBROK, "can't read %s to get used message queues "
>>>> +			"at %s:%d", sysvipc_file, file, lineno);
>>>>    	}
>> I also modify this info.
>> message queues =>  sysvipc resource total
> +1. nit: I'd also move "at" at the previous line (better for grep).
Sounds reasonable. Thanks.

Best Regards
Yang Xu
>
> Kind regards,
> Petr
>

[LTP] [PATCH v2 2/2] msgget03: don't depend on existed shared resources

[Petr Vorel]

Hi Cyril, all,

> Hi!
> > > I think that we allready discussed this in another thread:

> > > https://lists.linux.it/pipermail/ltp/2021-July/023831.html

> > Thanks, I forgot this. In that case my approach (not using <=, but count
> > segments in /proc/sysvipc/shm before testing) might be more precise.
> > But no strong feeling about that, both solutions fix the test, let's chose one
> > and merge.

> As I said previously, there are many SysV IPC tests that do expect that
> nobody will add/remove IPC shm/queue/semaphores during the testrun and
> some of the testcases cannot even be implemented without this
> expectation.

> Hence I wouldn't complicate the test here and just count how many
> segments are there at the start and be done with it.
Yes, that's what's done in "my approach" [1].

Kind regards,
Petr

[1] https://patchwork.ozlabs.org/project/ltp/patch/20210722073523.5099-1-pvorel@suse.cz/

[LTP] [PATCH v2 1/2] shmget03: don't depend on existed shared resources

[Li Wang]

On Mon, Jul 12, 2021 at 3:54 PM Alexey Kodanev <aleksei.kodanev@bell-sw.com>
wrote:

> It's unlikely, but still possible that some of them could be
> created/released during the test as well, so the patch only
> checks errno.
>
> Signed-off-by: Alexey Kodanev <aleksei.kodanev@bell-sw.com>
> ---
> v2: * Move the loop to the test run function and try to get
>       ENOSPC errno there.
>

I'm fine to go with this but move the loop to test run without any
limit will bring new fail if running with parameter '-i 2'.

We have to handle that situation (maybe add a judgment to skip
test for run more times) in case someone uses it like:

# ./shmget03 -i 2
tst_test.c:1344: TINFO: Timeout per run is 0h 05m 00s
shmget03.c:44: TPASS: Maximum number of segments reached (4096), used by
test 4096
shmget03.c:41: TFAIL: Failed to trigger ENOSPC error: EEXIST (17)

-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20210712/20653694/attachment.htm>

[LTP] [PATCH v2 1/2] shmget03: don't depend on existed shared resources

[Alexey Kodanev]

On 12.07.2021 11:28, Li Wang wrote:
> 
> 
> On Mon, Jul 12, 2021 at 3:54 PM Alexey Kodanev <aleksei.kodanev@bell-sw.com <mailto:aleksei.kodanev@bell-sw.com>> wrote:
> 
>     It's unlikely, but still possible that some of them could be
>     created/released during the test as well, so the patch only
>     checks errno.
> 
>     Signed-off-by: Alexey Kodanev <aleksei.kodanev@bell-sw.com <mailto:aleksei.kodanev@bell-sw.com>>
>     ---
>     v2: * Move the loop to the test run function and try to get
>     ? ? ? ENOSPC errno there.
> 
> 
> I'm fine to go with this but move?the loop to test run without any
> limit will bring new fail if running with parameter '-i 2'.
> 
> We have to handle that situation (maybe add a judgment to skip
> test for run?more times) in case someone uses it like:

Or just release them asap after tpass?

> 
> # ./shmget03 -i 2
> tst_test.c:1344: TINFO: Timeout per run is 0h 05m 00s
> shmget03.c:44: TPASS: Maximum number of segments reached (4096), used by test 4096
> shmget03.c:41: TFAIL: Failed to trigger ENOSPC error: EEXIST (17)
> 
> -- 
> Regards,
> Li Wang

[LTP] [PATCH v2 1/2] shmget03: don't depend on existed shared resources

[Li Wang]

On Mon, Jul 12, 2021 at 4:37 PM Alexey Kodanev <aleksei.kodanev@bell-sw.com>
wrote:

> On 12.07.2021 11:28, Li Wang wrote:
> >
> >
> > On Mon, Jul 12, 2021 at 3:54 PM Alexey Kodanev <
> aleksei.kodanev@bell-sw.com <mailto:aleksei.kodanev@bell-sw.com>> wrote:
> >
> >     It's unlikely, but still possible that some of them could be
> >     created/released during the test as well, so the patch only
> >     checks errno.
> >
> >     Signed-off-by: Alexey Kodanev <aleksei.kodanev@bell-sw.com <mailto:
> aleksei.kodanev@bell-sw.com>>
> >     ---
> >     v2: * Move the loop to the test run function and try to get
> >           ENOSPC errno there.
> >
> >
> > I'm fine to go with this but move the loop to test run without any
> > limit will bring new fail if running with parameter '-i 2'.
> >
> > We have to handle that situation (maybe add a judgment to skip
> > test for run more times) in case someone uses it like:
>
> Or just release them asap after tpass?


Sure, but looks a bit redundant.

Or we can just adding a global varible for saving num:

--- a/testcases/kernel/syscalls/ipc/shmget/shmget03.c
+++ b/testcases/kernel/syscalls/ipc/shmget/shmget03.c
@@ -22,7 +22,7 @@
 #include "libnewipc.h"

 static int *segments;
-static int maxshms, segments_cnt;
+static int number = 0, maxshms, segments_cnt;
 static key_t shmkey;

 static void verify_shmget(void)
@@ -30,7 +30,7 @@ static void verify_shmget(void)
        int res = 0, num;

        errno = 0;
-       for (num = 0; num <= maxshms; ++num) {
+       for (num = number; num <= maxshms; ++num) {
                res = shmget(shmkey + num, SHM_SIZE, IPC_CREAT | IPC_EXCL |
SHM_RW);
                if (res == -1)
                        break;
@@ -42,6 +42,8 @@ static void verify_shmget(void)

        tst_res(TPASS, "Maximum number of segments reached (%d), used by
test %d",
                maxshms, segments_cnt);
+
+       number = num;
 }

 static void setup(void)


-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20210712/22e4fe29/attachment.htm>

[LTP] [PATCH v2 1/2] shmget03: don't depend on existed shared resources

[Li Wang]

On Mon, Jul 12, 2021 at 4:42 PM Li Wang <liwang@redhat.com> wrote:

>
>
> On Mon, Jul 12, 2021 at 4:37 PM Alexey Kodanev <
> aleksei.kodanev@bell-sw.com> wrote:
>
>> On 12.07.2021 11:28, Li Wang wrote:
>> >
>> >
>> > On Mon, Jul 12, 2021 at 3:54 PM Alexey Kodanev <
>> aleksei.kodanev@bell-sw.com <mailto:aleksei.kodanev@bell-sw.com>> wrote:
>> >
>> >     It's unlikely, but still possible that some of them could be
>> >     created/released during the test as well, so the patch only
>> >     checks errno.
>> >
>> >     Signed-off-by: Alexey Kodanev <aleksei.kodanev@bell-sw.com <mailto:
>> aleksei.kodanev@bell-sw.com>>
>> >     ---
>> >     v2: * Move the loop to the test run function and try to get
>> >           ENOSPC errno there.
>> >
>> >
>> > I'm fine to go with this but move the loop to test run without any
>> > limit will bring new fail if running with parameter '-i 2'.
>> >
>> > We have to handle that situation (maybe add a judgment to skip
>> > test for run more times) in case someone uses it like:
>>
>> Or just release them asap after tpass?
>
>
> Sure, but looks a bit redundant.
>
> Or we can just adding a global varible for saving num:
>
> --- a/testcases/kernel/syscalls/ipc/shmget/shmget03.c
> +++ b/testcases/kernel/syscalls/ipc/shmget/shmget03.c
> @@ -22,7 +22,7 @@
>  #include "libnewipc.h"
>
>  static int *segments;
> -static int maxshms, segments_cnt;
> +static int number = 0, maxshms, segments_cnt;
>  static key_t shmkey;
>
>  static void verify_shmget(void)
> @@ -30,7 +30,7 @@ static void verify_shmget(void)
>         int res = 0, num;
>
>         errno = 0;
> -       for (num = 0; num <= maxshms; ++num) {
> +       for (num = number; num <= maxshms; ++num) {
>

Oh, this method is thoughtless, because if the test gets ENOSPC at
the last looping time, which means num == maxshms, then the global
number will be larger than maxshms, so the test won't fall into
for() loop next time and report FAIL again.

So, let's go with your way: release them after TPASS. This is safer.



>                 res = shmget(shmkey + num, SHM_SIZE, IPC_CREAT | IPC_EXCL
> | SHM_RW);
>                 if (res == -1)
>                         break;
> @@ -42,6 +42,8 @@ static void verify_shmget(void)
>
>         tst_res(TPASS, "Maximum number of segments reached (%d), used by
> test %d",
>                 maxshms, segments_cnt);
> +
> +       number = num;
>  }
>
>  static void setup(void)
>
>
> --
> Regards,
> Li Wang
>


-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20210712/fa877490/attachment-0001.htm>