Re: [PATCH V2] Stop searching for free slots in an inode chunk when there are none

[Brian Foster <bfoster@redhat.com>]

On Thu, Aug 10, 2017 at 09:56:18PM +1000, Dave Chinner wrote:
> On Mon, Aug 07, 2017 at 11:19:56AM -0400, Brian Foster wrote:
> > On Mon, Aug 07, 2017 at 12:17:10PM +0200, Carlos Maiolino wrote:
> > Also note that this has potential for performance side effects in the
> > common (non-corruption) case.
> 
> I'm not seeing an issue here once the code is corrected. Probably
> just me being dumb again, but could you point it out for me, Brian?
> 

Oh, I'm just pointing out that this version tweaks the normal runtime
algorithm (by further limiting the record search window in certain
cases) whereas the previous version did not and I didn't see any mention
as to why that is safe. The first sentence below explains why I think
the change has minimal performance impact, if any, and is probably fine.
I'm basically just asking that if we fix this by tweaking the
optimization algorithm, we add a brief justification for why this does
not impact normal runtime performance in the commit log (if my
understanding is correct, feel free to steal the text below).

> > It looks to me that it shouldn't be a major problem because it only
> > affects the situation where the cached search "wraps" to the outside of
> > the tree, and that probably doesn't happen often with a search distance
> > of 10 records and a large tree. I am a bit curious where the
> > searchdistance of 10 comes from though (we fit many more records in a
> > single inobt leaf block)..?
> 
> It was chosen based on CPU profiles and performance measurement to
> limit the CPU usage of the problem case the finobt now solves. i.e.
> finding the frees inode in a tree that indexes several million
> allocated inodes and the free inodes are few and far between. It was
> chosen to cap inode allocation performance degradation when free
> inodes were extremely sparse at around 50% of the "lots of free
> inodes that are easy to find" performance.
> 

Ah, Ok. So it was more of a CPU oriented optimization than an I/O one.
That makes sense, thanks.

Brian

> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html