On Wed, Mar 25, 2020 at 09:00:15PM +0300, Alexey Dobriyan wrote: > On Tue, Mar 24, 2020 at 02:21:59PM -0700, Linus Torvalds wrote: > > On Tue, Mar 24, 2020 at 1:46 PM Alexey Gladkov <gladkov.alexey@xxxxxxxxx> wrote: > > > > > > +/* definitions for hide_pid field */ > > > +enum { > > > + HIDEPID_OFF = 0, > > > + HIDEPID_NO_ACCESS = 1, > > > + HIDEPID_INVISIBLE = 2, > > > +}; > > > > Should this enum be named... > > > > > struct proc_fs_info { > > > struct pid_namespace *pid_ns; > > > struct dentry *proc_self; /* For /proc/self */ > > > struct dentry *proc_thread_self; /* For /proc/thread-self */ > > > + kgid_t pid_gid; > > > + int hide_pid; > > > }; > > > > .. and then used here instead of "int"? > > > > Same goes for 'struct proc_fs_context' too, for that matter? > > > > And maybe in the function declarations and definitions too? In things > > like 'has_pid_permissions()' (the series adds some other cases later, > > like hidepid2str() etc) > > > > Yeah, enums and ints are kind of interchangeable in C, but even if it > > wouldn't give us any more typechecking (except perhaps with sparse if > > you mark it so), it would be documenting the use. > > > > Or am I missing something? > > > > Anyway, I continue to think the series looks fine, bnut would love to > > see it in -next and perhaps comments from Al and Alexey Dobriyan.. > > Patches are OK, except the part where "pid" is named "pidfs" and > the suffix doesn't convey any information. I will fix this in the final version. > mount -t proc -o subset=pid,sysctl,misc I have not yet figured out how to implement this. I mean subset=meminfo,misc. -- Rgrds, legion