Hi,
To be clear, I add more details to explain this patch. A simple example
call stack is below:
Thread #1
nfs_readpages
nfs_readpages_from_fscache
|__nfs_readpages_from_fscache
|__fscache_read_or_alloc_pages(...)
| op = fscache_alloc_retrieval(...) (op->usage=1)
| cachefiles_read_or_alloc_pages
| cachefiles_read_backing_file
| op->op.processor = cache_files_read_copier
| for each netpage in list of netpages
| fscache_get_retreival(op) (op->usage ++)
| add_page_wait_queue(monitor->monitor, cachefiles_read_waiter)
| fscache_put_retrieval(op) (op->usage —)
Thread #2
cachefiles_read_waiter
| fscache_enqueue_retrieval
| add the monitor to op->to_do list
| fscache_enqueue_operation
| fscache_get_retreival(op) (op->usage ++)
| queue_work(fscache_op_wq, &op->work) op->processor is set to
cachefiles_read_copier
Thread #3
fscache_op_work_func
| cachefiles_read_copier
| while (op->to_do) --> monitor {
| copy_highpage(monitor->netfs_page, monitor->back_page);
| put_page(monitor->back_page);
| fscache_end_io(op, monitor->netfs_page, error); --> notify page copy
done
| fscache_retrieval_complete(op, 1);
| fscache_put_retrieval(op); (op->usage—)
| kfree(monitor);
| fscache_put_retrieval(op); (op->usage --)
A potential race condition is happened on thread #2. Assume we have two or
more pages, the second fscache_enqueue_operation may be blocked, but the
second page monitor is added in to op->to_do list.
So the thread #3 may be started before the second fscache_enqueue_operation
func, then the fscache_put_opertion may check a nil op or the op's state is
changed.
Best Regards,
-Lei
On 22 February 2018 at 15:33, <carmark.dlut@xxxxxxxxx> wrote:
> From: Lei Xue <carmark.dlut@xxxxxxxxx>
>
> There is a potential race in fscache operation enqueuing for reading and
> copying multiple pages from cachefiles to netfs.
> Under some heavy load system, it will happen very often.
>
> If this race occurs, an oops similar to the following is seen:
>
> kernel BUG at fs/fscache/operation.c:69!
> invalid opcode: 0000 [#1] SMP
> …
> #0 [ffff883fff0838d8] machine_kexec at ffffffff81051beb
> #1 [ffff883fff083938] crash_kexec at ffffffff810f2542
> #2 [ffff883fff083a08] oops_end at ffffffff8163e1a8
> #3 [ffff883fff083a30] die at ffffffff8101859b
> #4 [ffff883fff083a60] do_trap at ffffffff8163d860
> #5 [ffff883fff083ab0] do_invalid_op at ffffffff81015204
> #6 [ffff883fff083b60] invalid_op at ffffffff8164701e
> [exception RIP: fscache_enqueue_operation+246]
> RIP: ffffffffa0b793c6 RSP: ffff883fff083c18 RFLAGS: 00010046
> RAX: 0000000000000019 RBX: ffff8832ed1a9ec0 RCX: 0000000000000006
> RDX: 0000000000000000 RSI: 0000000000000046 RDI: 0000000000000046
> RBP: ffff883fff083c20 R8: 0000000000000086 R9: 000000000000178f
> R10: ffffffff816aeb00 R11: ffff883fff08392e R12: ffff8802f0525620
> R13: ffff88407ffc01d8 R14: 0000000000000000 R15: 0000000000000003
> ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0000
> #7 [ffff883fff083c10] fscache_enqueue_operation at ffffffffa0b793c6
> #8 [ffff883fff083c28] cachefiles_read_waiter at ffffffffa0b15a48
> #9 [ffff883fff083c48] __wake_up_common at ffffffff810af028
>
> Signed-off-by: Lei Xue <carmark.dlut@xxxxxxxxx>
> ---
> fs/cachefiles/rdwr.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/cachefiles/rdwr.c b/fs/cachefiles/rdwr.c
> index 883bc7bb12c5..9d5d13e150fb 100644
> --- a/fs/cachefiles/rdwr.c
> +++ b/fs/cachefiles/rdwr.c
> @@ -58,9 +58,9 @@ static int cachefiles_read_waiter(wait_queue_entry_t
> *wait, unsigned mode,
>
> spin_lock(&object->work_lock);
> list_add_tail(&monitor->op_link, &monitor->op->to_do);
> + fscache_enqueue_retrieval(monitor->op);
> spin_unlock(&object->work_lock);
>
> - fscache_enqueue_retrieval(monitor->op);
> return 0;
> }
>
> --
> 2.14.3 (Apple Git-98)
>
>
--
Linux-cachefs mailing list
Linux-cachefs@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/linux-cachefs
