All Virtuozzo development lists (kernel + QEMU)
 help / color / mirror / Atom feed
From: Konstantin Khorenko <khorenko@virtuozzo.com>
To: "Denis V. Lunev" <den@openvz.org>
Cc: devel@openvz.org
Subject: Re: [Devel] [PATCH hci-8.1 0/4] dm-ploop: fix the pio submission stop around cmds
Date: Wed, 04 Nov 2026 18:14:19 +0100	[thread overview]
Message-ID: <179381245947.2686233.5146578609245409826.b4-review@b4> (raw)
In-Reply-To: <20261103083642.130885-1-den@openvz.org>

I won't apply this version, please send v2 with the changes below.

On Tue, 03 Nov 2026 09:36:38 +0100, Denis V. Lunev wrote:
> Denis V. Lunev (4):
>   dm-ploop: fix pio submission racing with the suspend and resume of a
>     cmd
>   dm-ploop: fix a lost wakeup of the ploop_allocator() thread
>   dm-ploop: wait for the ploop_allocator() thread before changing
>     deltas[]
>   dm-ploop: fix a failed cluster copy on grow leaving the BAT page busy

What to change for v2:

1. Patch 1, commit message only, small: in the first race add
   notify_merged to "a merge, resize or delta index update". The code
   stays as is.

2. Patch 2, commit message only, small:
   - drop "until some later request wakes the thread", the stall does
     not end by itself;
   - add one sentence that the same reorder also fixes a lost
     kthread_stop() wakeup.
   The code stays as is.

3. Patch 3, code and commit message:
   - code: also wait for ploop_no_pending_prealloc() in
     ploop_flip_upper_deltas() before process_flip_upper_deltas(), keep
     the wait in ploop_postsuspend();
   - message: "merge and notify_merged_backward" becomes "merge and
     notify_merged", both directions move the top delta.

4. Patch 4: no changes, resend it as is.

5. One more old bug found, not part of this series: the forward
   notify_merged stores the new file_size into the wrong deltas[] slot
   (details below). Please file a Jira task for it and fix it with a
   separate patch, not in v2.

6. Cover letter of v2:
   - "cutting the live image to the preallocation size" becomes
     "cutting the live image";
   - reword "Patch 3 is the one losing data".

Details on each item are in the replies to the patches. Below are the
details on items 5 and 6.

Item 5, the old bug. In ploop_delta_clusters_merged() the forward case
stores file_size, file_preallocated_area_start and nr_be into
deltas[level + 1] after notify_delta_merged() has already shifted the
slots down, so the delta that took the clusters is at deltas[level] by
then. With level == top - 2 that overwrites the top delta's file_size,
and the next preallocation can truncate the live top image. It has been
there since the target was added.

> - the ploop_allocator() thread is not stopped at all. It truncates the
>   top delta while merge or flip moves it in deltas[], cutting the live
>   image to the preallocation size (patch 3). The thread can also miss a

Item 6. Only a merge cuts the image to the preallocation size. A flip
cuts the old top image to the size of the other delta plus the
preallocation.

> Patch 3 is the one losing data: a snapshot delete under write load is
> enough.

Item 6. Race A of patch 1 loses data too: a pio can read and write
through a delta that merge has already released. Patch 3 is the easiest
one to hit, not the only one losing data.

-- 
Konstantin Khorenko,
Virtuozzo Linux Kernel Team

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

  parent reply	other threads:[~2026-11-04 17:15 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-03  8:36 Denis V. Lunev
2026-11-03  8:36 ` [Devel] [PATCH hci-8.1 1/4] dm-ploop: fix pio submission racing with the suspend and resume of a cmd Denis V. Lunev
2026-11-03  8:40   ` no-reply
2026-11-04 17:14   ` Konstantin Khorenko
2026-11-03  8:36 ` [Devel] [PATCH hci-8.1 2/4] dm-ploop: fix a lost wakeup of the ploop_allocator() thread Denis V. Lunev
2026-11-03  8:39   ` no-reply
2026-11-04 17:14   ` Konstantin Khorenko
2026-11-03  8:36 ` [Devel] [PATCH hci-8.1 3/4] dm-ploop: wait for the ploop_allocator() thread before changing deltas[] Denis V. Lunev
2026-11-03  8:42   ` no-reply
2026-11-04 17:14   ` Konstantin Khorenko
2026-11-03  8:36 ` [Devel] [PATCH hci-8.1 4/4] dm-ploop: fix a failed cluster copy on grow leaving the BAT page busy Denis V. Lunev
2026-11-03  8:42   ` no-reply
2026-11-04 17:14   ` Konstantin Khorenko
2026-11-04 13:24 ` [Devel] [PATCH hci-8.1 0/4] dm-ploop: fix the pio submission stop around cmds Andrey Zhadchenko
2026-11-04 17:14 ` Konstantin Khorenko [this message]
  -- strict thread matches above, loose matches on Subject: below --
2026-11-03  8:36 Denis V. Lunev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=179381245947.2686233.5146578609245409826.b4-review@b4 \
    --to=khorenko@virtuozzo.com \
    --cc=den@openvz.org \
    --cc=devel@openvz.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.