linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v13 1/3] /proc/pid/status: Add support for architecture specific output
@ 2019-02-24  4:43 Aubrey Li
  2019-02-24  4:43 ` [PATCH v13 2/3] x86,/proc/pid/status: Add AVX-512 usage elapsed time Aubrey Li
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Aubrey Li @ 2019-02-24  4:43 UTC (permalink / raw)
  To: tglx, mingo, peterz, hpa
  Cc: ak, tim.c.chen, dave.hansen, arjan, aubrey.li, linux-kernel, Aubrey Li

The architecture specific information of the running processes could
be useful to the userland. Add support to examine process architecture
specific information externally.

Signed-off-by: Aubrey Li <aubrey.li@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Arjan van de Ven <arjan@linux.intel.com>
---
 fs/proc/array.c         | 5 +++++
 include/linux/proc_fs.h | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/fs/proc/array.c b/fs/proc/array.c
index 9d428d5a0ac8..ea7a981f289c 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -401,6 +401,10 @@ static inline void task_thp_status(struct seq_file *m, struct mm_struct *mm)
 	seq_printf(m, "THP_enabled:\t%d\n", thp_enabled);
 }
 
+void __weak arch_proc_pid_status(struct seq_file *m, struct task_struct *task)
+{
+}
+
 int proc_pid_status(struct seq_file *m, struct pid_namespace *ns,
 			struct pid *pid, struct task_struct *task)
 {
@@ -424,6 +428,7 @@ int proc_pid_status(struct seq_file *m, struct pid_namespace *ns,
 	task_cpus_allowed(m, task);
 	cpuset_task_status_allowed(m, task);
 	task_context_switch_counts(m, task);
+	arch_proc_pid_status(m, task);
 	return 0;
 }
 
diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index d0e1f1522a78..1de9ba1b064f 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -73,6 +73,8 @@ struct proc_dir_entry *proc_create_net_single_write(const char *name, umode_t mo
 						    int (*show)(struct seq_file *, void *),
 						    proc_write_t write,
 						    void *data);
+/* Add support for architecture specific output in /proc/pid/status */
+extern void arch_proc_pid_status(struct seq_file *m, struct task_struct *task);
 
 #else /* CONFIG_PROC_FS */
 
-- 
2.17.1


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

* [PATCH v13 2/3] x86,/proc/pid/status: Add AVX-512 usage elapsed time
  2019-02-24  4:43 [PATCH v13 1/3] /proc/pid/status: Add support for architecture specific output Aubrey Li
@ 2019-02-24  4:43 ` Aubrey Li
  2019-04-05 20:27   ` Jann Horn
  2019-02-24  4:44 ` [PATCH v13 3/3] Documentation/filesystems/proc.txt: add AVX512_elapsed_ms Aubrey Li
  2019-04-05 19:32 ` [PATCH v13 1/3] /proc/pid/status: Add support for architecture specific output Thomas Gleixner
  2 siblings, 1 reply; 14+ messages in thread
From: Aubrey Li @ 2019-02-24  4:43 UTC (permalink / raw)
  To: tglx, mingo, peterz, hpa
  Cc: ak, tim.c.chen, dave.hansen, arjan, aubrey.li, linux-kernel, Aubrey Li

AVX-512 components use could cause core turbo frequency drop. So
it's useful to expose AVX-512 usage elapsed time as a heuristic hint
for the user space job scheduler to cluster the AVX-512 using tasks
together.

Tensorflow example:
$ while [ 1 ]; do cat /proc/pid/status | grep AVX; sleep 1; done
AVX512_elapsed_ms:	4
AVX512_elapsed_ms:	8
AVX512_elapsed_ms:	4

This means that 4 milliseconds have elapsed since the AVX512 usage
of tensorflow task was detected when the task was scheduled out.

Or:
$ cat /proc/pid/status | grep AVX512_elapsed_ms
AVX512_elapsed_ms:      -1

The number '-1' indicates that no AVX512 usage recorded before
thus the task unlikely has frequency drop issue.

User space tools may want to further check by:

$ perf stat --pid <pid> -e core_power.lvl2_turbo_license -- sleep 1

 Performance counter stats for process id '3558':

     3,251,565,961      core_power.lvl2_turbo_license

       1.004031387 seconds time elapsed

Non-zero counter value confirms that the task causes frequency drop.

Signed-off-by: Aubrey Li <aubrey.li@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Arjan van de Ven <arjan@linux.intel.com>
---
 arch/x86/kernel/fpu/xstate.c | 42 ++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 9cc108456d0b..e480a535eeb2 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -7,6 +7,8 @@
 #include <linux/cpu.h>
 #include <linux/mman.h>
 #include <linux/pkeys.h>
+#include <linux/seq_file.h>
+#include <linux/proc_fs.h>
 
 #include <asm/fpu/api.h>
 #include <asm/fpu/internal.h>
@@ -1243,3 +1245,43 @@ int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf)
 
 	return 0;
 }
+
+/*
+ * Report the amount of time elapsed in millisecond since last AVX512
+ * use in the task.
+ */
+static void avx512_status(struct seq_file *m, struct task_struct *task)
+{
+	unsigned long timestamp = task->thread.fpu.avx512_timestamp;
+	long delta;
+
+	if (!timestamp) {
+		/*
+		 * Report -1 if no AVX512 usage
+		 */
+		delta = -1;
+	} else {
+		delta = (long)(jiffies - timestamp);
+		/*
+		 * Cap to LONG_MAX if time difference > LONG_MAX
+		 */
+		if (delta < 0)
+			delta = LONG_MAX;
+		delta = jiffies_to_msecs(delta);
+	}
+
+	seq_put_decimal_ll(m, "AVX512_elapsed_ms:\t", delta);
+	seq_putc(m, '\n');
+}
+
+/*
+ * Report architecture specific information
+ */
+void arch_proc_pid_status(struct seq_file *m, struct task_struct *task)
+{
+	/*
+	 * Report AVX512 state if the processor and build option supported.
+	 */
+	if (cpu_feature_enabled(X86_FEATURE_AVX512F))
+		avx512_status(m, task);
+}
-- 
2.17.1


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

* [PATCH v13 3/3] Documentation/filesystems/proc.txt: add AVX512_elapsed_ms
  2019-02-24  4:43 [PATCH v13 1/3] /proc/pid/status: Add support for architecture specific output Aubrey Li
  2019-02-24  4:43 ` [PATCH v13 2/3] x86,/proc/pid/status: Add AVX-512 usage elapsed time Aubrey Li
@ 2019-02-24  4:44 ` Aubrey Li
  2019-04-05 19:32 ` [PATCH v13 1/3] /proc/pid/status: Add support for architecture specific output Thomas Gleixner
  2 siblings, 0 replies; 14+ messages in thread
From: Aubrey Li @ 2019-02-24  4:44 UTC (permalink / raw)
  To: tglx, mingo, peterz, hpa
  Cc: ak, tim.c.chen, dave.hansen, arjan, aubrey.li, linux-kernel, Aubrey Li

Added AVX512_elapsed_ms in /proc/<pid>/status. Report it
in Documentation/filesystems/proc.txt

Signed-off-by: Aubrey Li <aubrey.li@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Arjan van de Ven <arjan@linux.intel.com>
---
 Documentation/filesystems/proc.txt | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/Documentation/filesystems/proc.txt b/Documentation/filesystems/proc.txt
index 66cad5c86171..c4a9e48681ad 100644
--- a/Documentation/filesystems/proc.txt
+++ b/Documentation/filesystems/proc.txt
@@ -207,6 +207,7 @@ read the file /proc/PID/status:
   Speculation_Store_Bypass:       thread vulnerable
   voluntary_ctxt_switches:        0
   nonvoluntary_ctxt_switches:     1
+  AVX512_elapsed_ms:	8
 
 This shows you nearly the same information you would get if you viewed it with
 the ps  command.  In  fact,  ps  uses  the  proc  file  system  to  obtain its
@@ -224,7 +225,7 @@ asynchronous manner and the value may not be very precise. To see a precise
 snapshot of a moment, you can see /proc/<pid>/smaps file and scan page table.
 It's slow but very precise.
 
-Table 1-2: Contents of the status files (as of 4.19)
+Table 1-2: Contents of the status files (as of 5.1)
 ..............................................................................
  Field                       Content
  Name                        filename of the executable
@@ -289,6 +290,32 @@ Table 1-2: Contents of the status files (as of 4.19)
  Mems_allowed_list           Same as previous, but in "list format"
  voluntary_ctxt_switches     number of voluntary context switches
  nonvoluntary_ctxt_switches  number of non voluntary context switches
+ AVX512_elapsed_ms           time elapsed since last AVX512 usage recorded
+
+ AVX512_elapsed_ms:
+ ------------------
+  If AVX512 is supported on the machine, this entry shows the milliseconds
+  elapsed since the last time AVX512 usage was recorded. The recording
+  happens on a best effort basis when a task is scheduled out. This means
+  that the value depends on two factors:
+
+    1) The time which the task spent on the CPU without being scheduled
+       out. With CPU isolation and a single runnable task this can take
+       several seconds.
+
+    2) The time since the task was scheduled out last. Depending on the
+       reason for being scheduled out (time slice exhausted, syscall ...)
+       this can be arbitrary long time.
+
+  As a consequence the value cannot be considered precise and authoritative
+  information. The application which uses this information has to be aware
+  of the overall scenario on the system in order to determine whether a
+  task is a real AVX512 user or not.
+
+  A special value of '-1' indicates that no AVX512 usage was recorded, thus
+  the task is unlikely an AVX512 user, but depends on the workload and the
+  scheduling scenario, it also could be a false negative mentioned above.
+
 ..............................................................................
 
 Table 1-3: Contents of the statm files (as of 2.6.8-rc3)
-- 
2.17.1


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

* Re: [PATCH v13 1/3] /proc/pid/status: Add support for architecture specific output
  2019-02-24  4:43 [PATCH v13 1/3] /proc/pid/status: Add support for architecture specific output Aubrey Li
  2019-02-24  4:43 ` [PATCH v13 2/3] x86,/proc/pid/status: Add AVX-512 usage elapsed time Aubrey Li
  2019-02-24  4:44 ` [PATCH v13 3/3] Documentation/filesystems/proc.txt: add AVX512_elapsed_ms Aubrey Li
@ 2019-04-05 19:32 ` Thomas Gleixner
  2019-04-06 21:41   ` Alexey Dobriyan
  2019-04-07 17:34   ` Andy Lutomirski
  2 siblings, 2 replies; 14+ messages in thread
From: Thomas Gleixner @ 2019-04-05 19:32 UTC (permalink / raw)
  To: Aubrey Li
  Cc: mingo, Peter Zijlstra, H. Peter Anvin, ak, tim.c.chen,
	dave.hansen, arjan, aubrey.li, LKML, Linux API, Alexey Dobriyan,
	Andrew Morton

On Sun, 24 Feb 2019, Aubrey Li wrote:

> The architecture specific information of the running processes could
> be useful to the userland. Add support to examine process architecture
> specific information externally.
> 
> Signed-off-by: Aubrey Li <aubrey.li@linux.intel.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Andi Kleen <ak@linux.intel.com>
> Cc: Tim Chen <tim.c.chen@linux.intel.com>
> Cc: Dave Hansen <dave.hansen@intel.com>
> Cc: Arjan van de Ven <arjan@linux.intel.com>

This really lacks

Cc: Linux API <linux-api@vger.kernel.org>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>

Cc'ed now.

> ---
>  fs/proc/array.c         | 5 +++++
>  include/linux/proc_fs.h | 2 ++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/fs/proc/array.c b/fs/proc/array.c
> index 9d428d5a0ac8..ea7a981f289c 100644
> --- a/fs/proc/array.c
> +++ b/fs/proc/array.c
> @@ -401,6 +401,10 @@ static inline void task_thp_status(struct seq_file *m, struct mm_struct *mm)
>  	seq_printf(m, "THP_enabled:\t%d\n", thp_enabled);
>  }
>  
> +void __weak arch_proc_pid_status(struct seq_file *m, struct task_struct *task)
> +{
> +}
> +
>  int proc_pid_status(struct seq_file *m, struct pid_namespace *ns,
>  			struct pid *pid, struct task_struct *task)
>  {
> @@ -424,6 +428,7 @@ int proc_pid_status(struct seq_file *m, struct pid_namespace *ns,
>  	task_cpus_allowed(m, task);
>  	cpuset_task_status_allowed(m, task);
>  	task_context_switch_counts(m, task);
> +	arch_proc_pid_status(m, task);
>  	return 0;
>  }
>  
> diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
> index d0e1f1522a78..1de9ba1b064f 100644
> --- a/include/linux/proc_fs.h
> +++ b/include/linux/proc_fs.h
> @@ -73,6 +73,8 @@ struct proc_dir_entry *proc_create_net_single_write(const char *name, umode_t mo
>  						    int (*show)(struct seq_file *, void *),
>  						    proc_write_t write,
>  						    void *data);
> +/* Add support for architecture specific output in /proc/pid/status */
> +extern void arch_proc_pid_status(struct seq_file *m, struct task_struct *task);
>  
>  #else /* CONFIG_PROC_FS */
>  
> -- 
> 2.17.1
> 
> 

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

* Re: [PATCH v13 2/3] x86,/proc/pid/status: Add AVX-512 usage elapsed time
  2019-02-24  4:43 ` [PATCH v13 2/3] x86,/proc/pid/status: Add AVX-512 usage elapsed time Aubrey Li
@ 2019-04-05 20:27   ` Jann Horn
  2019-04-06 15:41     ` Li, Aubrey
  0 siblings, 1 reply; 14+ messages in thread
From: Jann Horn @ 2019-04-05 20:27 UTC (permalink / raw)
  To: Aubrey Li
  Cc: Thomas Gleixner, Ingo Molnar, Peter Zijlstra, H . Peter Anvin,
	Andi Kleen, tim.c.chen, Dave Hansen, Arjan van de Ven, aubrey.li,
	kernel list, Linux API, Alexey Dobriyan, Andrew Morton

On Fri, Apr 5, 2019 at 10:02 PM Aubrey Li <aubrey.li@linux.intel.com> wrote:
> AVX-512 components use could cause core turbo frequency drop. So
> it's useful to expose AVX-512 usage elapsed time as a heuristic hint
> for the user space job scheduler to cluster the AVX-512 using tasks
> together.
>
> Tensorflow example:
> $ while [ 1 ]; do cat /proc/pid/status | grep AVX; sleep 1; done
> AVX512_elapsed_ms:      4
> AVX512_elapsed_ms:      8
> AVX512_elapsed_ms:      4
>
> This means that 4 milliseconds have elapsed since the AVX512 usage
> of tensorflow task was detected when the task was scheduled out.
>
> Or:
> $ cat /proc/pid/status | grep AVX512_elapsed_ms
> AVX512_elapsed_ms:      -1

(Very nitpicky, feel free to ignore: If you change the /proc/pid to
/proc/tid in the commit message, it becomes clearer that this status
is really per-task/thread, not per-process/threadgroup.)

[...]
> +
> +/*
> + * Report the amount of time elapsed in millisecond since last AVX512
> + * use in the task.
> + */
> +static void avx512_status(struct seq_file *m, struct task_struct *task)
> +{
> +       unsigned long timestamp = task->thread.fpu.avx512_timestamp;

This is theoretically a data race, right? Should this have a READ_ONCE() on it?

Is there something that zeroes out the avx512_timestamp on
fork()/clone(), or will every child inherit the avx512 timestamp? As
far as I can tell, the timestamp is inherited; I think it would be
nicer to zero it out at that point. Either way, It might be worth
documenting this decision.

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

* Re: [PATCH v13 2/3] x86,/proc/pid/status: Add AVX-512 usage elapsed time
  2019-04-05 20:27   ` Jann Horn
@ 2019-04-06 15:41     ` Li, Aubrey
  0 siblings, 0 replies; 14+ messages in thread
From: Li, Aubrey @ 2019-04-06 15:41 UTC (permalink / raw)
  To: Jann Horn
  Cc: Thomas Gleixner, Ingo Molnar, Peter Zijlstra, H . Peter Anvin,
	Andi Kleen, tim.c.chen, Dave Hansen, Arjan van de Ven, aubrey.li,
	kernel list, Linux API, Alexey Dobriyan, Andrew Morton

On 2019/4/6 4:27, Jann Horn wrote:
> On Fri, Apr 5, 2019 at 10:02 PM Aubrey Li <aubrey.li@linux.intel.com> wrote:
>> AVX-512 components use could cause core turbo frequency drop. So
>> it's useful to expose AVX-512 usage elapsed time as a heuristic hint
>> for the user space job scheduler to cluster the AVX-512 using tasks
>> together.
>>
>> Tensorflow example:
>> $ while [ 1 ]; do cat /proc/pid/status | grep AVX; sleep 1; done
>> AVX512_elapsed_ms:      4
>> AVX512_elapsed_ms:      8
>> AVX512_elapsed_ms:      4
>>
>> This means that 4 milliseconds have elapsed since the AVX512 usage
>> of tensorflow task was detected when the task was scheduled out.
>>
>> Or:
>> $ cat /proc/pid/status | grep AVX512_elapsed_ms
>> AVX512_elapsed_ms:      -1
> 
> (Very nitpicky, feel free to ignore: If you change the /proc/pid to
> /proc/tid in the commit message, it becomes clearer that this status
> is really per-task/thread, not per-process/threadgroup.)

Thanks, I'll refine.

> 
> [...]
>> +
>> +/*
>> + * Report the amount of time elapsed in millisecond since last AVX512
>> + * use in the task.
>> + */
>> +static void avx512_status(struct seq_file *m, struct task_struct *task)
>> +{
>> +       unsigned long timestamp = task->thread.fpu.avx512_timestamp;
> 
> This is theoretically a data race, right? Should this have a READ_ONCE() on it?

Thanks, I'll refine.

> 
> Is there something that zeroes out the avx512_timestamp on
> fork()/clone(), or will every child inherit the avx512 timestamp? As
> far as I can tell, the timestamp is inherited; I think it would be
> nicer to zero it out at that point. Either way, It might be worth
> documenting this decision.
> 

This timestamp is not inherited, see below:

_do_fork()
->copy_process()
-->dup_task_struct()
--->arch_dup_task_struct()
---->fpu__copy()

Thanks,
-Aubrey

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

* Re: [PATCH v13 1/3] /proc/pid/status: Add support for architecture specific output
  2019-04-05 19:32 ` [PATCH v13 1/3] /proc/pid/status: Add support for architecture specific output Thomas Gleixner
@ 2019-04-06 21:41   ` Alexey Dobriyan
  2019-04-07 13:02     ` Li, Aubrey
  2019-04-07 17:34   ` Andy Lutomirski
  1 sibling, 1 reply; 14+ messages in thread
From: Alexey Dobriyan @ 2019-04-06 21:41 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Aubrey Li, mingo, Peter Zijlstra, H. Peter Anvin, ak, tim.c.chen,
	dave.hansen, arjan, aubrey.li, LKML, Linux API, Andrew Morton

On Fri, Apr 05, 2019 at 09:32:35PM +0200, Thomas Gleixner wrote:
> > +/* Add support for architecture specific output in /proc/pid/status */
> > +extern void arch_proc_pid_status(struct seq_file *m, struct task_struct *task);
     ^^^^^^

Unnecessary extern.

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

* Re: [PATCH v13 1/3] /proc/pid/status: Add support for architecture specific output
  2019-04-06 21:41   ` Alexey Dobriyan
@ 2019-04-07 13:02     ` Li, Aubrey
  2019-04-07 15:46       ` Alexey Dobriyan
  0 siblings, 1 reply; 14+ messages in thread
From: Li, Aubrey @ 2019-04-07 13:02 UTC (permalink / raw)
  To: Alexey Dobriyan, Thomas Gleixner
  Cc: mingo, Peter Zijlstra, H. Peter Anvin, ak, tim.c.chen,
	dave.hansen, arjan, aubrey.li, LKML, Linux API, Andrew Morton

On 2019/4/7 5:41, Alexey Dobriyan wrote:
> On Fri, Apr 05, 2019 at 09:32:35PM +0200, Thomas Gleixner wrote:
>>> +/* Add support for architecture specific output in /proc/pid/status */
>>> +extern void arch_proc_pid_status(struct seq_file *m, struct task_struct *task);
>      ^^^^^^
> 
> Unnecessary extern.
> 
The linkage is default extern, but with this functions and variables
can be treated the same way.

Is it mandatory not to use it explicitly? ./script/checkpatch.pl did
not report this.

Thanks,
-Aubrey

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

* Re: [PATCH v13 1/3] /proc/pid/status: Add support for architecture specific output
  2019-04-07 13:02     ` Li, Aubrey
@ 2019-04-07 15:46       ` Alexey Dobriyan
  2019-04-08  0:45         ` Li, Aubrey
  0 siblings, 1 reply; 14+ messages in thread
From: Alexey Dobriyan @ 2019-04-07 15:46 UTC (permalink / raw)
  To: Li, Aubrey
  Cc: Thomas Gleixner, mingo, Peter Zijlstra, H. Peter Anvin, ak,
	tim.c.chen, dave.hansen, arjan, aubrey.li, LKML, Linux API,
	Andrew Morton

On Sun, Apr 07, 2019 at 09:02:38PM +0800, Li, Aubrey wrote:
> On 2019/4/7 5:41, Alexey Dobriyan wrote:
> > On Fri, Apr 05, 2019 at 09:32:35PM +0200, Thomas Gleixner wrote:
> >>> +/* Add support for architecture specific output in /proc/pid/status */
> >>> +extern void arch_proc_pid_status(struct seq_file *m, struct task_struct *task);
> >      ^^^^^^
> > 
> > Unnecessary extern.
> > 
> The linkage is default extern, but with this functions and variables
> can be treated the same way.
> 
> Is it mandatory not to use it explicitly? ./script/checkpatch.pl did
> not report this.

"extern" is only necessary for variables. For prototypes, it is more typing
and more characters on the screen.

checkpatch.pl doesn't report it because opening floodgates is not the plan.

Yours,
	Extern Police.

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

* Re: [PATCH v13 1/3] /proc/pid/status: Add support for architecture specific output
  2019-04-05 19:32 ` [PATCH v13 1/3] /proc/pid/status: Add support for architecture specific output Thomas Gleixner
  2019-04-06 21:41   ` Alexey Dobriyan
@ 2019-04-07 17:34   ` Andy Lutomirski
  2019-04-08  0:38     ` Li, Aubrey
  1 sibling, 1 reply; 14+ messages in thread
From: Andy Lutomirski @ 2019-04-07 17:34 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Aubrey Li, Ingo Molnar, Peter Zijlstra, H. Peter Anvin,
	Andi Kleen, Tim Chen, Dave Hansen, Arjan van de Ven, aubrey.li,
	LKML, Linux API, Alexey Dobriyan, Andrew Morton

On Fri, Apr 5, 2019 at 12:32 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> On Sun, 24 Feb 2019, Aubrey Li wrote:
>
> > The architecture specific information of the running processes could
> > be useful to the userland. Add support to examine process architecture
> > specific information externally.
> >
> > Signed-off-by: Aubrey Li <aubrey.li@linux.intel.com>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Andi Kleen <ak@linux.intel.com>
> > Cc: Tim Chen <tim.c.chen@linux.intel.com>
> > Cc: Dave Hansen <dave.hansen@intel.com>
> > Cc: Arjan van de Ven <arjan@linux.intel.com>
>
> This really lacks
>
> Cc: Linux API <linux-api@vger.kernel.org>
> Cc: Alexey Dobriyan <adobriyan@gmail.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
>
> Cc'ed now.
>

I certainly understand why you want to expose this info, but would it
make more sense to instead add an arch_status file in /proc with
architecture-specific info?  Or maybe an x86_status field for x86
status, etc.

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

* Re: [PATCH v13 1/3] /proc/pid/status: Add support for architecture specific output
  2019-04-07 17:34   ` Andy Lutomirski
@ 2019-04-08  0:38     ` Li, Aubrey
  2019-04-08  1:52       ` Andy Lutomirski
  0 siblings, 1 reply; 14+ messages in thread
From: Li, Aubrey @ 2019-04-08  0:38 UTC (permalink / raw)
  To: Andy Lutomirski, Thomas Gleixner
  Cc: Ingo Molnar, Peter Zijlstra, H. Peter Anvin, Andi Kleen,
	Tim Chen, Dave Hansen, Arjan van de Ven, aubrey.li, LKML,
	Linux API, Alexey Dobriyan, Andrew Morton

On 2019/4/8 1:34, Andy Lutomirski wrote:
> On Fri, Apr 5, 2019 at 12:32 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>>
>> On Sun, 24 Feb 2019, Aubrey Li wrote:
>>
>>> The architecture specific information of the running processes could
>>> be useful to the userland. Add support to examine process architecture
>>> specific information externally.
>>>
>>> Signed-off-by: Aubrey Li <aubrey.li@linux.intel.com>
>>> Cc: Peter Zijlstra <peterz@infradead.org>
>>> Cc: Andi Kleen <ak@linux.intel.com>
>>> Cc: Tim Chen <tim.c.chen@linux.intel.com>
>>> Cc: Dave Hansen <dave.hansen@intel.com>
>>> Cc: Arjan van de Ven <arjan@linux.intel.com>
>>
>> This really lacks
>>
>> Cc: Linux API <linux-api@vger.kernel.org>
>> Cc: Alexey Dobriyan <adobriyan@gmail.com>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>
>> Cc'ed now.
>>
> 
> I certainly understand why you want to expose this info, but would it
> make more sense to instead add an arch_status file in /proc with
> architecture-specific info?  Or maybe an x86_status field for x86
> status, etc.
> 

I tried this, but no other architecture showed interest in arch_status
under /proc.

Thanks,
-Aubrey

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

* Re: [PATCH v13 1/3] /proc/pid/status: Add support for architecture specific output
  2019-04-07 15:46       ` Alexey Dobriyan
@ 2019-04-08  0:45         ` Li, Aubrey
  0 siblings, 0 replies; 14+ messages in thread
From: Li, Aubrey @ 2019-04-08  0:45 UTC (permalink / raw)
  To: Alexey Dobriyan
  Cc: Thomas Gleixner, mingo, Peter Zijlstra, H. Peter Anvin, ak,
	tim.c.chen, dave.hansen, arjan, aubrey.li, LKML, Linux API,
	Andrew Morton

On 2019/4/7 23:46, Alexey Dobriyan wrote:
> On Sun, Apr 07, 2019 at 09:02:38PM +0800, Li, Aubrey wrote:
>> On 2019/4/7 5:41, Alexey Dobriyan wrote:
>>> On Fri, Apr 05, 2019 at 09:32:35PM +0200, Thomas Gleixner wrote:
>>>>> +/* Add support for architecture specific output in /proc/pid/status */
>>>>> +extern void arch_proc_pid_status(struct seq_file *m, struct task_struct *task);
>>>      ^^^^^^
>>>
>>> Unnecessary extern.
>>>
>> The linkage is default extern, but with this functions and variables
>> can be treated the same way.
>>
>> Is it mandatory not to use it explicitly? ./script/checkpatch.pl did
>> not report this.
> 
> "extern" is only necessary for variables. For prototypes, it is more typing
> and more characters on the screen.
> 
> checkpatch.pl doesn't report it because opening floodgates is not the plan.
> 
> Yours,
> 	Extern Police.
>
 
No problem, Mr. Police, I can remove it in the next version.

Thanks,
-Aubrey

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

* Re: [PATCH v13 1/3] /proc/pid/status: Add support for architecture specific output
  2019-04-08  0:38     ` Li, Aubrey
@ 2019-04-08  1:52       ` Andy Lutomirski
  2019-04-08  2:33         ` Li, Aubrey
  0 siblings, 1 reply; 14+ messages in thread
From: Andy Lutomirski @ 2019-04-08  1:52 UTC (permalink / raw)
  To: Li, Aubrey
  Cc: Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Peter Zijlstra,
	H. Peter Anvin, Andi Kleen, Tim Chen, Dave Hansen,
	Arjan van de Ven, aubrey.li, LKML, Linux API, Alexey Dobriyan,
	Andrew Morton

On Sun, Apr 7, 2019 at 5:38 PM Li, Aubrey <aubrey.li@linux.intel.com> wrote:
>
> On 2019/4/8 1:34, Andy Lutomirski wrote:
> > On Fri, Apr 5, 2019 at 12:32 PM Thomas Gleixner <tglx@linutronix.de> wrote:
> >>
> >> On Sun, 24 Feb 2019, Aubrey Li wrote:
> >>
> >>> The architecture specific information of the running processes could
> >>> be useful to the userland. Add support to examine process architecture
> >>> specific information externally.
> >>>
> >>> Signed-off-by: Aubrey Li <aubrey.li@linux.intel.com>
> >>> Cc: Peter Zijlstra <peterz@infradead.org>
> >>> Cc: Andi Kleen <ak@linux.intel.com>
> >>> Cc: Tim Chen <tim.c.chen@linux.intel.com>
> >>> Cc: Dave Hansen <dave.hansen@intel.com>
> >>> Cc: Arjan van de Ven <arjan@linux.intel.com>
> >>
> >> This really lacks
> >>
> >> Cc: Linux API <linux-api@vger.kernel.org>
> >> Cc: Alexey Dobriyan <adobriyan@gmail.com>
> >> Cc: Andrew Morton <akpm@linux-foundation.org>
> >>
> >> Cc'ed now.
> >>
> >
> > I certainly understand why you want to expose this info, but would it
> > make more sense to instead add an arch_status file in /proc with
> > architecture-specific info?  Or maybe an x86_status field for x86
> > status, etc.
> >
>
> I tried this, but no other architecture showed interest in arch_status
> under /proc.
>

Why is that a problem?  It could exist on x86 and not exist on other
arches until needed.

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

* Re: [PATCH v13 1/3] /proc/pid/status: Add support for architecture specific output
  2019-04-08  1:52       ` Andy Lutomirski
@ 2019-04-08  2:33         ` Li, Aubrey
  0 siblings, 0 replies; 14+ messages in thread
From: Li, Aubrey @ 2019-04-08  2:33 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Thomas Gleixner, Ingo Molnar, Peter Zijlstra, H. Peter Anvin,
	Andi Kleen, Tim Chen, Dave Hansen, Arjan van de Ven, aubrey.li,
	LKML, Linux API, Alexey Dobriyan, Andrew Morton

On 2019/4/8 9:52, Andy Lutomirski wrote:
> On Sun, Apr 7, 2019 at 5:38 PM Li, Aubrey <aubrey.li@linux.intel.com> wrote:
>>
>> On 2019/4/8 1:34, Andy Lutomirski wrote:
>>> On Fri, Apr 5, 2019 at 12:32 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>>>>
>>>> On Sun, 24 Feb 2019, Aubrey Li wrote:
>>>>
>>>>> The architecture specific information of the running processes could
>>>>> be useful to the userland. Add support to examine process architecture
>>>>> specific information externally.
>>>>>
>>>>> Signed-off-by: Aubrey Li <aubrey.li@linux.intel.com>
>>>>> Cc: Peter Zijlstra <peterz@infradead.org>
>>>>> Cc: Andi Kleen <ak@linux.intel.com>
>>>>> Cc: Tim Chen <tim.c.chen@linux.intel.com>
>>>>> Cc: Dave Hansen <dave.hansen@intel.com>
>>>>> Cc: Arjan van de Ven <arjan@linux.intel.com>
>>>>
>>>> This really lacks
>>>>
>>>> Cc: Linux API <linux-api@vger.kernel.org>
>>>> Cc: Alexey Dobriyan <adobriyan@gmail.com>
>>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>>>
>>>> Cc'ed now.
>>>>
>>>
>>> I certainly understand why you want to expose this info, but would it
>>> make more sense to instead add an arch_status file in /proc with
>>> architecture-specific info?  Or maybe an x86_status field for x86
>>> status, etc.
>>>
>>
>> I tried this, but no other architecture showed interest in arch_status
>> under /proc.
>>
> 
> Why is that a problem?  It could exist on x86 and not exist on other
> arches until needed.
> 

I placed it in tid_base_stuff, under live_patch entry, so it exists for
all arches, is there a better way to do this?

Thanks,
-Aubrey

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

end of thread, other threads:[~2019-04-08  2:33 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-24  4:43 [PATCH v13 1/3] /proc/pid/status: Add support for architecture specific output Aubrey Li
2019-02-24  4:43 ` [PATCH v13 2/3] x86,/proc/pid/status: Add AVX-512 usage elapsed time Aubrey Li
2019-04-05 20:27   ` Jann Horn
2019-04-06 15:41     ` Li, Aubrey
2019-02-24  4:44 ` [PATCH v13 3/3] Documentation/filesystems/proc.txt: add AVX512_elapsed_ms Aubrey Li
2019-04-05 19:32 ` [PATCH v13 1/3] /proc/pid/status: Add support for architecture specific output Thomas Gleixner
2019-04-06 21:41   ` Alexey Dobriyan
2019-04-07 13:02     ` Li, Aubrey
2019-04-07 15:46       ` Alexey Dobriyan
2019-04-08  0:45         ` Li, Aubrey
2019-04-07 17:34   ` Andy Lutomirski
2019-04-08  0:38     ` Li, Aubrey
2019-04-08  1:52       ` Andy Lutomirski
2019-04-08  2:33         ` Li, Aubrey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).