public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: gregkh@linuxfoundation.org
Cc: linux-kernel@vger.kernel.org, schwidefsky@de.ibm.com,
	heiko.carstens@de.ibm.com, stern@rowland.harvard.edu,
	JBottomley@parallels.com, bhelgaas@google.com,
	Tejun Heo <tj@kernel.org>
Subject: [PATCH 07/12] kernfs: remove KERNFS_REMOVED
Date: Tue,  7 Jan 2014 12:59:45 -0500	[thread overview]
Message-ID: <1389117590-25705-8-git-send-email-tj@kernel.org> (raw)
In-Reply-To: <1389117590-25705-1-git-send-email-tj@kernel.org>

KERNFS_REMOVED is used to mark half-initialized and dying nodes so
that they don't show up in lookups and deny adding new nodes under or
renaming it.  Marking half-initialized nodes is unnecessary as linking
to the hierarchy is always atomic w.r.t. kernfs_mutex as are the
lookups.

It's necessary at least to deny addition of new children while removal
is in progress; however, this role considerably intersects with
deactivation - KERNFS_REMOVED prevents new children while deactivation
prevents new file operations.  There's no reason to have them separate
making things more complex than necessary.  This patch removes
KERNFS_REMOVED and gives its function to deactivation.

* KERNFS_REMOVED diddling in the addition path removed.

* A new helper kernfs_active() which tests whether kn->active >= 0 is
  added for convenience and lockdep annotation.  All KERNFS_REMOVED
  tests are replaced with negated kernfs_active() tests.

* __kernfs_remove() is updated to deactivate, but not drain, all nodes
  in the subtree instead of setting KERNFS_REMOVED.  This removes
  deactivation from kernfs_deactivate(), which is now renamed to
  kernfs_drain().

* Sanity check on KERNFS_REMOVED in kernfs_put() is replaced with
  checks on the active ref.

* Some comment style updates in the affected area.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 fs/kernfs/dir.c        | 66 +++++++++++++++++++++++++-------------------------
 include/linux/kernfs.h |  1 -
 2 files changed, 33 insertions(+), 34 deletions(-)

diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index 8abdf07..71faef9 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -22,6 +22,12 @@ DEFINE_MUTEX(kernfs_mutex);
 
 #define rb_to_kn(X) rb_entry((X), struct kernfs_node, rb)
 
+static bool kernfs_active(struct kernfs_node *kn)
+{
+	lockdep_assert_held(&kernfs_mutex);
+	return atomic_read(&kn->active) >= 0;
+}
+
 static bool kernfs_lockdep(struct kernfs_node *kn)
 {
 #ifdef CONFIG_DEBUG_LOCK_ALLOC
@@ -183,25 +189,20 @@ void kernfs_put_active(struct kernfs_node *kn)
 }
 
 /**
- *	kernfs_deactivate - deactivate kernfs_node
- *	@kn: kernfs_node to deactivate
+ * kernfs_drain - drain kernfs_node
+ * @kn: kernfs_node to drain
  *
- *	Deny new active references, drain existing ones and nuke all
- *	existing mmaps.  Mutiple removers may invoke this function
- *	concurrently on @kn and all will return after deactivation and
- *	draining are complete.
+ * Drain existing usages and nuke all existing mmaps of @kn.  Mutiple
+ * removers may invoke this function concurrently on @kn and all will
+ * return after draining is complete.
  */
-static void kernfs_deactivate(struct kernfs_node *kn)
+static void kernfs_drain(struct kernfs_node *kn)
 	__releases(&kernfs_mutex) __acquires(&kernfs_mutex)
 {
 	struct kernfs_root *root = kernfs_root(kn);
 
 	lockdep_assert_held(&kernfs_mutex);
-	BUG_ON(!(kn->flags & KERNFS_REMOVED));
-
-	/* only the first invocation on @kn should deactivate it */
-	if (atomic_read(&kn->active) >= 0)
-		atomic_add(KN_DEACTIVATED_BIAS, &kn->active);
+	WARN_ON_ONCE(kernfs_active(kn));
 
 	mutex_unlock(&kernfs_mutex);
 
@@ -253,13 +254,16 @@ void kernfs_put(struct kernfs_node *kn)
 		return;
 	root = kernfs_root(kn);
  repeat:
-	/* Moving/renaming is always done while holding reference.
+	/*
+	 * Moving/renaming is always done while holding reference.
 	 * kn->parent won't change beneath us.
 	 */
 	parent = kn->parent;
 
-	WARN(!(kn->flags & KERNFS_REMOVED), "kernfs: free using entry: %s/%s\n",
-	     parent ? parent->name : "", kn->name);
+	WARN_ONCE(atomic_read(&kn->active) != 0 &&
+		  atomic_read(&kn->active) != KN_DEACTIVATED_BIAS,
+		  "kernfs_put: %s/%s: released with incorrect active_ref %d\n",
+		  parent ? parent->name : "", kn->name, atomic_read(&kn->active));
 
 	if (kernfs_type(kn) == KERNFS_LINK)
 		kernfs_put(kn->symlink.target_kn);
@@ -301,8 +305,8 @@ static int kernfs_dop_revalidate(struct dentry *dentry, unsigned int flags)
 	kn = dentry->d_fsdata;
 	mutex_lock(&kernfs_mutex);
 
-	/* The kernfs node has been deleted */
-	if (kn->flags & KERNFS_REMOVED)
+	/* The kernfs node has been deactivated */
+	if (!kernfs_active(kn))
 		goto out_bad;
 
 	/* The kernfs node has been moved? */
@@ -375,7 +379,7 @@ struct kernfs_node *kernfs_new_node(struct kernfs_root *root, const char *name,
 
 	kn->name = name;
 	kn->mode = mode;
-	kn->flags = flags | KERNFS_REMOVED;
+	kn->flags = flags;
 
 	return kn;
 
@@ -417,7 +421,7 @@ int kernfs_add_one(struct kernfs_node *kn, struct kernfs_node *parent)
 		goto out_unlock;
 
 	ret = -ENOENT;
-	if (parent->flags & KERNFS_REMOVED)
+	if (!kernfs_active(parent))
 		goto out_unlock;
 
 	kn->hash = kernfs_name_hash(kn->name, kn->ns);
@@ -434,9 +438,6 @@ int kernfs_add_one(struct kernfs_node *kn, struct kernfs_node *parent)
 		struct iattr *ps_iattrs = &ps_iattr->ia_iattr;
 		ps_iattrs->ia_ctime = ps_iattrs->ia_mtime = CURRENT_TIME;
 	}
-
-	/* Mark the entry added into directory tree */
-	kn->flags &= ~KERNFS_REMOVED;
 	ret = 0;
 out_unlock:
 	mutex_unlock(&kernfs_mutex);
@@ -535,7 +536,6 @@ struct kernfs_root *kernfs_create_root(struct kernfs_dir_ops *kdops, void *priv)
 		return ERR_PTR(-ENOMEM);
 	}
 
-	kn->flags &= ~KERNFS_REMOVED;
 	kn->priv = priv;
 	kn->dir.root = root;
 
@@ -750,13 +750,13 @@ static void __kernfs_remove(struct kernfs_node *kn)
 
 	pr_debug("kernfs %s: removing\n", kn->name);
 
-	/* disable lookup and node creation under @kn */
+	/* prevent any new usage under @kn by deactivating all nodes */
 	next = NULL;
 	do {
 		pos = next;
 		next = kernfs_next_descendant_post(pos, kn);
-		if (pos)
-			pos->flags |= KERNFS_REMOVED;
+		if (pos && kernfs_active(pos))
+			atomic_add(KN_DEACTIVATED_BIAS, &pos->active);
 	} while (next);
 
 	/* deactivate and unlink the subtree node-by-node */
@@ -764,14 +764,14 @@ static void __kernfs_remove(struct kernfs_node *kn)
 		pos = kernfs_leftmost_descendant(kn);
 
 		/*
-		 * kernfs_deactivate() drops kernfs_mutex temporarily and
-		 * @pos's base ref could have been put by someone else by
-		 * the time the function returns.  Make sure it doesn't go
-		 * away underneath us.
+		 * kernfs_drain() drops kernfs_mutex temporarily and @pos's
+		 * base ref could have been put by someone else by the time
+		 * the function returns.  Make sure it doesn't go away
+		 * underneath us.
 		 */
 		kernfs_get(pos);
 
-		kernfs_deactivate(pos);
+		kernfs_drain(pos);
 
 		/*
 		 * kernfs_unlink_sibling() succeeds once per node.  Use it
@@ -855,7 +855,7 @@ int kernfs_rename_ns(struct kernfs_node *kn, struct kernfs_node *new_parent,
 	mutex_lock(&kernfs_mutex);
 
 	error = -ENOENT;
-	if ((kn->flags | new_parent->flags) & KERNFS_REMOVED)
+	if (!kernfs_active(kn) || !kernfs_active(new_parent))
 		goto out;
 
 	error = 0;
@@ -915,7 +915,7 @@ static struct kernfs_node *kernfs_dir_pos(const void *ns,
 	struct kernfs_node *parent, loff_t hash, struct kernfs_node *pos)
 {
 	if (pos) {
-		int valid = !(pos->flags & KERNFS_REMOVED) &&
+		int valid = kernfs_active(pos) &&
 			pos->parent == parent && hash == pos->hash;
 		kernfs_put(pos);
 		if (!valid)
diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
index bff6e08..1238a66 100644
--- a/include/linux/kernfs.h
+++ b/include/linux/kernfs.h
@@ -37,7 +37,6 @@ enum kernfs_node_type {
 #define KERNFS_FLAG_MASK	~KERNFS_TYPE_MASK
 
 enum kernfs_node_flag {
-	KERNFS_REMOVED		= 0x0010,
 	KERNFS_NS		= 0x0020,
 	KERNFS_HAS_SEQ_SHOW	= 0x0040,
 	KERNFS_HAS_MMAP		= 0x0080,
-- 
1.8.4.2


  parent reply	other threads:[~2014-01-07 18:02 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-07 17:59 [PATCHSET driver-core-next] kernfs, sysfs, driver-core: implement synchronous self-removal Tejun Heo
2014-01-07 17:59 ` [PATCH 01/12] kernfs: fix get_active failure handling in kernfs_seq_*() Tejun Heo
2014-01-10 13:48   ` Sasha Levin
2014-01-07 17:59 ` [PATCH 02/12] kernfs: replace kernfs_node->u.completion with kernfs_root->deactivate_waitq Tejun Heo
2014-01-07 17:59 ` [PATCH 03/12] kernfs: restructure removal path to fix possible premature return Tejun Heo
2014-01-07 17:59 ` [PATCH 04/12] kernfs: invoke kernfs_unmap_bin_file() directly from kernfs_deactivate() Tejun Heo
2014-01-07 17:59 ` [PATCH 05/12] kernfs: remove kernfs_addrm_cxt Tejun Heo
2014-01-07 17:59 ` [PATCH 06/12] kernfs: remove KERNFS_ACTIVE_REF and add kernfs_lockdep() Tejun Heo
2014-01-07 17:59 ` Tejun Heo [this message]
2014-01-07 17:59 ` [PATCH 08/12] kernfs, sysfs, driver-core: implement kernfs_remove_self() and its wrappers Tejun Heo
2014-01-07 18:36   ` [PATCH v2 " Tejun Heo
2014-01-07 17:59 ` [PATCH 09/12] pci: use device_remove_file_self() instead of device_schedule_callback() Tejun Heo
2014-01-07 17:59 ` [PATCH 10/12] scsi: " Tejun Heo
2014-01-07 17:59 ` [PATCH 11/12] s390: " Tejun Heo
2014-01-07 17:59 ` [PATCH 12/12] remove-unused-callback-mechanism Tejun Heo
2014-01-07 18:04   ` [PATCH REPOST 12/12] sysfs, driver-core: remove unused {sysfs|device}_schedule_callback_owner() Tejun Heo
2014-01-07 21:41 ` [PATCHSET driver-core-next] kernfs, sysfs, driver-core: implement synchronous self-removal Tejun Heo
2014-01-08  1:01   ` Greg KH
  -- strict thread matches above, loose matches on Subject: below --
2014-02-03 19:02 [PATCHSET v5 " Tejun Heo
2014-02-03 19:03 ` [PATCH 07/12] kernfs: remove KERNFS_REMOVED Tejun Heo

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=1389117590-25705-8-git-send-email-tj@kernel.org \
    --to=tj@kernel.org \
    --cc=JBottomley@parallels.com \
    --cc=bhelgaas@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=heiko.carstens@de.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=schwidefsky@de.ibm.com \
    --cc=stern@rowland.harvard.edu \
    /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