ACK/cmnt: [SRU X/B][PATCH 0/1] LP: #1797314 - fix another fscache crash

[Daniel Axtens]

On Fri, Oct 12, 2018 at 6:12 PM Khaled Elmously
<khalid.elmously at canonical.com> wrote:
>
> On 2018-10-12 12:11:30 , Daniel Axtens wrote:
> > SRU Justification
> > =================
> >
> > [Impact]
> >
> > A kernel BUG is sometimes observed when using fscache:
> >     [4740718.880898] FS-Cache:
> >     [4740718.880920] FS-Cache: Assertion failed
> >     [4740718.880934] FS-Cache: 0 > 0 is false
> >     [4740718.881001] ------------[ cut here ]------------
> >     [4740718.881017] kernel BUG at /usr/src/linux-4.4.0/fs/fscache/operation.c:449!
> >     [4740718.881040] invalid opcode: 0000 [#1] SMP
> >
> >     [4740718.892659] Call Trace:
> >     [4740718.893506] [<ffffffffc1464cf9>] cachefiles_read_copier+0x3a9/0x410 [cachefiles]
> >     [4740718.894374] [<ffffffffc037e272>] fscache_op_work_func+0x22/0x50 [fscache]
> >     [4740718.895180] [<ffffffff81096da0>] process_one_work+0x150/0x3f0
> >     [4740718.895966] [<ffffffff8109751a>] worker_thread+0x11a/0x470
> >     [4740718.896753] [<ffffffff81808e59>] ? __schedule+0x359/0x980
> >     [4740718.897783] [<ffffffff81097400>] ? rescuer_thread+0x310/0x310
> >     [4740718.898581] [<ffffffff8109cdd6>] kthread+0xd6/0xf0
> >     [4740718.899469] [<ffffffff8109cd00>] ? kthread_park+0x60/0x60
> >     [4740718.900477] [<ffffffff8180d0cf>] ret_from_fork+0x3f/0x70
> >     [4740718.901514] [<ffffffff8109cd00>] ? kthread_park+0x60/0x60
> >
> > [Problem]
> > In include/linux/fscache-cache.h, fscache_retrieval_complete reads, in
> > part:
> >
> >             atomic_sub(n_pages, &op->n_pages);
> >             if (atomic_read(&op->n_pages) <= 0)
> >                     fscache_op_complete(&op->op, true);
> >
> > The code is using atomic_sub followed by an atomic_read. This causes
> > two threads doing a decrement of pages to race with each other seeing
> > the op->refcount <= 0 at same time, and end up calling
> > fscache_op_complete in both the threads leading to the OOPS.
> >
> > [Fix]
> > The fix is trivial to use atomic_sub_return instead of two calls.
> >
> > [Testcase]
> > I believe the user has tested the patch successfully on their fscache/cachefiles setup.
> >
> > [Regression Potential]
> > Limited to fscache. Small, comprehensible change.
> >
> > Kiran Kumar Modukuri (1):
> >   UBUNTU: SAUCE: fscache: Fix race in decrementing refcount of
> >     op->npages
> >
> >  include/linux/fscache-cache.h | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >

> Acked-by: Khalid Elmously <khalid.elmously at canonical.com>
Thanks!

> Nice and simple - I wonder why there was no feedback for it on the mailing list.
This has been my consistent experience with the list - this is I think
the 3rd or 4th patch set we've tried to get upstream with basically no
luck. Feedback is sporadic at best.

> Did you intentionally CC the patch author (kiran.modukuri at gmail.com)?
Yes, Kiran has been working with us in support engineering as a
customer contact. The customer is keen to have the patches included in
Ubuntu kernels.

Regards,
Daniel