Re: FAILED: patch "[PATCH] dm snapshot: rework COW throttling to fix deadlock" failed to apply to 5.3-stable tree

[Mike Snitzer <snitzer@redhat.com>]

On Mon, Oct 28 2019 at 10:38am -0400,
Sasha Levin <sashal@kernel.org> wrote:

> On Mon, Oct 28, 2019 at 12:46:57PM +0100, Greg KH wrote:
> >On Mon, Oct 28, 2019 at 05:39:28AM -0400, Sasha Levin wrote:
> >>On Sun, Oct 27, 2019 at 04:37:28PM +0100, gregkh@linuxfoundation.org wrote:
> >>>
> >>> The patch below does not apply to the 5.3-stable tree.
> >>> If someone wants it applied there, or to any other stable or longterm
> >>> tree, then please email the backport, including the original git commit
> >>> id to <stable@vger.kernel.org>.
> >>>
> >>> thanks,
> >>>
> >>> greg k-h
> >>>
> >>> ------------------ original commit in Linus's tree ------------------
> >>>
> >>> > From b21555786f18cd77f2311ad89074533109ae3ffa Mon Sep 17 00:00:00 2001
> >>> From: Mikulas Patocka <mpatocka@redhat.com>
> >>> Date: Wed, 2 Oct 2019 06:15:53 -0400
> >>> Subject: [PATCH] dm snapshot: rework COW throttling to fix deadlock
> >>>
> >>> Commit 721b1d98fb517a ("dm snapshot: Fix excessive memory usage and
> >>> workqueue stalls") introduced a semaphore to limit the maximum number of
> >>> in-flight kcopyd (COW) jobs.
> >>>
> >>> The implementation of this throttling mechanism is prone to a deadlock:
> >>>
> >>> 1. One or more threads write to the origin device causing COW, which is
> >>>   performed by kcopyd.
> >>>
> >>> 2. At some point some of these threads might reach the s->cow_count
> >>>   semaphore limit and block in down(&s->cow_count), holding a read lock
> >>>   on _origins_lock.
> >>>
> >>> 3. Someone tries to acquire a write lock on _origins_lock, e.g.,
> >>>   snapshot_ctr(), which blocks because the threads at step (2) already
> >>>   hold a read lock on it.
> >>>
> >>> 4. A COW operation completes and kcopyd runs dm-snapshot's completion
> >>>   callback, which ends up calling pending_complete().
> >>>   pending_complete() tries to resubmit any deferred origin bios. This
> >>>   requires acquiring a read lock on _origins_lock, which blocks.
> >>>
> >>>   This happens because the read-write semaphore implementation gives
> >>>   priority to writers, meaning that as soon as a writer tries to enter
> >>>   the critical section, no readers will be allowed in, until all
> >>>   writers have completed their work.
> >>>
> >>>   So, pending_complete() waits for the writer at step (3) to acquire
> >>>   and release the lock. This writer waits for the readers at step (2)
> >>>   to release the read lock and those readers wait for
> >>>   pending_complete() (the kcopyd thread) to signal the s->cow_count
> >>>   semaphore: DEADLOCK.
> >>>
> >>> The above was thoroughly analyzed and documented by Nikos Tsironis as
> >>> part of his initial proposal for fixing this deadlock, see:
> >>> https://www.redhat.com/archives/dm-devel/2019-October/msg00001.html
> >>>
> >>> Fix this deadlock by reworking COW throttling so that it waits without
> >>> holding any locks. Add a variable 'in_progress' that counts how many
> >>> kcopyd jobs are running. A function wait_for_in_progress() will sleep if
> >>> 'in_progress' is over the limit. It drops _origins_lock in order to
> >>> avoid the deadlock.
> >>>
> >>> Reported-by: Guruswamy Basavaiah <guru2018@gmail.com>
> >>> Reported-by: Nikos Tsironis <ntsironis@arrikto.com>
> >>> Reviewed-by: Nikos Tsironis <ntsironis@arrikto.com>
> >>> Tested-by: Nikos Tsironis <ntsironis@arrikto.com>
> >>> Fixes: 721b1d98fb51 ("dm snapshot: Fix excessive memory usage and workqueue stalls")
> >>> Cc: stable@vger.kernel.org # v5.0+
> >>> Depends-on: 4a3f111a73a8c ("dm snapshot: introduce account_start_copy() and account_end_copy()")
> >>> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> >>> Signed-off-by: Mike Snitzer <snitzer@redhat.com>
> >>
> >>Grabbing the listed dependency solved it for 5.3-4.19. For 4.14 and
> >>older I've also grabbed the semaphore->mutex conversion.
> >
> >Ugh, I missed that it said that there.  I'll do this for 4.19, unless
> >you have these ready to go for when the tree "opens up" again.
> 
> I'll have all of these ready to go when the tree opens up.

Not sure what all the trees are but a sufficiently older kernel could
have problems with these fixes if they are missing commit d3775354
(specifically the change to use kzalloc in dm-snap.c).

This came up in the context of 4.4 recently.  But hopefully none of your
staable branches are missing commit d3775354.

Thanks,
Mike