aboutsummaryrefslogtreecommitdiffstats
path: root/fs/fscache
diff options
context:
space:
mode:
authorDavid Howells <dhowells@redhat.com>2015-02-24 05:05:28 -0500
committerDavid Howells <dhowells@redhat.com>2015-04-02 09:28:53 -0400
commit418b7eb9e1011bc11220a03ad0045885d04698d2 (patch)
treed39aa74ea6c7251a396ccbd5d220a54b1cdcf140 /fs/fscache
parent87021526300f1a292dd966e141e183630ac95317 (diff)
FS-Cache: Permit fscache_cancel_op() to cancel in-progress operations too
Currently, fscache_cancel_op() only cancels pending operations - attempts to cancel in-progress operations are ignored. This leads to a problem in fscache_wait_for_operation_activation() whereby the wait is terminated, but the object has been killed. The check at the end of the function now triggers because it's no longer contingent on the cache having produced an I/O error since the commit that fixed the logic error in fscache_object_is_dead(). The result of the check is that it tries to cancel the operation - but since the object may not be pending by this point, the cancellation request may be ignored - with the result that the the object is just put by the caller and fscache_put_operation has an assertion failure because the operation isn't in either the COMPLETE or the CANCELLED states. To fix this, we permit in-progress ops to be cancelled under some circumstances. The bug results in an oops that looks something like this: FS-Cache: fscache_wait_for_operation_activation() = -ENOBUFS [obj dead 3] FS-Cache: FS-Cache: Assertion failed FS-Cache: 3 == 5 is false ------------[ cut here ]------------ kernel BUG at ../fs/fscache/operation.c:432! ... RIP: 0010:[<ffffffffa0088574>] fscache_put_operation+0xf2/0x2cd Call Trace: [<ffffffffa008b92a>] __fscache_read_or_alloc_pages+0x2ec/0x3b3 [<ffffffffa00b761f>] __nfs_readpages_from_fscache+0x59/0xbf [nfs] [<ffffffffa00b06c5>] nfs_readpages+0x10c/0x185 [nfs] [<ffffffff81124925>] ? alloc_pages_current+0x119/0x13e [<ffffffff810ee5fd>] ? __page_cache_alloc+0xfb/0x10a [<ffffffff810f87f8>] __do_page_cache_readahead+0x188/0x22c [<ffffffff810f8b3a>] ondemand_readahead+0x29e/0x2af [<ffffffff810f8c92>] page_cache_sync_readahead+0x38/0x3a [<ffffffff810ef337>] generic_file_read_iter+0x1a2/0x55a [<ffffffffa00a9dff>] ? nfs_revalidate_mapping+0xd6/0x288 [nfs] [<ffffffffa00a6a23>] nfs_file_read+0x49/0x70 [nfs] [<ffffffff811363be>] new_sync_read+0x78/0x9c [<ffffffff81137164>] __vfs_read+0x13/0x38 [<ffffffff8113721e>] vfs_read+0x95/0x121 [<ffffffff811372f6>] SyS_read+0x4c/0x8a [<ffffffff81557a52>] system_call_fastpath+0x12/0x17 Signed-off-by: David Howells <dhowells@redhat.com> Reviewed-by: Steve Dickson <steved@redhat.com> Acked-by: Jeff Layton <jeff.layton@primarydata.com>
Diffstat (limited to 'fs/fscache')
-rw-r--r--fs/fscache/internal.h3
-rw-r--r--fs/fscache/operation.c20
-rw-r--r--fs/fscache/page.c4
3 files changed, 21 insertions, 6 deletions
diff --git a/fs/fscache/internal.h b/fs/fscache/internal.h
index 3063a58b7d3d..87c4544ec912 100644
--- a/fs/fscache/internal.h
+++ b/fs/fscache/internal.h
@@ -125,7 +125,8 @@ extern int fscache_submit_exclusive_op(struct fscache_object *,
125extern int fscache_submit_op(struct fscache_object *, 125extern int fscache_submit_op(struct fscache_object *,
126 struct fscache_operation *); 126 struct fscache_operation *);
127extern int fscache_cancel_op(struct fscache_operation *, 127extern int fscache_cancel_op(struct fscache_operation *,
128 void (*)(struct fscache_operation *)); 128 void (*)(struct fscache_operation *),
129 bool);
129extern void fscache_cancel_all_ops(struct fscache_object *); 130extern void fscache_cancel_all_ops(struct fscache_object *);
130extern void fscache_abort_object(struct fscache_object *); 131extern void fscache_abort_object(struct fscache_object *);
131extern void fscache_start_operations(struct fscache_object *); 132extern void fscache_start_operations(struct fscache_object *);
diff --git a/fs/fscache/operation.c b/fs/fscache/operation.c
index 18658fffbba1..67594a8d791a 100644
--- a/fs/fscache/operation.c
+++ b/fs/fscache/operation.c
@@ -312,9 +312,11 @@ void fscache_start_operations(struct fscache_object *object)
312 * cancel an operation that's pending on an object 312 * cancel an operation that's pending on an object
313 */ 313 */
314int fscache_cancel_op(struct fscache_operation *op, 314int fscache_cancel_op(struct fscache_operation *op,
315 void (*do_cancel)(struct fscache_operation *)) 315 void (*do_cancel)(struct fscache_operation *),
316 bool cancel_in_progress_op)
316{ 317{
317 struct fscache_object *object = op->object; 318 struct fscache_object *object = op->object;
319 bool put = false;
318 int ret; 320 int ret;
319 321
320 _enter("OBJ%x OP%x}", op->object->debug_id, op->debug_id); 322 _enter("OBJ%x OP%x}", op->object->debug_id, op->debug_id);
@@ -328,8 +330,19 @@ int fscache_cancel_op(struct fscache_operation *op,
328 ret = -EBUSY; 330 ret = -EBUSY;
329 if (op->state == FSCACHE_OP_ST_PENDING) { 331 if (op->state == FSCACHE_OP_ST_PENDING) {
330 ASSERT(!list_empty(&op->pend_link)); 332 ASSERT(!list_empty(&op->pend_link));
331 fscache_stat(&fscache_n_op_cancelled);
332 list_del_init(&op->pend_link); 333 list_del_init(&op->pend_link);
334 put = true;
335 fscache_stat(&fscache_n_op_cancelled);
336 if (do_cancel)
337 do_cancel(op);
338 op->state = FSCACHE_OP_ST_CANCELLED;
339 if (test_bit(FSCACHE_OP_EXCLUSIVE, &op->flags))
340 object->n_exclusive--;
341 if (test_and_clear_bit(FSCACHE_OP_WAITING, &op->flags))
342 wake_up_bit(&op->flags, FSCACHE_OP_WAITING);
343 ret = 0;
344 } else if (op->state == FSCACHE_OP_ST_IN_PROGRESS && cancel_in_progress_op) {
345 fscache_stat(&fscache_n_op_cancelled);
333 if (do_cancel) 346 if (do_cancel)
334 do_cancel(op); 347 do_cancel(op);
335 op->state = FSCACHE_OP_ST_CANCELLED; 348 op->state = FSCACHE_OP_ST_CANCELLED;
@@ -337,10 +350,11 @@ int fscache_cancel_op(struct fscache_operation *op,
337 object->n_exclusive--; 350 object->n_exclusive--;
338 if (test_and_clear_bit(FSCACHE_OP_WAITING, &op->flags)) 351 if (test_and_clear_bit(FSCACHE_OP_WAITING, &op->flags))
339 wake_up_bit(&op->flags, FSCACHE_OP_WAITING); 352 wake_up_bit(&op->flags, FSCACHE_OP_WAITING);
340 fscache_put_operation(op);
341 ret = 0; 353 ret = 0;
342 } 354 }
343 355
356 if (put)
357 fscache_put_operation(op);
344 spin_unlock(&object->lock); 358 spin_unlock(&object->lock);
345 _leave(" = %d", ret); 359 _leave(" = %d", ret);
346 return ret; 360 return ret;
diff --git a/fs/fscache/page.c b/fs/fscache/page.c
index d0805e31361c..433cae927eca 100644
--- a/fs/fscache/page.c
+++ b/fs/fscache/page.c
@@ -359,7 +359,7 @@ int fscache_wait_for_operation_activation(struct fscache_object *object,
359 fscache_stat(stat_op_waits); 359 fscache_stat(stat_op_waits);
360 if (wait_on_bit(&op->flags, FSCACHE_OP_WAITING, 360 if (wait_on_bit(&op->flags, FSCACHE_OP_WAITING,
361 TASK_INTERRUPTIBLE) != 0) { 361 TASK_INTERRUPTIBLE) != 0) {
362 ret = fscache_cancel_op(op, do_cancel); 362 ret = fscache_cancel_op(op, do_cancel, false);
363 if (ret == 0) 363 if (ret == 0)
364 return -ERESTARTSYS; 364 return -ERESTARTSYS;
365 365
@@ -380,7 +380,7 @@ check_if_dead:
380 if (unlikely(fscache_object_is_dying(object) || 380 if (unlikely(fscache_object_is_dying(object) ||
381 fscache_cache_is_broken(object))) { 381 fscache_cache_is_broken(object))) {
382 enum fscache_operation_state state = op->state; 382 enum fscache_operation_state state = op->state;
383 fscache_cancel_op(op, do_cancel); 383 fscache_cancel_op(op, do_cancel, true);
384 if (stat_object_dead) 384 if (stat_object_dead)
385 fscache_stat(stat_object_dead); 385 fscache_stat(stat_object_dead);
386 _leave(" = -ENOBUFS [obj dead %d]", state); 386 _leave(" = -ENOBUFS [obj dead %d]", state);