syscalls/getpriority01: exclude default priority check for PRIO_USER.

PATCH New Guangwen Feng 2 participants 7 messages Nov 17, 2016 – Nov 24, 2016

lore

Share & tools

lore

Guangwen Feng

author

2016-11-17 06:02 UTC #0 lore raw

PATCH 1/1 syscalls/getpriority01: exclude default priority check for PRIO_USER.

The current user priority is various when testing, we should not check it as default value.

Signed-off-by Guangwen Feng <[email protected]>

---
 testcases/kernel/syscalls/getpriority/getpriority01.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

testcases/kernel/syscalls/getpriority/getpriority01.c +7 -5

index cca88a7..9a9a364 100644
@@ -19,7 +19,8 @@
 
 /*
  * Verify that getpriority(2) succeeds get the scheduling priority of
- * the current process, process group or user.
+ * the current process, process group or user, the default priority
+ * of process and process group is 0.
  */
 
 #include <errno.h>

@@ -29,10 +30,11 @@
 
 static struct tcase {
 	int which;
+	int defprio_strict;
 } tcases[] = {
-	{PRIO_PROCESS},
-	{PRIO_PGRP},
-	{PRIO_USER}
+	{PRIO_PROCESS, 1},
+	{PRIO_PGRP, 1},
+	{PRIO_USER, 0}
 };
 
 static void verify_getpriority(unsigned int n)

@@ -41,7 +43,7 @@ static void verify_getpriority(unsigned int n)
 
 	TEST(getpriority(tc->which, 0));
 
-	if (TEST_RETURN != 0 || TEST_ERRNO != 0) {
+	if ((tc->defprio_strict && TEST_RETURN != 0) || TEST_ERRNO != 0) {
 		tst_res(TFAIL | TTERRNO, "getpriority(%d, 0) returned %ld",
 			tc->which, TEST_RETURN);
 		return;

Signature · 1 line

1.8.4.2

Cyril Hrubis

2016-11-22 08:56 UTC #1 lore raw

PATCH 1/1 syscalls/getpriority01: exclude default priority check for PRIO_USER.

Hi!

Quoted 2 lines

The current user priority is various when testing, we should not check it as default value.

What system does this happen on? It should probably be mentioned in the commit message.

And we should probably as well just revert the change I've did and check only that errno wasn't set since the value may be set even to -1.

Signature · 2 lines

Cyril Hrubis
[email protected]

Guangwen Feng

author

2016-11-22 09:34 UTC #2 lore raw

PATCH 1/1 syscalls/getpriority01: exclude default priority check for PRIO_USER.

Hi!

Thanks for your review.

Quoted 6 lines · Cyril Hrubis

Hi!
Quoted 2 lines
The current user priority is various when testing, we should not check it as default value.
What system does this happen on? It should probably be mentioned in the commit message.

It happens on Fedora20, RHEL5.11GA, RHEL6.8GA, RNEL7.3GA, testing returns -20 and -11 for PRIO_USER as root and nobody respectively.

Quoted 3 lines

And we should probably as well just revert the change I've did and check only that errno wasn't set since the value may be set even to -1.

I think so, checking "TEST_ERRNO != 0" is adequate for the case.

Best Regards,
Guangwen Feng

Cyril Hrubis

2016-11-22 09:37 UTC #3 lore raw

PATCH 1/1 syscalls/getpriority01: exclude default priority check for PRIO_USER.

Hi!

Quoted 8 lines

Quoted 5 lines
Quoted 2 lines
The current user priority is various when testing, we should not check it as default value.
What system does this happen on? It should probably be mentioned in the commit message.
It happens on Fedora20, RHEL5.11GA, RHEL6.8GA, RNEL7.3GA, testing returns -20 and -11 for PRIO_USER as root and nobody respectively.

Hmm, are you sure that this is not a bug? Since -20 is the highest priority for a process, it does not make much sense to run all root processes like that.

Also the nice value gets inherited via fork, so it may be that if you are connecting to the machine via ssh the ssh process gets priority so that the machine stays responsive even under load, which is inherited by the shell and then inherited by the test. But even then the -20 sounds a bit too much.

Signature · 2 lines

Cyril Hrubis
[email protected]

Guangwen Feng

author

2016-11-23 06:26 UTC #4 lore raw

PATCH 1/1 syscalls/getpriority01: exclude default priority check for PRIO_USER.

Hi!

Quoted 7 lines · Cyril Hrubis

Hi!
Quoted 6 lines
Quoted 5 lines
Quoted 2 lines
The current user priority is various when testing, we should not check it as default value.
What system does this happen on? It should probably be mentioned in the commit message.

Quoted 6 lines

Quoted 2 lines
It happens on Fedora20, RHEL5.11GA, RHEL6.8GA, RNEL7.3GA, testing returns -20 and -11 for PRIO_USER as root and nobody respectively.
Hmm, are you sure that this is not a bug? Since -20 is the highest priority for a process, it does not make much sense to run all root processes like that.

Yes, not all root processes are -20 on my system, but sorry, I am afraid it's not a bug, because man page says that the getpriority() call returns the highest priority (lowest numerical value) enjoyed by any of the specified processes, so if only there is one process is -20, the getpriority() will return -20.

Best Regards,
Guangwen Feng

Quoted 7 lines

Also the nice value gets inherited via fork, so it may be that if you are connecting to the machine via ssh the ssh process gets priority so that the machine stays responsive even under load, which is inherited by the shell and then inherited by the test. But even then the -20 sounds a bit too much.

Cyril Hrubis

2016-11-23 15:04 UTC #5 lore raw

PATCH 1/1 syscalls/getpriority01: exclude default priority check for PRIO_USER.

Hi!

Quoted 7 lines

Quoted 6 lines
Quoted 2 lines
It happens on Fedora20, RHEL5.11GA, RHEL6.8GA, RNEL7.3GA, testing returns -20 and -11 for PRIO_USER as root and nobody respectively.
Hmm, are you sure that this is not a bug? Since -20 is the highest priority for a process, it does not make much sense to run all root processes like that.

Quoted 5 lines

Yes, not all root processes are -20 on my system, but sorry, I am afraid it's not a bug, because man page says that the getpriority() call returns the highest priority (lowest numerical value) enjoyed by any of the specified processes, so if only there is one process is -20, the getpriority() will return -20.

Ah, my bad, sure we can't really say if there is a process that belongs to the same user with priority lower than 0.

So what about we define a range in which the return value should be and set it to 0 .. 0 for PRIO_PROCESS and PRIO_PGRP and to -20 .. 0 for PRIO_USER?

Signature · 2 lines

Cyril Hrubis
[email protected]

Guangwen Feng

author

2016-11-24 07:02 UTC #6 lore raw

PATCH 1/1 syscalls/getpriority01: exclude default priority check for PRIO_USER.

Hi!

Quoted 14 lines · Cyril Hrubis

Hi!
Quoted 7 lines
Quoted 6 lines
Quoted 2 lines
It happens on Fedora20, RHEL5.11GA, RHEL6.8GA, RNEL7.3GA, testing returns -20 and -11 for PRIO_USER as root and nobody respectively.
Hmm, are you sure that this is not a bug? Since -20 is the highest priority for a process, it does not make much sense to run all root processes like that.
Quoted 5 lines
Yes, not all root processes are -20 on my system, but sorry, I am afraid it's not a bug, because man page says that the getpriority() call returns the highest priority (lowest numerical value) enjoyed by any of the specified processes, so if only there is one process is -20, the getpriority() will return -20.

Quoted 6 lines

Ah, my bad, sure we can't really say if there is a process that belongs to the same user with priority lower than 0.
So what about we define a range in which the return value should be and set it to 0 .. 0 for PRIO_PROCESS and PRIO_PGRP and to -20 .. 0 for PRIO_USER?

Sounds good, I will rewrite the patch, thanks.

Best Regards,
Guangwen Feng