On Thu, Nov 02, 2017 at 12:54:19PM -0400, Brian Foster wrote: > > <shrug> Same feeling here. TBH I wonder about how costly those > > unaligned be64 accesses are on things like sparc such that we should > > minimize the number of calls and just check the (probably aligned) > > incore version instead... > > > > I'm not totally sure.. I guess it's potentially a function call where an > immediate memory copy would suffice..? Anyways, that's pretty much what > caused the function to stand out to me as starting to look a little too > busy/ugly for its own good. We now have a validation helper that has to > handle unaligned accesses because its caller may or may not refer to > unaligned memory. It's not even totally clear to me when we can expect > the memory to be unaligned.. when we're referring to an on-disk inode > fork? We are iterating over each possibly unaligned on-disk extent, so we already are taking the hit. In all modern CPUs unaligned access don't cause a function call. They either require a slightly different load instruction or multiple small (e.g. byte) loads. I'd really like to see anyone who can demonstrate a difference vs reading the inode or bmap btree from disk. > So rather than start to propagate that logic, much more clean to me is > to do the unaligned accesses only where necessary and fix up the error > checks to examine the read values. I don't see any logical difference > between checking the bit vs. the extent state since the unwritten bit > basically maps directly to XFS_EXT_UNWRITTEN/XFS_EXT_NORM in the incore > record. With the new incore extent list I wanted to keep the incore extent format private to the xfs_iext_tree.c file, and so far succeeded. With the final version of this tree I could switch to checking the xfs_bmbt_irec structure, which makes a whole lot sense, but I can't think of an easy way to stage this. Let me thing if I can come up with something, else I'll leave this patch as-is and will change it again in the end. -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
