linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v9 1/3] /proc/pid/status: Add support for architecture specific output
@ 2019-02-11 18:59 Aubrey Li
  2019-02-11 18:59 ` [PATCH v9 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-11 18:59 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 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/proc/array.c b/fs/proc/array.c
index 0ceb3b6b37e7..d8cb5b5fd7bb 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -392,6 +392,10 @@ static inline void task_core_dumping(struct seq_file *m, struct mm_struct *mm)
 	seq_putc(m, '\n');
 }
 
+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)
 {
@@ -414,6 +418,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;
 }
 
-- 
2.17.1


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

* [PATCH v9 2/3] x86,/proc/pid/status: Add AVX-512 usage elapsed time
  2019-02-11 18:59 [PATCH v9 1/3] /proc/pid/status: Add support for architecture specific output Aubrey Li
@ 2019-02-11 18:59 ` Aubrey Li
  2019-02-12  8:22   ` Thomas Gleixner
  2019-02-11 18:59 ` [PATCH v9 3/3] Documentation/filesystems/proc.txt: add AVX512_elapsed_ms Aubrey Li
  2019-02-12  8:05 ` [PATCH v9 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-11 18:59 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.

Example:
$ cat /proc/pid/status | grep AVX512_elapsed_ms
AVX512_elapsed_ms:      1020

The number '1020' denotes 1020 millisecond elapsed since last time
context switch the off-CPU task using AVX-512 components, thus the
task could cause core frequency drop.

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

The number '-1' indicates the task didn't use AVX-512 components
before thus 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/include/asm/processor.h |  2 ++
 arch/x86/kernel/fpu/xstate.c     | 41 ++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index d53c54b842da..60ee932070fe 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -996,5 +996,7 @@ enum l1tf_mitigations {
 };
 
 extern enum l1tf_mitigations l1tf_mitigation;
+/* Add support for architecture specific output in /proc/pid/status */
+extern void arch_proc_pid_status(struct seq_file *m, struct task_struct *task);
 
 #endif /* _ASM_X86_PROCESSOR_H */
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 87a57b7642d3..dfd7cf847bc3 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -7,6 +7,7 @@
 #include <linux/cpu.h>
 #include <linux/mman.h>
 #include <linux/pkeys.h>
+#include <linux/seq_file.h>
 
 #include <asm/fpu/api.h>
 #include <asm/fpu/internal.h>
@@ -1245,3 +1246,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.
+ */
+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 v9 3/3] Documentation/filesystems/proc.txt: add AVX512_elapsed_ms
  2019-02-11 18:59 [PATCH v9 1/3] /proc/pid/status: Add support for architecture specific output Aubrey Li
  2019-02-11 18:59 ` [PATCH v9 2/3] x86,/proc/pid/status: Add AVX-512 usage elapsed time Aubrey Li
@ 2019-02-11 18:59 ` Aubrey Li
  2019-02-12  8:05 ` [PATCH v9 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-11 18:59 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 | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/filesystems/proc.txt b/Documentation/filesystems/proc.txt
index 520f6a84cf50..8275ccd766e4 100644
--- a/Documentation/filesystems/proc.txt
+++ b/Documentation/filesystems/proc.txt
@@ -197,6 +197,7 @@ read the file /proc/PID/status:
   Seccomp:        0
   voluntary_ctxt_switches:        0
   nonvoluntary_ctxt_switches:     1
+  AVX512_elapsed_ms:	1020
 
 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
@@ -214,7 +215,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.8)
+Table 1-2: Contents of the status files (as of 5.1)
 ..............................................................................
  Field                       Content
  Name                        filename of the executable
@@ -275,6 +276,7 @@ Table 1-2: Contents of the status files (as of 4.8)
  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 use in millisecond
 ..............................................................................
 
 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 v9 1/3] /proc/pid/status: Add support for architecture specific output
  2019-02-11 18:59 [PATCH v9 1/3] /proc/pid/status: Add support for architecture specific output Aubrey Li
  2019-02-11 18:59 ` [PATCH v9 2/3] x86,/proc/pid/status: Add AVX-512 usage elapsed time Aubrey Li
  2019-02-11 18:59 ` [PATCH v9 3/3] Documentation/filesystems/proc.txt: add AVX512_elapsed_ms Aubrey Li
@ 2019-02-12  8:05 ` Thomas Gleixner
  2019-02-12  8:15   ` Thomas Gleixner
  2 siblings, 1 reply; 14+ messages in thread
From: Thomas Gleixner @ 2019-02-12  8:05 UTC (permalink / raw)
  To: Aubrey Li
  Cc: mingo, peterz, hpa, ak, tim.c.chen, dave.hansen, arjan,
	linux-kernel, Aubrey Li

On Tue, 12 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>
> ---
>  fs/proc/array.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/fs/proc/array.c b/fs/proc/array.c
> index 0ceb3b6b37e7..d8cb5b5fd7bb 100644
> --- a/fs/proc/array.c
> +++ b/fs/proc/array.c
> @@ -392,6 +392,10 @@ static inline void task_core_dumping(struct seq_file *m, struct mm_struct *mm)
>  	seq_putc(m, '\n');
>  }
>  
> +void __weak arch_proc_pid_status(struct seq_file *m, struct task_struct *task)
> +{
> +}

This still lacks a prototype.

Thanks,

	tglx

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

* Re: [PATCH v9 1/3] /proc/pid/status: Add support for architecture specific output
  2019-02-12  8:05 ` [PATCH v9 1/3] /proc/pid/status: Add support for architecture specific output Thomas Gleixner
@ 2019-02-12  8:15   ` Thomas Gleixner
  0 siblings, 0 replies; 14+ messages in thread
From: Thomas Gleixner @ 2019-02-12  8:15 UTC (permalink / raw)
  To: Aubrey Li
  Cc: mingo, peterz, hpa, ak, tim.c.chen, dave.hansen, arjan,
	linux-kernel, Aubrey Li

On Tue, 12 Feb 2019, Thomas Gleixner wrote:

> On Tue, 12 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>
> > ---
> >  fs/proc/array.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/fs/proc/array.c b/fs/proc/array.c
> > index 0ceb3b6b37e7..d8cb5b5fd7bb 100644
> > --- a/fs/proc/array.c
> > +++ b/fs/proc/array.c
> > @@ -392,6 +392,10 @@ static inline void task_core_dumping(struct seq_file *m, struct mm_struct *mm)
> >  	seq_putc(m, '\n');
> >  }
> >  
> > +void __weak arch_proc_pid_status(struct seq_file *m, struct task_struct *task)
> > +{
> > +}
> 
> This still lacks a prototype.

Aside of that this doesn't apply to tip or Linus tree....

Thanks,

	tglx

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

* Re: [PATCH v9 2/3] x86,/proc/pid/status: Add AVX-512 usage elapsed time
  2019-02-11 18:59 ` [PATCH v9 2/3] x86,/proc/pid/status: Add AVX-512 usage elapsed time Aubrey Li
@ 2019-02-12  8:22   ` Thomas Gleixner
  2019-02-12  9:14     ` Li, Aubrey
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas Gleixner @ 2019-02-12  8:22 UTC (permalink / raw)
  To: Aubrey Li
  Cc: mingo, peterz, hpa, ak, tim.c.chen, dave.hansen, arjan,
	linux-kernel, Aubrey Li

On Tue, 12 Feb 2019, Aubrey Li wrote:
> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> index d53c54b842da..60ee932070fe 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -996,5 +996,7 @@ enum l1tf_mitigations {
>  };
>  
>  extern enum l1tf_mitigations l1tf_mitigation;
> +/* Add support for architecture specific output in /proc/pid/status */
> +extern void arch_proc_pid_status(struct seq_file *m, struct task_struct *task);

Sigh. This is absolutely the wrong place. The weak function is declared and
used in fs/proc/... So the prototype wants to be in a header which is
included from there independent of x86...

Thanks,

	tglx

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

* Re: [PATCH v9 2/3] x86,/proc/pid/status: Add AVX-512 usage elapsed time
  2019-02-12  8:22   ` Thomas Gleixner
@ 2019-02-12  9:14     ` Li, Aubrey
  2019-02-12  9:49       ` Li, Aubrey
  2019-02-12 11:19       ` Thomas Gleixner
  0 siblings, 2 replies; 14+ messages in thread
From: Li, Aubrey @ 2019-02-12  9:14 UTC (permalink / raw)
  To: Thomas Gleixner, Aubrey Li
  Cc: mingo, peterz, hpa, ak, tim.c.chen, dave.hansen, arjan, linux-kernel

On 2019/2/12 16:22, Thomas Gleixner wrote:
> On Tue, 12 Feb 2019, Aubrey Li wrote:
>> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
>> index d53c54b842da..60ee932070fe 100644
>> --- a/arch/x86/include/asm/processor.h
>> +++ b/arch/x86/include/asm/processor.h
>> @@ -996,5 +996,7 @@ enum l1tf_mitigations {
>>  };
>>  
>>  extern enum l1tf_mitigations l1tf_mitigation;
>> +/* Add support for architecture specific output in /proc/pid/status */
>> +extern void arch_proc_pid_status(struct seq_file *m, struct task_struct *task);
> 
> Sigh. This is absolutely the wrong place. The weak function is declared and
> used in fs/proc/... So the prototype wants to be in a header which is
> included from there independent of x86...

Can the prototype be in the architecture header if they want to call the function?
Like the following? arch_report_meminfo() is used in fs/proc/... as well.

$ find . -name *.h | xargs grep arch_report_meminfo
./arch/s390/include/asm/pgtable.h:void arch_report_meminfo(struct seq_file *m);
./arch/x86/include/asm/pgtable_types.h:extern void arch_report_meminfo(struct seq_file *m);
./arch/parisc/include/asm/pgtable.h:extern void arch_report_meminfo(struct seq_file *m);

Thanks,
-Aubrey

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

* Re: [PATCH v9 2/3] x86,/proc/pid/status: Add AVX-512 usage elapsed time
  2019-02-12  9:14     ` Li, Aubrey
@ 2019-02-12  9:49       ` Li, Aubrey
  2019-02-12 11:27         ` Thomas Gleixner
  2019-02-12 11:19       ` Thomas Gleixner
  1 sibling, 1 reply; 14+ messages in thread
From: Li, Aubrey @ 2019-02-12  9:49 UTC (permalink / raw)
  To: Thomas Gleixner, Aubrey Li
  Cc: mingo, peterz, hpa, ak, tim.c.chen, dave.hansen, arjan, linux-kernel

On 2019/2/12 17:14, Li, Aubrey wrote:
> On 2019/2/12 16:22, Thomas Gleixner wrote:
>> On Tue, 12 Feb 2019, Aubrey Li wrote:
>>> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
>>> index d53c54b842da..60ee932070fe 100644
>>> --- a/arch/x86/include/asm/processor.h
>>> +++ b/arch/x86/include/asm/processor.h
>>> @@ -996,5 +996,7 @@ enum l1tf_mitigations {
>>>  };
>>>  
>>>  extern enum l1tf_mitigations l1tf_mitigation;
>>> +/* Add support for architecture specific output in /proc/pid/status */
>>> +extern void arch_proc_pid_status(struct seq_file *m, struct task_struct *task);
>>
>> Sigh. This is absolutely the wrong place. The weak function is declared and
>> used in fs/proc/... So the prototype wants to be in a header which is
>> included from there independent of x86...
> 
> Can the prototype be in the architecture header if they want to call the function?
> Like the following? arch_report_meminfo() is used in fs/proc/... as well.
> 
> $ find . -name *.h | xargs grep arch_report_meminfo
> ./arch/s390/include/asm/pgtable.h:void arch_report_meminfo(struct seq_file *m);
> ./arch/x86/include/asm/pgtable_types.h:extern void arch_report_meminfo(struct seq_file *m);
> ./arch/parisc/include/asm/pgtable.h:extern void arch_report_meminfo(struct seq_file *m);
> 

Actually both way exist in the current kernel,the reason I chose to put the prototype
into architecture header file is that I found some architectures rename the function
name by a micro definition while others use prototype. See below:

$ find . -name *.h | xargs grep arch_irq_stat
./arch/arm64/include/asm/hardirq.h:#define arch_irq_stat_cpu	smp_irq_stat_cpu
./arch/arm/include/asm/hardirq.h:#define arch_irq_stat_cpu	smp_irq_stat_cpu
./arch/x86/include/asm/hardirq.h:extern u64 arch_irq_stat_cpu(unsigned int cpu);

This looks more flexible than it in the common header file.

Anyway, putting the prototype into the common header file like include/linux/proc_fs.h
is also acceptable to me if you persist, please just let me know, :)

Thanks,
-Aubrey

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

* Re: [PATCH v9 2/3] x86,/proc/pid/status: Add AVX-512 usage elapsed time
  2019-02-12  9:14     ` Li, Aubrey
  2019-02-12  9:49       ` Li, Aubrey
@ 2019-02-12 11:19       ` Thomas Gleixner
  2019-02-12 11:51         ` Li, Aubrey
  1 sibling, 1 reply; 14+ messages in thread
From: Thomas Gleixner @ 2019-02-12 11:19 UTC (permalink / raw)
  To: Li, Aubrey
  Cc: Aubrey Li, mingo, peterz, hpa, ak, tim.c.chen, dave.hansen,
	arjan, linux-kernel

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

On Tue, 12 Feb 2019, Li, Aubrey wrote:
> On 2019/2/12 16:22, Thomas Gleixner wrote:
> > On Tue, 12 Feb 2019, Aubrey Li wrote:
> >> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> >> index d53c54b842da..60ee932070fe 100644
> >> --- a/arch/x86/include/asm/processor.h
> >> +++ b/arch/x86/include/asm/processor.h
> >> @@ -996,5 +996,7 @@ enum l1tf_mitigations {
> >>  };
> >>  
> >>  extern enum l1tf_mitigations l1tf_mitigation;
> >> +/* Add support for architecture specific output in /proc/pid/status */
> >> +extern void arch_proc_pid_status(struct seq_file *m, struct task_struct *task);
> > 
> > Sigh. This is absolutely the wrong place. The weak function is declared and
> > used in fs/proc/... So the prototype wants to be in a header which is
> > included from there independent of x86...
> 
> Can the prototype be in the architecture header if they want to call the
> function?

Basic C programming course:

 The prototype must be available before the declaration of the global
 function.

fs/proc/array.c:404:13: warning: no previous prototype for ‘arch_proc_pid_status’ [-Wmissing-prototypes]
 void __weak arch_proc_pid_status(struct seq_file *m, struct task_struct *task)

Oh well....

Thanks,

	tglx

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

* Re: [PATCH v9 2/3] x86,/proc/pid/status: Add AVX-512 usage elapsed time
  2019-02-12  9:49       ` Li, Aubrey
@ 2019-02-12 11:27         ` Thomas Gleixner
  2019-02-12 11:52           ` Li, Aubrey
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas Gleixner @ 2019-02-12 11:27 UTC (permalink / raw)
  To: Li, Aubrey
  Cc: Aubrey Li, mingo, peterz, hpa, ak, tim.c.chen, dave.hansen,
	arjan, linux-kernel

On Tue, 12 Feb 2019, Li, Aubrey wrote:
> $ find . -name *.h | xargs grep arch_irq_stat
> ./arch/arm64/include/asm/hardirq.h:#define arch_irq_stat_cpu	smp_irq_stat_cpu
> ./arch/arm/include/asm/hardirq.h:#define arch_irq_stat_cpu	smp_irq_stat_cpu
> ./arch/x86/include/asm/hardirq.h:extern u64 arch_irq_stat_cpu(unsigned int cpu);
> 
> This looks more flexible than it in the common header file.

Looking more flexible does not make it more correct.

Thanks,

	tglx

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

* Re: [PATCH v9 2/3] x86,/proc/pid/status: Add AVX-512 usage elapsed time
  2019-02-12 11:19       ` Thomas Gleixner
@ 2019-02-12 11:51         ` Li, Aubrey
  2019-02-12 11:55           ` Thomas Gleixner
  0 siblings, 1 reply; 14+ messages in thread
From: Li, Aubrey @ 2019-02-12 11:51 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Aubrey Li, mingo, peterz, hpa, ak, tim.c.chen, dave.hansen,
	arjan, linux-kernel

On 2019/2/12 19:19, Thomas Gleixner wrote:
> On Tue, 12 Feb 2019, Li, Aubrey wrote:
>> On 2019/2/12 16:22, Thomas Gleixner wrote:
>>> On Tue, 12 Feb 2019, Aubrey Li wrote:
>>>> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
>>>> index d53c54b842da..60ee932070fe 100644
>>>> --- a/arch/x86/include/asm/processor.h
>>>> +++ b/arch/x86/include/asm/processor.h
>>>> @@ -996,5 +996,7 @@ enum l1tf_mitigations {
>>>>  };
>>>>  
>>>>  extern enum l1tf_mitigations l1tf_mitigation;
>>>> +/* Add support for architecture specific output in /proc/pid/status */
>>>> +extern void arch_proc_pid_status(struct seq_file *m, struct task_struct *task);
>>>
>>> Sigh. This is absolutely the wrong place. The weak function is declared and
>>> used in fs/proc/... So the prototype wants to be in a header which is
>>> included from there independent of x86...
>>
>> Can the prototype be in the architecture header if they want to call the
>> function?
> 
> Basic C programming course:
> 
>  The prototype must be available before the declaration of the global
>  function.
> 
> fs/proc/array.c:404:13: warning: no previous prototype for ‘arch_proc_pid_status’ [-Wmissing-prototypes]
>  void __weak arch_proc_pid_status(struct seq_file *m, struct task_struct *task)
> 
> Oh well....

Is this because patch 1/3 applied alone? If the whole patch set are applied,
the prototype is included in <asm/processor.h>, which is at the beginning of
array.c file, so it is available before the declaration.

Thanks,
-Aubrey

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

* Re: [PATCH v9 2/3] x86,/proc/pid/status: Add AVX-512 usage elapsed time
  2019-02-12 11:27         ` Thomas Gleixner
@ 2019-02-12 11:52           ` Li, Aubrey
  0 siblings, 0 replies; 14+ messages in thread
From: Li, Aubrey @ 2019-02-12 11:52 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Aubrey Li, mingo, peterz, hpa, ak, tim.c.chen, dave.hansen,
	arjan, linux-kernel

On 2019/2/12 19:27, Thomas Gleixner wrote:
> On Tue, 12 Feb 2019, Li, Aubrey wrote:
>> $ find . -name *.h | xargs grep arch_irq_stat
>> ./arch/arm64/include/asm/hardirq.h:#define arch_irq_stat_cpu	smp_irq_stat_cpu
>> ./arch/arm/include/asm/hardirq.h:#define arch_irq_stat_cpu	smp_irq_stat_cpu
>> ./arch/x86/include/asm/hardirq.h:extern u64 arch_irq_stat_cpu(unsigned int cpu);
>>
>> This looks more flexible than it in the common header file.
> 
> Looking more flexible does not make it more correct.

Okay, will cook a new version to put it into the common header.

Thanks,
-Aubrey

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

* Re: [PATCH v9 2/3] x86,/proc/pid/status: Add AVX-512 usage elapsed time
  2019-02-12 11:51         ` Li, Aubrey
@ 2019-02-12 11:55           ` Thomas Gleixner
  2019-02-12 12:02             ` Li, Aubrey
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas Gleixner @ 2019-02-12 11:55 UTC (permalink / raw)
  To: Li, Aubrey
  Cc: Aubrey Li, mingo, peterz, hpa, ak, tim.c.chen, dave.hansen,
	arjan, linux-kernel

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

On Tue, 12 Feb 2019, Li, Aubrey wrote:
> On 2019/2/12 19:19, Thomas Gleixner wrote:
> > On Tue, 12 Feb 2019, Li, Aubrey wrote:
> >> On 2019/2/12 16:22, Thomas Gleixner wrote:
> >>> On Tue, 12 Feb 2019, Aubrey Li wrote:
> >>>> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> >>>> index d53c54b842da..60ee932070fe 100644
> >>>> --- a/arch/x86/include/asm/processor.h
> >>>> +++ b/arch/x86/include/asm/processor.h
> >>>> @@ -996,5 +996,7 @@ enum l1tf_mitigations {
> >>>>  };
> >>>>  
> >>>>  extern enum l1tf_mitigations l1tf_mitigation;
> >>>> +/* Add support for architecture specific output in /proc/pid/status */
> >>>> +extern void arch_proc_pid_status(struct seq_file *m, struct task_struct *task);
> >>>
> >>> Sigh. This is absolutely the wrong place. The weak function is declared and
> >>> used in fs/proc/... So the prototype wants to be in a header which is
> >>> included from there independent of x86...
> >>
> >> Can the prototype be in the architecture header if they want to call the
> >> function?
> > 
> > Basic C programming course:
> > 
> >  The prototype must be available before the declaration of the global
> >  function.
> > 
> > fs/proc/array.c:404:13: warning: no previous prototype for ‘arch_proc_pid_status’ [-Wmissing-prototypes]
> >  void __weak arch_proc_pid_status(struct seq_file *m, struct task_struct *task)
> > 
> > Oh well....
> 
> Is this because patch 1/3 applied alone? If the whole patch set are applied,
> the prototype is included in <asm/processor.h>, which is at the beginning of
> array.c file, so it is available before the declaration.

1) Each patch has to be correct stand alone

2) This file is compiled for every architecture the kernel supports and how
   many of them are including arch/x86/include/asm/processor.h ?

   There is a world outside x86 and it's rather large.

Thanks,

	tglx

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

* Re: [PATCH v9 2/3] x86,/proc/pid/status: Add AVX-512 usage elapsed time
  2019-02-12 11:55           ` Thomas Gleixner
@ 2019-02-12 12:02             ` Li, Aubrey
  0 siblings, 0 replies; 14+ messages in thread
From: Li, Aubrey @ 2019-02-12 12:02 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Aubrey Li, mingo, peterz, hpa, ak, tim.c.chen, dave.hansen,
	arjan, linux-kernel

On 2019/2/12 19:55, Thomas Gleixner wrote:
> On Tue, 12 Feb 2019, Li, Aubrey wrote:
>> On 2019/2/12 19:19, Thomas Gleixner wrote:
>>> On Tue, 12 Feb 2019, Li, Aubrey wrote:
>>>> On 2019/2/12 16:22, Thomas Gleixner wrote:
>>>>> On Tue, 12 Feb 2019, Aubrey Li wrote:
>>>>>> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
>>>>>> index d53c54b842da..60ee932070fe 100644
>>>>>> --- a/arch/x86/include/asm/processor.h
>>>>>> +++ b/arch/x86/include/asm/processor.h
>>>>>> @@ -996,5 +996,7 @@ enum l1tf_mitigations {
>>>>>>  };
>>>>>>  
>>>>>>  extern enum l1tf_mitigations l1tf_mitigation;
>>>>>> +/* Add support for architecture specific output in /proc/pid/status */
>>>>>> +extern void arch_proc_pid_status(struct seq_file *m, struct task_struct *task);
>>>>>
>>>>> Sigh. This is absolutely the wrong place. The weak function is declared and
>>>>> used in fs/proc/... So the prototype wants to be in a header which is
>>>>> included from there independent of x86...
>>>>
>>>> Can the prototype be in the architecture header if they want to call the
>>>> function?
>>>
>>> Basic C programming course:
>>>
>>>  The prototype must be available before the declaration of the global
>>>  function.
>>>
>>> fs/proc/array.c:404:13: warning: no previous prototype for ‘arch_proc_pid_status’ [-Wmissing-prototypes]
>>>  void __weak arch_proc_pid_status(struct seq_file *m, struct task_struct *task)
>>>
>>> Oh well....
>>
>> Is this because patch 1/3 applied alone? If the whole patch set are applied,
>> the prototype is included in <asm/processor.h>, which is at the beginning of
>> array.c file, so it is available before the declaration.
> 
> 1) Each patch has to be correct stand alone
> 
> 2) This file is compiled for every architecture the kernel supports and how
>    many of them are including arch/x86/include/asm/processor.h ?
> 
>    There is a world outside x86 and it's rather large.

Got it, thanks!

> 
> Thanks,
> 
> 	tglx
> 


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

end of thread, other threads:[~2019-02-12 12:02 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-11 18:59 [PATCH v9 1/3] /proc/pid/status: Add support for architecture specific output Aubrey Li
2019-02-11 18:59 ` [PATCH v9 2/3] x86,/proc/pid/status: Add AVX-512 usage elapsed time Aubrey Li
2019-02-12  8:22   ` Thomas Gleixner
2019-02-12  9:14     ` Li, Aubrey
2019-02-12  9:49       ` Li, Aubrey
2019-02-12 11:27         ` Thomas Gleixner
2019-02-12 11:52           ` Li, Aubrey
2019-02-12 11:19       ` Thomas Gleixner
2019-02-12 11:51         ` Li, Aubrey
2019-02-12 11:55           ` Thomas Gleixner
2019-02-12 12:02             ` Li, Aubrey
2019-02-11 18:59 ` [PATCH v9 3/3] Documentation/filesystems/proc.txt: add AVX512_elapsed_ms Aubrey Li
2019-02-12  8:05 ` [PATCH v9 1/3] /proc/pid/status: Add support for architecture specific output Thomas Gleixner
2019-02-12  8:15   ` Thomas Gleixner

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