From 683bb2761fbf123b24aed03a1c0d5d7556ec3018 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Mon, 13 Jan 2014 14:49:01 -0800 Subject: [PATCH] Revert "kernfs: fix get_active failure handling in kernfs_seq_*()" This reverts commit d92d2e6bd72b653f9811e0c9c46307c743b3fc58. Tejun writes: I'm sorry but can you please revert the whole series? get_active() waiting while a node is deactivated has potential to lead to deadlock and that deactivate/reactivate interface is something fundamentally flawed and that cgroup will have to work with the remove_self() like everybody else. IOW, I think the first posting was correct. Cc: Tejun Heo Signed-off-by: Greg Kroah-Hartman --- fs/kernfs/file.c | 51 +++++++----------------------------------------- 1 file changed, 7 insertions(+), 44 deletions(-) diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c index bdd38854ef65..316604cc3a1c 100644 --- a/fs/kernfs/file.c +++ b/fs/kernfs/file.c @@ -54,38 +54,6 @@ 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; @@ -101,11 +69,7 @@ static void *kernfs_seq_start(struct seq_file *sf, loff_t *ppos) ops = kernfs_ops(of->kn); if (ops->seq_start) { - 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; + return ops->seq_start(sf, ppos); } else { /* * The same behavior and code as single_open(). Returns @@ -121,11 +85,7 @@ 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) { - 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; + return ops->seq_next(sf, v, ppos); } else { /* * The same behavior and code as single_open(), always @@ -139,9 +99,12 @@ 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 (v != ERR_PTR(-ENODEV)) - kernfs_seq_stop_active(sf, v); + if (ops->seq_stop) + ops->seq_stop(sf, v); + + kernfs_put_active(of->kn); mutex_unlock(&of->mutex); } -- 2.30.2