Re: [PATCH 6/8] xfs: use accessor functions for bitmap words

["Darrick J. Wong" <djwong@kernel.org>]

On Fri, Oct 13, 2023 at 06:28:33AM +0200, Christoph Hellwig wrote:
> On Thu, Oct 12, 2023 at 03:11:06PM -0700, Darrick J. Wong wrote:
> > Hmm, so you want to go from:
> > 
> > 	union xfs_rtword_ondisk *start, *end, *b;
> > 
> > 	start = b = xfs_rbmblock_wordptr(bp, startword);
> > 	end = xfs_rbmblock_wordptr(bp, endword);
> > 
> > 	while (b < end) {
> > 		somevalue = xfs_rtbitmap_getword(mp, b);
> > 		somevalue |= somemask;
> > 		xfs_rtbitmap_setword(mp, b, somevalue);
> > 		b++;
> > 	}
> > 
> > 	xfs_trans_log_buf(tp, bp, start - bp->b_addr, b - bp->b_addr);
> > 
> > to something like:
> > 
> > 	for (word = startword; word <= endword; word++) {
> > 		somevalue = xfs_rtbitmap_getword(mp, b);
> > 		somevalue |= somemask;
> > 		xfs_rtbitmap_setword(mp, bp, word, somevalue);
> > 	}
> > 	xfs_rtbitmap_log_buf(tp, bp, startword, endword);
> 
> Yes. (although xfs_rtbitmap_log_buf can't just take the words directly
> of course, and the xfs_rtbitmap_getword needs word and not the now
> not existing b).
> 
> > I think that could be done with relatively little churn, though it's
> > unfortunate that the second version does 2x(shift + addition) each time
> > it goes through the loop instead of the pointer increment that the first
> > version does.
> 
> I don't really think it matter compared to all the other overhead,
> and it keeps a much nicer API.

<nod> I suppose one could go the horrid iter function route to get
around the multiply, though at this point there's an awful lot of code
to do something very simple:

struct xfs_rbmword_cur {
	struct xfs_buf	*bp;
	xfs_rtword_t	*wordptr;
	unsigned int	endword;
	unsigned int	word;
};

static inline bool
xfs_rtbitmap_word_iter(
	struct xfs_rbmword_cur	*cur,
	xfs_rtword_t		*val)
{
	if (cur->word >= cur->endword)
		return false;

	if (!cur->wordptr)
		cur->wordptr = xfs_rbmblock_wordptr(cur->bp, cur->word);

	*val = *cur->wordptr;
	cur->word++;
	cur->wordptr++;
	return true;
}

static inline void
xfs_rtbitmap_word_set(
	struct xfs_rbmword_cur	*cur,
	xfs_rtword_t		val)
{
	*(cur->wordptr - 1) = val;
}

Usage:

	struct xfs_rbword_cur	cur = {
		.bp		= bp,
		.word		= startword,
		.endword	= endword,
	};
	xfs_rtword_t		val;

	while (xfs_rtbitmap_word_iter(&cur, &val)) {
		val |= somemask;
		xfs_rtbitmap_word_set(cur, val);
	}
	xfs_rtbitmap_log_buf(tp, bp, startword, endword);

--D