All of lore.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 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.