From: Al Viro <viro@ZenIV.linux.org.uk>
To: Konstantin Khlebnikov <koct9i@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Konstantin Khlebnikov <khlebnikov@yandex-team.ru>,
"Eric W. Biederman" <ebiederm@xmission.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] proc/sysctl: drop unregistered stale dentries as soon as possible
Date: Thu, 9 Feb 2017 08:40:16 +0000 [thread overview]
Message-ID: <20170209084016.GL13195@ZenIV.linux.org.uk> (raw)
In-Reply-To: <CALYGNiMU1a_P+uMspAfj6UYbsy1ahq_ab_OPZpKFF_cUhSKT1A@mail.gmail.com>
On Thu, Feb 09, 2017 at 10:36:15AM +0300, Konstantin Khlebnikov wrote:
> Ok, Thank you. I've expected that this fix isn't sane,
>
> Maybe we could minimize changes for now. For example: keep these
> stale dentries in memory but silently unhash them in ->d_compare().
> Memory processure and reclaimer will kill them later.
->d_compare() is called by the code walking the hash chains. What's worse,
in the most common case all we have is rcu_read_lock(). Modifying the chain
in rcu reader is no-go. Turning __d_lookup_rcu() into a writer on the
off-chance that we'll walk onto a visibly stale sysctl dentry - even more so.
If you want to deal with that, do it right, please. Have sysctl inodes
on a list of some kind anchored in struct ctl_table_header; insert them
there in proc_sys_make_inode(), remove - in proc_evict_inode() (or
have it pass the inode to sysctl_head_put() and do the removal there).
Use sysctl_lock for serialization.
In start_unregistering(), just before the erase_header() call, check
if the list is non-empty and if it is -
grab sysctl_lock
last = NULL
walk the list
igrab(inode we are looking at)
if succeeded
drop sysctl_lock
iput(last)
last = that inode
d_prune_aliases(last)
retake sysctl_lock
// inode is still not evicted, so it's still on the list
drop sysctl_lock
iput(last)
list would pass through struct proc_inode, and I would probably use
hlist rather than the normal one; might be more convenient to initialize
that way. Getting from containing struct proc_inode to inode - &ei->vfs_inode.
It's not that much work; if you have time - go for it, or remind me after
-rc1...
next prev parent reply other threads:[~2017-02-09 9:02 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-08 10:48 [PATCH] proc/sysctl: drop unregistered stale dentries as soon as possible Konstantin Khlebnikov
2017-02-08 21:48 ` Andrew Morton
2017-02-09 3:53 ` Al Viro
2017-02-09 7:36 ` Konstantin Khlebnikov
2017-02-09 8:40 ` Al Viro [this message]
2017-02-10 7:35 ` [PATCH] proc/sysctl: prune stale dentries during unregistering Konstantin Khlebnikov
2017-02-10 7:47 ` Al Viro
2017-02-10 7:54 ` Konstantin Khlebnikov
2017-02-13 9:54 ` Eric W. Biederman
2017-02-18 18:55 ` Konstantin Khlebnikov
2017-02-19 8:42 ` Al Viro
2017-02-21 1:41 ` [REVIEW][PATCH] proc/sysctl: Don't grab i_lock under sysctl_lock Eric W. Biederman
2017-02-21 8:40 ` Konstantin Khlebnikov
2017-02-21 19:29 ` Eric W. Biederman
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=20170209084016.GL13195@ZenIV.linux.org.uk \
--to=viro@zeniv.linux.org.uk \
--cc=akpm@linux-foundation.org \
--cc=ebiederm@xmission.com \
--cc=khlebnikov@yandex-team.ru \
--cc=koct9i@gmail.com \
--cc=linux-kernel@vger.kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.