diff options
author | Miklos Szeredi <miklos@szeredi.hu> | 2006-02-05 02:27:40 -0500 |
---|---|---|
committer | Linus Torvalds <torvalds@g5.osdl.org> | 2006-02-05 14:06:51 -0500 |
commit | 7128ec2a747d7a5f3c764c37bef17081ccc2374c (patch) | |
tree | 10781a63d46811789e1cd26964f1d0a9eb963ce2 | |
parent | e22bec266cd6f540da2a61db216914c3473135cc (diff) |
[PATCH] fuse: fix request_end() vs fuse_reset_request() race
The last fix for this function in fact opened up a much more often
triggering race.
It was uncommented tricky code, that was buggy. Add comment, make it less
tricky and fix bug.
Signed-off-by: Miklos Szeredi <miklos@szeredi.hu>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
-rw-r--r-- | fs/fuse/dev.c | 40 |
1 files changed, 29 insertions, 11 deletions
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c index 4526da8907c6..f556a0d5c0d3 100644 --- a/fs/fuse/dev.c +++ b/fs/fuse/dev.c | |||
@@ -120,9 +120,9 @@ struct fuse_req *fuse_get_request(struct fuse_conn *fc) | |||
120 | return do_get_request(fc); | 120 | return do_get_request(fc); |
121 | } | 121 | } |
122 | 122 | ||
123 | /* Must be called with fuse_lock held */ | ||
123 | static void fuse_putback_request(struct fuse_conn *fc, struct fuse_req *req) | 124 | static void fuse_putback_request(struct fuse_conn *fc, struct fuse_req *req) |
124 | { | 125 | { |
125 | spin_lock(&fuse_lock); | ||
126 | if (req->preallocated) { | 126 | if (req->preallocated) { |
127 | atomic_dec(&fc->num_waiting); | 127 | atomic_dec(&fc->num_waiting); |
128 | list_add(&req->list, &fc->unused_list); | 128 | list_add(&req->list, &fc->unused_list); |
@@ -134,11 +134,19 @@ static void fuse_putback_request(struct fuse_conn *fc, struct fuse_req *req) | |||
134 | fc->outstanding_debt--; | 134 | fc->outstanding_debt--; |
135 | else | 135 | else |
136 | up(&fc->outstanding_sem); | 136 | up(&fc->outstanding_sem); |
137 | spin_unlock(&fuse_lock); | ||
138 | } | 137 | } |
139 | 138 | ||
140 | void fuse_put_request(struct fuse_conn *fc, struct fuse_req *req) | 139 | void fuse_put_request(struct fuse_conn *fc, struct fuse_req *req) |
141 | { | 140 | { |
141 | if (atomic_dec_and_test(&req->count)) { | ||
142 | spin_lock(&fuse_lock); | ||
143 | fuse_putback_request(fc, req); | ||
144 | spin_unlock(&fuse_lock); | ||
145 | } | ||
146 | } | ||
147 | |||
148 | static void fuse_put_request_locked(struct fuse_conn *fc, struct fuse_req *req) | ||
149 | { | ||
142 | if (atomic_dec_and_test(&req->count)) | 150 | if (atomic_dec_and_test(&req->count)) |
143 | fuse_putback_request(fc, req); | 151 | fuse_putback_request(fc, req); |
144 | } | 152 | } |
@@ -163,26 +171,36 @@ void fuse_release_background(struct fuse_req *req) | |||
163 | * still waiting), the 'end' callback is called if given, else the | 171 | * still waiting), the 'end' callback is called if given, else the |
164 | * reference to the request is released | 172 | * reference to the request is released |
165 | * | 173 | * |
174 | * Releasing extra reference for foreground requests must be done | ||
175 | * within the same locked region as setting state to finished. This | ||
176 | * is because fuse_reset_request() may be called after request is | ||
177 | * finished and it must be the sole possessor. If request is | ||
178 | * interrupted and put in the background, it will return with an error | ||
179 | * and hence never be reset and reused. | ||
180 | * | ||
166 | * Called with fuse_lock, unlocks it | 181 | * Called with fuse_lock, unlocks it |
167 | */ | 182 | */ |
168 | static void request_end(struct fuse_conn *fc, struct fuse_req *req) | 183 | static void request_end(struct fuse_conn *fc, struct fuse_req *req) |
169 | { | 184 | { |
170 | void (*end) (struct fuse_conn *, struct fuse_req *) = req->end; | ||
171 | req->end = NULL; | ||
172 | list_del(&req->list); | 185 | list_del(&req->list); |
173 | req->state = FUSE_REQ_FINISHED; | 186 | req->state = FUSE_REQ_FINISHED; |
174 | spin_unlock(&fuse_lock); | 187 | if (!req->background) { |
175 | if (req->background) { | 188 | wake_up(&req->waitq); |
189 | fuse_put_request_locked(fc, req); | ||
190 | spin_unlock(&fuse_lock); | ||
191 | } else { | ||
192 | void (*end) (struct fuse_conn *, struct fuse_req *) = req->end; | ||
193 | req->end = NULL; | ||
194 | spin_unlock(&fuse_lock); | ||
176 | down_read(&fc->sbput_sem); | 195 | down_read(&fc->sbput_sem); |
177 | if (fc->mounted) | 196 | if (fc->mounted) |
178 | fuse_release_background(req); | 197 | fuse_release_background(req); |
179 | up_read(&fc->sbput_sem); | 198 | up_read(&fc->sbput_sem); |
199 | if (end) | ||
200 | end(fc, req); | ||
201 | else | ||
202 | fuse_put_request(fc, req); | ||
180 | } | 203 | } |
181 | wake_up(&req->waitq); | ||
182 | if (end) | ||
183 | end(fc, req); | ||
184 | else | ||
185 | fuse_put_request(fc, req); | ||
186 | } | 204 | } |
187 | 205 | ||
188 | /* | 206 | /* |