* [LTP] [PATCH] cpuacct_task: exit cleanly on SIGVTALRM
@ 2016-08-31 10:21 Stanislav Kholmanskikh
2016-08-31 13:34 ` Cyril Hrubis
0 siblings, 1 reply; 4+ messages in thread
From: Stanislav Kholmanskikh @ 2016-08-31 10:21 UTC (permalink / raw)
To: ltp
We don't handle the SIGVTALRM signal in cpuacct_task,
so the process gets terminated, and it makes cpuacct.sh generate
a "Virtual timer expired" message for each cpuacct_task process.
In my opinion, these messages have no real value, and there is no
need in keeping them in the output.
Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
.../kernel/controllers/cpuacct/cpuacct_task.c | 23 +++++++++++++++++++-
1 files changed, 22 insertions(+), 1 deletions(-)
diff --git a/testcases/kernel/controllers/cpuacct/cpuacct_task.c b/testcases/kernel/controllers/cpuacct/cpuacct_task.c
index 0ca01d1..94aa4e6 100644
--- a/testcases/kernel/controllers/cpuacct/cpuacct_task.c
+++ b/testcases/kernel/controllers/cpuacct/cpuacct_task.c
@@ -25,11 +25,21 @@
#include <sys/time.h>
#include <sys/types.h>
#include <unistd.h>
+#include <signal.h>
#include <stdlib.h>
+#include <string.h>
+
+static volatile int got_signal;
+
+static void sig_handler(int signo)
+{
+ got_signal = 1;
+}
int main(int argc, char **argv)
{
FILE *f;
+ struct sigaction sa;
if (argc != 2) {
fprintf(stderr, "Usage: %s /cgroup/.../tasks\n", argv[0]);
@@ -44,9 +54,20 @@ int main(int argc, char **argv)
fprintf(f, "%i\n", getpid());
fclose(f);
+
+ memset(&sa, 0, sizeof(sa));
+ sigemptyset(&sa.sa_mask);
+ sa.sa_handler = sig_handler;
+
+ if (sigaction(SIGVTALRM, &sa, NULL)) {
+ perror("sigaction failed");
+ return 1;
+ }
+
struct itimerval it = {.it_value = {.tv_sec = 0, .tv_usec = 10000}};
setitimer(ITIMER_VIRTUAL, &it, NULL);
- for (;;);
+ while (!got_signal)
+ ;
return 0;
}
--
1.7.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [LTP] [PATCH] cpuacct_task: exit cleanly on SIGVTALRM
2016-08-31 10:21 [LTP] [PATCH] cpuacct_task: exit cleanly on SIGVTALRM Stanislav Kholmanskikh
@ 2016-08-31 13:34 ` Cyril Hrubis
2016-08-31 14:26 ` Stanislav Kholmanskikh
0 siblings, 1 reply; 4+ messages in thread
From: Cyril Hrubis @ 2016-08-31 13:34 UTC (permalink / raw)
To: ltp
Hi!
> We don't handle the SIGVTALRM signal in cpuacct_task,
> so the process gets terminated, and it makes cpuacct.sh generate
> a "Virtual timer expired" message for each cpuacct_task process.
>
> In my opinion, these messages have no real value, and there is no
> need in keeping them in the output.
I was just too lazy to install the signal handler in this case given
that the default action for the signal is to terminate the process...
> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
> ---
> .../kernel/controllers/cpuacct/cpuacct_task.c | 23 +++++++++++++++++++-
> 1 files changed, 22 insertions(+), 1 deletions(-)
>
> diff --git a/testcases/kernel/controllers/cpuacct/cpuacct_task.c b/testcases/kernel/controllers/cpuacct/cpuacct_task.c
> index 0ca01d1..94aa4e6 100644
> --- a/testcases/kernel/controllers/cpuacct/cpuacct_task.c
> +++ b/testcases/kernel/controllers/cpuacct/cpuacct_task.c
> @@ -25,11 +25,21 @@
> #include <sys/time.h>
> #include <sys/types.h>
> #include <unistd.h>
> +#include <signal.h>
> #include <stdlib.h>
> +#include <string.h>
> +
> +static volatile int got_signal;
> +
> +static void sig_handler(int signo)
> +{
> + got_signal = 1;
> +}
We can as well do _exit(0); here. Then we don't have to add the global
variable and the main loop will stay as for (;;);
> int main(int argc, char **argv)
> {
> FILE *f;
> + struct sigaction sa;
>
> if (argc != 2) {
> fprintf(stderr, "Usage: %s /cgroup/.../tasks\n", argv[0]);
> @@ -44,9 +54,20 @@ int main(int argc, char **argv)
>
> fprintf(f, "%i\n", getpid());
> fclose(f);
> +
> + memset(&sa, 0, sizeof(sa));
> + sigemptyset(&sa.sa_mask);
> + sa.sa_handler = sig_handler;
> +
> + if (sigaction(SIGVTALRM, &sa, NULL)) {
> + perror("sigaction failed");
> + return 1;
> + }
I would just use simpler signal(SIGVTALRM, sig_handler); here.
> struct itimerval it = {.it_value = {.tv_sec = 0, .tv_usec = 10000}};
>
> setitimer(ITIMER_VIRTUAL, &it, NULL);
> - for (;;);
> + while (!got_signal)
> + ;
> return 0;
> }
Other than that it's fine.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 4+ messages in thread
* [LTP] [PATCH] cpuacct_task: exit cleanly on SIGVTALRM
2016-08-31 13:34 ` Cyril Hrubis
@ 2016-08-31 14:26 ` Stanislav Kholmanskikh
2016-09-02 7:57 ` Stanislav Kholmanskikh
0 siblings, 1 reply; 4+ messages in thread
From: Stanislav Kholmanskikh @ 2016-08-31 14:26 UTC (permalink / raw)
To: ltp
Hi!
On 08/31/2016 04:34 PM, Cyril Hrubis wrote:
> Hi!
>> We don't handle the SIGVTALRM signal in cpuacct_task,
>> so the process gets terminated, and it makes cpuacct.sh generate
>> a "Virtual timer expired" message for each cpuacct_task process.
>>
>> In my opinion, these messages have no real value, and there is no
>> need in keeping them in the output.
>
> I was just too lazy to install the signal handler in this case given
> that the default action for the signal is to terminate the process...
>
>> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
>> ---
>> .../kernel/controllers/cpuacct/cpuacct_task.c | 23 +++++++++++++++++++-
>> 1 files changed, 22 insertions(+), 1 deletions(-)
>>
>> diff --git a/testcases/kernel/controllers/cpuacct/cpuacct_task.c b/testcases/kernel/controllers/cpuacct/cpuacct_task.c
>> index 0ca01d1..94aa4e6 100644
>> --- a/testcases/kernel/controllers/cpuacct/cpuacct_task.c
>> +++ b/testcases/kernel/controllers/cpuacct/cpuacct_task.c
>> @@ -25,11 +25,21 @@
>> #include <sys/time.h>
>> #include <sys/types.h>
>> #include <unistd.h>
>> +#include <signal.h>
>> #include <stdlib.h>
>> +#include <string.h>
>> +
>> +static volatile int got_signal;
>> +
>> +static void sig_handler(int signo)
>> +{
>> + got_signal = 1;
>> +}
>
> We can as well do _exit(0); here. Then we don't have to add the global
> variable and the main loop will stay as for (;;);
Ok
>
>> int main(int argc, char **argv)
>> {
>> FILE *f;
>> + struct sigaction sa;
>>
>> if (argc != 2) {
>> fprintf(stderr, "Usage: %s /cgroup/.../tasks\n", argv[0]);
>> @@ -44,9 +54,20 @@ int main(int argc, char **argv)
>>
>> fprintf(f, "%i\n", getpid());
>> fclose(f);
>> +
>> + memset(&sa, 0, sizeof(sa));
>> + sigemptyset(&sa.sa_mask);
>> + sa.sa_handler = sig_handler;
>> +
>> + if (sigaction(SIGVTALRM, &sa, NULL)) {
>> + perror("sigaction failed");
>> + return 1;
>> + }
>
> I would just use simpler signal(SIGVTALRM, sig_handler); here.
I tend to use sigaction() everywhere, since the man page for signal()
gives preference to sigaction() for the general case.
>
>> struct itimerval it = {.it_value = {.tv_sec = 0, .tv_usec = 10000}};
>>
>> setitimer(ITIMER_VIRTUAL, &it, NULL);
>> - for (;;);
>> + while (!got_signal)
>> + ;
>> return 0;
>> }
>
> Other than that it's fine.
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [LTP] [PATCH] cpuacct_task: exit cleanly on SIGVTALRM
2016-08-31 14:26 ` Stanislav Kholmanskikh
@ 2016-09-02 7:57 ` Stanislav Kholmanskikh
0 siblings, 0 replies; 4+ messages in thread
From: Stanislav Kholmanskikh @ 2016-09-02 7:57 UTC (permalink / raw)
To: ltp
On 08/31/2016 05:26 PM, Stanislav Kholmanskikh wrote:
> Hi!
>
> On 08/31/2016 04:34 PM, Cyril Hrubis wrote:
>> Hi!
>>> We don't handle the SIGVTALRM signal in cpuacct_task,
>>> so the process gets terminated, and it makes cpuacct.sh generate
>>> a "Virtual timer expired" message for each cpuacct_task process.
>>>
>>> In my opinion, these messages have no real value, and there is no
>>> need in keeping them in the output.
>>
>> I was just too lazy to install the signal handler in this case given
>> that the default action for the signal is to terminate the process...
>>
>>> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
>>> ---
>>> .../kernel/controllers/cpuacct/cpuacct_task.c | 23 +++++++++++++++++++-
>>> 1 files changed, 22 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/testcases/kernel/controllers/cpuacct/cpuacct_task.c b/testcases/kernel/controllers/cpuacct/cpuacct_task.c
>>> index 0ca01d1..94aa4e6 100644
>>> --- a/testcases/kernel/controllers/cpuacct/cpuacct_task.c
>>> +++ b/testcases/kernel/controllers/cpuacct/cpuacct_task.c
>>> @@ -25,11 +25,21 @@
>>> #include <sys/time.h>
>>> #include <sys/types.h>
>>> #include <unistd.h>
>>> +#include <signal.h>
>>> #include <stdlib.h>
>>> +#include <string.h>
>>> +
>>> +static volatile int got_signal;
>>> +
>>> +static void sig_handler(int signo)
>>> +{
>>> + got_signal = 1;
>>> +}
>>
>> We can as well do _exit(0); here. Then we don't have to add the global
>> variable and the main loop will stay as for (;;);
>
> Ok
>
>>
>>> int main(int argc, char **argv)
>>> {
>>> FILE *f;
>>> + struct sigaction sa;
>>>
>>> if (argc != 2) {
>>> fprintf(stderr, "Usage: %s /cgroup/.../tasks\n", argv[0]);
>>> @@ -44,9 +54,20 @@ int main(int argc, char **argv)
>>>
>>> fprintf(f, "%i\n", getpid());
>>> fclose(f);
>>> +
>>> + memset(&sa, 0, sizeof(sa));
>>> + sigemptyset(&sa.sa_mask);
>>> + sa.sa_handler = sig_handler;
>>> +
>>> + if (sigaction(SIGVTALRM, &sa, NULL)) {
>>> + perror("sigaction failed");
>>> + return 1;
>>> + }
>>
>> I would just use simpler signal(SIGVTALRM, sig_handler); here.
>
> I tend to use sigaction() everywhere, since the man page for signal()
> gives preference to sigaction() for the general case.
>
>>
>>> struct itimerval it = {.it_value = {.tv_sec = 0, .tv_usec = 10000}};
>>>
>>> setitimer(ITIMER_VIRTUAL, &it, NULL);
>>> - for (;;);
>>> + while (!got_signal)
>>> + ;
>>> return 0;
>>> }
>>
>> Other than that it's fine.
>>
>
Updated the patch to use _exit(0) and pushed it.
Thank you.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-09-02 7:57 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-31 10:21 [LTP] [PATCH] cpuacct_task: exit cleanly on SIGVTALRM Stanislav Kholmanskikh
2016-08-31 13:34 ` Cyril Hrubis
2016-08-31 14:26 ` Stanislav Kholmanskikh
2016-09-02 7:57 ` Stanislav Kholmanskikh
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.