All of lore.kernel.org
 help / color / mirror / Atom feed
From: Qu Wenruo <wqu@suse.com>
To: linux-btrfs@vger.kernel.org, linux-fsdevel@vger.kernel.org
Cc: viro@zeniv.linux.org.uk, brauner@kernel.org, jack@suse.cz
Subject: [PATCH RFC 0/2] fs: fully sync all fsese even for an emergency sync
Date: Mon,  3 Nov 2025 14:37:27 +1030	[thread overview]
Message-ID: <cover.1762142636.git.wqu@suse.com> (raw)

The first patch is a cleanup related to sync_inodes_one_sb() callback.
Since it always wait for the writeback, there is no need to pass any
parameter for it.

The second patch is a fix mostly affecting btrfs, as btrfs requires a
explicit sync_fc() call with wait == 1, to commit its super blocks,
and sync_bdevs() won't cut it at all.

However the current emergency sync never passes wait == 1, it means
btrfs will writeback all dirty data and metadata, but still no super
block update, resulting everything still pointing back to the old
data/metadata.

This lead to a problem where btrfs doesn't seem to do anything during
emergency sync.

The second patch fixes the problem by passing wait == 1 for the second
iteration of sync_fs_one_sb().

[REASON FOR RFC]
I am not sure which way should I fix the bug.

I can definitely put btrfs to ignore the @wait parameter and always do
transaction commit, that will definitely fix the bug, but btrfs will do
two transaction commits for emergency sync.
Which may or may not be a problem for emergency sync itself, but will
definitely cause a lot of unnessary small transactions during regular
sync_fs() calls and degrade the peroformance.

On the other hand, I also didn't see why we can not follow the common
pattern inside emergency_sync(), all other call sites are syncing the fs
first with nowait, then wait.
(E.g. sync_filesyastem() and ksys_sync()).

I know it's an emergency sync thus we don't want to wait, but please
also remember that sync_inodes_one_sb() is always waiting, and I'm
pretty sure we spend most of the time inside sync_inodes_one_sb(), thus
it looks more sane to fix the only exception inside fs/sync.c.

Qu Wenruo (2):
  fs: do not pass a parameter for sync_inodes_one_sb()
  fs: fully sync all fses even for an emergency sync

 fs/sync.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

-- 
2.51.2


             reply	other threads:[~2025-11-03  4:07 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-03  4:07 Qu Wenruo [this message]
2025-11-03  4:07 ` [PATCH RFC 1/2] fs: do not pass a parameter for sync_inodes_one_sb() Qu Wenruo
2025-11-03 11:51   ` Christoph Hellwig
2025-11-04  8:42     ` Jan Kara
2025-11-05 18:50       ` Daniel Vacek
2025-11-03  4:07 ` [PATCH RFC 2/2] fs: fully sync all fses even for an emergency sync Qu Wenruo
2025-11-03 11:56   ` Christoph Hellwig
2025-11-03 12:04     ` Christoph Hellwig
2025-11-03 20:46       ` Qu Wenruo
2025-11-03 20:55     ` Qu Wenruo
2025-11-04  8:28       ` Jan Kara
2025-11-04  8:39         ` Jan Kara
2025-11-05 11:32           ` Christian Brauner
2025-11-04  8:43         ` Qu Wenruo
2025-11-04 12:42           ` Jan Kara
2025-11-04 15:48             ` Christoph Hellwig
2025-11-03 16:00 ` [PATCH RFC 0/2] fs: fully sync all fsese " Askar Safin
2025-11-05 11:31 ` Christian Brauner

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=cover.1762142636.git.wqu@suse.com \
    --to=wqu@suse.com \
    --cc=brauner@kernel.org \
    --cc=jack@suse.cz \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /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.