Re: [PATCH v17 1/3] proc: add /proc/<pid>/arch_status

On 24.04.19 23:18, Thomas Gleixner wrote:

Hi,

quoted

+config PROC_PID_ARCH_STATUS
+	bool "Enable /proc/<pid>/arch_status file"

Why is this switchable? x86 selects it if PROC_FS is enabled and all other
architectures are absolutely not interested in this.

IMHO, it's good to have a switch, but that way doesn't make much sense.
Instead, I'd do it the other way round: make that switch depending on
those archs that actually support it. Something like this:

config PROC_PID_ARCH_STATUS
	bool "Enable /proc/<pid>/arch_status file"
	depends on PROC_FS
	depends on BROKEN

When x86 comes in, it would change to:

config PROC_PID_ARCH_STATUS
	bool "Enable /proc/<pid>/arch_status file"
	depends on PROC_FS
	depends on X86

And later arm coming in:

config PROC_PID_ARCH_STATUS
	bool "Enable /proc/<pid>/arch_status file"
	depends on PROC_FS
	depends on X86 || ARM

quoted

+	default n
+	help
+	  Provides a way to examine process architecture specific information.
+	  See <file:Documentation/filesystems/proc.txt> for more information.

Which contains zero information about this file when only this patch is
applied. 

hmm, the patch alone doesn't do anything useful anyway. it only becomes
useful with subsequent patches that add some arch. I wonder if there's
anything more useful to document at that point.

quoted

+/*
+ * Add support for task architecture specific output in /proc/pid/arch_status.
+ * task_arch_status() must be defined in asm/processor.h
+ */
+#ifdef CONFIG_PROC_PID_ARCH_STATUS
+# ifndef task_arch_status
+# define task_arch_status(m, task)
+# endif

What exactly is the point of this macro mess? If an architecture selects
CONFIG_PROC_PID_ARCH_STATUS then it has to provide proc_task_arch_status()
and the prototype should be in include/linux/proc_fs.h.

ACK.

quoted

+static int proc_pid_arch_status(struct seq_file *m, struct pid_namespace *ns,
+				struct pid *pid, struct task_struct *task)
+{
+	task_arch_status(m, task);
+	return 0;
+}

Is that wrapper really neeeded ?


--mtx

-- 
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287