diff options
author | Greg Kroah-Hartman <gregkh@linuxfoundation.org> | 2014-01-13 17:49:01 -0500 |
---|---|---|
committer | Greg Kroah-Hartman <gregkh@linuxfoundation.org> | 2014-01-13 17:49:01 -0500 |
commit | 683bb2761fbf123b24aed03a1c0d5d7556ec3018 (patch) | |
tree | f6bb7a039fa1bf9387f70dfef9bec5474178547c /fs/kernfs | |
parent | 87da149343c8c93f6984c0f4b9da7651709624f7 (diff) |
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 <tj@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Diffstat (limited to 'fs/kernfs')
-rw-r--r-- | fs/kernfs/file.c | 51 |
1 files 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) | |||
54 | return kn->attr.ops; | 54 | return kn->attr.ops; |
55 | } | 55 | } |
56 | 56 | ||
57 | /* | ||
58 | * As kernfs_seq_stop() is also called after kernfs_seq_start() or | ||
59 | * kernfs_seq_next() failure, it needs to distinguish whether it's stopping | ||
60 | * a seq_file iteration which is fully initialized with an active reference | ||
61 | * or an aborted kernfs_seq_start() due to get_active failure. The | ||
62 | * position pointer is the only context for each seq_file iteration and | ||
63 | * thus the stop condition should be encoded in it. As the return value is | ||
64 | * directly visible to userland, ERR_PTR(-ENODEV) is the only acceptable | ||
65 | * choice to indicate get_active failure. | ||
66 | * | ||
67 | * Unfortunately, this is complicated due to the optional custom seq_file | ||
68 | * operations which may return ERR_PTR(-ENODEV) too. kernfs_seq_stop() | ||
69 | * can't distinguish whether ERR_PTR(-ENODEV) is from get_active failure or | ||
70 | * custom seq_file operations and thus can't decide whether put_active | ||
71 | * should be performed or not only on ERR_PTR(-ENODEV). | ||
72 | * | ||
73 | * This is worked around by factoring out the custom seq_stop() and | ||
74 | * put_active part into kernfs_seq_stop_active(), skipping it from | ||
75 | * kernfs_seq_stop() if ERR_PTR(-ENODEV) while invoking it directly after | ||
76 | * custom seq_file operations fail with ERR_PTR(-ENODEV) - this ensures | ||
77 | * that kernfs_seq_stop_active() is skipped only after get_active failure. | ||
78 | */ | ||
79 | static void kernfs_seq_stop_active(struct seq_file *sf, void *v) | ||
80 | { | ||
81 | struct kernfs_open_file *of = sf->private; | ||
82 | const struct kernfs_ops *ops = kernfs_ops(of->kn); | ||
83 | |||
84 | if (ops->seq_stop) | ||
85 | ops->seq_stop(sf, v); | ||
86 | kernfs_put_active(of->kn); | ||
87 | } | ||
88 | |||
89 | static void *kernfs_seq_start(struct seq_file *sf, loff_t *ppos) | 57 | static void *kernfs_seq_start(struct seq_file *sf, loff_t *ppos) |
90 | { | 58 | { |
91 | struct kernfs_open_file *of = sf->private; | 59 | struct kernfs_open_file *of = sf->private; |
@@ -101,11 +69,7 @@ static void *kernfs_seq_start(struct seq_file *sf, loff_t *ppos) | |||
101 | 69 | ||
102 | ops = kernfs_ops(of->kn); | 70 | ops = kernfs_ops(of->kn); |
103 | if (ops->seq_start) { | 71 | if (ops->seq_start) { |
104 | void *next = ops->seq_start(sf, ppos); | 72 | return ops->seq_start(sf, ppos); |
105 | /* see the comment above kernfs_seq_stop_active() */ | ||
106 | if (next == ERR_PTR(-ENODEV)) | ||
107 | kernfs_seq_stop_active(sf, next); | ||
108 | return next; | ||
109 | } else { | 73 | } else { |
110 | /* | 74 | /* |
111 | * The same behavior and code as single_open(). Returns | 75 | * 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) | |||
121 | const struct kernfs_ops *ops = kernfs_ops(of->kn); | 85 | const struct kernfs_ops *ops = kernfs_ops(of->kn); |
122 | 86 | ||
123 | if (ops->seq_next) { | 87 | if (ops->seq_next) { |
124 | void *next = ops->seq_next(sf, v, ppos); | 88 | return ops->seq_next(sf, v, ppos); |
125 | /* see the comment above kernfs_seq_stop_active() */ | ||
126 | if (next == ERR_PTR(-ENODEV)) | ||
127 | kernfs_seq_stop_active(sf, next); | ||
128 | return next; | ||
129 | } else { | 89 | } else { |
130 | /* | 90 | /* |
131 | * The same behavior and code as single_open(), always | 91 | * 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) | |||
139 | static void kernfs_seq_stop(struct seq_file *sf, void *v) | 99 | static void kernfs_seq_stop(struct seq_file *sf, void *v) |
140 | { | 100 | { |
141 | struct kernfs_open_file *of = sf->private; | 101 | struct kernfs_open_file *of = sf->private; |
102 | const struct kernfs_ops *ops = kernfs_ops(of->kn); | ||
142 | 103 | ||
143 | if (v != ERR_PTR(-ENODEV)) | 104 | if (ops->seq_stop) |
144 | kernfs_seq_stop_active(sf, v); | 105 | ops->seq_stop(sf, v); |
106 | |||
107 | kernfs_put_active(of->kn); | ||
145 | mutex_unlock(&of->mutex); | 108 | mutex_unlock(&of->mutex); |
146 | } | 109 | } |
147 | 110 | ||