Re: [PATCH 02/39] proc: introduce proc_create_seq{,_data}
On Thu, Apr 19, 2018 at 02:41:03PM +0200, Christoph Hellwig wrote:
> Variants of proc_create{,_data} that directly take a struct seq_operations
> argument and drastically reduces the boilerplate code in the callers.
> +static int proc_seq_open(struct inode *inode, struct file *file)
> +{
> + struct proc_dir_entry *de = PDE(inode);
> +
> + return seq_open(file, de->seq_ops);
> +}
> +
> +static const struct file_operations proc_seq_fops = {
> + .open = proc_seq_open,
> + .read = seq_read,
> + .llseek = seq_lseek,
> + .release= seq_release,
> +};
> +
> +struct proc_dir_entry *proc_create_seq_data(const char *name, umode_t mode,
> + struct proc_dir_entry *parent, const struct seq_operations *ops,
> + void *data)
> +{
> + struct proc_dir_entry *p;
> +
> + p = proc_create_data(name, mode, parent, &proc_seq_fops, data);
> + if (p)
> + p->seq_ops = ops;
> + return p;
> +}
Should be oopsable.
Once proc_create_data() returns, entry is live, ->open can be called.
> --- a/fs/proc/internal.h
> +++ b/fs/proc/internal.h
> @@ -44,6 +44,7 @@ struct proc_dir_entry {
> struct completion *pde_unload_completion;
> const struct inode_operations *proc_iops;
> const struct file_operations *proc_fops;
> + const struct seq_operations *seq_ops;
> void *data;
> unsigned int low_ino;
> nlink_t nlink;
"struct proc_dir_entry is 192/128 bytes now.
If someone knows how to pad array to certain size without union
please tell.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 14/39] proc: introduce proc_create_net_single
On Thu, Apr 19, 2018 at 02:41:15PM +0200, Christoph Hellwig wrote:
> Variant of proc_create_data that directly take a seq_file show
> +struct proc_dir_entry *proc_create_net_single(const char *name, umode_t mode,
> + struct proc_dir_entry *parent,
> + int (*show)(struct seq_file *, void *), void *data)
> +{
> + struct proc_dir_entry *p;
> +
> + p = proc_create_data(name, mode, parent, &proc_net_single_fops, data);
> + if (p)
> + p->single_show = show;
> + return p;
> +}
Ditto, should be oopsable.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 03/39] proc: introduce proc_create_seq_private
On Thu, Apr 19, 2018 at 02:41:04PM +0200, Christoph Hellwig wrote:
> Variant of proc_create_data that directly take a struct seq_operations
> --- a/fs/proc/internal.h
> +++ b/fs/proc/internal.h
> @@ -45,6 +45,7 @@ struct proc_dir_entry {
> const struct inode_operations *proc_iops;
> const struct file_operations *proc_fops;
> const struct seq_operations *seq_ops;
> + size_t state_size;
"unsigned int" please.
Where have you seen 4GB priv states?
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: simplify procfs code for seq_file instances
> git://git.infradead.org/users/hch/misc.git proc_create I want to ask if it is time to start using poorman function overloading with _b_c_e(). There are millions of allocation functions for example, all slightly difference, and people will add more. Seeing /proc interfaces doubled like this is painful. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: simplify procfs code for seq_file instances
On Tue, Apr 24, 2018 at 06:06:53PM +0200, Christoph Hellwig wrote: > On Tue, Apr 24, 2018 at 08:19:16AM -0700, Andrew Morton wrote: > > > > I want to ask if it is time to start using poorman function overloading > > > > with _b_c_e(). There are millions of allocation functions for example, > > > > all slightly difference, and people will add more. Seeing /proc > > > > interfaces > > > > doubled like this is painful. > > > > > > Function overloading is totally unacceptable. > > > > > > And I very much disagree with a tradeoff that keeps 5000 lines of > > > code vs a few new helpers. > > > > OK, the curiosity and suspense are killing me. What the heck is > > "function overloading with _b_c_e()"? > > The way I understood Alexey was to use have a proc_create macro > that can take different ops types. Although the short cut for > __builtin_types_compatible_p would be _b_t_c or similar, so maybe > I misunderstood him. That's correct. I also think that several dozens kmalloc signatures are a problem. And there will be more with pmalloc* stuff and more 2D/3D array checked allocations and who knows what. And I want to add typed kmalloc! ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: simplify procfs code for seq_file instances V2
On Wed, Apr 25, 2018 at 05:47:47PM +0200, Christoph Hellwig wrote:
> Changes since V1:
> - open code proc_create_data to avoid setting not fully initialized
>entries live
> - use unsigned int for state_size
Need this to maintain sizeof(struct proc_dir_entry):
Otherwise ACK fs/proc/ part.
diff --git a/fs/proc/internal.h b/fs/proc/internal.h
index 6d171485c45b..a318ae5b36b4 100644
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -48,8 +48,8 @@ struct proc_dir_entry {
const struct seq_operations *seq_ops;
int (*single_show)(struct seq_file *, void *);
};
- unsigned int state_size;
void *data;
+ unsigned int state_size;
unsigned int low_ino;
nlink_t nlink;
kuid_t uid;
@@ -62,9 +62,9 @@ struct proc_dir_entry {
umode_t mode;
u8 namelen;
#ifdef CONFIG_64BIT
-#define SIZEOF_PDE_INLINE_NAME (192-139)
+#define SIZEOF_PDE_INLINE_NAME (192-155)
#else
-#define SIZEOF_PDE_INLINE_NAME (128-87)
+#define SIZEOF_PDE_INLINE_NAME (128-95)
#endif
char inline_name[SIZEOF_PDE_INLINE_NAME];
} __randomize_layout;
diff --git a/fs/proc/proc_net.c b/fs/proc/proc_net.c
index baf1994289ce..7d94fa005b0d 100644
--- a/fs/proc/proc_net.c
+++ b/fs/proc/proc_net.c
@@ -40,7 +40,7 @@ static struct net *get_proc_net(const struct inode *inode)
static int seq_open_net(struct inode *inode, struct file *file)
{
- size_t state_size = PDE(inode)->state_size;
+ unsigned int state_size = PDE(inode)->state_size;
struct seq_net_private *p;
struct net *net;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: simplify procfs code for seq_file instances V2
On Sun, May 06, 2018 at 06:45:31PM +0100, Al Viro wrote:
> On Sun, May 06, 2018 at 08:19:49PM +0300, Alexey Dobriyan wrote:
> > @@ -62,9 +62,9 @@ struct proc_dir_entry {
> > umode_t mode;
> > u8 namelen;
> > #ifdef CONFIG_64BIT
> > -#define SIZEOF_PDE_INLINE_NAME (192-139)
> > +#define SIZEOF_PDE_INLINE_NAME (192-155)
> > #else
> > -#define SIZEOF_PDE_INLINE_NAME (128-87)
> > +#define SIZEOF_PDE_INLINE_NAME (128-95)
>
> > #endif
> > char inline_name[SIZEOF_PDE_INLINE_NAME];
> > } __randomize_layout;
>
> *UGH*
I agree. Maintaining these numbers is a pain point.
Who knew people were going to start adding fields right away.
> Both to the original state and that kind of "adjustments".
> Incidentally, with __bugger_layout in there these expressions
> are simply wrong.
Struct randomization is exempt from maintaining sizeof as they are already
screwing cachelines and everything. But if patch works with lockdep and
randomization -- even better.
> If nothing else, I would suggest turning the last one into
> char inline_name[];
> in hope that layout won't get... randomized that much and
> used
>
> #ifdef CONFIG_64BIT
> #define PDE_SIZE 192
> #else
> #define PDE_SIZE 128
> #endif
>
> union __proc_dir_entry {
> char pad[PDE_SIZE];
> struct proc_dir_entry real;
> };
>
> #define SIZEOF_PDE_INLINE_NAME (PDE_SIZE - offsetof(struct proc_dir_entry,
> inline_name))
>
> for constants, adjusted sizeof and sizeof_field when creating
> proc_dir_entry_cache and turned proc_root into
>
> union __proc_dir_entry __proc_root = { .real = {
> .low_ino= PROC_ROOT_INO,
> .namelen= 5,
> .mode = S_IFDIR | S_IRUGO | S_IXUGO,
> .nlink = 2,
> .refcnt = REFCOUNT_INIT(1),
> .proc_iops = &proc_root_inode_operations,
> .proc_fops = &proc_root_operations,
> .parent = &__proc_root.real,
> .subdir = RB_ROOT,
> .name = __proc_root.real.inline_name,
> .inline_name= "/proc",
> }};
>
> #define proc_root __proc_root.real
> (or actually used __proc_root.real in all of a 6 places where it remains).
>
> > - size_t state_size = PDE(inode)->state_size;
> > + unsigned int state_size = PDE(inode)->state_size;
>
>
> You and your "size_t is evil" crusade...
[nods]
unsigned long flags; /* error bits */
unsigned long i_state;
unsigned long s_blocksize;
unsigned long s_flags;
unsigned long s_iflags; /* internal SB_I_* flags */
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
