All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH] syscalls/getpriority01: exclude default priority check for PRIO_USER.
@ 2016-11-17  6:02 Guangwen Feng
  2016-11-22  8:56 ` Cyril Hrubis
  0 siblings, 1 reply; 7+ messages in thread
From: Guangwen Feng @ 2016-11-17  6:02 UTC (permalink / raw)
  To: ltp

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

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 testcases/kernel/syscalls/getpriority/getpriority01.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/testcases/kernel/syscalls/getpriority/getpriority01.c b/testcases/kernel/syscalls/getpriority/getpriority01.c
index cca88a7..9a9a364 100644
--- a/testcases/kernel/syscalls/getpriority/getpriority01.c
+++ b/testcases/kernel/syscalls/getpriority/getpriority01.c
@@ -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;
-- 
1.8.4.2




^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [LTP] [PATCH] syscalls/getpriority01: exclude default priority check for PRIO_USER.
  2016-11-17  6:02 [LTP] [PATCH] syscalls/getpriority01: exclude default priority check for PRIO_USER Guangwen Feng
@ 2016-11-22  8:56 ` Cyril Hrubis
  2016-11-22  9:34   ` Guangwen Feng
  0 siblings, 1 reply; 7+ messages in thread
From: Cyril Hrubis @ 2016-11-22  8:56 UTC (permalink / raw)
  To: ltp

Hi!
> 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.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [LTP] [PATCH] syscalls/getpriority01: exclude default priority check for PRIO_USER.
  2016-11-22  8:56 ` Cyril Hrubis
@ 2016-11-22  9:34   ` Guangwen Feng
  2016-11-22  9:37     ` Cyril Hrubis
  0 siblings, 1 reply; 7+ messages in thread
From: Guangwen Feng @ 2016-11-22  9:34 UTC (permalink / raw)
  To: ltp

Hi!

Thanks for your review.

On 11/22/2016 04:56 PM, Cyril Hrubis wrote:
> Hi!
>> 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.

> 
> 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



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [LTP] [PATCH] syscalls/getpriority01: exclude default priority check for PRIO_USER.
  2016-11-22  9:34   ` Guangwen Feng
@ 2016-11-22  9:37     ` Cyril Hrubis
  2016-11-23  6:26       ` Guangwen Feng
  0 siblings, 1 reply; 7+ messages in thread
From: Cyril Hrubis @ 2016-11-22  9:37 UTC (permalink / raw)
  To: ltp

Hi!
> >> 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.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [LTP] [PATCH] syscalls/getpriority01: exclude default priority check for PRIO_USER.
  2016-11-22  9:37     ` Cyril Hrubis
@ 2016-11-23  6:26       ` Guangwen Feng
  2016-11-23 15:04         ` Cyril Hrubis
  0 siblings, 1 reply; 7+ messages in thread
From: Guangwen Feng @ 2016-11-23  6:26 UTC (permalink / raw)
  To: ltp

Hi!

On 11/22/2016 05:37 PM, Cyril Hrubis wrote:
> Hi!
>>>> 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.

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

> 
> 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.
> 



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [LTP] [PATCH] syscalls/getpriority01: exclude default priority check for PRIO_USER.
  2016-11-23  6:26       ` Guangwen Feng
@ 2016-11-23 15:04         ` Cyril Hrubis
  2016-11-24  7:02           ` Guangwen Feng
  0 siblings, 1 reply; 7+ messages in thread
From: Cyril Hrubis @ 2016-11-23 15:04 UTC (permalink / raw)
  To: ltp

Hi!
> >> 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.

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?

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [LTP] [PATCH] syscalls/getpriority01: exclude default priority check for PRIO_USER.
  2016-11-23 15:04         ` Cyril Hrubis
@ 2016-11-24  7:02           ` Guangwen Feng
  0 siblings, 0 replies; 7+ messages in thread
From: Guangwen Feng @ 2016-11-24  7:02 UTC (permalink / raw)
  To: ltp

Hi!

On 11/23/2016 11:04 PM, Cyril Hrubis wrote:
> Hi!
>>>> 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.
> 
> 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



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2016-11-24  7:02 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-17  6:02 [LTP] [PATCH] syscalls/getpriority01: exclude default priority check for PRIO_USER Guangwen Feng
2016-11-22  8:56 ` Cyril Hrubis
2016-11-22  9:34   ` Guangwen Feng
2016-11-22  9:37     ` Cyril Hrubis
2016-11-23  6:26       ` Guangwen Feng
2016-11-23 15:04         ` Cyril Hrubis
2016-11-24  7:02           ` Guangwen Feng

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.