From: David Sterba <dsterba@suse.cz>
To: Qu Wenruo <wqu@suse.com>
Cc: Kai Krakow <kai@kaishome.de>,
linux-btrfs@vger.kernel.org, Eli Venter <eli@genedx.com>
Subject: Re: [PATCH] btrfs: harden __reserve_bytes() with space_info==NULL
Date: Mon, 15 Dec 2025 20:28:25 +0100 [thread overview]
Message-ID: <20251215192825.GI3195@twin.jikos.cz> (raw)
In-Reply-To: <684a6f99-0df8-428a-8e57-294a38b8788e@suse.com>
On Sun, Dec 14, 2025 at 09:01:41AM +1030, Qu Wenruo wrote:
>
>
> 在 2025/12/14 08:13, Kai Krakow 写道:
> > Am Sa., 13. Dez. 2025 um 22:15 Uhr schrieb Qu Wenruo <wqu@suse.com>:
> [...]
> >>
> >> So if you really want to change a member, do a proper in-memory update
> >> only, and find a way (e.g. a new dirty dev list) to tell the fs to
> >> update the device item at commit time.
> >
> > I don't want to steal your time,
>
> I'm totally fine discussing the implementation details here.
>
> > but is this a better approach?
> > https://gist.github.com/kakra/aa3eb8473cc05c1d3dd000160a5ee481
>
> Unfortunately as long as if you're trying to do any metadata
> modification, e.g. calling btrfs_update_device(), it will be a huge change.
>
>
> My idea would be something like this:
>
> btrfs_dev_info_type_store()
> {
> btrfs_device *device = container_of();
>
> /* Do the proper locking. */
>
> WRITE_ONCE(device->type, type);
> if (!list_empty(&dev->dirty_list))
> list_add_tail(&fs_info->dirty_dev_list, &dev->dirty_list);
set_bit(BTRFS_FS_NEED_TRANS_COMMIT, &fs_info->flags);
wake_up_process(fs_info->transaction_kthread);
I don't think it would be wise to wait until the transaction is
committed before returning from the store handler. We don't do that for
other similar changes either so the write becomes permanent after a
sync, using eg. 'btrfs fi sync'.
> return len;
> }
>
>
> Then inside btrfs_commit_transaction(), I do not yet have a good idea on
> the timing, but I guess it can done before btrfs_start_delalloc_flush().
>
> Do something like this to write those dirty devices to chunk tree:
>
> btrfs_commit_transaction()
> {
> list_for_each_entry(dev, &fs_info->dirty_dev_list, dirty_list) {
> ret = btrfs_update_device(dev);
> }
>
> /* The remaining code. */
> ret = btrfs_start_delalloc_flush();
> }
This should work. The placement depends on what is changed and how it's
related to the current transaction.
prev parent reply other threads:[~2025-12-15 19:28 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-12-13 20:09 [PATCH] btrfs: harden __reserve_bytes() with space_info==NULL Kai Krakow
2025-12-13 20:48 ` Qu Wenruo
2025-12-13 21:04 ` Qu Wenruo
2025-12-13 21:14 ` Kai Krakow
2025-12-13 21:10 ` Kai Krakow
2025-12-13 21:15 ` Qu Wenruo
2025-12-13 21:43 ` Kai Krakow
2025-12-13 22:31 ` Qu Wenruo
2025-12-15 19:28 ` David Sterba [this message]
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=20251215192825.GI3195@twin.jikos.cz \
--to=dsterba@suse.cz \
--cc=eli@genedx.com \
--cc=kai@kaishome.de \
--cc=linux-btrfs@vger.kernel.org \
--cc=wqu@suse.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox