On Wed, 19 Feb 2020 22:11:27 +0300 Alexey Dobriyan <adobriyan@xxxxxxxxx> wrote:
> Now that "struct proc_ops" exist we can start putting there stuff which
> could not fly with VFS "struct file_operations"...
>
> Most of fs/proc/inode.c file is dedicated to make open/read/.../close reliable
> in the event of disappearing /proc entries which usually happens if module is
> getting removed. Files like /proc/cpuinfo which never disappear simply do not
> need such protection.
>
> Save 2 atomic ops, 1 allocation, 1 free per open/read/close sequence for such
> "permanent" files.
>
> Enable "permanent" flag for
>
> /proc/cpuinfo
> /proc/kmsg
> /proc/modules
> /proc/slabinfo
> /proc/stat
> /proc/sysvipc/*
> /proc/swaps
>
> More will come once I figure out foolproof way to prevent out module
> authors from marking their stuff "permanent" for performance reasons
> when it is not.
>
> This should help with scalability: benchmark is "read /proc/cpuinfo R times
> by N threads scattered over the system".
>
> N R t, s (before) t, s (after)
> -----------------------------------------------------
> 64 4096 1.582458 1.530502 -3.2%
> 256 4096 6.371926 6.125168 -3.9%
> 1024 4096 25.64888 24.47528 -4.6%
I guess that's significant.
> --- a/fs/proc/internal.h
> +++ b/fs/proc/internal.h
> @@ -61,6 +61,7 @@ struct proc_dir_entry {
> struct rb_node subdir_node;
> char *name;
> umode_t mode;
> + u8 flags;
Add a comment describing what this is?
> u8 namelen;
> char inline_name[];
> } __randomize_layout;
>
> ...
>
> --- a/include/linux/proc_fs.h
> +++ b/include/linux/proc_fs.h
> @@ -5,6 +5,7 @@
> #ifndef _LINUX_PROC_FS_H
> #define _LINUX_PROC_FS_H
>
> +#include <linux/compiler.h>
> #include <linux/types.h>
> #include <linux/fs.h>
>
> @@ -12,7 +13,21 @@ struct proc_dir_entry;
> struct seq_file;
> struct seq_operations;
>
> +enum {
> + /*
> + * All /proc entries using this ->proc_ops instance are never removed.
> + *
> + * If in doubt, ignore this flag.
> + */
> +#ifdef MODULE
> + PROC_ENTRY_PERMANENT = 0U,
> +#else
> + PROC_ENTRY_PERMANENT = 1U << 0,
> +#endif
> +};
That feels quite hacky. Is it really needed? Any module which uses
this is simply buggy?
Can we just leave this undefined if MODULE and break the build?
> struct proc_ops {
> + unsigned int proc_flags;
> int (*proc_open)(struct inode *, struct file *);
> ssize_t (*proc_read)(struct file *, char __user *, size_t, loff_t *);
> ssize_t (*proc_write)(struct file *, const char __user *, size_t, loff_t *);
> @@ -25,7 +40,7 @@ struct proc_ops {
> #endif
> int (*proc_mmap)(struct file *, struct vm_area_struct *);
> unsigned long (*proc_get_unmapped_area)(struct file *, unsigned long, unsigned long, unsigned long, unsigned long);
> -};
> +} __randomize_layout;
Unchangelogged, unrelated?
> #ifdef CONFIG_PROC_FS
>
