On Thu, Oct 12, 2023 at 08:19:16AM +0200, Christoph Hellwig wrote:
> On Wed, Oct 11, 2023 at 11:07:48AM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@xxxxxxxxxx>
> >
> > Create get and set functions for rtbitmap words so that we can redefine
> > the ondisk format with a specific endianness. Note that this requires
> > the definition of a distinct type for ondisk rtbitmap words so that the
> > compiler can perform proper typechecking as we go back and forth.
> >
> > In the upcoming rtgroups feature, we're going to fix the problem that
> > rtwords are written in host endian order, which means we'll need the
> > distinct rtword/rtword_raw types.
>
> I've been looking over this and I have to say I kinda hate the
> abstraction level.
>
> Having to deal with both the union xfs_rtword_ondisk, and the
> normal in-memory rtword just feels cumbersome.
>
> I'd go for an API that gets/sets the values based on [bp, word] indices
> instead. That would also need helpers for logging the buffer ranges
> based on indices, which seems helpful for the code quality anyway.
>
> I don't really want to burden that on you and would offer to do that
> work myself after we work before this merged.
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);
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.
--D
