From: "Darrick J. Wong" <djwong@kernel.org>
To: Christoph Hellwig <hch@lst.de>
Cc: osandov@fb.com, linux-xfs@vger.kernel.org
Subject: Re: [PATCH 4/4] xfs: use accessor functions for summary info words
Date: Tue, 17 Oct 2023 22:31:27 -0700 [thread overview]
Message-ID: <20231018053127.GE3195650@frogsfrogsfrogs> (raw)
In-Reply-To: <20231018051934.GF15759@lst.de>
On Wed, Oct 18, 2023 at 07:19:34AM +0200, Christoph Hellwig wrote:
> On Tue, Oct 17, 2023 at 07:10:42PM -0700, Darrick J. Wong wrote:
> > +static inline union xfs_suminfo_raw *
> > xfs_rsumblock_infoptr(
> > struct xfs_buf *bp,
> > unsigned int index)
> > {
> > - xfs_suminfo_t *info = bp->b_addr;
> > + union xfs_suminfo_raw *info = bp->b_addr;
> >
> > return info + index;
> > }
> >
> > +/* Get the current value of a summary counter. */
> > +static inline xfs_suminfo_t
> > +xfs_suminfo_get(
> > + struct xfs_buf *bp,
> > + unsigned int index)
> > +{
> > + union xfs_suminfo_raw *info = xfs_rsumblock_infoptr(bp, index);
> > +
> > + return info->old;
> > +}
>
> Same nitpick as for the bitmap version.
>
> Otherwise this looks good:
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Assuming you also meant "Reviewed-by" here too? :)
> ... to actually understand the mess in xfs_rtmodify_summary_int I had
> to do the (untested) refactoring below. I'll probably resubmit it after
> the whole series which touches a lot of this:
I'll take a look tomorrow, but yeah, xfs_rtmodify_summary_int is
confusing.
Annoyingly, I think there's a bug in new logging helpers, because all
the shutdown tests are falling all over themselves:
https://djwong.org/fstests/output/.c6b00f80b1e68bd5a3b17a7f7fbe97bab28dab740d7acf9e3fa879c3ae0c56c0/.02425ea8cdb6100e408b20dceac80a46f53f6fa1587fb4af7fba2810f2b8d0fd/?C=M;O=A
Will look at /that/ first thing...
--D
>
> diff --git a/fs/xfs/libxfs/xfs_rtbitmap.c b/fs/xfs/libxfs/xfs_rtbitmap.c
> index 2118c6f177a135..7b09caa747a720 100644
> --- a/fs/xfs/libxfs/xfs_rtbitmap.c
> +++ b/fs/xfs/libxfs/xfs_rtbitmap.c
> @@ -433,78 +433,69 @@ xfs_trans_log_rtsummary(
> * Summary information is returned in *sum if specified.
> * If no delta is specified, returns summary only.
> */
> -int
> -xfs_rtmodify_summary_int(
> +static int
> +xfs_rtmodify_summary_find(
> xfs_mount_t *mp, /* file system mount structure */
> xfs_trans_t *tp, /* transaction pointer */
> int log, /* log2 of extent size */
> xfs_fileoff_t bbno, /* bitmap block number */
> - int delta, /* change to make to summary info */
> struct xfs_buf **rbpp, /* in/out: summary block buffer */
> xfs_fileoff_t *rsb, /* in/out: summary block number */
> - xfs_suminfo_t *sum) /* out: summary info for this block */
> + unsigned int *word)
> {
> struct xfs_buf *bp; /* buffer for the summary block */
> int error; /* error value */
> - xfs_fileoff_t sb; /* summary fsblock */
> xfs_rtsumoff_t so; /* index into the summary file */
> + xfs_fileoff_t sb; /* summary fsblock */
>
> /*
> * Compute entry number in the summary file.
> */
> so = xfs_rtsumoffs(mp, log, bbno);
> +
> /*
> * Compute the block number in the summary file.
> */
> sb = xfs_rtsumoffs_to_block(mp, so);
> +
> + /*
> + * Compute the word index into the summary.
> + */
> + *word = xfs_rtsumoffs_to_infoword(mp, so);
> +
> /*
> * If we have an old buffer, and the block number matches, use that.
> */
> if (*rbpp && *rsb == sb)
> - bp = *rbpp;
> + return 0;
> +
> /*
> - * Otherwise we have to get the buffer.
> + * Otherwise we have to get a new buffer.
> + * If there was an old one, get rid of it first.
> */
> - else {
> - /*
> - * If there was an old one, get rid of it first.
> - */
> - if (*rbpp)
> - xfs_trans_brelse(tp, *rbpp);
> - error = xfs_rtbuf_get(mp, tp, sb, 1, &bp);
> - if (error) {
> - return error;
> - }
> - /*
> - * Remember this buffer and block for the next call.
> - */
> - *rbpp = bp;
> - *rsb = sb;
> - }
> + if (*rbpp)
> + xfs_trans_brelse(tp, *rbpp);
> + error = xfs_rtbuf_get(mp, tp, sb, 1, &bp);
> + if (error)
> + return error;
> +
> /*
> - * Point to the summary information, modify/log it, and/or copy it out.
> + * Remember this buffer and block for the next call.
> */
> - if (delta) {
> - unsigned int infoword = xfs_rtsumoffs_to_infoword(mp, so);
> - xfs_suminfo_t val = xfs_suminfo_add(bp, infoword, delta);
> -
> - if (mp->m_rsum_cache) {
> - if (val == 0 && log == mp->m_rsum_cache[bbno])
> - mp->m_rsum_cache[bbno]++;
> - if (val != 0 && log < mp->m_rsum_cache[bbno])
> - mp->m_rsum_cache[bbno] = log;
> - }
> - xfs_trans_log_rtsummary(tp, bp, infoword);
> - if (sum)
> - *sum = val;
> - } else if (sum) {
> - unsigned int infoword = xfs_rtsumoffs_to_infoword(mp, so);
> -
> - *sum = xfs_suminfo_get(bp, infoword);
> - }
> + *rbpp = bp;
> + *rsb = sb;
> return 0;
> }
>
> +/*
> + * Read and/or modify the summary information for a given extent size,
> + * bitmap block combination.
> + * Keeps track of a current summary block, so we don't keep reading
> + * it from the buffer cache.
> + *
> + * Summary information is returned in *sum if specified.
> + * If no delta is specified, returns summary only.
> + */
> int
> xfs_rtmodify_summary(
> xfs_mount_t *mp, /* file system mount structure */
> @@ -515,8 +506,51 @@ xfs_rtmodify_summary(
> struct xfs_buf **rbpp, /* in/out: summary block buffer */
> xfs_fileoff_t *rsb) /* in/out: summary block number */
> {
> - return xfs_rtmodify_summary_int(mp, tp, log, bbno,
> - delta, rbpp, rsb, NULL);
> + int error;
> + unsigned int word;
> + xfs_suminfo_t val;
> +
> + error = xfs_rtmodify_summary_find(mp, tp, log, bbno, rbpp, rsb, &word);
> + if (error)
> + return error;
> +
> + /*
> + * Modify and log the summary information.
> + */
> + val = xfs_suminfo_add(*rbpp, word, delta);
> + if (mp->m_rsum_cache) {
> + if (val == 0 && log == mp->m_rsum_cache[bbno])
> + mp->m_rsum_cache[bbno]++;
> + if (val != 0 && log < mp->m_rsum_cache[bbno])
> + mp->m_rsum_cache[bbno] = log;
> + }
> + xfs_trans_log_rtsummary(tp, *rbpp, word);
> + return 0;
> +}
> +
> +/*
> + * Read and return the summary information for a given extent size,
> + * bitmap block combination.
> + * Keeps track of a current summary block, so we don't keep reading
> + * it from the buffer cache.
> + */
> +int
> +xfs_rtget_summary(
> + xfs_mount_t *mp, /* file system mount structure */
> + xfs_trans_t *tp, /* transaction pointer */
> + int log, /* log2 of extent size */
> + xfs_fileoff_t bbno, /* bitmap block number */
> + struct xfs_buf **rbpp, /* in/out: summary block buffer */
> + xfs_fileoff_t *rsb, /* in/out: summary block number */
> + xfs_suminfo_t *sum) /* out: summary info for this block */
> +{
> + int error;
> + unsigned int word;
> +
> + error = xfs_rtmodify_summary_find(mp, tp, log, bbno, rbpp, rsb, &word);
> + if (!error)
> + *sum = xfs_suminfo_get(*rbpp, word);
> + return error;
> }
>
> /* Log rtbitmap block from the word @from to the byte before @next. */
> diff --git a/fs/xfs/libxfs/xfs_rtbitmap.h b/fs/xfs/libxfs/xfs_rtbitmap.h
> index fdfa98e0ee52f7..6d23a77def50dd 100644
> --- a/fs/xfs/libxfs/xfs_rtbitmap.h
> +++ b/fs/xfs/libxfs/xfs_rtbitmap.h
> @@ -294,13 +294,12 @@ int xfs_rtfind_forw(struct xfs_mount *mp, struct xfs_trans *tp,
> xfs_rtxnum_t *rtblock);
> int xfs_rtmodify_range(struct xfs_mount *mp, struct xfs_trans *tp,
> xfs_rtxnum_t start, xfs_rtxlen_t len, int val);
> -int xfs_rtmodify_summary_int(struct xfs_mount *mp, struct xfs_trans *tp,
> - int log, xfs_fileoff_t bbno, int delta,
> - struct xfs_buf **rbpp, xfs_fileoff_t *rsb,
> - xfs_suminfo_t *sum);
> int xfs_rtmodify_summary(struct xfs_mount *mp, struct xfs_trans *tp, int log,
> xfs_fileoff_t bbno, int delta, struct xfs_buf **rbpp,
> xfs_fileoff_t *rsb);
> +int xfs_rtget_summary(struct xfs_mount *mp, struct xfs_trans *tp, int log,
> + xfs_fileoff_t bbno, struct xfs_buf **rbpp,
> + xfs_fileoff_t *rsb, xfs_suminfo_t *sum);
> int xfs_rtfree_range(struct xfs_mount *mp, struct xfs_trans *tp,
> xfs_rtxnum_t start, xfs_rtxlen_t len,
> struct xfs_buf **rbpp, xfs_fileoff_t *rsb);
> diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
> index 3be6bda2fd920c..22bc8b3b724a5b 100644
> --- a/fs/xfs/xfs_rtalloc.c
> +++ b/fs/xfs/xfs_rtalloc.c
> @@ -21,25 +21,6 @@
> #include "xfs_sb.h"
> #include "xfs_rtbitmap.h"
>
> -/*
> - * Read and return the summary information for a given extent size,
> - * bitmap block combination.
> - * Keeps track of a current summary block, so we don't keep reading
> - * it from the buffer cache.
> - */
> -static int
> -xfs_rtget_summary(
> - xfs_mount_t *mp, /* file system mount structure */
> - xfs_trans_t *tp, /* transaction pointer */
> - int log, /* log2 of extent size */
> - xfs_fileoff_t bbno, /* bitmap block number */
> - struct xfs_buf **rbpp, /* in/out: summary block buffer */
> - xfs_fileoff_t *rsb, /* in/out: summary block number */
> - xfs_suminfo_t *sum) /* out: summary info for this block */
> -{
> - return xfs_rtmodify_summary_int(mp, tp, log, bbno, 0, rbpp, rsb, sum);
> -}
> -
> /*
> * Return whether there are any free extents in the size range given
> * by low and high, for the bitmap block bbno.
next prev parent reply other threads:[~2023-10-18 5:31 UTC|newest]
Thread overview: 86+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-17 15:37 [RFC v1.1] xfs-linux: rtalloc-speedups-6.7 updated to b67199695696 Darrick J. Wong
2023-10-17 15:43 ` Darrick J. Wong
2023-10-17 15:45 ` [PATCHSET RFC v1.1 0/4] xfs: minor bugfixes for rt stuff Darrick J. Wong
2023-10-17 15:47 ` [PATCH 1/4] xfs: bump max fsgeom struct version Darrick J. Wong
2023-10-17 15:47 ` [PATCH 2/4] xfs: hoist freeing of rt data fork extent mappings Darrick J. Wong
2023-10-17 16:00 ` Christoph Hellwig
2023-10-17 15:47 ` [PATCH 3/4] xfs: prevent rt growfs when quota is enabled Darrick J. Wong
2023-10-17 15:47 ` [PATCH 4/4] xfs: rt stubs should return negative errnos when rt disabled Darrick J. Wong
2023-10-17 15:46 ` [PATCHSET RFC v1.1 0/8] xfs: clean up realtime type usage Darrick J. Wong
2023-10-17 15:48 ` [PATCH 1/8] xfs: fix units conversion error in xfs_bmap_del_extent_delay Darrick J. Wong
2023-10-17 16:01 ` Christoph Hellwig
2023-10-17 15:48 ` [PATCH 2/8] xfs: make sure maxlen is still congruent with prod when rounding down Darrick J. Wong
2023-10-17 15:48 ` [PATCH 3/8] xfs: move the xfs_rtbitmap.c declarations to xfs_rtbitmap.h Darrick J. Wong
2023-10-17 15:48 ` [PATCH 4/8] xfs: convert xfs_extlen_t to xfs_rtxlen_t in the rt allocator Darrick J. Wong
2023-10-17 15:49 ` [PATCH 5/8] xfs: convert rt bitmap/summary block numbers to xfs_fileoff_t Darrick J. Wong
2023-10-17 15:49 ` [PATCH 6/8] xfs: convert rt bitmap extent lengths to xfs_rtbxlen_t Darrick J. Wong
2023-10-17 15:49 ` [PATCH 7/8] xfs: rename xfs_verify_rtext to xfs_verify_rtbext Darrick J. Wong
2023-10-17 15:49 ` [PATCH 8/8] xfs: convert rt extent numbers to xfs_rtxnum_t Darrick J. Wong
2023-10-17 15:46 ` [PATCHSET RFC v1.1 0/7] xfs: refactor rt extent unit conversions Darrick J. Wong
2023-10-17 15:50 ` [PATCH 1/7] xfs: create a helper to convert rtextents to rtblocks Darrick J. Wong
2023-10-17 15:50 ` [PATCH 2/7] xfs: create a helper to compute leftovers of realtime extents Darrick J. Wong
2023-10-17 15:50 ` [PATCH 3/7] xfs: create a helper to convert extlen to rtextlen Darrick J. Wong
2023-10-17 15:50 ` [PATCH 4/7] xfs: create helpers to convert rt block numbers to rt extent numbers Darrick J. Wong
2023-10-17 16:04 ` Christoph Hellwig
2023-10-17 15:51 ` [PATCH 5/7] xfs: convert do_div calls to xfs_rtb_to_rtx helper calls Darrick J. Wong
2023-10-17 16:05 ` Christoph Hellwig
2023-10-17 15:51 ` [PATCH 6/7] xfs: create rt extent rounding helpers for realtime extent blocks Darrick J. Wong
2023-10-17 15:51 ` [PATCH 7/7] xfs: use shifting and masking when converting rt extents, if possible Darrick J. Wong
2023-10-17 15:46 ` [PATCHSET RFC v1.1 0/8] xfs: refactor rtbitmap/summary macros Darrick J. Wong
2023-10-17 15:52 ` [PATCH 1/8] xfs: convert the rtbitmap block and bit macros to static inline functions Darrick J. Wong
2023-10-17 15:52 ` [PATCH 2/8] xfs: remove XFS_BLOCKWSIZE and XFS_BLOCKWMASK macros Darrick J. Wong
2023-10-17 15:52 ` [PATCH 3/8] xfs: convert open-coded xfs_rtword_t pointer accesses to helper Darrick J. Wong
2023-10-17 16:31 ` Christoph Hellwig
2023-10-17 15:52 ` [PATCH 4/8] xfs: convert rt summary macros to helpers Darrick J. Wong
2023-10-17 15:53 ` [PATCH 5/8] xfs: create helpers for rtbitmap block/wordcount computations Darrick J. Wong
2023-10-17 16:33 ` Christoph Hellwig
2023-10-17 15:53 ` [PATCH 6/8] xfs: use accessor functions for bitmap words Darrick J. Wong
2023-10-17 18:53 ` Christoph Hellwig
2023-10-18 2:01 ` Darrick J. Wong
2023-10-18 4:50 ` Christoph Hellwig
2023-10-17 15:53 ` [PATCH 7/8] xfs: create helpers for rtsummary block/wordcount computations Darrick J. Wong
2023-10-17 15:53 ` [PATCH 8/8] xfs: use accessor functions for summary info words Darrick J. Wong
2023-10-18 2:10 ` [PATCHSET RFC v1.2 0/4] xfs: refactor rtbitmap/summary accessors Darrick J. Wong
2023-10-18 2:10 ` [PATCH 1/4] xfs: create a helper to handle logging parts of rt bitmap blocks Darrick J. Wong
2023-10-18 4:52 ` Christoph Hellwig
2023-10-18 2:10 ` [PATCH 2/4] xfs: use accessor functions for bitmap words Darrick J. Wong
2023-10-18 2:19 ` Darrick J. Wong
2023-10-18 4:54 ` Christoph Hellwig
2023-10-18 4:54 ` Christoph Hellwig
2023-10-18 16:27 ` Darrick J. Wong
2023-10-18 16:28 ` Christoph Hellwig
2023-10-18 2:10 ` [PATCH 3/4] xfs: create helpers for rtsummary block/wordcount computations Darrick J. Wong
2023-10-18 2:10 ` [PATCH 4/4] xfs: use accessor functions for summary info words Darrick J. Wong
2023-10-18 5:19 ` Christoph Hellwig
2023-10-18 5:31 ` Darrick J. Wong [this message]
2023-10-18 5:35 ` Christoph Hellwig
2023-10-18 6:16 ` Darrick J. Wong
2023-10-17 15:46 ` [PATCHSET RFC 2.1 0/7] xfs: CPU usage optimizations for realtime allocator Darrick J. Wong
2023-10-17 15:54 ` [PATCH 1/7] xfs: consolidate realtime allocation arguments Darrick J. Wong
2023-10-18 6:16 ` Christoph Hellwig
2023-10-18 17:04 ` Darrick J. Wong
2023-10-17 15:54 ` [PATCH 2/7] xfs: cache last bitmap block in realtime allocator Darrick J. Wong
2023-10-18 6:19 ` Christoph Hellwig
2023-10-18 16:33 ` Darrick J. Wong
2023-10-19 0:00 ` Darrick J. Wong
2023-10-18 18:28 ` Darrick J. Wong
2023-10-17 15:54 ` [PATCH 3/7] xfs: invert the realtime summary cache Darrick J. Wong
2023-10-17 15:54 ` [PATCH 4/7] xfs: return maximum free size from xfs_rtany_summary() Darrick J. Wong
2023-10-17 15:55 ` [PATCH 5/7] xfs: limit maxlen based on available space in xfs_rtallocate_extent_near() Darrick J. Wong
2023-10-17 15:55 ` [PATCH 6/7] xfs: don't try redundant allocations " Darrick J. Wong
2023-10-17 15:55 ` [PATCH 7/7] xfs: don't look for end of extent further than necessary " Darrick J. Wong
2023-10-19 0:00 ` [PATCHSET v2.2 0/9] xfs: CPU usage optimizations for realtime allocator Darrick J. Wong
2023-10-19 0:00 ` [PATCH 1/9] xfs: consolidate realtime allocation arguments Darrick J. Wong
2023-10-19 5:08 ` Christoph Hellwig
2023-10-19 0:01 ` [PATCH 2/9] xfs: cache last bitmap block in realtime allocator Darrick J. Wong
2023-10-19 0:01 ` [PATCH 3/9] xfs: simplify xfs_rtbuf_get calling conventions Darrick J. Wong
2023-10-19 5:11 ` Christoph Hellwig
2023-10-19 0:01 ` [PATCH 4/9] xfs: simplify rt bitmap/summary block accessor functions Darrick J. Wong
2023-10-19 5:13 ` Christoph Hellwig
2023-10-19 16:13 ` Darrick J. Wong
2023-10-19 0:01 ` [PATCH 5/9] xfs: invert the realtime summary cache Darrick J. Wong
2023-10-19 0:01 ` [PATCH 6/9] xfs: return maximum free size from xfs_rtany_summary() Darrick J. Wong
2023-10-19 0:01 ` [PATCH 7/9] xfs: limit maxlen based on available space in xfs_rtallocate_extent_near() Darrick J. Wong
2023-10-19 0:01 ` [PATCH 8/9] xfs: don't try redundant allocations " Darrick J. Wong
2023-10-19 0:01 ` [PATCH 9/9] xfs: don't look for end of extent further than necessary " Darrick J. Wong
-- strict thread matches above, loose matches on Subject: below --
2023-10-19 16:21 [PATCHSET v1.2 0/4] xfs: refactor rtbitmap/summary accessors Darrick J. Wong
2023-10-19 16:28 ` [PATCH 4/4] xfs: use accessor functions for summary info words Darrick J. Wong
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=20231018053127.GE3195650@frogsfrogsfrogs \
--to=djwong@kernel.org \
--cc=hch@lst.de \
--cc=linux-xfs@vger.kernel.org \
--cc=osandov@fb.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