| Message ID | 20210712075223.10682-2-aleksei.kodanev@bell-sw.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/2] shmget03: don't depend on existed shared resources | expand |
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); > } ...
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
> 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
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.
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 >
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/
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
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 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") 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; 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; 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; 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");
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 >
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>
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
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 > >
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
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 >
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)
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(-)