public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Josef Bacik <josef@toxicpanda.com>
To: Qu Wenruo <quwenruo.btrfs@gmx.com>
Cc: Johannes Thumshirn <jth@kernel.org>, Chris Mason <clm@fb.com>,
	David Sterba <dsterba@suse.com>,
	"open list:BTRFS FILE SYSTEM" <linux-btrfs@vger.kernel.org>,
	open list <linux-kernel@vger.kernel.org>,
	Qu Wenruo <wqu@suse.com>, Naohiro Aota <naohiro.aota@wdc.com>,
	Johannes Thumshirn <johannes.thumshirn@wdc.com>
Subject: Re: [PATCH] btrfs: also add stripe entries for NOCOW writes
Date: Mon, 23 Sep 2024 11:20:46 -0400	[thread overview]
Message-ID: <20240923152046.GA159452@perftesting> (raw)
In-Reply-To: <71088008-c105-4eb9-9199-882091eafe07@gmx.com>

On Mon, Sep 23, 2024 at 04:58:34PM +0930, Qu Wenruo wrote:
> 
> 
> 在 2024/9/23 16:15, Johannes Thumshirn 写道:
> > From: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> > 
> > NOCOW writes do not generate stripe_extent entries in the RAID stripe
> > tree, as the RAID stripe-tree feature initially was designed with a
> > zoned filesystem in mind and on a zoned filesystem, we do not allow NOCOW
> > writes. But the RAID stripe-tree feature is independent from the zoned
> > feature, so we must also allow NOCOW writes for zoned filesystems.
> > 
> > Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> 
> Sorry I'm going to repeat myself again, I still believe if we insert an
> RST entry at falloc() time, it will be more consistent with the non-RST
> code.
> 
> Yes, I known preallocated space will not need any read nor search RST
> entry, and we just fill the page cache with zero at read time.
> 
> But the point of proper (not just dummy) RST entry for the whole
> preallocated space is, we do not need to touch the RST entry anymore for
> NOCOW/PREALLOCATED write at all.
> 
> This makes the RST NOCOW/PREALLOC writes behavior to align with the
> non-RST code, which doesn't update any extent item, but only modify the
> file extent for PREALLOC writes.

I see what you're getting at here, but it creates a huge amount of problems
later down the line.

I prealloc an extent, I map that logical extent to a physical extent and then I
insert a RST entry for that mapping.  Now I rip out one of my disks, and now I
have to update RST entries for extents I'm not going to rewrite because they're
prealloc.

RST is a logical->physical mapping.  We do not need to update or insert anything
until we create that logical->physical mapping.  Keeping the rules consistent
across the different layers will make it easier to reason about and easier to
maintain.  Adding an index at endio time for NOCOW makes sense, we now have
created a thing on disk that we need to have a mapping for.  The same goes for
prealloc, adding an entry at prealloc time doesn't make logical sense as we
haven't yet instantiated that space on disk.  Thanks,

Josef

  parent reply	other threads:[~2024-09-23 15:20 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-23  6:45 [PATCH] btrfs: also add stripe entries for NOCOW writes Johannes Thumshirn
2024-09-23  7:28 ` Qu Wenruo
2024-09-23  7:40   ` Johannes Thumshirn
2024-09-23  7:56     ` Qu Wenruo
2024-09-23  8:15       ` Johannes Thumshirn
2024-09-23  8:53         ` Qu Wenruo
2024-09-23 14:41           ` Johannes Thumshirn
2024-09-23 22:23             ` Qu Wenruo
2024-09-23 15:20   ` Josef Bacik [this message]
2024-09-23 22:32     ` Qu Wenruo
2024-09-24  0:46       ` Qu Wenruo
2024-09-24  7:07 ` Naohiro Aota

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=20240923152046.GA159452@perftesting \
    --to=josef@toxicpanda.com \
    --cc=clm@fb.com \
    --cc=dsterba@suse.com \
    --cc=johannes.thumshirn@wdc.com \
    --cc=jth@kernel.org \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=naohiro.aota@wdc.com \
    --cc=quwenruo.btrfs@gmx.com \
    --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