Re: [linus:master] [xfs] 2edf06a50f: fsmark.files_per_sec -5.7% regression

[Dave Chinner]

On Tue, May 09, 2023 at 04:54:33PM +1000, Dave Chinner wrote:
> On Tue, May 09, 2023 at 10:13:19AM +0800, kernel test robot wrote:
> > 
> > 
> > Hello,
> > 
> > kernel test robot noticed a -5.7% regression of fsmark.files_per_sec on:
> > 
> > 
> > commit: 2edf06a50f5bbe664283f3c55c480fc013221d70 ("xfs: factor xfs_alloc_vextent_this_ag() for  _iterate_ags()")
> > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git master
> 
> This is just a refactoring patch and doesn't change any logic.
> Hence I'm sceptical that it actually resulted in a performance
> regression. Indeed, the profile indicates a significant change of
> behaviour in the allocator and I can't see how the commit above
> would cause anything like that.
> 
> Was this a result of a bisect? If so, what were the original kernel
> versions where the regression was detected?

Oh, CONFIG_XFS_DEBUG=y, which means:

static int
xfs_alloc_ag_vextent_lastblock(
        struct xfs_alloc_arg    *args,
        struct xfs_alloc_cur    *acur,
        xfs_agblock_t           *bno,
        xfs_extlen_t            *len,
        bool                    *allocated)
{
        int                     error;
        int                     i;

#ifdef DEBUG
        /* Randomly don't execute the first algorithm. */
        if (get_random_u32_below(2))
                return 0;
#endif

We randomly chose a near block allocation strategy to use to improve
code coverage, not the optimal one for IO performance. Hence the CPU
usage and allocation patterns that impact IO performance are simply
not predictable or reproducable from run to run. So, yeah, trying to
bisect a minor difference in performance as a result of this
randomness will not be reliable....

Cheers,

Dave.

-- 
Dave Chinner
david@xxxxxxxxxxxxx