From: Christoph Hellwig <hch@lst.de>
To: "Darrick J. Wong" <djwong@kernel.org>
Cc: Christoph Hellwig <hch@lst.de>,
Chandan Babu R <chandan.babu@oracle.com>,
Dave Chinner <david@fromorbit.com>,
"open list:XFS FILESYSTEM" <linux-xfs@vger.kernel.org>
Subject: Re: [PATCH 8/8] xfs: do not allocate the entire delalloc extent in xfs_bmapi_write
Date: Wed, 10 Apr 2024 06:07:50 +0200 [thread overview]
Message-ID: <20240410040750.GA2099@lst.de> (raw)
In-Reply-To: <20240409231601.GJ6390@frogsfrogsfrogs>
On Tue, Apr 09, 2024 at 04:16:01PM -0700, Darrick J. Wong wrote:
> On Mon, Apr 08, 2024 at 04:54:54PM +0200, Christoph Hellwig wrote:
> > While trying to convert the entire delalloc extent is a good decision
> > for regular writeback as it leads to larger contigous on-disk extents,
> > but for other callers of xfs_bmapi_write is is rather questionable as
> > it forced them to loop creating new transactions just in case there
> > is no large enough contiguous extent to cover the whole delalloc
> > reservation.
> >
> > Change xfs_bmapi_write to only allocate the passed in range instead.
>
> Looking at this... I guess xfs_map_blocks -> xfs_convert_blocks ->
> xfs_bmapi_convert_delalloc -> xfs_bmapi_allocate is now how writeback
> converts delalloc extents before scheduling writeout. This is how the
> mass-conversions of large da reservations got done before this series,
> and that's still how it works, right?
Yes and yes.
> Whereas xfs_bmapi_write is for targeted conversions only?
targeted is one way to describe it, the other way to look at it is
that xfs_bmapi_write is used where the callers want to allocate
real (or unwritten) extents for a range, which just happens to
convert delalloc as a side-effect as those callers don't want to
deal with delalloc extents.
>
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
>
> If yes and yes, then:
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
So as mentioned in the cover letter I'm quite worried about the
new behavior we expose here as we always converted delalloc
extents from the start and tried to convert it to the end,
and this now changes that. So while the changes looks quite
simple they expose a lot of previously untested code and behavior.
It's probably the right thing to do but quite risky, let me know
what you think about it.
prev parent reply other threads:[~2024-04-10 4:07 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-08 14:54 RFC: extended version of the xfs_bmapi_write retval fix Christoph Hellwig
2024-04-08 14:54 ` [PATCH 1/8] xfs: fix error returns from xfs_bmapi_write Christoph Hellwig
2024-04-09 23:19 ` Darrick J. Wong
2024-04-10 4:02 ` Christoph Hellwig
2024-04-08 14:54 ` [PATCH 2/8] xfs: remove the unusued tmp_logflags variable in xfs_bmapi_allocate Christoph Hellwig
2024-04-09 23:17 ` Darrick J. Wong
2024-04-08 14:54 ` [PATCH 3/8] xfs: lifr a xfs_valid_startblock into xfs_bmapi_allocate Christoph Hellwig
2024-04-09 23:17 ` Darrick J. Wong
2024-04-08 14:54 ` [PATCH 4/8] xfs: don't open code XFS_FILBLKS_MIN in xfs_bmapi_write Christoph Hellwig
2024-04-09 23:17 ` Darrick J. Wong
2024-04-08 14:54 ` [PATCH 5/8] xfs: pass the actual offset and len to allocate to xfs_bmapi_allocate Christoph Hellwig
2024-04-09 23:20 ` Darrick J. Wong
2024-04-10 4:03 ` Christoph Hellwig
2024-04-08 14:54 ` [PATCH 6/8] xfs: remove the xfs_iext_peek_prev_extent call in xfs_bmapi_allocate Christoph Hellwig
2024-04-09 23:16 ` Darrick J. Wong
2024-04-08 14:54 ` [PATCH 7/8] xfs: fix xfs_bmap_add_extent_delay_real for partial conversions Christoph Hellwig
2024-04-09 23:16 ` Darrick J. Wong
2024-04-10 4:04 ` Christoph Hellwig
2024-04-08 14:54 ` [PATCH 8/8] xfs: do not allocate the entire delalloc extent in xfs_bmapi_write Christoph Hellwig
2024-04-09 23:16 ` Darrick J. Wong
2024-04-10 4:07 ` Christoph Hellwig [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=20240410040750.GA2099@lst.de \
--to=hch@lst.de \
--cc=chandan.babu@oracle.com \
--cc=david@fromorbit.com \
--cc=djwong@kernel.org \
--cc=linux-xfs@vger.kernel.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 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 read-only IMAP folder(s) and NNTP newsgroup(s).