linux.kernel - 26 new messages in 16 topics - digest

linux.kernel
http://groups.google.com/group/linux.kernel?hl=en

linux.kernel@googlegroups.com

Today's topics:

* kernfs: remove kernfs_addrm_cxt - 6 messages, 1 author
http://groups.google.com/group/linux.kernel/t/b72ccc5edcdfb313?hl=en
* hwmon/sensors: fix SENSORS_LM75 dependencies - 1 messages, 1 author
http://groups.google.com/group/linux.kernel/t/0b19a469dc611e35?hl=en
* [RESEND] platform: x86: New BayTrail IOSF-SB MBI driver - 3 messages, 2
authors
http://groups.google.com/group/linux.kernel/t/89a0fed1f99e307a?hl=en
* kobject: provide kobject_put_wait to fix module unload race - 2 messages, 2
authors
http://groups.google.com/group/linux.kernel/t/f1e8d59a892ab84a?hl=en
* change close_files() to use rcu_lock_acquire() to shut up RCU-lockdep - 3
messages, 1 author
http://groups.google.com/group/linux.kernel/t/c6ec2159d16edee2?hl=en
* pciehp: Don't disable the link permanently, during removal - 1 messages, 1
author
http://groups.google.com/group/linux.kernel/t/f4fad40308dd17fc?hl=en
* lib/vsprintf: add %pT[C012] format specifier - 1 messages, 1 author
http://groups.google.com/group/linux.kernel/t/40f2db64b3df6ee9?hl=en
* Lockdep problem - 1 messages, 1 author
http://groups.google.com/group/linux.kernel/t/984c15a736c6685f?hl=en
* vfio: Use new interfaces for MSI/MSI-X enablement - 1 messages, 1 author
http://groups.google.com/group/linux.kernel/t/6035bbf945f5ceba?hl=en
* dt-bindings: pci: xgene pcie device tree bindings - 1 messages, 1 author
http://groups.google.com/group/linux.kernel/t/bb793bfb4a89dad9?hl=en
* OMAPDSS: DISPC: horizontal timing too tight errors - 1 messages, 1 author
http://groups.google.com/group/linux.kernel/t/f2b09789ae626fc0?hl=en
* [INCOMPLETE] ARM: make return_address available for ARM_UNWIND - 1 messages,
1 author
http://groups.google.com/group/linux.kernel/t/d81d184392f0d82e?hl=en
* Terrible performance of sequential O_DIRECT 4k writes in SAN environment. ~3
times slower then Solars 10 with the same HBA/Storage. - 1 messages, 1 author
http://groups.google.com/group/linux.kernel/t/c2d4038a5c79058e?hl=en
* PCI: Allocate 64-bit BARs above 4G when possible - 1 messages, 1 author
http://groups.google.com/group/linux.kernel/t/c274f90c943e3a33?hl=en
* mm/mempolicy: correct putback method for isolate pages if failed - 1
messages, 1 author
http://groups.google.com/group/linux.kernel/t/0449b5a1e27f3cb7?hl=en
* x86: Add check for number of available vectors before CPU down [v6] - 1
messages, 1 author
http://groups.google.com/group/linux.kernel/t/3ccfd69cb2196147?hl=en

==============================================================================
TOPIC: kernfs: remove kernfs_addrm_cxt
http://groups.google.com/group/linux.kernel/t/b72ccc5edcdfb313?hl=en
==============================================================================

== 1 of 6 ==
Date: Tues, Jan 7 2014 10:10 am
From: Tejun Heo


kernfs_addrm_cxt and the accompanying kernfs_addrm_start/finish() were
added because there were operations which should be performed outside
kernfs_mutex after adding and removing kernfs_nodes. The necessary
operations were recorded in kernfs_addrm_cxt and performed by
kernfs_addrm_finish(); however, after the recent changes which
relocated deactivation and unmapping so that they're performed
directly during removal, the only operation kernfs_addrm_finish()
performs is kernfs_put(), which can be moved inside the removal path
too.

This patch moves the kernfs_put() of the base ref to __kernfs_remove()
and remove kernfs_addrm_cxt and kernfs_addrm_start/finish().

* kernfs_add_one() is updated to grab and release kernfs_mutex itself.
sysfs_addrm_start/finish() invocations around it are removed from
all users.

* __kernfs_remove() puts an unlinked node directly instead of chaining
it to kernfs_addrm_cxt. Its callers are updated to grab and release
kernfs_mutex instead of calling kernfs_addrm_start/finish() around
it.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
fs/kernfs/dir.c | 110 ++++++++++----------------------------------
fs/kernfs/file.c | 6 +--
fs/kernfs/kernfs-internal.h | 12 +----
fs/kernfs/symlink.c | 6 +--
include/linux/kernfs.h | 4 --
5 files changed, 28 insertions(+), 110 deletions(-)

diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index ff633d2..07cf22c 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -377,28 +377,7 @@ struct kernfs_node *kernfs_new_node(struct kernfs_root *root, const char *name,
}

/**
- * kernfs_addrm_start - prepare for kernfs_node add/remove
- * @acxt: pointer to kernfs_addrm_cxt to be used
- *
- * This function is called when the caller is about to add or remove
- * kernfs_node. This function acquires kernfs_mutex. @acxt is used
- * to keep and pass context to other addrm functions.
- *
- * LOCKING:
- * Kernel thread context (may sleep). kernfs_mutex is locked on
- * return.
- */
-void kernfs_addrm_start(struct kernfs_addrm_cxt *acxt)
- __acquires(kernfs_mutex)
-{
- memset(acxt, 0, sizeof(*acxt));
-
- mutex_lock(&kernfs_mutex);
-}
-
-/**
* kernfs_add_one - add kernfs_node to parent without warning
- * @acxt: addrm context to use
* @kn: kernfs_node to be added
* @parent: the parent kernfs_node to add @kn to
*
@@ -406,35 +385,30 @@ void kernfs_addrm_start(struct kernfs_addrm_cxt *acxt)
* parent inode if @kn is a directory and link into the children list
* of the parent.
*
- * This function should be called between calls to
- * kernfs_addrm_start() and kernfs_addrm_finish() and should be passed
- * the same @acxt as passed to kernfs_addrm_start().
- *
- * LOCKING:
- * Determined by kernfs_addrm_start().
- *
* RETURNS:
* 0 on success, -EEXIST if entry with the given name already
* exists.
*/
-int kernfs_add_one(struct kernfs_addrm_cxt *acxt, struct kernfs_node *kn,
- struct kernfs_node *parent)
+int kernfs_add_one(struct kernfs_node *kn, struct kernfs_node *parent)
{
- bool has_ns = kernfs_ns_enabled(parent);
struct kernfs_iattrs *ps_iattr;
+ bool has_ns;
int ret;

- if (has_ns != (bool)kn->ns) {
- WARN(1, KERN_WARNING "kernfs: ns %s in '%s' for '%s'\n",
- has_ns ? "required" : "invalid", parent->name, kn->name);
- return -EINVAL;
- }
+ mutex_lock(&kernfs_mutex);
+
+ ret = -EINVAL;
+ has_ns = kernfs_ns_enabled(parent);
+ if (WARN(has_ns != (bool)kn->ns, KERN_WARNING "kernfs: ns %s in '%s' for '%s'\n",
+ has_ns ? "required" : "invalid", parent->name, kn->name))
+ goto out_unlock;

if (kernfs_type(parent) != KERNFS_DIR)
- return -EINVAL;
+ goto out_unlock;

+ ret = -ENOENT;
if (parent->flags & KERNFS_REMOVED)
- return -ENOENT;
+ goto out_unlock;

kn->hash = kernfs_name_hash(kn->name, kn->ns);
kn->parent = parent;
@@ -442,7 +416,7 @@ int kernfs_add_one(struct kernfs_addrm_cxt *acxt, struct kernfs_node *kn,

ret = kernfs_link_sibling(kn);
if (ret)
- return ret;
+ goto out_unlock;

/* Update timestamps on the parent */
ps_iattr = parent->iattr;
@@ -453,35 +427,10 @@ int kernfs_add_one(struct kernfs_addrm_cxt *acxt, struct kernfs_node *kn,

/* Mark the entry added into directory tree */
kn->flags &= ~KERNFS_REMOVED;
-
- return 0;
-}
-
-/**
- * kernfs_addrm_finish - finish up kernfs_node add/remove
- * @acxt: addrm context to finish up
- *
- * Finish up kernfs_node add/remove. Resources acquired by
- * kernfs_addrm_start() are released and removed kernfs_nodes are
- * cleaned up.
- *
- * LOCKING:
- * kernfs_mutex is released.
- */
-void kernfs_addrm_finish(struct kernfs_addrm_cxt *acxt)
- __releases(kernfs_mutex)
-{
- /* release resources acquired by kernfs_addrm_start() */
+ ret = 0;
+out_unlock:
mutex_unlock(&kernfs_mutex);
-
- /* kill removed kernfs_nodes */
- while (acxt->removed) {
- struct kernfs_node *kn = acxt->removed;
-
- acxt->removed = kn->u.removed_list;
-
- kernfs_put(kn);
- }
+ return ret;
}

/**
@@ -613,7 +562,6 @@ struct kernfs_node *kernfs_create_dir_ns(struct kernfs_node *parent,
const char *name, umode_t mode,
void *priv, const void *ns)
{
- struct kernfs_addrm_cxt acxt;
struct kernfs_node *kn;
int rc;

@@ -628,10 +576,7 @@ struct kernfs_node *kernfs_create_dir_ns(struct kernfs_node *parent,
kn->priv = priv;

/* link in */
- kernfs_addrm_start(&acxt);
- rc = kernfs_add_one(&acxt, kn, parent);
- kernfs_addrm_finish(&acxt);
-
+ rc = kernfs_add_one(kn, parent);
if (!rc)
return kn;

@@ -784,8 +729,7 @@ static struct kernfs_node *kernfs_next_descendant_post(struct kernfs_node *pos,
return pos->parent;
}

-static void __kernfs_remove(struct kernfs_addrm_cxt *acxt,
- struct kernfs_node *kn)
+static void __kernfs_remove(struct kernfs_node *kn)
{
struct kernfs_node *pos, *next;

@@ -832,8 +776,7 @@ static void __kernfs_remove(struct kernfs_addrm_cxt *acxt,
ps_iattr->ia_iattr.ia_mtime = CURRENT_TIME;
}

- pos->u.removed_list = acxt->removed;
- acxt->removed = pos;
+ kernfs_put(pos);
}

kernfs_put(pos);
@@ -848,11 +791,9 @@ static void __kernfs_remove(struct kernfs_addrm_cxt *acxt,
*/
void kernfs_remove(struct kernfs_node *kn)
{
- struct kernfs_addrm_cxt acxt;
-
- kernfs_addrm_start(&acxt);
- __kernfs_remove(&acxt, kn);
- kernfs_addrm_finish(&acxt);
+ mutex_lock(&kernfs_mutex);
+ __kernfs_remove(kn);
+ mutex_unlock(&kernfs_mutex);
}

/**
@@ -867,7 +808,6 @@ void kernfs_remove(struct kernfs_node *kn)
int kernfs_remove_by_name_ns(struct kernfs_node *parent, const char *name,
const void *ns)
{
- struct kernfs_addrm_cxt acxt;
struct kernfs_node *kn;

if (!parent) {
@@ -876,13 +816,13 @@ int kernfs_remove_by_name_ns(struct kernfs_node *parent, const char *name,
return -ENOENT;
}

- kernfs_addrm_start(&acxt);
+ mutex_lock(&kernfs_mutex);

kn = kernfs_find_ns(parent, name, ns);
if (kn)
- __kernfs_remove(&acxt, kn);
+ __kernfs_remove(kn);

- kernfs_addrm_finish(&acxt);
+ mutex_unlock(&kernfs_mutex);

if (kn)
return 0;
diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
index bdd3885..cd1022c 100644
--- a/fs/kernfs/file.c
+++ b/fs/kernfs/file.c
@@ -820,7 +820,6 @@ struct kernfs_node *__kernfs_create_file(struct kernfs_node *parent,
bool name_is_static,
struct lock_class_key *key)
{
- struct kernfs_addrm_cxt acxt;
struct kernfs_node *kn;
unsigned flags;
int rc;
@@ -856,10 +855,7 @@ struct kernfs_node *__kernfs_create_file(struct kernfs_node *parent,
if (ops->mmap)
kn->flags |= KERNFS_HAS_MMAP;

- kernfs_addrm_start(&acxt);
- rc = kernfs_add_one(&acxt, kn, parent);
- kernfs_addrm_finish(&acxt);
-
+ rc = kernfs_add_one(kn, parent);
if (rc) {
kernfs_put(kn);
return ERR_PTR(rc);
diff --git a/fs/kernfs/kernfs-internal.h b/fs/kernfs/kernfs-internal.h
index c6ba5bc..8af635b 100644
--- a/fs/kernfs/kernfs-internal.h
+++ b/fs/kernfs/kernfs-internal.h
@@ -45,13 +45,6 @@ static inline struct kernfs_root *kernfs_root(struct kernfs_node *kn)
}

/*
- * Context structure to be used while adding/removing nodes.
- */
-struct kernfs_addrm_cxt {
- struct kernfs_node *removed;
-};
-
-/*
* mount.c
*/
struct kernfs_super_info {
@@ -100,10 +93,7 @@ extern const struct inode_operations kernfs_dir_iops;

struct kernfs_node *kernfs_get_active(struct kernfs_node *kn);
void kernfs_put_active(struct kernfs_node *kn);
-void kernfs_addrm_start(struct kernfs_addrm_cxt *acxt);
-int kernfs_add_one(struct kernfs_addrm_cxt *acxt, struct kernfs_node *kn,
- struct kernfs_node *parent);
-void kernfs_addrm_finish(struct kernfs_addrm_cxt *acxt);
+int kernfs_add_one(struct kernfs_node *kn, struct kernfs_node *parent);
struct kernfs_node *kernfs_new_node(struct kernfs_root *root, const char *name,
umode_t mode, unsigned flags);

diff --git a/fs/kernfs/symlink.c b/fs/kernfs/symlink.c
index a03e260..3a939c2 100644
--- a/fs/kernfs/symlink.c
+++ b/fs/kernfs/symlink.c
@@ -27,7 +27,6 @@ struct kernfs_node *kernfs_create_link(struct kernfs_node *parent,
struct kernfs_node *target)
{
struct kernfs_node *kn;
- struct kernfs_addrm_cxt acxt;
int error;

kn = kernfs_new_node(kernfs_root(parent), name, S_IFLNK|S_IRWXUGO,
@@ -40,10 +39,7 @@ struct kernfs_node *kernfs_create_link(struct kernfs_node *parent,
kn->symlink.target_kn = target;
kernfs_get(target); /* ref owned by symlink */

- kernfs_addrm_start(&acxt);
- error = kernfs_add_one(&acxt, kn, parent);
- kernfs_addrm_finish(&acxt);
-
+ error = kernfs_add_one(kn, parent);
if (!error)
return kn;

diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
index 232f1a6..4b68b6f 100644
--- a/include/linux/kernfs.h
+++ b/include/linux/kernfs.h
@@ -90,10 +90,6 @@ struct kernfs_node {

struct rb_node rb;

- union {
- struct kernfs_node *removed_list;
- } u;
-
const void *ns; /* namespace tag */
unsigned int hash; /* ns + name hash */
union {
--
1.8.4.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/




== 2 of 6 ==
Date: Tues, Jan 7 2014 10:10 am
From: Tejun Heo


When kernfs_seq_start() fails to obtain an active reference, it
returns ERR_PTR(-ENODEV). kernfs_seq_stop() is then invoked with the
error pointer value; however, it still proceeds to invoke
kernfs_put_active() on the node leading to unbalanced put.

If kernfs_seq_stop() is called even after active ref failure, it
should skip invocation of @ops->seq_stop() and put_active.
Unfortunately, this is a bit complicated because active ref failure
isn't the only thing which may fail with ERR_PTR(-ENODEV).
@ops->seq_start/next() may also fail with the error value and
kernfs_seq_stop() doesn't have a way to tell apart those failures.

Work it around by factoring out the active part of kernfs_seq_stop()
into kernfs_seq_stop_active() and invoking it directly if
@ops->seq_start/next() fail with ERR_PTR(-ENODEV) and updating
kernfs_seq_stop() to skip kernfs_seq_stop_active() on
ERR_PTR(-ENODEV). This is a bit nasty but ensures that the active put
is skipped iff get_active failed in kernfs_seq_start().

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Sasha Levin <sasha.levin@oracle.com>
---
Sasha, I *think* this chould be the fix for the problem you reported
with trinity fuzzying. Is the problem reproducible? If so, can you
please test it with this patch applied?

Thanks!

fs/kernfs/file.c | 51 ++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 44 insertions(+), 7 deletions(-)

diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
index 316604c..bdd3885 100644
--- a/fs/kernfs/file.c
+++ b/fs/kernfs/file.c
@@ -54,6 +54,38 @@ static const struct kernfs_ops *kernfs_ops(struct kernfs_node *kn)
return kn->attr.ops;
}

+/*
+ * As kernfs_seq_stop() is also called after kernfs_seq_start() or
+ * kernfs_seq_next() failure, it needs to distinguish whether it's stopping
+ * a seq_file iteration which is fully initialized with an active reference
+ * or an aborted kernfs_seq_start() due to get_active failure. The
+ * position pointer is the only context for each seq_file iteration and
+ * thus the stop condition should be encoded in it. As the return value is
+ * directly visible to userland, ERR_PTR(-ENODEV) is the only acceptable
+ * choice to indicate get_active failure.
+ *
+ * Unfortunately, this is complicated due to the optional custom seq_file
+ * operations which may return ERR_PTR(-ENODEV) too. kernfs_seq_stop()
+ * can't distinguish whether ERR_PTR(-ENODEV) is from get_active failure or
+ * custom seq_file operations and thus can't decide whether put_active
+ * should be performed or not only on ERR_PTR(-ENODEV).
+ *
+ * This is worked around by factoring out the custom seq_stop() and
+ * put_active part into kernfs_seq_stop_active(), skipping it from
+ * kernfs_seq_stop() if ERR_PTR(-ENODEV) while invoking it directly after
+ * custom seq_file operations fail with ERR_PTR(-ENODEV) - this ensures
+ * that kernfs_seq_stop_active() is skipped only after get_active failure.
+ */
+static void kernfs_seq_stop_active(struct seq_file *sf, void *v)
+{
+ struct kernfs_open_file *of = sf->private;
+ const struct kernfs_ops *ops = kernfs_ops(of->kn);
+
+ if (ops->seq_stop)
+ ops->seq_stop(sf, v);
+ kernfs_put_active(of->kn);
+}
+
static void *kernfs_seq_start(struct seq_file *sf, loff_t *ppos)
{
struct kernfs_open_file *of = sf->private;
@@ -69,7 +101,11 @@ static void *kernfs_seq_start(struct seq_file *sf, loff_t *ppos)

ops = kernfs_ops(of->kn);
if (ops->seq_start) {
- return ops->seq_start(sf, ppos);
+ void *next = ops->seq_start(sf, ppos);
+ /* see the comment above kernfs_seq_stop_active() */
+ if (next == ERR_PTR(-ENODEV))
+ kernfs_seq_stop_active(sf, next);
+ return next;
} else {
/*
* The same behavior and code as single_open(). Returns
@@ -85,7 +121,11 @@ static void *kernfs_seq_next(struct seq_file *sf, void *v, loff_t *ppos)
const struct kernfs_ops *ops = kernfs_ops(of->kn);

if (ops->seq_next) {
- return ops->seq_next(sf, v, ppos);
+ void *next = ops->seq_next(sf, v, ppos);
+ /* see the comment above kernfs_seq_stop_active() */
+ if (next == ERR_PTR(-ENODEV))
+ kernfs_seq_stop_active(sf, next);
+ return next;
} else {
/*
* The same behavior and code as single_open(), always
@@ -99,12 +139,9 @@ static void *kernfs_seq_next(struct seq_file *sf, void *v, loff_t *ppos)
static void kernfs_seq_stop(struct seq_file *sf, void *v)
{
struct kernfs_open_file *of = sf->private;
- const struct kernfs_ops *ops = kernfs_ops(of->kn);

- if (ops->seq_stop)
- ops->seq_stop(sf, v);
-
- kernfs_put_active(of->kn);
+ if (v != ERR_PTR(-ENODEV))
+ kernfs_seq_stop_active(sf, v);
mutex_unlock(&of->mutex);
}

--
1.8.4.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/




== 3 of 6 ==
Date: Tues, Jan 7 2014 10:10 am
From: Tejun Heo


There currently are two mechanisms gating active ref lockdep
annotations - KERNFS_LOCKDEP flag and KERNFS_ACTIVE_REF type mask.
The former disables lockdep annotations in kernfs_get/put_active()
while the latter disables all of kernfs_deactivate().

While KERNFS_ACTIVE_REF also behaves as an optimization to skip the
deactivation step for non-file nodes, the benefit is marginal and it
needlessly diverges code paths. Let's drop KERNFS_ACTIVE_REF and use
KERNFS_LOCKDEP in kernfs_deactivate() too.

While at it, add a test helper kernfs_lockdep() to test KERNFS_LOCKDEP
flag so that it's more convenient and the related code can be compiled
out when not enabled.

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

diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index 07cf22c..8abdf07 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -22,6 +22,15 @@ DEFINE_MUTEX(kernfs_mutex);

#define rb_to_kn(X) rb_entry((X), struct kernfs_node, rb)

+static bool kernfs_lockdep(struct kernfs_node *kn)
+{
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+ return kn->flags & KERNFS_LOCKDEP;
+#else
+ return false;
+