diff mbox series

[1/4] xfs: create a helper to handle logging parts of rt bitmap/summary blocks

Message ID 169773211358.225711.13859802342332594222.stgit@frogsfrogsfrogs (mailing list archive)
State Superseded
Headers show
Series xfs: refactor rtbitmap/summary accessors | expand

Commit Message

Darrick J. Wong Oct. 19, 2023, 4:27 p.m. UTC 
From: Darrick J. Wong <djwong@kernel.org>

Create an explicit helper function to log parts of rt bitmap and summary
blocks.  While we're at it, fix an off-by-one error in two of the
rtbitmap logging calls that led to unnecessarily large log items but was
otherwise benign.

Note that the upcoming rtgroups patchset will add block headers to the
rtbitmap and rtsummary files.  The helpers in this and the next few
patches take a less than direct route through xfs_rbmblock_wordptr and
xfs_rsumblock_infoptr to avoid helper churn in that patchset.

Suggested-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_rtbitmap.c |   55 +++++++++++++++++++++++++++++++-----------
 1 file changed, 40 insertions(+), 15 deletions(-)

Comments

Christoph Hellwig Oct. 20, 2023, 6:20 a.m. UTC | #1 
On Thu, Oct 19, 2023 at 09:27:48AM -0700, Darrick J. Wong wrote:
> +	size_t			first, last;
> +
> +	first = (void *)xfs_rsumblock_infoptr(bp, infoword) - bp->b_addr;
> +	last = first + sizeof(xfs_suminfo_t) - 1;

> +	size_t			first, last;
> +
> +	first = (void *)xfs_rbmblock_wordptr(bp, from) - bp->b_addr;
> +	last = ((void *)xfs_rbmblock_wordptr(bp, next) - 1) - bp->b_addr;
> +
> +	xfs_trans_log_buf(tp, bp, first, last);

Going to pointers and back looks a bit confusing and rather inefficient
to me.  But given how late we are in the cycle I don't want to derail
your series, so let's keep this as-is for now, and I'll add a TODO
list item to my ever growing list to eventually lean this up.
Darrick J. Wong Oct. 20, 2023, 3:20 p.m. UTC | #2 
On Fri, Oct 20, 2023 at 08:20:23AM +0200, Christoph Hellwig wrote:
> On Thu, Oct 19, 2023 at 09:27:48AM -0700, Darrick J. Wong wrote:
> > +	size_t			first, last;
> > +
> > +	first = (void *)xfs_rsumblock_infoptr(bp, infoword) - bp->b_addr;
> > +	last = first + sizeof(xfs_suminfo_t) - 1;
> 
> > +	size_t			first, last;
> > +
> > +	first = (void *)xfs_rbmblock_wordptr(bp, from) - bp->b_addr;
> > +	last = ((void *)xfs_rbmblock_wordptr(bp, next) - 1) - bp->b_addr;
> > +
> > +	xfs_trans_log_buf(tp, bp, first, last);
> 
> Going to pointers and back looks a bit confusing and rather inefficient
> to me.  But given how late we are in the cycle I don't want to derail
> your series, so let's keep this as-is for now, and I'll add a TODO
> list item to my ever growing list to eventually lean this up.

<nod> I think this function ultimately becomes:

/* Log rtbitmap block from the word @from to the byte before @next. */
static inline void
xfs_trans_log_rtbitmap(
	struct xfs_rtalloc_args	*args,
	unsigned int		from,
	unsigned int		next)
{
	struct xfs_buf		*bp = args->rbmbp;
	size_t			first = from * sizeof(xfs_rtword_t);
	size_t			last = next * sizeof(xfs_rtword_t) - 1;

	if (xfs_has_rtgroup(args->mp)) {
		first += sizeof(struct xfs_rtbuf_blkinfo);
		last += sizeof(struct xfs_rtbuf_blkinfo);
	}

	xfs_trans_log_buf(args->tp, bp, first, last);
}

I'll go play with the compiler to see what asm it generates.

--D
diff mbox series

Patch

diff --git a/fs/xfs/libxfs/xfs_rtbitmap.c b/fs/xfs/libxfs/xfs_rtbitmap.c
index aefa2b0747a5..4f096348d4b2 100644
--- a/fs/xfs/libxfs/xfs_rtbitmap.c
+++ b/fs/xfs/libxfs/xfs_rtbitmap.c
@@ -432,6 +432,21 @@  xfs_rtfind_forw(
 	return 0;
 }
 
+/* Log rtsummary counter at @infoword. */
+static inline void
+xfs_trans_log_rtsummary(
+	struct xfs_trans	*tp,
+	struct xfs_buf		*bp,
+	unsigned int		infoword)
+{
+	size_t			first, last;
+
+	first = (void *)xfs_rsumblock_infoptr(bp, infoword) - bp->b_addr;
+	last = first + sizeof(xfs_suminfo_t) - 1;
+
+	xfs_trans_log_buf(tp, bp, first, last);
+}
+
 /*
  * Read and/or modify the summary information for a given extent size,
  * bitmap block combination.
@@ -497,8 +512,6 @@  xfs_rtmodify_summary_int(
 	infoword = xfs_rtsumoffs_to_infoword(mp, so);
 	sp = xfs_rsumblock_infoptr(bp, infoword);
 	if (delta) {
-		uint first = (uint)((char *)sp - (char *)bp->b_addr);
-
 		*sp += delta;
 		if (mp->m_rsum_cache) {
 			if (*sp == 0 && log == mp->m_rsum_cache[bbno])
@@ -506,7 +519,7 @@  xfs_rtmodify_summary_int(
 			if (*sp != 0 && log < mp->m_rsum_cache[bbno])
 				mp->m_rsum_cache[bbno] = log;
 		}
-		xfs_trans_log_buf(tp, bp, first, first + sizeof(*sp) - 1);
+		xfs_trans_log_rtsummary(tp, bp, infoword);
 	}
 	if (sum)
 		*sum = *sp;
@@ -527,6 +540,22 @@  xfs_rtmodify_summary(
 					delta, rbpp, rsb, NULL);
 }
 
+/* Log rtbitmap block from the word @from to the byte before @next. */
+static inline void
+xfs_trans_log_rtbitmap(
+	struct xfs_trans	*tp,
+	struct xfs_buf		*bp,
+	unsigned int		from,
+	unsigned int		next)
+{
+	size_t			first, last;
+
+	first = (void *)xfs_rbmblock_wordptr(bp, from) - bp->b_addr;
+	last = ((void *)xfs_rbmblock_wordptr(bp, next) - 1) - bp->b_addr;
+
+	xfs_trans_log_buf(tp, bp, first, last);
+}
+
 /*
  * Set the given range of bitmap bits to the given value.
  * Do whatever I/O and logging is required.
@@ -548,6 +577,7 @@  xfs_rtmodify_range(
 	int		i;		/* current bit number rel. to start */
 	int		lastbit;	/* last useful bit in word */
 	xfs_rtword_t	mask;		/* mask o frelevant bits for value */
+	unsigned int	firstword;	/* first word used in the buffer */
 	unsigned int	word;		/* word number in the buffer */
 
 	/*
@@ -565,7 +595,7 @@  xfs_rtmodify_range(
 	/*
 	 * Compute the starting word's address, and starting bit.
 	 */
-	word = xfs_rtx_to_rbmword(mp, start);
+	firstword = word = xfs_rtx_to_rbmword(mp, start);
 	first = b = xfs_rbmblock_wordptr(bp, word);
 	bit = (int)(start & (XFS_NBWORD - 1));
 	/*
@@ -599,15 +629,13 @@  xfs_rtmodify_range(
 			 * Log the changed part of this block.
 			 * Get the next one.
 			 */
-			xfs_trans_log_buf(tp, bp,
-				(uint)((char *)first - (char *)bp->b_addr),
-				(uint)((char *)b - (char *)bp->b_addr));
+			xfs_trans_log_rtbitmap(tp, bp, firstword, word);
 			error = xfs_rtbuf_get(mp, tp, ++block, 0, &bp);
 			if (error) {
 				return error;
 			}
 
-			word = 0;
+			firstword = word = 0;
 			first = b = xfs_rbmblock_wordptr(bp, word);
 		} else {
 			/*
@@ -640,15 +668,13 @@  xfs_rtmodify_range(
 			 * Log the changed part of this block.
 			 * Get the next one.
 			 */
-			xfs_trans_log_buf(tp, bp,
-				(uint)((char *)first - (char *)bp->b_addr),
-				(uint)((char *)b - (char *)bp->b_addr));
+			xfs_trans_log_rtbitmap(tp, bp, firstword, word);
 			error = xfs_rtbuf_get(mp, tp, ++block, 0, &bp);
 			if (error) {
 				return error;
 			}
 
-			word = 0;
+			firstword = word = 0;
 			first = b = xfs_rbmblock_wordptr(bp, word);
 		} else {
 			/*
@@ -673,15 +699,14 @@  xfs_rtmodify_range(
 			*b |= mask;
 		else
 			*b &= ~mask;
+		word++;
 		b++;
 	}
 	/*
 	 * Log any remaining changed bytes.
 	 */
 	if (b > first)
-		xfs_trans_log_buf(tp, bp,
-			(uint)((char *)first - (char *)bp->b_addr),
-			(uint)((char *)b - (char *)bp->b_addr - 1));
+		xfs_trans_log_rtbitmap(tp, bp, firstword, word);
 	return 0;
 }