Re: getpriority() and top display for priority is inconsistent

Re: getpriority() and top display for priority is inconsistent

[Lavrentiev, Anton (NIH/NLM/NCBI) [C] via cygwin]

> You seem to have worked it out already so please send a patch in
> git format-patch foramt to the cygwin-patches mailing list.

I tried :-/

<cygwin-patches@cygwin.com>:
Sorry, only subscribers may post. (#5.7.2)

I do not use git to pull Cygwin sources.  The last snapshot (that corresponds to the last-known-stable release I'm allowed to use) is a little old,
but here goes the diff patch:

$ diff -p cygwin-snapshot-20181226-{0,1}/winsup/cygwin/fhandler_process.cc
*** cygwin-snapshot-20181226-0/winsup/cygwin/fhandler_process.cc        2018-08-17 14:41:04.000000000 -0400
--- cygwin-snapshot-20181226-1/winsup/cygwin/fhandler_process.cc        2019-08-06 17:05:35.421073900 -0400
*************** format_process_stat (void *data, char *&
*** 1066,1072 ****
    unsigned long fault_count = 0UL,
                vmsize = 0UL, vmrss = 0UL, vmmaxrss = 0UL;
    uint64_t utime = 0ULL, stime = 0ULL, start_time = 0ULL;
!   int priority = 0;
    if (p->process_state & PID_EXITED)
      strcpy (cmd, "<defunct>");
    else
--- 1066,1072 ----
    unsigned long fault_count = 0UL,
                vmsize = 0UL, vmrss = 0UL, vmmaxrss = 0UL;
    uint64_t utime = 0ULL, stime = 0ULL, start_time = 0ULL;
!
    if (p->process_state & PID_EXITED)
      strcpy (cmd, "<defunct>");
    else
*************** format_process_stat (void *data, char *&
*** 1095,1101 ****
    HANDLE hProcess;
    VM_COUNTERS vmc;
    KERNEL_USER_TIMES put;
-   PROCESS_BASIC_INFORMATION pbi;
    QUOTA_LIMITS ql;
    SYSTEM_TIMEOFDAY_INFORMATION stodi;
    SYSTEM_PROCESSOR_PERFORMANCE_INFORMATION spt;
--- 1095,1100 ----
*************** format_process_stat (void *data, char *&
*** 1115,1123 ****
      status = NtQueryInformationProcess (hProcess, ProcessTimes,
                                        (PVOID) &put, sizeof put, NULL);
    if (NT_SUCCESS (status))
-     status = NtQueryInformationProcess (hProcess, ProcessBasicInformation,
-                                       (PVOID) &pbi, sizeof pbi, NULL);
-   if (NT_SUCCESS (status))
      status = NtQueryInformationProcess (hProcess, ProcessQuotaLimits,
                                        (PVOID) &ql, sizeof ql, NULL);
    CloseHandle (hProcess);
--- 1114,1119 ----
*************** format_process_stat (void *data, char *&
*** 1138,1154 ****
    stime = put.KernelTime.QuadPart * CLOCKS_PER_SEC / NS100PERSEC;
    start_time = (put.CreateTime.QuadPart - stodi.BootTime.QuadPart)
               * CLOCKS_PER_SEC / NS100PERSEC;
-   /* The BasePriority returned to a 32 bit process under WOW64 is
-      apparently broken, for 32 and 64 bit target processes.  64 bit
-      processes get the correct base priority, even for 32 bit processes. */
-   if (wincap.is_wow64 ())
-     priority = 8; /* Default value. */
-   else
-     priority = pbi.BasePriority;
    unsigned page_size = wincap.page_size ();
    vmsize = vmc.PagefileUsage;
    vmrss = vmc.WorkingSetSize / page_size;
    vmmaxrss = ql.MaximumWorkingSetSize / page_size;

    destbuf = (char *) crealloc_abort (destbuf, strlen (cmd) + 320);
    return __small_sprintf (destbuf, "%d (%s) %c "
--- 1134,1144 ----
    stime = put.KernelTime.QuadPart * CLOCKS_PER_SEC / NS100PERSEC;
    start_time = (put.CreateTime.QuadPart - stodi.BootTime.QuadPart)
               * CLOCKS_PER_SEC / NS100PERSEC;
    unsigned page_size = wincap.page_size ();
    vmsize = vmc.PagefileUsage;
    vmrss = vmc.WorkingSetSize / page_size;
    vmmaxrss = ql.MaximumWorkingSetSize / page_size;
+   int nice = winprio_to_nice(GetPriorityClass(hProcess));

    destbuf = (char *) crealloc_abort (destbuf, strlen (cmd) + 320);
    return __small_sprintf (destbuf, "%d (%s) %c "
*************** format_process_stat (void *data, char *&
*** 1160,1166 ****
                          p->pid, cmd, state,
                          p->ppid, p->pgid, p->sid, p->ctty, -1,
                          0, fault_count, fault_count, 0, 0, utime, stime,
!                         utime, stime, priority, 0, 0, 0,
                          start_time, vmsize,
                          vmrss, vmmaxrss
                          );
--- 1150,1156 ----
                          p->pid, cmd, state,
                          p->ppid, p->pgid, p->sid, p->ctty, -1,
                          0, fault_count, fault_count, 0, 0, utime, stime,
!                         utime, stime, NZERO + nice, nice, 0, 0,
                          start_time, vmsize,
                          vmrss, vmmaxrss
                          );


--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: getpriority() and top display for priority is inconsistent

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 912 bytes --]

On Aug  7 00:45, Lavrentiev, Anton (NIH/NLM/NCBI) [C] via cygwin wrote:
> > You seem to have worked it out already so please send a patch in
> > git format-patch foramt to the cygwin-patches mailing list.
> 
> I tried :-/
> 
> <cygwin-patches@cygwin.com>:
> Sorry, only subscribers may post. (#5.7.2)

There are no subscribtion restrictions, you can simply subscribe.

> I do not use git to pull Cygwin sources.  The last snapshot (that
> corresponds to the last-known-stable release I'm allowed to use) is a
> little old, but here goes the diff patch:

This is much better with git, because of the commit message and
the fact that the diff is in the right format (-up).  You can send
the git format-patch output to this list if you don't want to 
subscribe to the (very low traffic) cygwin-patches ML.  Please give
it a try.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: getpriority() and top display for priority is inconsistent

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 740 bytes --]

On Aug  7 19:27, Lavrentiev, Anton (NIH/NLM/NCBI) [C] via cygwin wrote:
> > Please give it a try.
> 
> diff --git a/winsup/cygwin/fhandler_process.cc b/winsup/cygwin/fhandler_process.cc
> index 2a06554..2305b53 100644
> --- a/winsup/cygwin/fhandler_process.cc
> +++ b/winsup/cygwin/fhandler_process.cc
> @@ -1076,7 +1076,7 @@ format_process_stat (void *data, char *&destbuf)
>    unsigned long fault_count = 0UL,
>                 vmsize = 0UL, vmrss = 0UL, vmmaxrss = 0UL;
>    uint64_t utime = 0ULL, stime = 0ULL, start_time = 0ULL;
> -  int priority = 0;
> +  
> [...]

Close enough for a start, thanks.  I pushed your patch with a short
commit message.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: getpriority() and top display for priority is inconsistent

[Lavrentiev, Anton (NIH/NLM/NCBI) [C] via cygwin]

> Please give it a try.

diff --git a/winsup/cygwin/fhandler_process.cc b/winsup/cygwin/fhandler_process.cc
index 2a06554..2305b53 100644
--- a/winsup/cygwin/fhandler_process.cc
+++ b/winsup/cygwin/fhandler_process.cc
@@ -1076,7 +1076,7 @@ format_process_stat (void *data, char *&destbuf)
   unsigned long fault_count = 0UL,
                vmsize = 0UL, vmrss = 0UL, vmmaxrss = 0UL;
   uint64_t utime = 0ULL, stime = 0ULL, start_time = 0ULL;
-  int priority = 0;
+  
   if (p->process_state & PID_EXITED)
     strcpy (cmd, "<defunct>");
   else
@@ -1105,7 +1105,6 @@ format_process_stat (void *data, char *&destbuf)
   HANDLE hProcess;
   VM_COUNTERS vmc = { 0 };
   KERNEL_USER_TIMES put = { 0 };
-  PROCESS_BASIC_INFORMATION pbi = { 0 };
   QUOTA_LIMITS ql = { 0 };
   SYSTEM_TIMEOFDAY_INFORMATION stodi = { 0 };
 
@@ -1134,11 +1133,6 @@ format_process_stat (void *data, char *&destbuf)
       if (!NT_SUCCESS (status))
        debug_printf ("NtQueryInformationProcess(ProcessTimes): status %y",
                      status);
-      status = NtQueryInformationProcess (hProcess, ProcessBasicInformation,
-                                         (PVOID) &pbi, sizeof pbi, NULL);
-      if (!NT_SUCCESS (status))
-       debug_printf ("NtQueryInformationProcess(ProcessBasicInformation): "
-                     "status %y", status);
       status = NtQueryInformationProcess (hProcess, ProcessQuotaLimits,
                                          (PVOID) &ql, sizeof ql, NULL);
       if (!NT_SUCCESS (status))
@@ -1159,17 +1153,11 @@ format_process_stat (void *data, char *&destbuf)
                 * CLOCKS_PER_SEC / NS100PERSEC;
   else
     start_time = (p->start_time - to_time_t (&stodi.BootTime)) * CLOCKS_PER_SEC;
-  /* The BasePriority returned to a 32 bit process under WOW64 is
-     apparently broken, for 32 and 64 bit target processes.  64 bit
-     processes get the correct base priority, even for 32 bit processes. */
-  if (wincap.is_wow64 ())
-    priority = 8; /* Default value. */
-  else
-    priority = pbi.BasePriority;
   unsigned page_size = wincap.page_size ();
   vmsize = vmc.PagefileUsage;
   vmrss = vmc.WorkingSetSize / page_size;
   vmmaxrss = ql.MaximumWorkingSetSize / page_size;
+  int nice = winprio_to_nice(GetPriorityClass(hProcess));
 
   destbuf = (char *) crealloc_abort (destbuf, strlen (cmd) + 320);
   return __small_sprintf (destbuf, "%d (%s) %c "
@@ -1181,7 +1169,7 @@ format_process_stat (void *data, char *&destbuf)
                          p->pid, cmd, state,
                          p->ppid, p->pgid, p->sid, p->ctty, -1,
                          0, fault_count, fault_count, 0, 0, utime, stime,
-                         utime, stime, priority, 0, 0, 0,
+                         utime, stime, NZERO + nice, nice, 0, 0,
                          start_time, vmsize,
                          vmrss, vmmaxrss
                          );

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: getpriority() and top display for priority is inconsistent

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 1510 bytes --]

On Aug  6 18:54, Lavrentiev, Anton (NIH/NLM/NCBI) [C] via cygwin wrote:
> I have noticed a discrepancy between the process priority shown by
> "top" vs. what getpriority() returns.
> 
> I'm using the procps-based "top", so it reads the priority value from
> /proc/PID/stat.  The value gets there via code found in
> "fhandler_process.cc":
> 
>   /* The BasePriority returned to a 32 bit process under WOW64 is
>   apparently broken, for 32 and 64 bit target processes.  64 bit
>   processes get the correct base priority, even for 32 bit processes.
>   */ if (wincap.is_wow64 ()) priority = 8; /* Default value. */ else
>   priority = pbi.BasePriority;
> 
> But that's an inconsistent way of generating the value, because it is
> supposed to be the one that "getpriority()" returns.
> 
> Also, it looks like the higher value in "pbi.BasePriority" corresponds
> to a higher process priority, while Unix priority is higher when the
> value is less (20 - nice, generally).
> 
> It looks like it should have been done by calling a utility function,
> winprio_to_nice(GetPriorityClass(CurrentProcess())), and setting up
> both the priority field (as "NZERO + winprio_to_nice()") and the
> following "nice" field (which is currently set to permanent zero) with
> what winprio_to_nice() returns.

You seem to have worked it out already so please send a patch in
git format-patch foramt to the cygwin-patches mailing list.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

getpriority() and top display for priority is inconsistent

[Lavrentiev, Anton (NIH/NLM/NCBI) [C] via cygwin]

I have noticed a discrepancy between the process priority shown by "top" vs. what getpriority() returns.

I'm using the procps-based "top", so it reads the priority value from /proc/PID/stat.  The value gets there via code found in "fhandler_process.cc":

  /* The BasePriority returned to a 32 bit process under WOW64 is
     apparently broken, for 32 and 64 bit target processes.  64 bit
     processes get the correct base priority, even for 32 bit processes. */
  if (wincap.is_wow64 ())
    priority = 8; /* Default value. */
  else
    priority = pbi.BasePriority;

But that's an inconsistent way of generating the value, because it is supposed to be the one that "getpriority()" returns.

Also, it looks like the higher value in "pbi.BasePriority" corresponds to a higher process priority, while Unix priority is higher when the value is less
(20 - nice, generally).

It looks like it should have been done by calling a utility function, winprio_to_nice(GetPriorityClass(CurrentProcess())), and setting up both the priority
field (as "NZERO + winprio_to_nice()") and the following "nice" field (which is currently set to permanent zero) with what winprio_to_nice() returns.


--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple