| Message ID | 871rqpaswu.fsf_-_@x220.int.ebiederm.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | proc: Dentry flushing without proc_mnt | expand |
On Thu, Feb 20, 2020 at 12:48 PM Eric W. Biederman <ebiederm@xmission.com> wrote: > > Linus, does this approach look like something you can stand? A couple of worries, although one of them seem to have already been resolved by Al. I think the real gatekeeper should be Al in general. But other than the small comments I had, I think this might work just fine. Al? Linus
On Thu, Feb 20, 2020 at 03:02:22PM -0800, Linus Torvalds wrote: > On Thu, Feb 20, 2020 at 12:48 PM Eric W. Biederman > <ebiederm@xmission.com> wrote: > > > > Linus, does this approach look like something you can stand? > > A couple of worries, although one of them seem to have already been > resolved by Al. > > I think the real gatekeeper should be Al in general. But other than > the small comments I had, I think this might work just fine. > > Al? I'll need to finish RTFS there; I have initially misread that patch, actually - Eric _is_ using that thing both for those directories and for sysctl inodes. And the prototype for that machinery (the one he'd pulled from proc_sysctl.c) is playing with pinning superblocks way too much; for per-pid directories that's not an issue, but for sysctl table removal you are very likely to hit a bunch of evictees on the same superblock...
Al Viro <viro@zeniv.linux.org.uk> writes: > On Thu, Feb 20, 2020 at 03:02:22PM -0800, Linus Torvalds wrote: >> On Thu, Feb 20, 2020 at 12:48 PM Eric W. Biederman >> <ebiederm@xmission.com> wrote: >> > >> > Linus, does this approach look like something you can stand? >> >> A couple of worries, although one of them seem to have already been >> resolved by Al. >> >> I think the real gatekeeper should be Al in general. But other than >> the small comments I had, I think this might work just fine. >> >> Al? > > I'll need to finish RTFS there; I have initially misread that patch, > actually - Eric _is_ using that thing both for those directories > and for sysctl inodes. And the prototype for that machinery (the > one he'd pulled from proc_sysctl.c) is playing with pinning superblocks > way too much; for per-pid directories that's not an issue, but > for sysctl table removal you are very likely to hit a bunch of > evictees on the same superblock... I saw that was possible. If the broad strokes look correct I don't have a problem at all with optimizing for the case where many of the entries are for inodes on the same superblock. I just had enough other details on my mind I was afraid if I got a little more clever I would have introduced a typo somewhere. I wish I could limit the sysctl parts to just directories, but unfortunately the sysctl tables don't always give a guarantee that a directory is what will be removed. But sysctls do have one name per inode invarant like fat. There is no way to express a sysctl table that doesn't have that invariant. As for d_find_alias/d_invalidate. Just for completeness I wanted to write a loop: while (dentry = d_find_alias(inode)) { d_invalidate(dentry); dput(dentry); } Unfortunately that breaks on directories, because for directories d_find_alias turns into d_find_any_alias, and continues to return aliases even when they are unhashed. It might be nice to write a cousin of d_prune_aliases call it d_invalidate_aliases that just does that loop the correct way in dcache.c Eric
Just because it is less of a fundamental change and less testing I went and looked at updating proc_flush_task to use a list as Al suggested. If we can stand an sget/deactivate_super pair for every dentry we want to invalidate I think I have something. Comments from anyone will be appreciated I gave this some light testing and the code is based on something similar already present in proc so I think there is a high chance this code is correct but I could easily be wrong. Linus, does this approach look like something you can stand? Eric Eric W. Biederman (7): proc: Rename in proc_inode rename sysctl_inodes sibling_inodes proc: Generalize proc_sys_prune_dcache into proc_prune_siblings_dcache proc: Mov rcu_read_(lock|unlock) in proc_prune_siblings_dcache proc: Use d_invalidate in proc_prune_siblings_dcache proc: Clear the pieces of proc_inode that proc_evict_inode cares about proc: Use a list of inodes to flush from proc proc: Ensure we see the exit of each process tid exactly once fs/exec.c | 5 +-- fs/proc/base.c | 111 ++++++++++++++++-------------------------------- fs/proc/inode.c | 60 +++++++++++++++++++++++--- fs/proc/internal.h | 4 +- fs/proc/proc_sysctl.c | 45 +++----------------- include/linux/pid.h | 2 + include/linux/proc_fs.h | 4 +- kernel/exit.c | 4 +- kernel/pid.c | 16 +++++++ 9 files changed, 124 insertions(+), 127 deletions(-)