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 23:16:13 -0700 [thread overview]
Message-ID: <20231018061613.GF3195650@frogsfrogsfrogs> (raw)
In-Reply-To: <20231018053127.GE3195650@frogsfrogsfrogs>
On Tue, Oct 17, 2023 at 10:31:27PM -0700, Darrick J. Wong wrote:
> 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...
Oh rats, I forgot to adjust the xfs_trans_log_rtbitmap offsets for the
block headers added in the rtgroups patchset.
--D
> --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 6:16 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
2023-10-18 5:35 ` Christoph Hellwig
2023-10-18 6:16 ` Darrick J. Wong [this message]
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=20231018061613.GF3195650@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