linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC 0/2] fs: fully sync all fsese even for an emergency sync
@ 2025-11-03  4:07 Qu Wenruo
  2025-11-03  4:07 ` [PATCH RFC 1/2] fs: do not pass a parameter for sync_inodes_one_sb() Qu Wenruo
                   ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: Qu Wenruo @ 2025-11-03  4:07 UTC (permalink / raw)
  To: linux-btrfs, linux-fsdevel; +Cc: viro, brauner, jack

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


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2025-11-05 18:50 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-03  4:07 [PATCH RFC 0/2] fs: fully sync all fsese even for an emergency sync Qu Wenruo
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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).