From: Jorik Cronenberg <jcronenberg@suse.de>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH 1/2] lib: Add timeout to TST_PROCESS_STATE_WAIT
Date: Thu, 23 Jan 2020 13:16:23 +0100 [thread overview]
Message-ID: <2cfda6ff-b02f-00c4-b256-eeded0d3edda@suse.de> (raw)
In-Reply-To: <a8618b2e-067c-9a66-02e7-3feceec1ff9c@cn.fujitsu.com>
Hi Yang, thanks for the review!
>
>> Add the possibility to add a timeout to TST_PROCESS_STATE_WAIT.
>> Like checkpoints add TST_PROCESS_STATE_WAIT2()
>> for specifying a timeout. The original TST_PROCESS_STATE_WAIT()
>> works the same as before. Timeout can be specified in milliseconds.
>>
> Hi Jorik
>
> We have tst_process_state_wait2 since commit dbf270c5 ("lib: Add
> tst_process_state_wait2()"), this api has same functions as
> tst_process_state_wait but only return error instead of TBROK.
>
> I think using TST_PROCESS_STATE_WAIT2 is confused and we can only expand
> tst_process_state_wait make it support sleep specifying in milliseconds.
>
> Best Regards
> Yang Xu
I don't think I quite understand what you mean. I can see that using
TST_PROCESS_STATE_WAIT2 is confusing. But I didn't want to touch the
existing TST_PROCESS_STATE_WAIT to ensure all older tests still run the
same. Are you saying i should go through all tests that use
TST_PROCESS_STATE_WAIT and specify that they use a timeout of 0(which
according to a git grep doesn't seem too many, so it wouldn't be too
much effort) and then change TST_PROCESS_STATE_WAIT to include a timeout
or should I just rename TST_PROCESS_STATE_WAIT2 to something that
seperates it more from tst_process_state_wait2?
regards,
Jorik Cronenberg
>> Signed-off-by: Jorik Cronenberg <jcronenberg@suse.de>
>> ---
>> ? include/tst_process_state.h | 12 ++++++++----
>> ? lib/tst_process_state.c???? | 19 ++++++++++++++-----
>> ? 2 files changed, 22 insertions(+), 9 deletions(-)
>>
>> diff --git a/include/tst_process_state.h b/include/tst_process_state.h
>> index fab0491d9..27a8ffc36 100644
>> --- a/include/tst_process_state.h
>> +++ b/include/tst_process_state.h
>> @@ -47,9 +47,13 @@
>> ?? */
>> ? #ifdef TST_TEST_H__
>> ? +#define TST_PROCESS_STATE_WAIT2(pid, state, msec_timeout) \
>> +??? tst_process_state_wait(__FILE__, __LINE__, NULL, \
>> +?????????????????????????? (pid), (state), msec_timeout)
>> +
>> ? #define TST_PROCESS_STATE_WAIT(pid, state) \
>> ????? tst_process_state_wait(__FILE__, __LINE__, NULL, \
>> -?????????????????????????? (pid), (state))
>> +?????????????????????????? (pid), (state), 0)
>> ? #else
>> ? /*
>> ?? * The same as above but does not use tst_brkm() interface.
>> @@ -65,8 +69,8 @@ int tst_process_state_wait2(pid_t pid, const char
>> state);
>> ????????????????????????????? (pid), (state))
>> ? #endif
>> ? -void tst_process_state_wait(const char *file, const int lineno,
>> -??????????????????????????? void (*cleanup_fn)(void),
>> -??????????????????????????? pid_t pid, const char state);
>> +int tst_process_state_wait(const char *file, const int lineno,
>> +??????????????????????????? void (*cleanup_fn)(void), pid_t pid,
>> +??????????????? const char state, unsigned int msec_timeout);
>> ? ? #endif /* TST_PROCESS_STATE__ */
>> diff --git a/lib/tst_process_state.c b/lib/tst_process_state.c
>> index 7a7824959..32b44992c 100644
>> --- a/lib/tst_process_state.c
>> +++ b/lib/tst_process_state.c
>> @@ -28,11 +28,12 @@
>> ? #include "test.h"
>> ? #include "tst_process_state.h"
>> ? -void tst_process_state_wait(const char *file, const int lineno,
>> -??????????????????????????? void (*cleanup_fn)(void),
>> -??????????????????????????? pid_t pid, const char state)
>> +int tst_process_state_wait(const char *file, const int lineno,
>> +??????????????????????????? void (*cleanup_fn)(void), pid_t pid,
>> +??????????????? const char state, unsigned int msec_timeout)
>> ? {
>> ????? char proc_path[128], cur_state;
>> +??? unsigned int msecs = 0;
>> ? ????? snprintf(proc_path, sizeof(proc_path), "/proc/%i/stat", pid);
>> ? @@ -41,10 +42,18 @@ void tst_process_state_wait(const char *file,
>> const int lineno,
>> ????????????????????????? "%*i %*s %c", &cur_state);
>> ? ????????? if (state == cur_state)
>> -??????????? return;
>> +??????????? break;
>> ? -??????? usleep(10000);
>> +??????? usleep(1000);
>> +??????? msecs += 1;
>> +
>> +??????? if (msecs >= msec_timeout) {
>> +??????????? errno = ETIMEDOUT;
>> +??????????? return -1;
>> +??????? }
>> ????? }
>> +
>> +??? return 0;
>> ? }
>> ? ? int tst_process_state_wait2(pid_t pid, const char state)
>>
>
>
next prev parent reply other threads:[~2020-01-23 12:16 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-22 13:42 [LTP] [PATCH 1/2] lib: Add timeout to TST_PROCESS_STATE_WAIT Jorik Cronenberg
2020-01-22 13:42 ` [LTP] [PATCH 2/2] syscalls/vmsplice: Add NONBLOCK testcase Jorik Cronenberg
2020-01-23 6:57 ` Yang Xu
2020-01-23 6:21 ` [LTP] [PATCH 1/2] lib: Add timeout to TST_PROCESS_STATE_WAIT Yang Xu
2020-01-23 12:16 ` Jorik Cronenberg [this message]
2020-01-23 13:17 ` Yang Xu
2020-01-23 13:33 ` Cyril Hrubis
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2cfda6ff-b02f-00c4-b256-eeded0d3edda@suse.de \
--to=jcronenberg@suse.de \
--cc=ltp@lists.linux.it \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox