All Virtuozzo development lists (kernel + QEMU)
 help / color / mirror / Atom feed
From: Konstantin Khorenko <khorenko@virtuozzo.com>
To: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
Cc: devel@openvz.org
Subject: Re: [Devel] [PATCH hci-8.1 2/7] drivers/md/dm-qcow2: check MD_DIRTY and md->wbd under lock in perform_rw_mapped()
Date: Wed, 04 Nov 2026 22:35:09 +0100	[thread overview]
Message-ID: <179382810903.2794215.10567595584481536659.b4-review@b4> (raw)
In-Reply-To: <20261029140346.465768-3-andrey.zhadchenko@virtuozzo.com>

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

On Thu, 29 Oct 2026 17:03:41 +0300, Andrey Zhadchenko wrote:
> -	if (rw == WRITE && (md->status & MD_DIRTY) &&
> -	    test_bit(index_in_page, md->wbd->changed_indexes)) {
> +	if (rw == WRITE) {
>  		spin_lock_irqsave(&qcow2->md_pages_lock, flags);
> -		md->wbd->nr_submitted++;
> -		qio->data = md->wbd;
> +		if ((md->status & MD_DIRTY) &&
> +		    test_bit(index_in_page, md->wbd->changed_indexes)) {
> +			md->wbd->nr_submitted++;
> +			qio->data = md->wbd;
> +		}
>  		spin_unlock_irqrestore(&qcow2->md_pages_lock, flags);

Item 1 of the cover letter reply: under this lock, when MD_WRITEBACK
is set and the index is in changed_indexes, park the qio on
md->wait_list, as parse_l2() does. Do not attach it to a wbd under
writeback: after a failed writeback complete_wbd() frees the wbd.

Why: the lock makes this check consistent, but parse_l2() decided
under an earlier lock hold that the write need not wait. With the fast
path of patch 5:

  main kwork                      fast worker
  allocates cluster X,
  L2 page DIRTY
                                  parse_l2(): write to X, page DIRTY,
                                  not delayed, unlock
  submit_metadata_writeback():
  DIRTY -> WRITEBACK
                                  perform_rw_mapped(): no MD_DIRTY,
                                  qio->data = NULL, submit

The write is acked as soon as its data is written. A FLUSH after it
finds QLIST_COMPLETED_WB empty and does only vfs_fsync(), it does not
wait for the L2 write in flight. A crash then loses a write that was
acked and flushed. If the L2 write fails instead, the entry is reverted
and X freed while the write already returned success. The base is safe
only because parse, submit and the DIRTY -> WRITEBACK switch happen in
one do_qcow2_work() pass.

-- 
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 21:36 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-29 14:03 [Devel] [PATCH 0/7] drivers/md/dm-qcow2: per-qio fast path Andrey Zhadchenko
2026-10-29 14:03 ` [Devel] [PATCH hci-8.1 1/7] drivers/md/dm-qcow2: repeat md page lookup if it was inserted in parallel Andrey Zhadchenko
2026-10-29 14:13   ` no-reply
2026-11-04 21:35   ` Konstantin Khorenko
2026-10-29 14:03 ` [Devel] [PATCH hci-8.1 2/7] drivers/md/dm-qcow2: check MD_DIRTY and md->wbd under lock in perform_rw_mapped() Andrey Zhadchenko
2026-10-29 14:13   ` no-reply
2026-11-04 21:35   ` Konstantin Khorenko [this message]
2026-10-29 14:03 ` [Devel] [PATCH hci-8.1 3/7] drivers/md/dm-qcow2: prepare process_one_qio() for fastpath Andrey Zhadchenko
2026-10-29 14:16   ` no-reply
2026-11-02 17:28   ` Alexander Ivanov
2026-11-04 21:35   ` Konstantin Khorenko
2026-10-29 14:03 ` [Devel] [PATCH hci-8.1 4/7] drivers/md/dm-qcow2: add per-qio work infrastructure Andrey Zhadchenko
2026-10-29 14:18   ` no-reply
2026-11-04 21:35   ` Konstantin Khorenko
2026-10-29 14:03 ` [Devel] [PATCH hci-8.1 5/7] drivers/md/dm-qcow2: implement per-qio fast processing Andrey Zhadchenko
2026-10-29 14:22   ` no-reply
2026-11-04 21:35   ` Konstantin Khorenko
2026-10-29 14:03 ` [Devel] [PATCH hci-8.1 6/7] drivers/md/dm-qcow2: declare DM_TARGET_NOWAIT Andrey Zhadchenko
2026-10-29 14:19   ` no-reply
2026-11-03  8:33   ` Alexander Ivanov
2026-11-04 21:35   ` Konstantin Khorenko
2026-10-29 14:03 ` [Devel] [PATCH hci-8.1 7/7] drivers/md/dm-qcow2: process fast path qios in per-cpu batches on a workqueue Andrey Zhadchenko
2026-10-29 14:22   ` no-reply
2026-11-02 10:49     ` Andrey Zhadchenko
2026-11-04 21:35   ` Konstantin Khorenko
2026-11-03  9:36 ` [Devel] [PATCH 0/7] drivers/md/dm-qcow2: per-qio fast path Alexander Ivanov
2026-11-04 21:35 ` Konstantin Khorenko

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=179382810903.2794215.10567595584481536659.b4-review@b4 \
    --to=khorenko@virtuozzo.com \
    --cc=andrey.zhadchenko@virtuozzo.com \
    --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.