Re: [PATCH 3/6] xfs: merge xfs_buf_find() and xfs_buf_get_map()

From: Dave Chinner <david@fromorbit.com>
To: "Darrick J. Wong" <djwong@kernel.org>
Cc: Christoph Hellwig <hch@infradead.org>, linux-xfs@vger.kernel.org
Subject: Re: [PATCH 3/6] xfs: merge xfs_buf_find() and xfs_buf_get_map()
Date: Thu, 7 Jul 2022 22:39:09 +1000	[thread overview]
Message-ID: <20220707123909.GN227878@dread.disaster.area> (raw)
In-Reply-To: <YrzM57Xg2LU4pEha@magnolia>

On Wed, Jun 29, 2022 at 03:06:31PM -0700, Darrick J. Wong wrote:
> On Wed, Jun 29, 2022 at 12:40:08AM -0700, Christoph Hellwig wrote:
> > >  
> > > -static inline struct xfs_buf *
> > > -xfs_buf_find_fast(
> > > -	struct xfs_perag	*pag,
> > > -	struct xfs_buf_map	*map)
> > > -{
> > > -	struct xfs_buf          *bp;
> > > -
> > > -	bp = rhashtable_lookup(&pag->pag_buf_hash, map, xfs_buf_hash_params);
> > > -	if (!bp)
> > > -		return NULL;
> > > -	atomic_inc(&bp->b_hold);
> > > -	return bp;
> > > -}
> > 
> > > -static int
> > > -xfs_buf_find_insert(
> > > -	struct xfs_buftarg	*btp,
> > > -	struct xfs_perag	*pag,
> > 
> > Adding the function just in the last patch and moving them around
> > here and slighty changing seems a little counter productive.
> > I think just merging the two might actually end up with a result
> > that is easier to review.
> 
> I read the second patch and it makes sense, but I'm also curious if
> hch's suggestion here would make this change easier to read?

I moved the initial placement of these functions around and it took
a big chunk out of the diff in this patch. That should make it
easier to read without combining the two patches together...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com