Re: [PATCH v14 1/3] /proc/pid/status: Add support for architecture specific output
On 2019/4/11 9:02, Li, Aubrey wrote: > On 2019/4/10 22:54, Andy Lutomirski wrote: >> On Tue, Apr 9, 2019 at 8:40 PM Li, Aubrey wrote: >>> >>> On 2019/4/10 10:36, Li, Aubrey wrote: On 2019/4/10 10:25, Andy Lutomirski wrote: > On Tue, Apr 9, 2019 at 7:20 PM Li, Aubrey > wrote: >> >> On 2019/4/10 9:58, Andy Lutomirski wrote: >>> On Tue, Apr 9, 2019 at 6:55 PM 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 Cc: Peter Zijlstra Cc: Andi Kleen Cc: Tim Chen Cc: Dave Hansen Cc: Arjan van de Ven Cc: Linux API Cc: Alexey Dobriyan Cc: Andrew Morton --- 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 2edbb657f859..331592a61718 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) +{ +} >>> >>> This pointlessly bloats other architectures. Do this instead in an >>> appropriate header: >>> >>> #ifndef arch_proc_pid_status >>> static inline void arch_proc_pid_status(...) >>> { >>> } >>> #endif >>> >> >> I saw a bunch of similar weak functions, is it not acceptable? >> >> fs/proc$ grep weak *.c >> cpuinfo.c:__weak void arch_freq_prepare_all(void) >> meminfo.c:void __attribute__((weak)) arch_report_meminfo(struct seq_file >> *m) >> vmcore.c:int __weak elfcorehdr_alloc(unsigned long long *addr, unsigned >> long long *size) >> vmcore.c:void __weak elfcorehdr_free(unsigned long long addr) >> vmcore.c:ssize_t __weak elfcorehdr_read(char *buf, size_t count, u64 >> *ppos) >> vmcore.c:ssize_t __weak elfcorehdr_read_notes(char *buf, size_t count, >> u64 *ppos) >> vmcore.c:int __weak remap_oldmem_pfn_range(struct vm_area_struct *vma, >> vmcore.c:ssize_t __weak > > I think they're acceptable, but I don't personally like them. > okay, let me try to see if I can refine it in an appropriate way. >>> >>> Hi Andy, >>> >>> Is this what you want? >>> >>> Thanks, >>> -Aubrey >>> >>> >>> diff --git a/arch/x86/include/asm/processor.h >>> b/arch/x86/include/asm/processor.h >>> index 2bb3a648fc12..82d77d3aefff 100644 >>> --- a/arch/x86/include/asm/processor.h >>> +++ b/arch/x86/include/asm/processor.h >>> @@ -990,5 +990,8 @@ enum l1tf_mitigations { >>> }; >>> >>> extern enum l1tf_mitigations l1tf_mitigation; >>> +/* Add support for architecture specific output in /proc/pid/status */ >>> +void arch_proc_pid_status(struct seq_file *m, struct task_struct *task); >>> +#define arch_proc_pid_status arch_proc_pid_status >>> >>> #endif /* _ASM_X86_PROCESSOR_H */ >>> diff --git a/fs/proc/array.c b/fs/proc/array.c >>> index 2edbb657f859..fd65a6ba2864 100644 >>> --- a/fs/proc/array.c >>> +++ b/fs/proc/array.c >>> @@ -401,6 +401,11 @@ static inline void task_thp_status(struct seq_file *m, >>> struct mm_struct *mm) >>> seq_printf(m, "THP_enabled:\t%d\n", thp_enabled); >>> } >>> >>> +/* Add support for architecture specific output in /proc/pid/status */ >>> +#ifndef arch_proc_pid_status >>> +#define arch_proc_pid_status(m, task) >>> +#endif >>> + >>> int proc_pid_status(struct seq_file *m, struct pid_namespace *ns, >>> struct pid *pid, struct task_struct *task) >>> { >>> @@ -424,6 +429,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; >>> } >>> >> >> Yes. But I still think it would be nicer to separate the arch stuff >> into its own file. Others might reasonably disagree with me. >> > I like arch_status, I proposed but no other arch shows interesting in it. > > I think the problem is similar for x86_status, it does not make sense for > those x86 platform without AVX512 to have an empty arch file. I personally > don't like [arch]_status because the code may become unclean if more arches > added in future. > > Maybe it's too early to have a separated arch staff file for now. Hi Andy, Is it acceptable to you if I make the above change and post v15? Thanks, -Aubrey
Re: [PATCH v14 1/3] /proc/pid/status: Add support for architecture specific output
On 2019/4/10 22:54, Andy Lutomirski wrote: > On Tue, Apr 9, 2019 at 8:40 PM Li, Aubrey wrote: >> >> On 2019/4/10 10:36, Li, Aubrey wrote: >>> On 2019/4/10 10:25, Andy Lutomirski wrote: On Tue, Apr 9, 2019 at 7:20 PM Li, Aubrey wrote: > > On 2019/4/10 9:58, Andy Lutomirski wrote: >> On Tue, Apr 9, 2019 at 6:55 PM 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 >>> Cc: Peter Zijlstra >>> Cc: Andi Kleen >>> Cc: Tim Chen >>> Cc: Dave Hansen >>> Cc: Arjan van de Ven >>> Cc: Linux API >>> Cc: Alexey Dobriyan >>> Cc: Andrew Morton >>> --- >>> 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 2edbb657f859..331592a61718 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) >>> +{ >>> +} >> >> This pointlessly bloats other architectures. Do this instead in an >> appropriate header: >> >> #ifndef arch_proc_pid_status >> static inline void arch_proc_pid_status(...) >> { >> } >> #endif >> > > I saw a bunch of similar weak functions, is it not acceptable? > > fs/proc$ grep weak *.c > cpuinfo.c:__weak void arch_freq_prepare_all(void) > meminfo.c:void __attribute__((weak)) arch_report_meminfo(struct seq_file > *m) > vmcore.c:int __weak elfcorehdr_alloc(unsigned long long *addr, unsigned > long long *size) > vmcore.c:void __weak elfcorehdr_free(unsigned long long addr) > vmcore.c:ssize_t __weak elfcorehdr_read(char *buf, size_t count, u64 > *ppos) > vmcore.c:ssize_t __weak elfcorehdr_read_notes(char *buf, size_t count, > u64 *ppos) > vmcore.c:int __weak remap_oldmem_pfn_range(struct vm_area_struct *vma, > vmcore.c:ssize_t __weak I think they're acceptable, but I don't personally like them. >>> >>> okay, let me try to see if I can refine it in an appropriate way. >> >> Hi Andy, >> >> Is this what you want? >> >> Thanks, >> -Aubrey >> >> >> diff --git a/arch/x86/include/asm/processor.h >> b/arch/x86/include/asm/processor.h >> index 2bb3a648fc12..82d77d3aefff 100644 >> --- a/arch/x86/include/asm/processor.h >> +++ b/arch/x86/include/asm/processor.h >> @@ -990,5 +990,8 @@ enum l1tf_mitigations { >> }; >> >> extern enum l1tf_mitigations l1tf_mitigation; >> +/* Add support for architecture specific output in /proc/pid/status */ >> +void arch_proc_pid_status(struct seq_file *m, struct task_struct *task); >> +#define arch_proc_pid_status arch_proc_pid_status >> >> #endif /* _ASM_X86_PROCESSOR_H */ >> diff --git a/fs/proc/array.c b/fs/proc/array.c >> index 2edbb657f859..fd65a6ba2864 100644 >> --- a/fs/proc/array.c >> +++ b/fs/proc/array.c >> @@ -401,6 +401,11 @@ static inline void task_thp_status(struct seq_file *m, >> struct mm_struct *mm) >> seq_printf(m, "THP_enabled:\t%d\n", thp_enabled); >> } >> >> +/* Add support for architecture specific output in /proc/pid/status */ >> +#ifndef arch_proc_pid_status >> +#define arch_proc_pid_status(m, task) >> +#endif >> + >> int proc_pid_status(struct seq_file *m, struct pid_namespace *ns, >> struct pid *pid, struct task_struct *task) >> { >> @@ -424,6 +429,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; >> } >> > > Yes. But I still think it would be nicer to separate the arch stuff > into its own file. Others might reasonably disagree with me. > I like arch_status, I proposed but no other arch shows interesting in it. I think the problem is similar for x86_status, it does not make sense for those x86 platform without AVX512 to have an empty arch file. I personally don't like [arch]_status because the code may become unclean if more arches added in future. Maybe it's too early to have a separated arch staff file for now. Thanks, -Aubrey
Re: [PATCH v14 1/3] /proc/pid/status: Add support for architecture specific output
On Tue, Apr 9, 2019 at 8:40 PM Li, Aubrey wrote: > > On 2019/4/10 10:36, Li, Aubrey wrote: > > On 2019/4/10 10:25, Andy Lutomirski wrote: > >> On Tue, Apr 9, 2019 at 7:20 PM Li, Aubrey > >> wrote: > >>> > >>> On 2019/4/10 9:58, Andy Lutomirski wrote: > On Tue, Apr 9, 2019 at 6:55 PM 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 > > Cc: Peter Zijlstra > > Cc: Andi Kleen > > Cc: Tim Chen > > Cc: Dave Hansen > > Cc: Arjan van de Ven > > Cc: Linux API > > Cc: Alexey Dobriyan > > Cc: Andrew Morton > > --- > > 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 2edbb657f859..331592a61718 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) > > +{ > > +} > > This pointlessly bloats other architectures. Do this instead in an > appropriate header: > > #ifndef arch_proc_pid_status > static inline void arch_proc_pid_status(...) > { > } > #endif > > >>> > >>> I saw a bunch of similar weak functions, is it not acceptable? > >>> > >>> fs/proc$ grep weak *.c > >>> cpuinfo.c:__weak void arch_freq_prepare_all(void) > >>> meminfo.c:void __attribute__((weak)) arch_report_meminfo(struct seq_file > >>> *m) > >>> vmcore.c:int __weak elfcorehdr_alloc(unsigned long long *addr, unsigned > >>> long long *size) > >>> vmcore.c:void __weak elfcorehdr_free(unsigned long long addr) > >>> vmcore.c:ssize_t __weak elfcorehdr_read(char *buf, size_t count, u64 > >>> *ppos) > >>> vmcore.c:ssize_t __weak elfcorehdr_read_notes(char *buf, size_t count, > >>> u64 *ppos) > >>> vmcore.c:int __weak remap_oldmem_pfn_range(struct vm_area_struct *vma, > >>> vmcore.c:ssize_t __weak > >> > >> I think they're acceptable, but I don't personally like them. > >> > > > > okay, let me try to see if I can refine it in an appropriate way. > > Hi Andy, > > Is this what you want? > > Thanks, > -Aubrey > > > diff --git a/arch/x86/include/asm/processor.h > b/arch/x86/include/asm/processor.h > index 2bb3a648fc12..82d77d3aefff 100644 > --- a/arch/x86/include/asm/processor.h > +++ b/arch/x86/include/asm/processor.h > @@ -990,5 +990,8 @@ enum l1tf_mitigations { > }; > > extern enum l1tf_mitigations l1tf_mitigation; > +/* Add support for architecture specific output in /proc/pid/status */ > +void arch_proc_pid_status(struct seq_file *m, struct task_struct *task); > +#define arch_proc_pid_status arch_proc_pid_status > > #endif /* _ASM_X86_PROCESSOR_H */ > diff --git a/fs/proc/array.c b/fs/proc/array.c > index 2edbb657f859..fd65a6ba2864 100644 > --- a/fs/proc/array.c > +++ b/fs/proc/array.c > @@ -401,6 +401,11 @@ static inline void task_thp_status(struct seq_file *m, > struct mm_struct *mm) > seq_printf(m, "THP_enabled:\t%d\n", thp_enabled); > } > > +/* Add support for architecture specific output in /proc/pid/status */ > +#ifndef arch_proc_pid_status > +#define arch_proc_pid_status(m, task) > +#endif > + > int proc_pid_status(struct seq_file *m, struct pid_namespace *ns, > struct pid *pid, struct task_struct *task) > { > @@ -424,6 +429,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; > } > Yes. But I still think it would be nicer to separate the arch stuff into its own file. Others might reasonably disagree with me.
Re: [PATCH v14 1/3] /proc/pid/status: Add support for architecture specific output
On 2019/4/10 10:36, Li, Aubrey wrote: > On 2019/4/10 10:25, Andy Lutomirski wrote: >> On Tue, Apr 9, 2019 at 7:20 PM Li, Aubrey wrote: >>> >>> On 2019/4/10 9:58, Andy Lutomirski wrote: On Tue, Apr 9, 2019 at 6:55 PM 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 > Cc: Peter Zijlstra > Cc: Andi Kleen > Cc: Tim Chen > Cc: Dave Hansen > Cc: Arjan van de Ven > Cc: Linux API > Cc: Alexey Dobriyan > Cc: Andrew Morton > --- > 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 2edbb657f859..331592a61718 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) > +{ > +} This pointlessly bloats other architectures. Do this instead in an appropriate header: #ifndef arch_proc_pid_status static inline void arch_proc_pid_status(...) { } #endif >>> >>> I saw a bunch of similar weak functions, is it not acceptable? >>> >>> fs/proc$ grep weak *.c >>> cpuinfo.c:__weak void arch_freq_prepare_all(void) >>> meminfo.c:void __attribute__((weak)) arch_report_meminfo(struct seq_file *m) >>> vmcore.c:int __weak elfcorehdr_alloc(unsigned long long *addr, unsigned >>> long long *size) >>> vmcore.c:void __weak elfcorehdr_free(unsigned long long addr) >>> vmcore.c:ssize_t __weak elfcorehdr_read(char *buf, size_t count, u64 *ppos) >>> vmcore.c:ssize_t __weak elfcorehdr_read_notes(char *buf, size_t count, u64 >>> *ppos) >>> vmcore.c:int __weak remap_oldmem_pfn_range(struct vm_area_struct *vma, >>> vmcore.c:ssize_t __weak >> >> I think they're acceptable, but I don't personally like them. >> > > okay, let me try to see if I can refine it in an appropriate way. Hi Andy, Is this what you want? Thanks, -Aubrey diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h index 2bb3a648fc12..82d77d3aefff 100644 --- a/arch/x86/include/asm/processor.h +++ b/arch/x86/include/asm/processor.h @@ -990,5 +990,8 @@ enum l1tf_mitigations { }; extern enum l1tf_mitigations l1tf_mitigation; +/* Add support for architecture specific output in /proc/pid/status */ +void arch_proc_pid_status(struct seq_file *m, struct task_struct *task); +#define arch_proc_pid_status arch_proc_pid_status #endif /* _ASM_X86_PROCESSOR_H */ diff --git a/fs/proc/array.c b/fs/proc/array.c index 2edbb657f859..fd65a6ba2864 100644 --- a/fs/proc/array.c +++ b/fs/proc/array.c @@ -401,6 +401,11 @@ static inline void task_thp_status(struct seq_file *m, struct mm_struct *mm) seq_printf(m, "THP_enabled:\t%d\n", thp_enabled); } +/* Add support for architecture specific output in /proc/pid/status */ +#ifndef arch_proc_pid_status +#define arch_proc_pid_status(m, task) +#endif + int proc_pid_status(struct seq_file *m, struct pid_namespace *ns, struct pid *pid, struct task_struct *task) { @@ -424,6 +429,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; }
Re: [PATCH v14 1/3] /proc/pid/status: Add support for architecture specific output
On 2019/4/10 10:25, Andy Lutomirski wrote: > On Tue, Apr 9, 2019 at 7:20 PM Li, Aubrey wrote: >> >> On 2019/4/10 9:58, Andy Lutomirski wrote: >>> On Tue, Apr 9, 2019 at 6:55 PM 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 Cc: Peter Zijlstra Cc: Andi Kleen Cc: Tim Chen Cc: Dave Hansen Cc: Arjan van de Ven Cc: Linux API Cc: Alexey Dobriyan Cc: Andrew Morton --- 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 2edbb657f859..331592a61718 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) +{ +} >>> >>> This pointlessly bloats other architectures. Do this instead in an >>> appropriate header: >>> >>> #ifndef arch_proc_pid_status >>> static inline void arch_proc_pid_status(...) >>> { >>> } >>> #endif >>> >> >> I saw a bunch of similar weak functions, is it not acceptable? >> >> fs/proc$ grep weak *.c >> cpuinfo.c:__weak void arch_freq_prepare_all(void) >> meminfo.c:void __attribute__((weak)) arch_report_meminfo(struct seq_file *m) >> vmcore.c:int __weak elfcorehdr_alloc(unsigned long long *addr, unsigned long >> long *size) >> vmcore.c:void __weak elfcorehdr_free(unsigned long long addr) >> vmcore.c:ssize_t __weak elfcorehdr_read(char *buf, size_t count, u64 *ppos) >> vmcore.c:ssize_t __weak elfcorehdr_read_notes(char *buf, size_t count, u64 >> *ppos) >> vmcore.c:int __weak remap_oldmem_pfn_range(struct vm_area_struct *vma, >> vmcore.c:ssize_t __weak > > I think they're acceptable, but I don't personally like them. > okay, let me try to see if I can refine it in an appropriate way. >> >>> Or add /proc/PID/x86_status, which sounds better in most respects to me. >>> >> >> I didn't figure out how to make /proc/PID/x86_status invisible to other >> architectures in an appropriate way, do you have any suggestions? > > #ifdef CONFIG_X86? > I'm not sure if everyone like adding an arch #ifdef in a common file, please allow me to wait a while to see if there are other comments. Thanks, -Aubrey
Re: [PATCH v14 1/3] /proc/pid/status: Add support for architecture specific output
On Tue, Apr 9, 2019 at 7:20 PM Li, Aubrey wrote: > > On 2019/4/10 9:58, Andy Lutomirski wrote: > > On Tue, Apr 9, 2019 at 6:55 PM 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 > >> Cc: Peter Zijlstra > >> Cc: Andi Kleen > >> Cc: Tim Chen > >> Cc: Dave Hansen > >> Cc: Arjan van de Ven > >> Cc: Linux API > >> Cc: Alexey Dobriyan > >> Cc: Andrew Morton > >> --- > >> 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 2edbb657f859..331592a61718 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) > >> +{ > >> +} > > > > This pointlessly bloats other architectures. Do this instead in an > > appropriate header: > > > > #ifndef arch_proc_pid_status > > static inline void arch_proc_pid_status(...) > > { > > } > > #endif > > > > I saw a bunch of similar weak functions, is it not acceptable? > > fs/proc$ grep weak *.c > cpuinfo.c:__weak void arch_freq_prepare_all(void) > meminfo.c:void __attribute__((weak)) arch_report_meminfo(struct seq_file *m) > vmcore.c:int __weak elfcorehdr_alloc(unsigned long long *addr, unsigned long > long *size) > vmcore.c:void __weak elfcorehdr_free(unsigned long long addr) > vmcore.c:ssize_t __weak elfcorehdr_read(char *buf, size_t count, u64 *ppos) > vmcore.c:ssize_t __weak elfcorehdr_read_notes(char *buf, size_t count, u64 > *ppos) > vmcore.c:int __weak remap_oldmem_pfn_range(struct vm_area_struct *vma, > vmcore.c:ssize_t __weak I think they're acceptable, but I don't personally like them. > > > Or add /proc/PID/x86_status, which sounds better in most respects to me. > > > > I didn't figure out how to make /proc/PID/x86_status invisible to other > architectures in an appropriate way, do you have any suggestions? #ifdef CONFIG_X86?
Re: [PATCH v14 1/3] /proc/pid/status: Add support for architecture specific output
On 2019/4/10 9:58, Andy Lutomirski wrote: > On Tue, Apr 9, 2019 at 6:55 PM 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 >> Cc: Peter Zijlstra >> Cc: Andi Kleen >> Cc: Tim Chen >> Cc: Dave Hansen >> Cc: Arjan van de Ven >> Cc: Linux API >> Cc: Alexey Dobriyan >> Cc: Andrew Morton >> --- >> 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 2edbb657f859..331592a61718 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) >> +{ >> +} > > This pointlessly bloats other architectures. Do this instead in an > appropriate header: > > #ifndef arch_proc_pid_status > static inline void arch_proc_pid_status(...) > { > } > #endif > I saw a bunch of similar weak functions, is it not acceptable? fs/proc$ grep weak *.c cpuinfo.c:__weak void arch_freq_prepare_all(void) meminfo.c:void __attribute__((weak)) arch_report_meminfo(struct seq_file *m) vmcore.c:int __weak elfcorehdr_alloc(unsigned long long *addr, unsigned long long *size) vmcore.c:void __weak elfcorehdr_free(unsigned long long addr) vmcore.c:ssize_t __weak elfcorehdr_read(char *buf, size_t count, u64 *ppos) vmcore.c:ssize_t __weak elfcorehdr_read_notes(char *buf, size_t count, u64 *ppos) vmcore.c:int __weak remap_oldmem_pfn_range(struct vm_area_struct *vma, vmcore.c:ssize_t __weak > Or add /proc/PID/x86_status, which sounds better in most respects to me. > I didn't figure out how to make /proc/PID/x86_status invisible to other architectures in an appropriate way, do you have any suggestions? Thanks, -Aubrey
Re: [PATCH v14 1/3] /proc/pid/status: Add support for architecture specific output
On Tue, Apr 9, 2019 at 6:55 PM 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 > Cc: Peter Zijlstra > Cc: Andi Kleen > Cc: Tim Chen > Cc: Dave Hansen > Cc: Arjan van de Ven > Cc: Linux API > Cc: Alexey Dobriyan > Cc: Andrew Morton > --- > 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 2edbb657f859..331592a61718 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) > +{ > +} This pointlessly bloats other architectures. Do this instead in an appropriate header: #ifndef arch_proc_pid_status static inline void arch_proc_pid_status(...) { } #endif Or add /proc/PID/x86_status, which sounds better in most respects to me.