Re: [PATCH 3/5] xfs: remove a superflous hash lookup when inserting new buffers

[Christoph Hellwig <hch@lst.de>]

On Wed, Apr 06, 2022 at 08:01:21AM +1000, Dave Chinner wrote:
> Agreed, but you're making two distinct, significant modifications in
> the one patchset. One is changing the way we use a generic library
> functionality, the other is changing the entire structure of the
> lookup path.
> 
> IOWs, I was not saying the end result was bad, I was (clumsily)
> trying to suggest that you should split these two modifications into
> separate patches because they are largely separate changes.
> 
> Once I thought about it that way, and
> looking that them that way made me want to structure the code quite
> differently.

Ok, I'll see if I can split things up a bit better.

> 
> e.g. Most of the complexity goes away if we factor out the buffer
> trylock/locking code into a helper (like we have in the iomap code)
> and then have xfs_buf_insert() call it when it finds an existing
> buffer. Then the -EEXIST return value can go away, and
> xfs_buf_insert can return a locked buffer exactly the same as if it
> inserted a new buffer. Have the newly allocated buffer take a new
> perag reference, too, instead of stealing the caller's reference,
> and then all the differences between insert and -EEXIST cases go
> away.

I actually had that earlier as well and really like the flow of
the single function.  So it certainly is doable.