On Mon, Jul 17, 2023 at 11:18:12AM -0700, Omar Sandoval wrote: > On Wed, Jul 12, 2023 at 11:29:26AM -0700, Darrick J. Wong wrote: > > On Tue, Jun 20, 2023 at 02:32:11PM -0700, Omar Sandoval wrote: > > > From: Omar Sandoval <osandov@xxxxxx> > > > > > > Profiling a workload on a highly fragmented realtime device showed a ton > > > of CPU cycles being spent in xfs_trans_read_buf() called by > > > xfs_rtbuf_get(). Further tracing showed that much of that was repeated > > > calls to xfs_rtbuf_get() for the same block of the realtime bitmap. > > > These come from xfs_rtallocate_extent_block(): as it walks through > > > ranges of free bits in the bitmap, each call to xfs_rtcheck_range() and > > > xfs_rtfind_{forw,back}() gets the same bitmap block. If the bitmap block > > > is very fragmented, then this is _a lot_ of buffer lookups. > > > > > > The realtime allocator already passes around a cache of the last used > > > realtime summary block to avoid repeated reads (the parameters rbpp and > > > rsb). We can do the same for the realtime bitmap. > > > > > > This replaces rbpp and rsb with a struct xfs_rtbuf_cache, which caches > > > the most recently used block for both the realtime bitmap and summary. > > > xfs_rtbuf_get() now handles the caching instead of the callers, which > > > requires plumbing xfs_rtbuf_cache to more functions but also makes sure > > > we don't miss anything. > > > > Hmm. I initially wondered why rtbitmap blocks wouldn't just stay > > attached to the transaction, but then I realized -- this is the > > /scanning/ phase that's repeatedly getting and releasing the rtbmp > > block, right? The allocator hasn't dirtied any rtbitmap blocks yet, so > > each xfs_trans_brelse actually drops the buffer, and that's how the > > xfs_rtbuf_get ends up pounding on the bmapi and then the buffer cache? > > Repeatedly? > > That's right. > > > > Signed-off-by: Omar Sandoval <osandov@xxxxxx> > > > --- > > > fs/xfs/libxfs/xfs_rtbitmap.c | 167 ++++++++++++++++++----------------- > > > fs/xfs/xfs_rtalloc.c | 107 ++++++++++------------ > > > fs/xfs/xfs_rtalloc.h | 28 ++++-- > > > 3 files changed, 154 insertions(+), 148 deletions(-) > > > > > > diff --git a/fs/xfs/libxfs/xfs_rtbitmap.c b/fs/xfs/libxfs/xfs_rtbitmap.c > > > index fa180ab66b73..1a832c9a412f 100644 > > > --- a/fs/xfs/libxfs/xfs_rtbitmap.c > > > +++ b/fs/xfs/libxfs/xfs_rtbitmap.c > > > @@ -46,6 +46,21 @@ const struct xfs_buf_ops xfs_rtbuf_ops = { > > > .verify_write = xfs_rtbuf_verify_write, > > > }; > > > > > > +static void > > > +xfs_rtbuf_cache_relse( > > > + xfs_trans_t *tp, /* transaction pointer */ > > > > Please don't introduce more typedef usage. > > Oops, I was just copying the surrounding code. I'll fix this in my > changes. > > > struct xfs_trans *tp, > > struct xfs_rtbuf_cache *cache) > > > > (Also there's a lot of broken indenting in the lines added by this > > patch.) > > I'll fix it in this new function. Should I also be reindenting all of > the parameter lists where I added this parameter? Others are likely to have opinions, but I only care about the grottier things like mixing tabs and spaces in an indent. At some point I'll push my patchset to fix all the ambiguous type usages like fsblock for fileoff, and mixing of rtblock for rtextent and the like. > > > + struct xfs_rtbuf_cache *cache) /* cached blocks */ > > > +{ > > > + if (cache->bbuf) { > > > + xfs_trans_brelse(tp, cache->bbuf); > > > + cache->bbuf = NULL; > > > + } > > > + if (cache->sbuf) { > > > + xfs_trans_brelse(tp, cache->sbuf); > > > + cache->sbuf = NULL; > > > + } > > > +} > > > + > > > /* > > > * Get a buffer for the bitmap or summary file block specified. > > > * The buffer is returned read and locked. > > > @@ -56,14 +71,35 @@ xfs_rtbuf_get( > > > xfs_trans_t *tp, /* transaction pointer */ > > > xfs_rtblock_t block, /* block number in bitmap or summary */ > > > int issum, /* is summary not bitmap */ > > > + struct xfs_rtbuf_cache *cache, /* in/out: cached blocks */ > > > struct xfs_buf **bpp) /* output: buffer for the block */ > > > { > > > + struct xfs_buf **cbpp; /* cached block buffer */ > > > + xfs_fsblock_t *cbp; /* cached block number */ > > > struct xfs_buf *bp; /* block buffer, result */ > > > xfs_inode_t *ip; /* bitmap or summary inode */ > > > xfs_bmbt_irec_t map; > > > int nmap = 1; > > > int error; /* error value */ > > > > > > + cbpp = issum ? &cache->bbuf : &cache->sbuf; > > > + cbp = issum ? &cache->bblock : &cache->sblock; > > > + /* > > > + * If we have a cached buffer, and the block number matches, use that. > > > + */ > > > + if (*cbpp && *cbp == block) { > > > + *bpp = *cbpp; > > > + return 0; > > > + } > > > + /* > > > + * Otherwise we have to have to get the buffer. If there was an old > > > + * one, get rid of it first. > > > + */ > > > + if (*cbpp) { > > > + xfs_trans_brelse(tp, *cbpp); > > > + *cbpp = NULL; > > > + } > > > + > > > ip = issum ? mp->m_rsumip : mp->m_rbmip; > > > > > > error = xfs_bmapi_read(ip, block, 1, &map, &nmap, 0); > > > @@ -82,7 +118,8 @@ xfs_rtbuf_get( > > > > > > xfs_trans_buf_set_type(tp, bp, issum ? XFS_BLFT_RTSUMMARY_BUF > > > : XFS_BLFT_RTBITMAP_BUF); > > > - *bpp = bp; > > > + *cbpp = *bpp = bp; > > > + *cbp = block; > > > return 0; > > > } > > > > > > @@ -96,6 +133,7 @@ xfs_rtfind_back( > > > xfs_trans_t *tp, /* transaction pointer */ > > > xfs_rtblock_t start, /* starting block to look at */ > > > xfs_rtblock_t limit, /* last block to look at */ > > > + struct xfs_rtbuf_cache *rtbufc, /* in/out: cache of realtime blocks */ > > > xfs_rtblock_t *rtblock) /* out: start block found */ > > > { > > > xfs_rtword_t *b; /* current word in buffer */ > > > @@ -116,7 +154,7 @@ xfs_rtfind_back( > > > * Compute and read in starting bitmap block for starting block. > > > */ > > > block = XFS_BITTOBLOCK(mp, start); > > > - error = xfs_rtbuf_get(mp, tp, block, 0, &bp); > > > + error = xfs_rtbuf_get(mp, tp, block, 0, rtbufc, &bp); > > > if (error) { > > > return error; > > > } > > > @@ -153,7 +191,6 @@ xfs_rtfind_back( > > > /* > > > * Different. Mark where we are and return. > > > */ > > > - xfs_trans_brelse(tp, bp); > > > i = bit - XFS_RTHIBIT(wdiff); > > > *rtblock = start - i + 1; > > > return 0; > > > @@ -167,8 +204,7 @@ xfs_rtfind_back( > > > /* > > > * If done with this block, get the previous one. > > > */ > > > - xfs_trans_brelse(tp, bp); > > > - error = xfs_rtbuf_get(mp, tp, --block, 0, &bp); > > > + error = xfs_rtbuf_get(mp, tp, --block, 0, rtbufc, &bp); > > > if (error) { > > > return error; > > > } > > > @@ -199,7 +235,6 @@ xfs_rtfind_back( > > > /* > > > * Different, mark where we are and return. > > > */ > > > - xfs_trans_brelse(tp, bp); > > > i += XFS_NBWORD - 1 - XFS_RTHIBIT(wdiff); > > > *rtblock = start - i + 1; > > > return 0; > > > @@ -213,8 +248,7 @@ xfs_rtfind_back( > > > /* > > > * If done with this block, get the previous one. > > > */ > > > - xfs_trans_brelse(tp, bp); > > > - error = xfs_rtbuf_get(mp, tp, --block, 0, &bp); > > > + error = xfs_rtbuf_get(mp, tp, --block, 0, rtbufc, &bp); > > > if (error) { > > > return error; > > > } > > > @@ -246,7 +280,6 @@ xfs_rtfind_back( > > > /* > > > * Different, mark where we are and return. > > > */ > > > - xfs_trans_brelse(tp, bp); > > > i += XFS_NBWORD - 1 - XFS_RTHIBIT(wdiff); > > > *rtblock = start - i + 1; > > > return 0; > > > @@ -256,7 +289,6 @@ xfs_rtfind_back( > > > /* > > > * No match, return that we scanned the whole area. > > > */ > > > - xfs_trans_brelse(tp, bp); > > > *rtblock = start - i + 1; > > > return 0; > > > } > > > @@ -271,6 +303,7 @@ xfs_rtfind_forw( > > > xfs_trans_t *tp, /* transaction pointer */ > > > xfs_rtblock_t start, /* starting block to look at */ > > > xfs_rtblock_t limit, /* last block to look at */ > > > + struct xfs_rtbuf_cache *rtbufc, /* in/out: cache of realtime blocks */ > > > xfs_rtblock_t *rtblock) /* out: start block found */ > > > { > > > xfs_rtword_t *b; /* current word in buffer */ > > > @@ -291,7 +324,7 @@ xfs_rtfind_forw( > > > * Compute and read in starting bitmap block for starting block. > > > */ > > > block = XFS_BITTOBLOCK(mp, start); > > > - error = xfs_rtbuf_get(mp, tp, block, 0, &bp); > > > + error = xfs_rtbuf_get(mp, tp, block, 0, rtbufc, &bp); > > > if (error) { > > > return error; > > > } > > > @@ -327,7 +360,6 @@ xfs_rtfind_forw( > > > /* > > > * Different. Mark where we are and return. > > > */ > > > - xfs_trans_brelse(tp, bp); > > > i = XFS_RTLOBIT(wdiff) - bit; > > > *rtblock = start + i - 1; > > > return 0; > > > @@ -341,8 +373,7 @@ xfs_rtfind_forw( > > > /* > > > * If done with this block, get the previous one. > > > */ > > > - xfs_trans_brelse(tp, bp); > > > - error = xfs_rtbuf_get(mp, tp, ++block, 0, &bp); > > > + error = xfs_rtbuf_get(mp, tp, ++block, 0, rtbufc, &bp); > > > if (error) { > > > return error; > > > } > > > @@ -372,7 +403,6 @@ xfs_rtfind_forw( > > > /* > > > * Different, mark where we are and return. > > > */ > > > - xfs_trans_brelse(tp, bp); > > > i += XFS_RTLOBIT(wdiff); > > > *rtblock = start + i - 1; > > > return 0; > > > @@ -386,8 +416,7 @@ xfs_rtfind_forw( > > > /* > > > * If done with this block, get the next one. > > > */ > > > - xfs_trans_brelse(tp, bp); > > > - error = xfs_rtbuf_get(mp, tp, ++block, 0, &bp); > > > + error = xfs_rtbuf_get(mp, tp, ++block, 0, rtbufc, &bp); > > > if (error) { > > > return error; > > > } > > > @@ -416,7 +445,6 @@ xfs_rtfind_forw( > > > /* > > > * Different, mark where we are and return. > > > */ > > > - xfs_trans_brelse(tp, bp); > > > i += XFS_RTLOBIT(wdiff); > > > *rtblock = start + i - 1; > > > return 0; > > > @@ -426,7 +454,6 @@ xfs_rtfind_forw( > > > /* > > > * No match, return that we scanned the whole area. > > > */ > > > - xfs_trans_brelse(tp, bp); > > > *rtblock = start + i - 1; > > > return 0; > > > } > > > @@ -447,8 +474,7 @@ xfs_rtmodify_summary_int( > > > int log, /* log2 of extent size */ > > > xfs_rtblock_t bbno, /* bitmap block number */ > > > int delta, /* change to make to summary info */ > > > - struct xfs_buf **rbpp, /* in/out: summary block buffer */ > > > - xfs_fsblock_t *rsb, /* in/out: summary block number */ > > > + struct xfs_rtbuf_cache *rtbufc, /* in/out: cache of realtime blocks */ > > > xfs_suminfo_t *sum) /* out: summary info for this block */ > > > { > > > struct xfs_buf *bp; /* buffer for the summary block */ > > > @@ -465,30 +491,9 @@ xfs_rtmodify_summary_int( > > > * Compute the block number in the summary file. > > > */ > > > sb = XFS_SUMOFFSTOBLOCK(mp, so); > > > - /* > > > - * If we have an old buffer, and the block number matches, use that. > > > - */ > > > - if (*rbpp && *rsb == sb) > > > - bp = *rbpp; > > > - /* > > > - * Otherwise we have to get the buffer. > > > - */ > > > - 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; > > > - } > > > + error = xfs_rtbuf_get(mp, tp, sb, 1, rtbufc, &bp); > > > + if (error) > > > + return error; > > > /* > > > * Point to the summary information, modify/log it, and/or copy it out. > > > */ > > > @@ -517,11 +522,9 @@ xfs_rtmodify_summary( > > > int log, /* log2 of extent size */ > > > xfs_rtblock_t bbno, /* bitmap block number */ > > > int delta, /* change to make to summary info */ > > > - struct xfs_buf **rbpp, /* in/out: summary block buffer */ > > > - xfs_fsblock_t *rsb) /* in/out: summary block number */ > > > + struct xfs_rtbuf_cache *rtbufc) /* in/out: cache of realtime blocks */ > > > { > > > - return xfs_rtmodify_summary_int(mp, tp, log, bbno, > > > - delta, rbpp, rsb, NULL); > > > + return xfs_rtmodify_summary_int(mp, tp, log, bbno, delta, rtbufc, NULL); > > > } > > > > > > /* > > > @@ -534,7 +537,8 @@ xfs_rtmodify_range( > > > xfs_trans_t *tp, /* transaction pointer */ > > > xfs_rtblock_t start, /* starting block to modify */ > > > xfs_extlen_t len, /* length of extent to modify */ > > > - int val) /* 1 for free, 0 for allocated */ > > > + int val, /* 1 for free, 0 for allocated */ > > > + struct xfs_rtbuf_cache *rtbufc) /* in/out: cache of realtime blocks */ > > > { > > > xfs_rtword_t *b; /* current word in buffer */ > > > int bit; /* bit number in the word */ > > > @@ -555,7 +559,7 @@ xfs_rtmodify_range( > > > /* > > > * Read the bitmap block, and point to its data. > > > */ > > > - error = xfs_rtbuf_get(mp, tp, block, 0, &bp); > > > + error = xfs_rtbuf_get(mp, tp, block, 0, rtbufc, &bp); > > > if (error) { > > > return error; > > > } > > > @@ -600,7 +604,7 @@ xfs_rtmodify_range( > > > xfs_trans_log_buf(tp, bp, > > > (uint)((char *)first - (char *)bufp), > > > (uint)((char *)b - (char *)bufp)); > > > - error = xfs_rtbuf_get(mp, tp, ++block, 0, &bp); > > > + error = xfs_rtbuf_get(mp, tp, ++block, 0, rtbufc, &bp); > > > if (error) { > > > return error; > > > } > > > @@ -640,7 +644,7 @@ xfs_rtmodify_range( > > > xfs_trans_log_buf(tp, bp, > > > (uint)((char *)first - (char *)bufp), > > > (uint)((char *)b - (char *)bufp)); > > > - error = xfs_rtbuf_get(mp, tp, ++block, 0, &bp); > > > + error = xfs_rtbuf_get(mp, tp, ++block, 0, rtbufc, &bp); > > > if (error) { > > > return error; > > > } > > > @@ -690,8 +694,7 @@ xfs_rtfree_range( > > > xfs_trans_t *tp, /* transaction pointer */ > > > xfs_rtblock_t start, /* starting block to free */ > > > xfs_extlen_t len, /* length to free */ > > > - struct xfs_buf **rbpp, /* in/out: summary block buffer */ > > > - xfs_fsblock_t *rsb) /* in/out: summary block number */ > > > + struct xfs_rtbuf_cache *rtbufc) /* in/out: cache of realtime blocks */ > > > { > > > xfs_rtblock_t end; /* end of the freed extent */ > > > int error; /* error value */ > > > @@ -702,7 +705,7 @@ xfs_rtfree_range( > > > /* > > > * Modify the bitmap to mark this extent freed. > > > */ > > > - error = xfs_rtmodify_range(mp, tp, start, len, 1); > > > + error = xfs_rtmodify_range(mp, tp, start, len, 1, rtbufc); > > > if (error) { > > > return error; > > > } > > > @@ -711,7 +714,7 @@ xfs_rtfree_range( > > > * We need to find the beginning and end of the extent so we can > > > * properly update the summary. > > > */ > > > - error = xfs_rtfind_back(mp, tp, start, 0, &preblock); > > > + error = xfs_rtfind_back(mp, tp, start, 0, rtbufc, &preblock); > > > if (error) { > > > return error; > > > } > > > @@ -719,7 +722,7 @@ xfs_rtfree_range( > > > * Find the next allocated block (end of allocated extent). > > > */ > > > error = xfs_rtfind_forw(mp, tp, end, mp->m_sb.sb_rextents - 1, > > > - &postblock); > > > + rtbufc, &postblock); > > > if (error) > > > return error; > > > /* > > > @@ -729,7 +732,7 @@ xfs_rtfree_range( > > > if (preblock < start) { > > > error = xfs_rtmodify_summary(mp, tp, > > > XFS_RTBLOCKLOG(start - preblock), > > > - XFS_BITTOBLOCK(mp, preblock), -1, rbpp, rsb); > > > + XFS_BITTOBLOCK(mp, preblock), -1, rtbufc); > > > if (error) { > > > return error; > > > } > > > @@ -741,7 +744,7 @@ xfs_rtfree_range( > > > if (postblock > end) { > > > error = xfs_rtmodify_summary(mp, tp, > > > XFS_RTBLOCKLOG(postblock - end), > > > - XFS_BITTOBLOCK(mp, end + 1), -1, rbpp, rsb); > > > + XFS_BITTOBLOCK(mp, end + 1), -1, rtbufc); > > > if (error) { > > > return error; > > > } > > > @@ -752,7 +755,7 @@ xfs_rtfree_range( > > > */ > > > error = xfs_rtmodify_summary(mp, tp, > > > XFS_RTBLOCKLOG(postblock + 1 - preblock), > > > - XFS_BITTOBLOCK(mp, preblock), 1, rbpp, rsb); > > > + XFS_BITTOBLOCK(mp, preblock), 1, rtbufc); > > > return error; > > > } > > > > > > @@ -767,6 +770,7 @@ xfs_rtcheck_range( > > > xfs_rtblock_t start, /* starting block number of extent */ > > > xfs_extlen_t len, /* length of extent */ > > > int val, /* 1 for free, 0 for allocated */ > > > + struct xfs_rtbuf_cache *rtbufc, /* in/out: cache of realtime blocks */ > > > xfs_rtblock_t *new, /* out: first block not matching */ > > > int *stat) /* out: 1 for matches, 0 for not */ > > > { > > > @@ -789,7 +793,7 @@ xfs_rtcheck_range( > > > /* > > > * Read the bitmap block. > > > */ > > > - error = xfs_rtbuf_get(mp, tp, block, 0, &bp); > > > + error = xfs_rtbuf_get(mp, tp, block, 0, rtbufc, &bp); > > > if (error) { > > > return error; > > > } > > > @@ -824,7 +828,6 @@ xfs_rtcheck_range( > > > /* > > > * Different, compute first wrong bit and return. > > > */ > > > - xfs_trans_brelse(tp, bp); > > > i = XFS_RTLOBIT(wdiff) - bit; > > > *new = start + i; > > > *stat = 0; > > > @@ -839,8 +842,7 @@ xfs_rtcheck_range( > > > /* > > > * If done with this block, get the next one. > > > */ > > > - xfs_trans_brelse(tp, bp); > > > - error = xfs_rtbuf_get(mp, tp, ++block, 0, &bp); > > > + error = xfs_rtbuf_get(mp, tp, ++block, 0, rtbufc, &bp); > > > if (error) { > > > return error; > > > } > > > @@ -870,7 +872,6 @@ xfs_rtcheck_range( > > > /* > > > * Different, compute first wrong bit and return. > > > */ > > > - xfs_trans_brelse(tp, bp); > > > i += XFS_RTLOBIT(wdiff); > > > *new = start + i; > > > *stat = 0; > > > @@ -885,8 +886,7 @@ xfs_rtcheck_range( > > > /* > > > * If done with this block, get the next one. > > > */ > > > - xfs_trans_brelse(tp, bp); > > > - error = xfs_rtbuf_get(mp, tp, ++block, 0, &bp); > > > + error = xfs_rtbuf_get(mp, tp, ++block, 0, rtbufc, &bp); > > > if (error) { > > > return error; > > > } > > > @@ -915,7 +915,6 @@ xfs_rtcheck_range( > > > /* > > > * Different, compute first wrong bit and return. > > > */ > > > - xfs_trans_brelse(tp, bp); > > > i += XFS_RTLOBIT(wdiff); > > > *new = start + i; > > > *stat = 0; > > > @@ -926,7 +925,6 @@ xfs_rtcheck_range( > > > /* > > > * Successful, return. > > > */ > > > - xfs_trans_brelse(tp, bp); > > > *new = start + i; > > > *stat = 1; > > > return 0; > > > @@ -941,20 +939,21 @@ xfs_rtcheck_alloc_range( > > > xfs_mount_t *mp, /* file system mount point */ > > > xfs_trans_t *tp, /* transaction pointer */ > > > xfs_rtblock_t bno, /* starting block number of extent */ > > > - xfs_extlen_t len) /* length of extent */ > > > + xfs_extlen_t len, /* length of extent */ > > > + struct xfs_rtbuf_cache *rtbufc) /* in/out: cached blocks */ > > > { > > > xfs_rtblock_t new; /* dummy for xfs_rtcheck_range */ > > > int stat; > > > int error; > > > > > > - error = xfs_rtcheck_range(mp, tp, bno, len, 0, &new, &stat); > > > + error = xfs_rtcheck_range(mp, tp, bno, len, 0, rtbufc, &new, &stat); > > > if (error) > > > return error; > > > ASSERT(stat); > > > return 0; > > > } > > > #else > > > -#define xfs_rtcheck_alloc_range(m,t,b,l) (0) > > > +#define xfs_rtcheck_alloc_range(m,t,b,l,r) (0) > > > #endif > > > /* > > > * Free an extent in the realtime subvolume. Length is expressed in > > > @@ -968,22 +967,21 @@ xfs_rtfree_extent( > > > { > > > int error; /* error value */ > > > xfs_mount_t *mp; /* file system mount structure */ > > > - xfs_fsblock_t sb; /* summary file block number */ > > > - struct xfs_buf *sumbp = NULL; /* summary file block buffer */ > > > + struct xfs_rtbuf_cache rtbufc = {}; /* cache of realtime blocks */ > > > > > > mp = tp->t_mountp; > > > > > > ASSERT(mp->m_rbmip->i_itemp != NULL); > > > ASSERT(xfs_isilocked(mp->m_rbmip, XFS_ILOCK_EXCL)); > > > > > > - error = xfs_rtcheck_alloc_range(mp, tp, bno, len); > > > + error = xfs_rtcheck_alloc_range(mp, tp, bno, len, &rtbufc); > > > if (error) > > > return error; > > > > > > /* > > > * Free the range of realtime blocks. > > > */ > > > - error = xfs_rtfree_range(mp, tp, bno, len, &sumbp, &sb); > > > + error = xfs_rtfree_range(mp, tp, bno, len, &rtbufc); > > > if (error) { > > > return error; > > > } > > > @@ -1021,6 +1019,7 @@ xfs_rtalloc_query_range( > > > xfs_rtblock_t high_key; > > > int is_free; > > > int error = 0; > > > + struct xfs_rtbuf_cache rtbufc = {}; > > > > > > if (low_rec->ar_startext > high_rec->ar_startext) > > > return -EINVAL; > > > @@ -1034,13 +1033,14 @@ xfs_rtalloc_query_range( > > > rtstart = low_rec->ar_startext; > > > while (rtstart <= high_key) { > > > /* Is the first block free? */ > > > - error = xfs_rtcheck_range(mp, tp, rtstart, 1, 1, &rtend, > > > - &is_free); > > > + error = xfs_rtcheck_range(mp, tp, rtstart, 1, 1, &rtbufc, > > > + &rtend, &is_free); > > > if (error) > > > break; > > > > > > /* How long does the extent go for? */ > > > - error = xfs_rtfind_forw(mp, tp, rtstart, high_key, &rtend); > > > + error = xfs_rtfind_forw(mp, tp, rtstart, high_key, &rtbufc, > > > + &rtend); > > > if (error) > > > break; > > > > > > @@ -1056,6 +1056,8 @@ xfs_rtalloc_query_range( > > > rtstart = rtend + 1; > > > } > > > > > > + xfs_rtbuf_cache_relse(tp, &rtbufc); > > > + > > > return error; > > > } > > > > > > @@ -1085,11 +1087,14 @@ xfs_rtalloc_extent_is_free( > > > xfs_extlen_t len, > > > bool *is_free) > > > { > > > + struct xfs_rtbuf_cache rtbufc = {}; > > > xfs_rtblock_t end; > > > int matches; > > > int error; > > > > > > - error = xfs_rtcheck_range(mp, tp, start, len, 1, &end, &matches); > > > + error = xfs_rtcheck_range(mp, tp, start, len, 1, &rtbufc, &end, > > > + &matches); > > > + xfs_rtbuf_cache_relse(tp, &rtbufc); > > > if (error) > > > return error; > > > > > > diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c > > > index 16534e9873f6..61ef13286654 100644 > > > --- a/fs/xfs/xfs_rtalloc.c > > > +++ b/fs/xfs/xfs_rtalloc.c > > > @@ -32,11 +32,10 @@ xfs_rtget_summary( > > > xfs_trans_t *tp, /* transaction pointer */ > > > int log, /* log2 of extent size */ > > > xfs_rtblock_t bbno, /* bitmap block number */ > > > - struct xfs_buf **rbpp, /* in/out: summary block buffer */ > > > - xfs_fsblock_t *rsb, /* in/out: summary block number */ > > > + struct xfs_rtbuf_cache *rtbufc, /* in/out: cache of realtime blocks */ > > > xfs_suminfo_t *sum) /* out: summary info for this block */ > > > { > > > - return xfs_rtmodify_summary_int(mp, tp, log, bbno, 0, rbpp, rsb, sum); > > > + return xfs_rtmodify_summary_int(mp, tp, log, bbno, 0, rtbufc, sum); > > > } > > > > > > /* > > > @@ -50,8 +49,7 @@ xfs_rtany_summary( > > > int low, /* low log2 extent size */ > > > int high, /* high log2 extent size */ > > > xfs_rtblock_t bbno, /* bitmap block number */ > > > - struct xfs_buf **rbpp, /* in/out: summary block buffer */ > > > - xfs_fsblock_t *rsb, /* in/out: summary block number */ > > > + struct xfs_rtbuf_cache *rtbufc, /* in/out: cache of realtime blocks */ > > > int *stat) /* out: any good extents here? */ > > > { > > > int error; /* error value */ > > > @@ -69,7 +67,7 @@ xfs_rtany_summary( > > > /* > > > * Get one summary datum. > > > */ > > > - error = xfs_rtget_summary(mp, tp, log, bbno, rbpp, rsb, &sum); > > > + error = xfs_rtget_summary(mp, tp, log, bbno, rtbufc, &sum); > > > if (error) { > > > return error; > > > } > > > @@ -104,29 +102,27 @@ xfs_rtcopy_summary( > > > xfs_trans_t *tp) /* transaction pointer */ > > > { > > > xfs_rtblock_t bbno; /* bitmap block number */ > > > - struct xfs_buf *bp; /* summary buffer */ > > > + struct xfs_rtbuf_cache rtbufc = {}; /* cache of realtime blocks */ > > > int error; /* error return value */ > > > int log; /* summary level number (log length) */ > > > xfs_suminfo_t sum; /* summary data */ > > > - xfs_fsblock_t sumbno; /* summary block number */ > > > > > > - bp = NULL; > > > for (log = omp->m_rsumlevels - 1; log >= 0; log--) { > > > for (bbno = omp->m_sb.sb_rbmblocks - 1; > > > (xfs_srtblock_t)bbno >= 0; > > > bbno--) { > > > - error = xfs_rtget_summary(omp, tp, log, bbno, &bp, > > > - &sumbno, &sum); > > > + error = xfs_rtget_summary(omp, tp, log, bbno, &rtbufc, > > > + &sum); > > > if (error) > > > return error; > > > if (sum == 0) > > > continue; > > > error = xfs_rtmodify_summary(omp, tp, log, bbno, -sum, > > > - &bp, &sumbno); > > > + &rtbufc); > > > if (error) > > > return error; > > > error = xfs_rtmodify_summary(nmp, tp, log, bbno, sum, > > > - &bp, &sumbno); > > > + &rtbufc); > > > if (error) > > > return error; > > > ASSERT(sum > 0); > > > @@ -144,8 +140,7 @@ xfs_rtallocate_range( > > > xfs_trans_t *tp, /* transaction pointer */ > > > xfs_rtblock_t start, /* start block to allocate */ > > > xfs_extlen_t len, /* length to allocate */ > > > - struct xfs_buf **rbpp, /* in/out: summary block buffer */ > > > - xfs_fsblock_t *rsb) /* in/out: summary block number */ > > > + struct xfs_rtbuf_cache *rtbufc) /* in/out: cache of realtime blocks */ > > > { > > > xfs_rtblock_t end; /* end of the allocated extent */ > > > int error; /* error value */ > > > @@ -158,14 +153,14 @@ xfs_rtallocate_range( > > > * We need to find the beginning and end of the extent so we can > > > * properly update the summary. > > > */ > > > - error = xfs_rtfind_back(mp, tp, start, 0, &preblock); > > > + error = xfs_rtfind_back(mp, tp, start, 0, rtbufc, &preblock); > > > if (error) { > > > return error; > > > } > > > /* > > > * Find the next allocated block (end of free extent). > > > */ > > > - error = xfs_rtfind_forw(mp, tp, end, mp->m_sb.sb_rextents - 1, > > > + error = xfs_rtfind_forw(mp, tp, end, mp->m_sb.sb_rextents - 1, rtbufc, > > > &postblock); > > > if (error) { > > > return error; > > > @@ -176,7 +171,7 @@ xfs_rtallocate_range( > > > */ > > > error = xfs_rtmodify_summary(mp, tp, > > > XFS_RTBLOCKLOG(postblock + 1 - preblock), > > > - XFS_BITTOBLOCK(mp, preblock), -1, rbpp, rsb); > > > + XFS_BITTOBLOCK(mp, preblock), -1, rtbufc); > > > if (error) { > > > return error; > > > } > > > @@ -187,7 +182,7 @@ xfs_rtallocate_range( > > > if (preblock < start) { > > > error = xfs_rtmodify_summary(mp, tp, > > > XFS_RTBLOCKLOG(start - preblock), > > > - XFS_BITTOBLOCK(mp, preblock), 1, rbpp, rsb); > > > + XFS_BITTOBLOCK(mp, preblock), 1, rtbufc); > > > if (error) { > > > return error; > > > } > > > @@ -199,7 +194,7 @@ xfs_rtallocate_range( > > > if (postblock > end) { > > > error = xfs_rtmodify_summary(mp, tp, > > > XFS_RTBLOCKLOG(postblock - end), > > > - XFS_BITTOBLOCK(mp, end + 1), 1, rbpp, rsb); > > > + XFS_BITTOBLOCK(mp, end + 1), 1, rtbufc); > > > if (error) { > > > return error; > > > } > > > @@ -207,7 +202,7 @@ xfs_rtallocate_range( > > > /* > > > * Modify the bitmap to mark this extent allocated. > > > */ > > > - error = xfs_rtmodify_range(mp, tp, start, len, 0); > > > + error = xfs_rtmodify_range(mp, tp, start, len, 0, rtbufc); > > > return error; > > > } > > > > > > @@ -226,8 +221,7 @@ xfs_rtallocate_extent_block( > > > xfs_extlen_t maxlen, /* maximum length to allocate */ > > > xfs_extlen_t *len, /* out: actual length allocated */ > > > xfs_rtblock_t *nextp, /* out: next block to try */ > > > - struct xfs_buf **rbpp, /* in/out: summary block buffer */ > > > - xfs_fsblock_t *rsb, /* in/out: summary block number */ > > > + struct xfs_rtbuf_cache *rtbufc, /* in/out: cache of realtime blocks */ > > > xfs_extlen_t prod, /* extent product factor */ > > > xfs_rtblock_t *rtblock) /* out: start block allocated */ > > > { > > > @@ -254,7 +248,8 @@ xfs_rtallocate_extent_block( > > > * See if there's a free extent of maxlen starting at i. > > > * If it's not so then next will contain the first non-free. > > > */ > > > - error = xfs_rtcheck_range(mp, tp, i, maxlen, 1, &next, &stat); > > > + error = xfs_rtcheck_range(mp, tp, i, maxlen, 1, rtbufc, &next, > > > + &stat); > > > if (error) { > > > return error; > > > } > > > @@ -262,8 +257,7 @@ xfs_rtallocate_extent_block( > > > /* > > > * i for maxlen is all free, allocate and return that. > > > */ > > > - error = xfs_rtallocate_range(mp, tp, i, maxlen, rbpp, > > > - rsb); > > > + error = xfs_rtallocate_range(mp, tp, i, maxlen, rtbufc); > > > if (error) { > > > return error; > > > } > > > @@ -290,7 +284,7 @@ xfs_rtallocate_extent_block( > > > * If not done yet, find the start of the next free space. > > > */ > > > if (next < end) { > > > - error = xfs_rtfind_forw(mp, tp, next, end, &i); > > > + error = xfs_rtfind_forw(mp, tp, next, end, rtbufc, &i); > > > if (error) { > > > return error; > > > } > > > @@ -315,7 +309,7 @@ xfs_rtallocate_extent_block( > > > /* > > > * Allocate besti for bestlen & return that. > > > */ > > > - error = xfs_rtallocate_range(mp, tp, besti, bestlen, rbpp, rsb); > > > + error = xfs_rtallocate_range(mp, tp, besti, bestlen, rtbufc); > > > if (error) { > > > return error; > > > } > > > @@ -344,9 +338,8 @@ xfs_rtallocate_extent_exact( > > > xfs_rtblock_t bno, /* starting block number to allocate */ > > > xfs_extlen_t minlen, /* minimum length to allocate */ > > > xfs_extlen_t maxlen, /* maximum length to allocate */ > > > + struct xfs_rtbuf_cache *rtbufc, /* in/out: cache of realtime blocks */ > > > xfs_extlen_t *len, /* out: actual length allocated */ > > > - struct xfs_buf **rbpp, /* in/out: summary block buffer */ > > > - xfs_fsblock_t *rsb, /* in/out: summary block number */ > > > xfs_extlen_t prod, /* extent product factor */ > > > xfs_rtblock_t *rtblock) /* out: start block allocated */ > > > { > > > @@ -359,7 +352,8 @@ xfs_rtallocate_extent_exact( > > > /* > > > * Check if the range in question (for maxlen) is free. > > > */ > > > - error = xfs_rtcheck_range(mp, tp, bno, maxlen, 1, &next, &isfree); > > > + error = xfs_rtcheck_range(mp, tp, bno, maxlen, 1, rtbufc, &next, > > > + &isfree); > > > if (error) { > > > return error; > > > } > > > @@ -367,7 +361,7 @@ xfs_rtallocate_extent_exact( > > > /* > > > * If it is, allocate it and return success. > > > */ > > > - error = xfs_rtallocate_range(mp, tp, bno, maxlen, rbpp, rsb); > > > + error = xfs_rtallocate_range(mp, tp, bno, maxlen, rtbufc); > > > if (error) { > > > return error; > > > } > > > @@ -402,7 +396,7 @@ xfs_rtallocate_extent_exact( > > > /* > > > * Allocate what we can and return it. > > > */ > > > - error = xfs_rtallocate_range(mp, tp, bno, maxlen, rbpp, rsb); > > > + error = xfs_rtallocate_range(mp, tp, bno, maxlen, rtbufc); > > > if (error) { > > > return error; > > > } > > > @@ -424,8 +418,7 @@ xfs_rtallocate_extent_near( > > > xfs_extlen_t minlen, /* minimum length to allocate */ > > > xfs_extlen_t maxlen, /* maximum length to allocate */ > > > xfs_extlen_t *len, /* out: actual length allocated */ > > > - struct xfs_buf **rbpp, /* in/out: summary block buffer */ > > > - xfs_fsblock_t *rsb, /* in/out: summary block number */ > > > + struct xfs_rtbuf_cache *rtbufc, /* in/out: cache of realtime blocks */ > > > xfs_extlen_t prod, /* extent product factor */ > > > xfs_rtblock_t *rtblock) /* out: start block allocated */ > > > { > > > @@ -456,8 +449,8 @@ xfs_rtallocate_extent_near( > > > /* > > > * Try the exact allocation first. > > > */ > > > - error = xfs_rtallocate_extent_exact(mp, tp, bno, minlen, maxlen, len, > > > - rbpp, rsb, prod, &r); > > > + error = xfs_rtallocate_extent_exact(mp, tp, bno, minlen, maxlen, rtbufc, > > > + len, prod, &r); > > > if (error) { > > > return error; > > > } > > > @@ -481,7 +474,7 @@ xfs_rtallocate_extent_near( > > > * starting in this bitmap block. > > > */ > > > error = xfs_rtany_summary(mp, tp, log2len, mp->m_rsumlevels - 1, > > > - bbno + i, rbpp, rsb, &any); > > > + bbno + i, rtbufc, &any); > > > if (error) { > > > return error; > > > } > > > @@ -499,8 +492,8 @@ xfs_rtallocate_extent_near( > > > * this block. > > > */ > > > error = xfs_rtallocate_extent_block(mp, tp, > > > - bbno + i, minlen, maxlen, len, &n, rbpp, > > > - rsb, prod, &r); > > > + bbno + i, minlen, maxlen, len, &n, > > > + rtbufc, prod, &r); > > > if (error) { > > > return error; > > > } > > > @@ -529,7 +522,7 @@ xfs_rtallocate_extent_near( > > > */ > > > error = xfs_rtany_summary(mp, tp, > > > log2len, mp->m_rsumlevels - 1, > > > - bbno + j, rbpp, rsb, &any); > > > + bbno + j, rtbufc, &any); > > > if (error) { > > > return error; > > > } > > > @@ -545,7 +538,7 @@ xfs_rtallocate_extent_near( > > > continue; > > > error = xfs_rtallocate_extent_block(mp, > > > tp, bbno + j, minlen, maxlen, > > > - len, &n, rbpp, rsb, prod, &r); > > > + len, &n, rtbufc, prod, &r); > > > if (error) { > > > return error; > > > } > > > @@ -566,8 +559,8 @@ xfs_rtallocate_extent_near( > > > * that we found. > > > */ > > > error = xfs_rtallocate_extent_block(mp, tp, > > > - bbno + i, minlen, maxlen, len, &n, rbpp, > > > - rsb, prod, &r); > > > + bbno + i, minlen, maxlen, len, &n, > > > + rtbufc, prod, &r); > > > if (error) { > > > return error; > > > } > > > @@ -626,8 +619,7 @@ xfs_rtallocate_extent_size( > > > xfs_extlen_t minlen, /* minimum length to allocate */ > > > xfs_extlen_t maxlen, /* maximum length to allocate */ > > > xfs_extlen_t *len, /* out: actual length allocated */ > > > - struct xfs_buf **rbpp, /* in/out: summary block buffer */ > > > - xfs_fsblock_t *rsb, /* in/out: summary block number */ > > > + struct xfs_rtbuf_cache *rtbufc, /* in/out: cache of realtime blocks */ > > > xfs_extlen_t prod, /* extent product factor */ > > > xfs_rtblock_t *rtblock) /* out: start block allocated */ > > > { > > > @@ -656,7 +648,7 @@ xfs_rtallocate_extent_size( > > > /* > > > * Get the summary for this level/block. > > > */ > > > - error = xfs_rtget_summary(mp, tp, l, i, rbpp, rsb, > > > + error = xfs_rtget_summary(mp, tp, l, i, rtbufc, > > > &sum); > > > if (error) { > > > return error; > > > @@ -670,7 +662,7 @@ xfs_rtallocate_extent_size( > > > * Try allocating the extent. > > > */ > > > error = xfs_rtallocate_extent_block(mp, tp, i, maxlen, > > > - maxlen, len, &n, rbpp, rsb, prod, &r); > > > + maxlen, len, &n, rtbufc, prod, &r); > > > if (error) { > > > return error; > > > } > > > @@ -715,7 +707,7 @@ xfs_rtallocate_extent_size( > > > /* > > > * Get the summary information for this level/block. > > > */ > > > - error = xfs_rtget_summary(mp, tp, l, i, rbpp, rsb, > > > + error = xfs_rtget_summary(mp, tp, l, i, rtbufc, > > > &sum); > > > if (error) { > > > return error; > > > @@ -733,7 +725,7 @@ xfs_rtallocate_extent_size( > > > error = xfs_rtallocate_extent_block(mp, tp, i, > > > XFS_RTMAX(minlen, 1 << l), > > > XFS_RTMIN(maxlen, (1 << (l + 1)) - 1), > > > - len, &n, rbpp, rsb, prod, &r); > > > + len, &n, rtbufc, prod, &r); > > > if (error) { > > > return error; > > > } > > > @@ -922,7 +914,6 @@ xfs_growfs_rt( > > > xfs_extlen_t rbmblocks; /* current number of rt bitmap blocks */ > > > xfs_extlen_t rsumblocks; /* current number of rt summary blks */ > > > xfs_sb_t *sbp; /* old superblock */ > > > - xfs_fsblock_t sumbno; /* summary block number */ > > > uint8_t *rsum_cache; /* old summary cache */ > > > > > > sbp = &mp->m_sb; > > > @@ -1025,6 +1016,7 @@ xfs_growfs_rt( > > > bmbno++) { > > > struct xfs_trans *tp; > > > xfs_rfsblock_t nrblocks_step; > > > + struct xfs_rtbuf_cache rtbufc = {}; /* cache of realtime blocks */ > > > > > > *nmp = *mp; > > > nsbp = &nmp->m_sb; > > > @@ -1111,9 +1103,8 @@ xfs_growfs_rt( > > > /* > > > * Free new extent. > > > */ > > > - bp = NULL; > > > error = xfs_rtfree_range(nmp, tp, sbp->sb_rextents, > > > - nsbp->sb_rextents - sbp->sb_rextents, &bp, &sumbno); > > > + nsbp->sb_rextents - sbp->sb_rextents, &rtbufc); > > > if (error) { > > > error_cancel: > > > xfs_trans_cancel(tp); > > > @@ -1185,8 +1176,7 @@ xfs_rtallocate_extent( > > > xfs_mount_t *mp = tp->t_mountp; > > > int error; /* error value */ > > > xfs_rtblock_t r; /* result allocated block */ > > > - xfs_fsblock_t sb; /* summary file block number */ > > > - struct xfs_buf *sumbp; /* summary file block buffer */ > > > + struct xfs_rtbuf_cache rtbufc = {}; /* cache of realtime blocks */ > > > > > > ASSERT(xfs_isilocked(mp->m_rbmip, XFS_ILOCK_EXCL)); > > > ASSERT(minlen > 0 && minlen <= maxlen); > > > @@ -1208,13 +1198,14 @@ xfs_rtallocate_extent( > > > } > > > > > > retry: > > > - sumbp = NULL; > > > + rtbufc.bbuf = NULL; > > > + rtbufc.sbuf = NULL; > > > > xfs_rtbuf_cache_relse? > > In this case, the cached blocks have been dirtied and attached to the > transaction, so it's not strictly necessary (which is also why the old > code didn't call xfs_trans_brelse()). But it's probably clearer to just > use xfs_rtbuf_cache_relse() anyways rather than needing to justify that. > > > > if (bno == 0) { > > > error = xfs_rtallocate_extent_size(mp, tp, minlen, maxlen, len, > > > - &sumbp, &sb, prod, &r); > > > + &rtbufc, prod, &r); > > > } else { > > > error = xfs_rtallocate_extent_near(mp, tp, bno, minlen, maxlen, > > > - len, &sumbp, &sb, prod, &r); > > > + len, &rtbufc, prod, &r); > > > } > > > > > > if (error) > > > diff --git a/fs/xfs/xfs_rtalloc.h b/fs/xfs/xfs_rtalloc.h > > > index 62c7ad79cbb6..888552c4f287 100644 > > > --- a/fs/xfs/xfs_rtalloc.h > > > +++ b/fs/xfs/xfs_rtalloc.h > > > @@ -101,29 +101,39 @@ xfs_growfs_rt( > > > /* > > > * From xfs_rtbitmap.c > > > */ > > > +struct xfs_rtbuf_cache { > > > + struct xfs_buf *bbuf; /* bitmap block buffer */ > > > + xfs_fsblock_t bblock; /* bitmap block number */ > > > + struct xfs_buf *sbuf; /* summary block buffer */ > > > + xfs_fsblock_t sblock; /* summary block number */ > > > > These are really offsets into the data fork, right? They ought to be > > xfs_fileoff_t, except that none of the rtalloc code uses the correct > > type? > > Yup, once again I was mimicking the surrounding code. I can fix it here. Ok. > Thanks! NP. Sorry I've been slow at getting back to this. :/ --D > > > And yes I do want to merge the fixes for that: > > https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux.git/log/?h=clean-up-realtime-units > > > > --D > > > > > +}; > > > + > > > int xfs_rtbuf_get(struct xfs_mount *mp, struct xfs_trans *tp, > > > - xfs_rtblock_t block, int issum, struct xfs_buf **bpp); > > > + xfs_rtblock_t block, int issum, struct xfs_rtbuf_cache *cache, > > > + struct xfs_buf **bpp); > > > int xfs_rtcheck_range(struct xfs_mount *mp, struct xfs_trans *tp, > > > xfs_rtblock_t start, xfs_extlen_t len, int val, > > > - xfs_rtblock_t *new, int *stat); > > > + struct xfs_rtbuf_cache *rtbufc, xfs_rtblock_t *new, > > > + int *stat); > > > int xfs_rtfind_back(struct xfs_mount *mp, struct xfs_trans *tp, > > > xfs_rtblock_t start, xfs_rtblock_t limit, > > > - xfs_rtblock_t *rtblock); > > > + struct xfs_rtbuf_cache *rtbufc, xfs_rtblock_t *rtblock); > > > int xfs_rtfind_forw(struct xfs_mount *mp, struct xfs_trans *tp, > > > xfs_rtblock_t start, xfs_rtblock_t limit, > > > - xfs_rtblock_t *rtblock); > > > + struct xfs_rtbuf_cache *rtbufc, xfs_rtblock_t *rtblock); > > > int xfs_rtmodify_range(struct xfs_mount *mp, struct xfs_trans *tp, > > > - xfs_rtblock_t start, xfs_extlen_t len, int val); > > > + xfs_rtblock_t start, xfs_extlen_t len, int val, > > > + struct xfs_rtbuf_cache *rtbufc); > > > int xfs_rtmodify_summary_int(struct xfs_mount *mp, struct xfs_trans *tp, > > > int log, xfs_rtblock_t bbno, int delta, > > > - struct xfs_buf **rbpp, xfs_fsblock_t *rsb, > > > + struct xfs_rtbuf_cache *rtbufc, > > > xfs_suminfo_t *sum); > > > int xfs_rtmodify_summary(struct xfs_mount *mp, struct xfs_trans *tp, int log, > > > - xfs_rtblock_t bbno, int delta, struct xfs_buf **rbpp, > > > - xfs_fsblock_t *rsb); > > > + xfs_rtblock_t bbno, int delta, > > > + struct xfs_rtbuf_cache *rtbufc); > > > int xfs_rtfree_range(struct xfs_mount *mp, struct xfs_trans *tp, > > > xfs_rtblock_t start, xfs_extlen_t len, > > > - struct xfs_buf **rbpp, xfs_fsblock_t *rsb); > > > + struct xfs_rtbuf_cache *rtbufc); > > > int xfs_rtalloc_query_range(struct xfs_mount *mp, struct xfs_trans *tp, > > > const struct xfs_rtalloc_rec *low_rec, > > > const struct xfs_rtalloc_rec *high_rec, > > > -- > > > 2.41.0 > > >