On Fri, Sep 15, 2023 at 07:48:24AM +1000, Dave Chinner wrote:
> On Fri, Sep 08, 2023 at 08:28:27AM -0700, Darrick J. Wong wrote:
> > On Fri, Sep 08, 2023 at 01:43:15PM +1000, Dave Chinner wrote:
> > > On Tue, Sep 05, 2023 at 02:51:52PM -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.
> > > >
> > > > Signed-off-by: Omar Sandoval <osandov@xxxxxx>
> > > > ---
> > > > fs/xfs/libxfs/xfs_rtbitmap.c | 179 ++++++++++++++++++-----------------
> > > > fs/xfs/xfs_rtalloc.c | 119 +++++++++++------------
> > > > fs/xfs/xfs_rtalloc.h | 30 ++++--
> > > > 3 files changed, 170 insertions(+), 158 deletions(-)
> > >
> > > ....
> > >
> > > > diff --git a/fs/xfs/xfs_rtalloc.h b/fs/xfs/xfs_rtalloc.h
> > > > index 62c7ad79cbb6..72f4261bb101 100644
> > > > --- a/fs/xfs/xfs_rtalloc.h
> > > > +++ b/fs/xfs/xfs_rtalloc.h
> > > > @@ -101,29 +101,40 @@ xfs_growfs_rt(
> > > > /*
> > > > * From xfs_rtbitmap.c
> > > > */
> > > > +struct xfs_rtbuf_cache {
> > > > + struct xfs_buf *bbuf; /* bitmap block buffer */
> > > > + xfs_fileoff_t bblock; /* bitmap block number */
> > > > + struct xfs_buf *sbuf; /* summary block buffer */
> > > > + xfs_fileoff_t sblock; /* summary block number */
> > >
> > > I don't think the block numbers are file offsets? Most of the code
> > > passes the bitmap and summary block numbers around as a
> > > xfs_fsblock_t...
> >
> > They're fed into xfs_bmapi_read as the xfs_fileoff_t parameter.
> >
> > I have a whole series cleaning up all of the units abuse in the rt code
> > part of the realtime modernization patchdeluge:
> > https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux.git/log/?h=clean-up-realtime-units
> >
> > Here's the specific patch that cleans up this one part:
> > https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux.git/commit/?h=realtime-groups&id=b344bd4bd655576f8bda193b0e33f471a6295b05
> ....
>
> > Could someone (Omar?) take a look at my other rt cleanups too? I'd very
> > much like to get them merged out of my dev tree.
>
> Can you post the series to the list? That makes it much easier to
> comment on them....
https://lore.kernel.org/linux-xfs/167243865605.709511.15650588946095003543.stgit@magnolia/
--D
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@xxxxxxxxxxxxx
