On Mar 24, 2008 17:13 -0500, Eric Sandeen wrote: > Extent data is shared with the i_block[] space in the inode, > but it is always swapped on access, not when the inode is read. > > In e2fsck/pass1.c we must be careful when checking validity > of the extents flag on the inode. If the flag was set when > the inode was read & swapped, then the extents data itself > (in ->i_block[]) was NOT swapped, so testing for a valid > extent header requires some swapping first. Then, if we > ultimately set the extents flag, all of i_block[] must be > re/un-swapped. This seems pretty awkward for any other users of the library. Having the i_block[] array NOT be swabbed if it is an extent file means that every place in the code which is accessing this array also needs to do the swabbing itself. This would break the abstraction that the in-memory inode is in host-endian order, and also forces every application to understand the difference between extent- and non-extent-mapped inodes, and the on-disk byte order. Ugh. IMHO, it would be better to swab the i_block[] array in ext2fs_swap_inode_full() if the EXTENTS_FL is set, and in the rare case of e2fsck needing to clear that flag then it should un-swap (if needed), clear the flag, and re-swap (if needed). This will very rarely happen, I think. Note that the Lustre extents patches did NOT do the swap+clear_swap operation when clearing the extents flags because we don't have any big-endian server systems (our PPC testing is limited to userspace "make check"), though I think that is the right thing to do. Cheers, Andreas -- Andreas Dilger Sr. Staff Engineer, Lustre Group Sun Microsystems of Canada, Inc. -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
