All of lore.kernel.org
 help / color / mirror / Atom feed
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...

  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.