public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: gregkh@linuxfoundation.org
Cc: kay@vrfy.org, linux-kernel@vger.kernel.org,
	ebiederm@xmission.com, bhelgaas@google.com,
	Tejun Heo <tj@kernel.org>
Subject: [PATCH 22/39] sysfs, kernfs: revamp sysfs_dirent active_ref lockdep annotation
Date: Sun,  3 Nov 2013 04:53:01 -0500	[thread overview]
Message-ID: <1383472398-17812-23-git-send-email-tj@kernel.org> (raw)
In-Reply-To: <1383472398-17812-1-git-send-email-tj@kernel.org>

Currently, sysfs_dirent active_ref lockdep annotation uses
attribute->[s]key as the lockdep key, which forces
kernfs_create_file_ns() to assume that sysfs_dirent->priv is pointing
to a struct attribute which may not be true for non-sysfs users.  This
patch restructures the lockdep annotation such that

* kernfs_ops contains lockdep_key which is used by default for files
  created kernfs_create_file_ns().

* kernfs_create_file_ns_key() is introduced which takes an extra @key
  argument.  The created file will use the specified key for
  active_ref lockdep annotation.  If NULL is specified, lockdep for
  the file is disabled.

* sysfs_add_file_mode_ns() is updated to use
  kernfs_create_file_ns_key() with the appropriate key from the
  attribute or NULL if ignore_lockdep is set.

This makes the lockdep annotation properly contained in kernfs while
allowing sysfs to cleanly keep its current behavior.  This patch
doesn't introduce any behavior differences.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 fs/sysfs/dir.c         |  4 ++--
 fs/sysfs/file.c        | 35 ++++++++++++++++++++++++-----------
 fs/sysfs/sysfs.h       | 32 +-------------------------------
 include/linux/kernfs.h | 37 +++++++++++++++++++++++++++++--------
 4 files changed, 56 insertions(+), 52 deletions(-)

diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index 14693aa..1933718 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -162,7 +162,7 @@ struct sysfs_dirent *sysfs_get_active(struct sysfs_dirent *sd)
 	if (!atomic_inc_unless_negative(&sd->s_active))
 		return NULL;
 
-	if (likely(!sysfs_ignore_lockdep(sd)))
+	if (sd->s_flags & SYSFS_FLAG_LOCKDEP)
 		rwsem_acquire_read(&sd->dep_map, 0, 1, _RET_IP_);
 	return sd;
 }
@@ -181,7 +181,7 @@ void sysfs_put_active(struct sysfs_dirent *sd)
 	if (unlikely(!sd))
 		return;
 
-	if (likely(!sysfs_ignore_lockdep(sd)))
+	if (sd->s_flags & SYSFS_FLAG_LOCKDEP)
 		rwsem_release(&sd->dep_map, 1, _RET_IP_);
 	v = atomic_dec_return(&sd->s_active);
 	if (likely(v != SD_DEACTIVATED_BIAS))
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index 1f99fe3..eabc1d2 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -58,7 +58,7 @@ static struct sysfs_open_file *sysfs_of(struct file *file)
  */
 static const struct kernfs_ops *kernfs_ops(struct sysfs_dirent *sd)
 {
-	if (!sysfs_ignore_lockdep(sd))
+	if (sd->s_flags & SYSFS_FLAG_LOCKDEP)
 		lockdep_assert_held(sd);
 	return sd->s_attr.ops;
 }
@@ -71,7 +71,7 @@ static const struct sysfs_ops *sysfs_file_ops(struct sysfs_dirent *sd)
 {
 	struct kobject *kobj = sd->s_parent->priv;
 
-	if (!sysfs_ignore_lockdep(sd))
+	if (sd->s_flags & SYSFS_FLAG_LOCKDEP)
 		lockdep_assert_held(sd);
 	return kobj->ktype ? kobj->ktype->sysfs_ops : NULL;
 }
@@ -925,6 +925,7 @@ int sysfs_add_file_mode_ns(struct sysfs_dirent *dir_sd,
 			   const struct attribute *attr, bool is_bin,
 			   umode_t mode, const void *ns)
 {
+	struct lock_class_key *key = NULL;
 	const struct kernfs_ops *ops;
 	struct sysfs_dirent *sd;
 	loff_t size;
@@ -964,8 +965,12 @@ int sysfs_add_file_mode_ns(struct sysfs_dirent *dir_sd,
 		size = battr->size;
 	}
 
-	sd = kernfs_create_file_ns(dir_sd, attr->name, mode, size,
-				   ops, (void *)attr, ns);
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+	if (!attr->ignore_lockdep)
+		key = attr->key ?: (struct lock_class_key *)&attr->skey;
+#endif
+	sd = kernfs_create_file_ns_key(dir_sd, attr->name, mode, size,
+				       ops, (void *)attr, ns, key);
 	if (IS_ERR(sd)) {
 		if (PTR_ERR(sd) == -EEXIST)
 			sysfs_warn_dup(dir_sd, attr->name);
@@ -975,7 +980,7 @@ int sysfs_add_file_mode_ns(struct sysfs_dirent *dir_sd,
 }
 
 /**
- * kernfs_create_file_ns - create a file
+ * kernfs_create_file_ns_key - create a file
  * @parent: directory to create the file in
  * @name: name of the file
  * @mode: mode of the file
@@ -983,14 +988,16 @@ int sysfs_add_file_mode_ns(struct sysfs_dirent *dir_sd,
  * @ops: kernfs operations for the file
  * @priv: private data for the file
  * @ns: optional namespace tag of the file
+ * @key: lockdep key for the file's active_ref, %NULL to disable lockdep
  *
  * Returns the created node on success, ERR_PTR() value on error.
  */
-struct sysfs_dirent *kernfs_create_file_ns(struct sysfs_dirent *parent,
-					   const char *name,
-					   umode_t mode, loff_t size,
-					   const struct kernfs_ops *ops,
-					   void *priv, const void *ns)
+struct sysfs_dirent *kernfs_create_file_ns_key(struct sysfs_dirent *parent,
+					       const char *name,
+					       umode_t mode, loff_t size,
+					       const struct kernfs_ops *ops,
+					       void *priv, const void *ns,
+					       struct lock_class_key *key)
 {
 	struct sysfs_addrm_cxt acxt;
 	struct sysfs_dirent *sd;
@@ -1005,7 +1012,13 @@ struct sysfs_dirent *kernfs_create_file_ns(struct sysfs_dirent *parent,
 	sd->s_attr.size = size;
 	sd->s_ns = ns;
 	sd->priv = priv;
-	sysfs_dirent_init_lockdep(sd);
+
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+	if (key) {
+		lockdep_init_map(&sd->dep_map, "s_active", key, 0);
+		sd->s_flags |= SYSFS_FLAG_LOCKDEP;
+	}
+#endif
 
 	/*
 	 * sd->s_attr.ops is accesible only while holding active ref.  We
diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
index 83f4708..bffaa5b 100644
--- a/fs/sysfs/sysfs.h
+++ b/fs/sysfs/sysfs.h
@@ -92,43 +92,13 @@ struct sysfs_dirent {
 #define SYSFS_FLAG_HAS_NS		0x0020
 #define SYSFS_FLAG_HAS_SEQ_SHOW		0x0040
 #define SYSFS_FLAG_HAS_MMAP		0x0080
+#define SYSFS_FLAG_LOCKDEP		0x0100
 
 static inline unsigned int sysfs_type(struct sysfs_dirent *sd)
 {
 	return sd->s_flags & SYSFS_TYPE_MASK;
 }
 
-#ifdef CONFIG_DEBUG_LOCK_ALLOC
-
-#define sysfs_dirent_init_lockdep(sd)				\
-do {								\
-	struct attribute *attr = sd->priv;			\
-	struct lock_class_key *key = attr->key;			\
-	if (!key)						\
-		key = &attr->skey;				\
-								\
-	lockdep_init_map(&sd->dep_map, "s_active", key, 0);	\
-} while (0)
-
-/* Test for attributes that want to ignore lockdep for read-locking */
-static inline bool sysfs_ignore_lockdep(struct sysfs_dirent *sd)
-{
-	struct attribute *attr = sd->priv;
-
-	return sysfs_type(sd) == SYSFS_KOBJ_ATTR && attr->ignore_lockdep;
-}
-
-#else
-
-#define sysfs_dirent_init_lockdep(sd) do {} while (0)
-
-static inline bool sysfs_ignore_lockdep(struct sysfs_dirent *sd)
-{
-	return true;
-}
-
-#endif
-
 /*
  * Context structure to be used while adding/removing nodes.
  */
diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
index 53cf787..9db44f2 100644
--- a/include/linux/kernfs.h
+++ b/include/linux/kernfs.h
@@ -11,6 +11,7 @@
 #include <linux/err.h>
 #include <linux/list.h>
 #include <linux/mutex.h>
+#include <linux/lockdep.h>
 
 struct file;
 struct iattr;
@@ -62,6 +63,10 @@ struct kernfs_ops {
 			 loff_t off);
 
 	int (*mmap)(struct sysfs_open_file *of, struct vm_area_struct *vma);
+
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+	struct lock_class_key	lockdep_key;
+#endif
 };
 
 #ifdef CONFIG_SYSFS
@@ -69,11 +74,12 @@ struct kernfs_ops {
 struct sysfs_dirent *kernfs_create_dir_ns(struct sysfs_dirent *parent,
 					  const char *name, void *priv,
 					  const void *ns);
-struct sysfs_dirent *kernfs_create_file_ns(struct sysfs_dirent *parent,
-					   const char *name,
-					   umode_t mode, loff_t size,
-					   const struct kernfs_ops *ops,
-					   void *priv, const void *ns);
+struct sysfs_dirent *kernfs_create_file_ns_key(struct sysfs_dirent *parent,
+					       const char *name,
+					       umode_t mode, loff_t size,
+					       const struct kernfs_ops *ops,
+					       void *priv, const void *ns,
+					       struct lock_class_key *key);
 struct sysfs_dirent *kernfs_create_link(struct sysfs_dirent *parent,
 					const char *name,
 					struct sysfs_dirent *target);
@@ -93,9 +99,10 @@ kernfs_create_dir_ns(struct sysfs_dirent *parent, const char *name, void *priv,
 { return ERR_PTR(-ENOSYS); }
 
 static inline struct sysfs_dirent *
-kernfs_create_file_ns(struct sysfs_dirent *parent, const char *name,
-		      umode_t mode, loff_t size, const struct kernfs_ops *ops,
-		      void *priv, const void *ns)
+kernfs_create_file_ns_key(struct sysfs_dirent *parent, const char *name,
+			  umode_t mode, loff_t size,
+			  const struct kernfs_ops *ops, void *priv,
+			  const void *ns, struct lock_class_key *key)
 { return ERR_PTR(-ENOSYS); }
 
 static inline struct sysfs_dirent *
@@ -129,6 +136,20 @@ kernfs_create_dir(struct sysfs_dirent *parent, const char *name, void *priv)
 }
 
 static inline struct sysfs_dirent *
+kernfs_create_file_ns(struct sysfs_dirent *parent, const char *name,
+		      umode_t mode, loff_t size, const struct kernfs_ops *ops,
+		      void *priv, const void *ns)
+{
+	struct lock_class_key *key = NULL;
+
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+	key = (struct lock_class_key *)&ops->lockdep_key;
+#endif
+	return kernfs_create_file_ns_key(parent, name, mode, size, ops, priv,
+					 ns, key);
+}
+
+static inline struct sysfs_dirent *
 kernfs_create_file(struct sysfs_dirent *parent, const char *name, umode_t mode,
 		   loff_t size, const struct kernfs_ops *ops, void *priv)
 {
-- 
1.8.3.1


  parent reply	other threads:[~2013-11-03  9:59 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-03  9:52 [PATCHSET driver-core-next] sysfs: separate out kernfs, take #2 Tejun Heo
2013-11-03  9:52 ` [PATCH 01/39] sysfs: make __sysfs_add_one() fail if the parent isn't a directory Tejun Heo
2013-11-03  9:52 ` [PATCH 02/39] sysfs, kernfs: add skeletons for kernfs Tejun Heo
2013-11-03  9:52 ` [PATCH 03/39] sysfs, kernfs: introduce kernfs_remove[_by_name[_ns]]() Tejun Heo
2013-11-03  9:52 ` [PATCH 04/39] sysfs, kernfs: introduce kernfs_create_link() Tejun Heo
2013-11-03  9:52 ` [PATCH 05/39] sysfs, kernfs: introduce kernfs_rename[_ns]() Tejun Heo
2013-11-03  9:52 ` [PATCH 06/39] sysfs, kernfs: introduce kernfs_setattr() Tejun Heo
2013-11-03  9:52 ` [PATCH 07/39] sysfs, kernfs: replace sysfs_dirent->s_dir.kobj and ->s_attr.[bin_]attr with ->priv Tejun Heo
2013-11-03  9:52 ` [PATCH 08/39] sysfs, kernfs: introduce kernfs_create_dir[_ns]() Tejun Heo
2013-11-03  9:52 ` [PATCH 09/39] sysfs, kernfs: prepare read path for kernfs Tejun Heo
2013-11-03  9:52 ` [PATCH 10/39] sysfs, kernfs: prepare write " Tejun Heo
2013-11-03  9:52 ` [PATCH 11/39] sysfs, kernfs: prepare mmap " Tejun Heo
2013-11-03  9:52 ` [PATCH 12/39] sysfs, kernfs: prepare open, release, poll paths " Tejun Heo
2013-11-03  9:52 ` [PATCH 13/39] sysfs, kernfs: move sysfs_open_file to include/linux/kernfs.h Tejun Heo
2013-11-03  9:52 ` [PATCH 14/39] sysfs, kernfs: introduce kernfs_ops Tejun Heo
2013-11-03  9:52 ` [PATCH 15/39] sysfs, kernfs: add sysfs_dirent->s_attr.size Tejun Heo
2013-11-03  9:52 ` [PATCH 16/39] sysfs, kernfs: remove SYSFS_KOBJ_BIN_ATTR Tejun Heo
2013-11-03  9:52 ` [PATCH 17/39] sysfs, kernfs: introduce kernfs_create_file[_ns]() Tejun Heo
2013-11-03  9:52 ` [PATCH 18/39] sysfs, kernfs: remove sysfs_add_one() Tejun Heo
2013-11-03  9:52 ` [PATCH 19/39] sysfs, kernfs: add kernfs_ops->seq_{start|next|stop}() Tejun Heo
2013-11-03  9:52 ` [PATCH 20/39] sysfs, kernfs: introduce kernfs_notify() Tejun Heo
2013-11-03  9:53 ` [PATCH 21/39] sysfs, kernfs: reorganize SYSFS_* constants Tejun Heo
2013-11-03  9:53 ` Tejun Heo [this message]
2013-11-03  9:53 ` [PATCH 23/39] sysfs, kernfs: introduce kernfs[_find_and]_get() and kernfs_put() Tejun Heo
2013-11-03  9:53 ` [PATCH 24/39] sysfs, kernfs: move internal decls to fs/kernfs/kernfs-internal.h Tejun Heo
2013-11-03  9:53 ` [PATCH 25/39] sysfs, kernfs: move inode code to fs/kernfs/inode.c Tejun Heo
2013-11-03  9:53 ` [PATCH 26/39] sysfs, kernfs: move dir core code to fs/kernfs/dir.c Tejun Heo
2013-11-03  9:53 ` [PATCH 27/39] sysfs, kernfs: move file core code to fs/kernfs/file.c Tejun Heo
2013-11-03  9:53 ` [PATCH 28/39] sysfs, kernfs: move symlink core code to fs/kernfs/symlink.c Tejun Heo
2013-11-03  9:53 ` [PATCH 29/39] sysfs, kernfs: drop unused params from sysfs_fill_super() Tejun Heo
2013-11-03  9:53 ` [PATCH 30/39] sysfs, kernfs: make sysfs_super_info->ns const Tejun Heo
2013-11-03  9:53 ` [PATCH 31/39] sysfs, kernfs: no need to kern_mount() sysfs from sysfs_init() Tejun Heo
2013-11-03  9:53 ` [PATCH 32/39] sysfs, kernfs: introduce sysfs_root_sd Tejun Heo
2013-11-03  9:53 ` [PATCH 33/39] sysfs, kernfs: implement kernfs_create/destroy_root() Tejun Heo
2013-11-03  9:53 ` [PATCH 34/39] sysfs, kernfs: make inode number ida per kernfs_root Tejun Heo
2013-11-03  9:53 ` [PATCH 35/39] sysfs, kernfs: make super_blocks bind to different kernfs_roots Tejun Heo
2013-11-03  9:53 ` [PATCH 36/39] sysfs, kernfs: prepare mount path for kernfs Tejun Heo
2013-11-03  9:53 ` [PATCH 37/39] sysfs, kernfs: move mount core code to fs/kernfs/mount.c Tejun Heo
2013-11-03  9:53 ` [PATCH 38/39] sysfs, kernfs: make sysfs_dirent definition public Tejun Heo
2013-11-03  9:53 ` [PATCH 39/39] sysfs, kernfs: remove cross inclusions of internal headers 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=1383472398-17812-23-git-send-email-tj@kernel.org \
    --to=tj@kernel.org \
    --cc=bhelgaas@google.com \
    --cc=ebiederm@xmission.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=kay@vrfy.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox