| Message ID | da373ada54c851b448cc7d41167458dc6bd6f8ea.1687296675.git.osandov@osandov.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xfs: CPU usage optimizations for realtime allocator | expand |
On Tue, Jun 20, 2023 at 02:32:14PM -0700, Omar Sandoval wrote: > From: Omar Sandoval <osandov@fb.com> > > xfs_rtallocate_extent_near() calls xfs_rtallocate_extent_block() with > the minlen and maxlen that were passed to it. > xfs_rtallocate_extent_block() then scans the bitmap block looking for a > free range of size maxlen. If there is none, it has to scan the whole > bitmap block before returning the largest range of at least size minlen. > For a fragmented realtime device and a large allocation request, it's > almost certain that this will have to search the whole bitmap block, > leading to high CPU usage. > > However, the realtime summary tells us the maximum size available in the > bitmap block. We can limit the search in xfs_rtallocate_extent_block() > to that size and often stop before scanning the whole bitmap block. > > Signed-off-by: Omar Sandoval <osandov@fb.com> > --- > fs/xfs/xfs_rtalloc.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c > index ba7d42e0090f..d079dfb77c73 100644 > --- a/fs/xfs/xfs_rtalloc.c > +++ b/fs/xfs/xfs_rtalloc.c > @@ -488,6 +488,8 @@ xfs_rtallocate_extent_near( > * allocating one. > */ > if (maxlog >= 0) { > + xfs_extlen_t maxavail = > + min(maxlen, ((xfs_extlen_t)1 << (maxlog + 1)) - 1); There can be up to 2^52rtx (realtime extents) in the filesystem, right? xfs_extlen_t is a u32, which will overflow this calculation if the realtime volume is seriously huge. IOWs, doesn't this need to be: xfs_extlen_t maxavail = max_t(xfs_rtblock_t, maxlen, (1ULL << (maxlog + 1)) - 1); (The rest of the patch looks ok) --D > /* > * On the positive side of the starting location. > */ > @@ -497,7 +499,7 @@ xfs_rtallocate_extent_near( > * this block. > */ > error = xfs_rtallocate_extent_block(mp, tp, > - bbno + i, minlen, maxlen, len, &n, > + bbno + i, minlen, maxavail, len, &n, > rtbufc, prod, &r); > if (error) { > return error; > @@ -542,7 +544,7 @@ xfs_rtallocate_extent_near( > if (maxlog >= 0) > continue; > error = xfs_rtallocate_extent_block(mp, > - tp, bbno + j, minlen, maxlen, > + tp, bbno + j, minlen, maxavail, > len, &n, rtbufc, prod, &r); > if (error) { > return error; > @@ -564,7 +566,7 @@ xfs_rtallocate_extent_near( > * that we found. > */ > error = xfs_rtallocate_extent_block(mp, tp, > - bbno + i, minlen, maxlen, len, &n, > + bbno + i, minlen, maxavail, len, &n, > rtbufc, prod, &r); > if (error) { > return error; > -- > 2.41.0 >
On Wed, Jul 12, 2023 at 04:01:59PM -0700, Darrick J. Wong wrote: > On Tue, Jun 20, 2023 at 02:32:14PM -0700, Omar Sandoval wrote: > > From: Omar Sandoval <osandov@fb.com> > > > > xfs_rtallocate_extent_near() calls xfs_rtallocate_extent_block() with > > the minlen and maxlen that were passed to it. > > xfs_rtallocate_extent_block() then scans the bitmap block looking for a > > free range of size maxlen. If there is none, it has to scan the whole > > bitmap block before returning the largest range of at least size minlen. > > For a fragmented realtime device and a large allocation request, it's > > almost certain that this will have to search the whole bitmap block, > > leading to high CPU usage. > > > > However, the realtime summary tells us the maximum size available in the > > bitmap block. We can limit the search in xfs_rtallocate_extent_block() > > to that size and often stop before scanning the whole bitmap block. > > > > Signed-off-by: Omar Sandoval <osandov@fb.com> > > --- > > fs/xfs/xfs_rtalloc.c | 8 +++++--- > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c > > index ba7d42e0090f..d079dfb77c73 100644 > > --- a/fs/xfs/xfs_rtalloc.c > > +++ b/fs/xfs/xfs_rtalloc.c > > @@ -488,6 +488,8 @@ xfs_rtallocate_extent_near( > > * allocating one. > > */ > > if (maxlog >= 0) { > > + xfs_extlen_t maxavail = > > + min(maxlen, ((xfs_extlen_t)1 << (maxlog + 1)) - 1); > > There can be up to 2^52rtx (realtime extents) in the filesystem, right? > xfs_extlen_t is a u32, which will overflow this calculation if the > realtime volume is seriously huge. IOWs, doesn't this need to be: > > xfs_extlen_t maxavail = max_t(xfs_rtblock_t, maxlen, > (1ULL << (maxlog + 1)) - 1); > > (The rest of the patch looks ok) min_t instead of max_t, but good catch, fixed.
diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c index ba7d42e0090f..d079dfb77c73 100644 --- a/fs/xfs/xfs_rtalloc.c +++ b/fs/xfs/xfs_rtalloc.c @@ -488,6 +488,8 @@ xfs_rtallocate_extent_near( * allocating one. */ if (maxlog >= 0) { + xfs_extlen_t maxavail = + min(maxlen, ((xfs_extlen_t)1 << (maxlog + 1)) - 1); /* * On the positive side of the starting location. */ @@ -497,7 +499,7 @@ xfs_rtallocate_extent_near( * this block. */ error = xfs_rtallocate_extent_block(mp, tp, - bbno + i, minlen, maxlen, len, &n, + bbno + i, minlen, maxavail, len, &n, rtbufc, prod, &r); if (error) { return error; @@ -542,7 +544,7 @@ xfs_rtallocate_extent_near( if (maxlog >= 0) continue; error = xfs_rtallocate_extent_block(mp, - tp, bbno + j, minlen, maxlen, + tp, bbno + j, minlen, maxavail, len, &n, rtbufc, prod, &r); if (error) { return error; @@ -564,7 +566,7 @@ xfs_rtallocate_extent_near( * that we found. */ error = xfs_rtallocate_extent_block(mp, tp, - bbno + i, minlen, maxlen, len, &n, + bbno + i, minlen, maxavail, len, &n, rtbufc, prod, &r); if (error) { return error;