diff mbox series

[04/14] libxfs: refactor libxfs_readbuf out of existence

Message ID 158216309405.603628.3732022870551516081.stgit@magnolia (mailing list archive)
State Superseded
Headers show
Series xfsprogs: make buffer functions return error codes | expand

Commit Message

Darrick J. Wong Feb. 20, 2020, 1:44 a.m. UTC 
From: Darrick J. Wong <darrick.wong@oracle.com>

The two libxfs_readbuf* functions are awfully similar, so refactor one
into the other to reduce duplicated code.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 libxfs/rdwr.c |   83 +++++++++++++++++++++------------------------------------
 1 file changed, 30 insertions(+), 53 deletions(-)

Comments

Christoph Hellwig Feb. 21, 2020, 3 p.m. UTC | #1 
On Wed, Feb 19, 2020 at 05:44:54PM -0800, Darrick J. Wong wrote:
> +	/*
> +	 * if the buffer was prefetched, it is likely that it was not validated.

Please capitalize the first character in multi-line comments.

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>
Darrick J. Wong Feb. 21, 2020, 10:13 p.m. UTC | #2 
On Fri, Feb 21, 2020 at 07:00:01AM -0800, Christoph Hellwig wrote:
> On Wed, Feb 19, 2020 at 05:44:54PM -0800, Darrick J. Wong wrote:
> > +	/*
> > +	 * if the buffer was prefetched, it is likely that it was not validated.
> 
> Please capitalize the first character in multi-line comments.

Will fix.

--D

> Otherwise looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
diff mbox series

Patch

diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
index 4e1665c0..6eaa3e69 100644
--- a/libxfs/rdwr.c
+++ b/libxfs/rdwr.c
@@ -972,53 +972,6 @@  libxfs_readbuf_verify(
 	return bp->b_error;
 }
 
-static struct xfs_buf *
-libxfs_readbuf(
-	struct xfs_buftarg	*btp,
-	xfs_daddr_t		blkno,
-	size_t			len,
-	int			flags,
-	const struct xfs_buf_ops *ops)
-{
-	struct xfs_buf		*bp;
-	int			error;
-
-	error = libxfs_getbuf_flags(btp, blkno, len, 0, &bp);
-	if (error)
-		return NULL;
-
-	/*
-	 * if the buffer was prefetched, it is likely that it was not validated.
-	 * Hence if we are supplied an ops function and the buffer is marked as
-	 * unchecked, we need to validate it now.
-	 *
-	 * We do this verification even if the buffer is dirty - the
-	 * verification is almost certainly going to fail the CRC check in this
-	 * case as a dirty buffer has not had the CRC recalculated. However, we
-	 * should not be dirtying unchecked buffers and therefore failing it
-	 * here because it's dirty and unchecked indicates we've screwed up
-	 * somewhere else.
-	 */
-	bp->b_error = 0;
-	if ((bp->b_flags & (LIBXFS_B_UPTODATE|LIBXFS_B_DIRTY))) {
-		if (bp->b_flags & LIBXFS_B_UNCHECKED)
-			libxfs_readbuf_verify(bp, ops);
-		return bp;
-	}
-
-	/*
-	 * Set the ops on a cache miss (i.e. first physical read) as the
-	 * verifier may change the ops to match the type of buffer it contains.
-	 * A cache hit might reset the verifier to the original type if we set
-	 * it again, but it won't get called again and set to match the buffer
-	 * contents. *cough* xfs_da_node_buf_ops *cough*.
-	 */
-	error = libxfs_readbufr(btp, blkno, bp, len, flags);
-	if (!error)
-		libxfs_readbuf_verify(bp, ops);
-	return bp;
-}
-
 int
 libxfs_readbufr_map(struct xfs_buftarg *btp, struct xfs_buf *bp, int flags)
 {
@@ -1059,20 +1012,44 @@  libxfs_buf_read_map(struct xfs_buftarg *btp, struct xfs_buf_map *map, int nmaps,
 	int		error = 0;
 
 	if (nmaps == 1)
-		return libxfs_readbuf(btp, map[0].bm_bn, map[0].bm_len,
-					flags, ops);
-
-	error = __libxfs_buf_get_map(btp, map, nmaps, 0, &bp);
+		error = libxfs_getbuf_flags(btp, map[0].bm_bn, map[0].bm_len,
+				0, &bp);
+	else
+		error = __libxfs_buf_get_map(btp, map, nmaps, 0, &bp);
 	if (error)
 		return NULL;
 
+	/*
+	 * if the buffer was prefetched, it is likely that it was not validated.
+	 * Hence if we are supplied an ops function and the buffer is marked as
+	 * unchecked, we need to validate it now.
+	 *
+	 * We do this verification even if the buffer is dirty - the
+	 * verification is almost certainly going to fail the CRC check in this
+	 * case as a dirty buffer has not had the CRC recalculated. However, we
+	 * should not be dirtying unchecked buffers and therefore failing it
+	 * here because it's dirty and unchecked indicates we've screwed up
+	 * somewhere else.
+	 */
 	bp->b_error = 0;
-	if ((bp->b_flags & (LIBXFS_B_UPTODATE|LIBXFS_B_DIRTY))) {
+	if (bp->b_flags & (LIBXFS_B_UPTODATE | LIBXFS_B_DIRTY)) {
 		if (bp->b_flags & LIBXFS_B_UNCHECKED)
 			libxfs_readbuf_verify(bp, ops);
 		return bp;
 	}
-	error = libxfs_readbufr_map(btp, bp, flags);
+
+	/*
+	 * Set the ops on a cache miss (i.e. first physical read) as the
+	 * verifier may change the ops to match the type of buffer it contains.
+	 * A cache hit might reset the verifier to the original type if we set
+	 * it again, but it won't get called again and set to match the buffer
+	 * contents. *cough* xfs_da_node_buf_ops *cough*.
+	 */
+	if (nmaps == 1)
+		error = libxfs_readbufr(btp, map[0].bm_bn, bp, map[0].bm_len,
+				flags);
+	else
+		error = libxfs_readbufr_map(btp, bp, flags);
 	if (!error)
 		libxfs_readbuf_verify(bp, ops);