linux-api.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
To: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
Cc: Al Viro <viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org>,
	LKML <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Kernel Hardening
	<kernel-hardening-ZwoEplunGu1jrUoiu81ncdBPR1lH4CV8@public.gmane.org>,
	Linux API <linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Linux FS Devel
	<linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Linux Security Module
	<linux-security-module-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Akinobu Mita
	<akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Alexey Dobriyan
	<adobriyan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Andrew Morton
	<akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>,
	Andy Lutomirski <luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Daniel Micay
	<danielmicay-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Djalal Harouni <tixxdz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	"Dmitry V . Levin" <ldv-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org>,
	Greg Kroah-Hartman
	<gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>,
	Ingo Molnar <mingo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	"J . Bruce Fields"
	<bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>,
	Jeff Layton <jlayton-vpEMnDpepFuMZCB2o+C8xQ@public.gmane.org>,
	Jonathan Corbet <corbet-T1hC0tSOHrs@public.gmane.org>,
	Kees
Subject: Re: [PATCH v8 07/11] proc: flush task dcache entries from all procfs instances
Date: Wed, 12 Feb 2020 16:48:14 -0800	[thread overview]
Message-ID: <CAHk-=wgmn9Qds0VznyphouSZW6e42GWDT5H1dpZg8pyGDGN+=w@mail.gmail.com> (raw)
In-Reply-To: <87lfp7h422.fsf-JOvCrm2gF+uungPnsOpG7nhyD016LWXt@public.gmane.org>

On Wed, Feb 12, 2020 at 1:48 PM Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org> wrote:
>
> The good news is proc_flush_task isn't exactly called from process exit.
> proc_flush_task is called during zombie clean up. AKA release_task.

Yeah, that at least avoids some of the nasty locking while dying debug problems.

But the one I was more worried about was actually the lock contention
issue with lots of processes. The lock is basically a single global
lock in many situations - yes, it's technically per-ns, but in a lot
of cases you really only have one namespace anyway.

And we've had problems with global locks in this area before, notably
the one you call out:

> Further after proc_flush_task does it's thing the code goes
> and does "write_lock_irq(&task_list_lock);"

Yeah, so it's not introducing a new issue, but it is potentially
making something we already know is bad even worse.

> What would be downside of having a mutex for a list of proc superblocks?
> A mutex that is taken for both reading and writing the list.

That's what the original patch actually was, and I was hoping we could
avoid that thing.

An rwsem would be possibly better, since most cases by far are likely
about reading.

And yes, I'm very aware of the task_list_lock, but it's literally why
I don't want to make a new one.

I'm _hoping_ we can some day come up with something better than task_list_lock.

            Linus

  parent reply	other threads:[~2020-02-13  0:48 UTC|newest]

Thread overview: 85+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-10 15:05 [PATCH v8 00/11] proc: modernize proc to support multiple private instances Alexey Gladkov
2020-02-10 15:05 ` [PATCH v8 01/11] proc: Rename struct proc_fs_info to proc_fs_opts Alexey Gladkov
2020-02-10 15:05 ` [PATCH v8 02/11] proc: add proc_fs_info struct to store proc information Alexey Gladkov
     [not found] ` <20200210150519.538333-1-gladkov.alexey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2020-02-10 15:05   ` [PATCH v8 03/11] proc: move /proc/{self|thread-self} dentries to proc_fs_info Alexey Gladkov
     [not found]     ` <20200210150519.538333-4-gladkov.alexey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2020-02-10 18:23       ` Andy Lutomirski
     [not found]         ` <CALCETrWGpRr86tVKJU-sEMcg+x0Yzp+TbiBhrAc71RaO8=DYGQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2020-02-12 15:00           ` Alexey Gladkov
2020-02-10 15:05   ` [PATCH v8 04/11] proc: move hide_pid, pid_gid from pid_namespace " Alexey Gladkov
2020-02-10 15:05   ` [PATCH v8 06/11] proc: support mounting procfs instances inside same pid namespace Alexey Gladkov
2020-02-10 15:05 ` [PATCH v8 05/11] proc: add helpers to set and get proc hidepid and gid mount options Alexey Gladkov
2020-02-10 18:30   ` Andy Lutomirski
2020-02-12 14:57     ` Alexey Gladkov
2020-02-10 15:05 ` [PATCH v8 07/11] proc: flush task dcache entries from all procfs instances Alexey Gladkov
2020-02-10 17:46   ` Linus Torvalds
2020-02-10 19:23     ` Al Viro
2020-02-11  1:36   ` Eric W. Biederman
     [not found]     ` <87v9odlxbr.fsf-JOvCrm2gF+uungPnsOpG7nhyD016LWXt@public.gmane.org>
2020-02-11  4:01       ` Eric W. Biederman
2020-02-12 14:49     ` Alexey Gladkov
2020-02-12 14:59       ` Eric W. Biederman
2020-02-12 17:08         ` Alexey Gladkov
2020-02-12 18:45         ` Linus Torvalds
     [not found]           ` <CAHk-=wg52stFtUxMOxs3afkwDWmWn1JXC7RJ7dPsTrJbnxpZVg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2020-02-12 19:16             ` Eric W. Biederman
2020-02-12 19:49               ` Linus Torvalds
2020-02-12 20:03                 ` Al Viro
2020-02-12 20:35                   ` Linus Torvalds
2020-02-12 20:38                     ` Al Viro
2020-02-12 20:41                       ` Al Viro
     [not found]                         ` <20200212204124.GR23230-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
2020-02-12 21:02                           ` Linus Torvalds
2020-02-12 21:46                             ` Eric W. Biederman
     [not found]                               ` <87lfp7h422.fsf-JOvCrm2gF+uungPnsOpG7nhyD016LWXt@public.gmane.org>
2020-02-13  0:48                                 ` Linus Torvalds [this message]
2020-02-13  4:37                                   ` Eric W. Biederman
2020-02-13  5:55                                     ` Al Viro
2020-02-13 21:30                                       ` Linus Torvalds
2020-02-13 22:23                                         ` Al Viro
2020-02-13 22:47                                           ` Linus Torvalds
2020-02-14 14:15                                             ` Eric W. Biederman
2020-02-14  3:48                                       ` Eric W. Biederman
2020-02-20 20:46                                     ` [PATCH 0/7] proc: Dentry flushing without proc_mnt Eric W. Biederman
2020-02-20 20:47                                       ` [PATCH 1/7] proc: Rename in proc_inode rename sysctl_inodes sibling_inodes Eric W. Biederman
2020-02-20 20:48                                       ` [PATCH 2/7] proc: Generalize proc_sys_prune_dcache into proc_prune_siblings_dcache Eric W. Biederman
2020-02-20 20:49                                       ` [PATCH 3/7] proc: Mov rcu_read_(lock|unlock) in proc_prune_siblings_dcache Eric W. Biederman
2020-02-20 22:33                                         ` Linus Torvalds
2020-02-20 20:49                                       ` [PATCH 4/7] proc: Use d_invalidate " Eric W. Biederman
2020-02-20 22:43                                         ` Linus Torvalds
2020-02-20 22:54                                         ` Al Viro
2020-02-20 23:00                                           ` Linus Torvalds
2020-02-20 23:03                                           ` Al Viro
2020-02-20 23:39                                             ` Eric W. Biederman
2020-02-20 20:51                                       ` [PATCH 5/7] proc: Clear the pieces of proc_inode that proc_evict_inode cares about Eric W. Biederman
2020-02-20 20:52                                       ` [PATCH 6/7] proc: Use a list of inodes to flush from proc Eric W. Biederman
2020-02-20 20:52                                       ` [PATCH 7/7] proc: Ensure we see the exit of each process tid exactly once Eric W. Biederman
2020-02-21 16:50                                         ` Oleg Nesterov
2020-02-22 15:46                                           ` Eric W. Biederman
2020-02-20 23:02                                       ` [PATCH 0/7] proc: Dentry flushing without proc_mnt Linus Torvalds
2020-02-20 23:07                                         ` Al Viro
2020-02-20 23:37                                           ` Eric W. Biederman
2020-02-24 16:25                                       ` [PATCH v2 0/6] " Eric W. Biederman
2020-02-24 16:26                                         ` [PATCH v2 1/6] proc: Rename in proc_inode rename sysctl_inodes sibling_inodes Eric W. Biederman
2020-02-24 16:27                                         ` [PATCH v2 2/6] proc: Generalize proc_sys_prune_dcache into proc_prune_siblings_dcache Eric W. Biederman
2020-02-24 16:27                                         ` [PATCH v2 3/6] proc: In proc_prune_siblings_dcache cache an aquired super block Eric W. Biederman
2020-02-24 16:28                                         ` [PATCH v2 4/6] proc: Use d_invalidate in proc_prune_siblings_dcache Eric W. Biederman
2020-02-24 16:28                                         ` [PATCH v2 5/6] proc: Clear the pieces of proc_inode that proc_evict_inode cares about Eric W. Biederman
2020-02-24 16:29                                         ` [PATCH v2 6/6] proc: Use a list of inodes to flush from proc Eric W. Biederman
2020-02-28 20:17                                         ` [PATCH 0/3] proc: Actually honor the mount options Eric W. Biederman
2020-02-28 20:18                                           ` [PATCH 1/3] uml: Don't consult current to find the proc_mnt in mconsole_proc Eric W. Biederman
2020-02-28 20:18                                           ` [PATCH 2/3] uml: Create a private mount of proc for mconsole Eric W. Biederman
2020-02-28 20:30                                             ` Christian Brauner
2020-02-28 21:28                                               ` Eric W. Biederman
2020-02-28 21:59                                                 ` Christian Brauner
2020-02-28 20:19                                           ` [PATCH 3/3] proc: Remove the now unnecessary internal mount of proc Eric W. Biederman
2020-02-28 20:39                                             ` Christian Brauner
2020-02-28 21:40                                               ` Eric W. Biederman
2020-02-28 22:34                                           ` [PATCH 4/3] pid: Improve the comment about waiting in zap_pid_ns_processes Eric W. Biederman
2020-02-29  2:59                                             ` Christian Brauner
2020-02-14  3:49                       ` [PATCH v8 07/11] proc: flush task dcache entries from all procfs instances Eric W. Biederman
2020-02-12 19:47           ` Al Viro
2020-02-11 22:45   ` Al Viro
2020-02-12 14:26     ` Alexey Gladkov
2020-02-10 15:05 ` [PATCH v8 08/11] proc: instantiate only pids that we can ptrace on 'hidepid=4' mount option Alexey Gladkov
     [not found]   ` <20200210150519.538333-9-gladkov.alexey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2020-02-10 16:29     ` Jordan Glover
2020-02-12 14:34       ` Alexey Gladkov
2020-02-10 15:05 ` [PATCH v8 09/11] proc: add option to mount only a pids subset Alexey Gladkov
2020-02-10 15:05 ` [PATCH v8 10/11] docs: proc: add documentation for "hidepid=4" and "subset=pidfs" options and new mount behavior Alexey Gladkov
2020-02-10 18:29   ` Andy Lutomirski
2020-02-12 16:03     ` Alexey Gladkov
2020-02-10 15:05 ` [PATCH v8 11/11] proc: Move hidepid values to uapi as they are user interface to mount Alexey Gladkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAHk-=wgmn9Qds0VznyphouSZW6e42GWDT5H1dpZg8pyGDGN+=w@mail.gmail.com' \
    --to=torvalds-de/tnxtf+jlsfhdxvbkv3wd2fqjk+8+b@public.gmane.org \
    --cc=adobriyan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org \
    --cc=bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org \
    --cc=corbet-T1hC0tSOHrs@public.gmane.org \
    --cc=danielmicay-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org \
    --cc=gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org \
    --cc=jlayton-vpEMnDpepFuMZCB2o+C8xQ@public.gmane.org \
    --cc=kernel-hardening-ZwoEplunGu1jrUoiu81ncdBPR1lH4CV8@public.gmane.org \
    --cc=ldv-u2l5PoMzF/Vg9hUCZPvPmw@public.gmane.org \
    --cc=linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-security-module-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=mingo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=tixxdz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).