Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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>

+21 -6
+2 -1
fs/fscache/internal.h
··· 125 125 extern int fscache_submit_op(struct fscache_object *, 126 126 struct fscache_operation *); 127 127 extern int fscache_cancel_op(struct fscache_operation *, 128 - void (*)(struct fscache_operation *)); 128 + void (*)(struct fscache_operation *), 129 + bool); 129 130 extern void fscache_cancel_all_ops(struct fscache_object *); 130 131 extern void fscache_abort_object(struct fscache_object *); 131 132 extern void fscache_start_operations(struct fscache_object *);
+17 -3
fs/fscache/operation.c
··· 312 312 * cancel an operation that's pending on an object 313 313 */ 314 314 int 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 318 struct fscache_object *object = op->object; 319 + bool put = false; 318 320 int ret; 319 321 320 322 _enter("OBJ%x OP%x}", op->object->debug_id, op->debug_id); ··· 330 328 ret = -EBUSY; 331 329 if (op->state == FSCACHE_OP_ST_PENDING) { 332 330 ASSERT(!list_empty(&op->pend_link)); 333 - fscache_stat(&fscache_n_op_cancelled); 334 331 list_del_init(&op->pend_link); 332 + put = true; 333 + fscache_stat(&fscache_n_op_cancelled); 335 334 if (do_cancel) 336 335 do_cancel(op); 337 336 op->state = FSCACHE_OP_ST_CANCELLED; ··· 340 337 object->n_exclusive--; 341 338 if (test_and_clear_bit(FSCACHE_OP_WAITING, &op->flags)) 342 339 wake_up_bit(&op->flags, FSCACHE_OP_WAITING); 343 - fscache_put_operation(op); 340 + ret = 0; 341 + } else if (op->state == FSCACHE_OP_ST_IN_PROGRESS && cancel_in_progress_op) { 342 + fscache_stat(&fscache_n_op_cancelled); 343 + if (do_cancel) 344 + do_cancel(op); 345 + op->state = FSCACHE_OP_ST_CANCELLED; 346 + if (test_bit(FSCACHE_OP_EXCLUSIVE, &op->flags)) 347 + object->n_exclusive--; 348 + if (test_and_clear_bit(FSCACHE_OP_WAITING, &op->flags)) 349 + wake_up_bit(&op->flags, FSCACHE_OP_WAITING); 344 350 ret = 0; 345 351 } 346 352 353 + if (put) 354 + fscache_put_operation(op); 347 355 spin_unlock(&object->lock); 348 356 _leave(" = %d", ret); 349 357 return ret;
+2 -2
fs/fscache/page.c
··· 359 359 fscache_stat(stat_op_waits); 360 360 if (wait_on_bit(&op->flags, FSCACHE_OP_WAITING, 361 361 TASK_INTERRUPTIBLE) != 0) { 362 - ret = fscache_cancel_op(op, do_cancel); 362 + ret = fscache_cancel_op(op, do_cancel, false); 363 363 if (ret == 0) 364 364 return -ERESTARTSYS; 365 365 ··· 380 380 if (unlikely(fscache_object_is_dying(object) || 381 381 fscache_cache_is_broken(object))) { 382 382 enum fscache_operation_state state = op->state; 383 - fscache_cancel_op(op, do_cancel); 383 + fscache_cancel_op(op, do_cancel, true); 384 384 if (stat_object_dead) 385 385 fscache_stat(stat_object_dead); 386 386 _leave(" = -ENOBUFS [obj dead %d]", state);