On Fri, Feb 06, 2015 at 02:52:55PM -0500, Brian Foster wrote: > The inobt record holemask field is a condensed data type designed to fit > into the existing on-disk record and is zero based (allocated regions > are set to 0, sparse regions are set to 1) to provide backwards > compatibility. This makes the type somewhat complex for use in higher > level inode manipulations such as individual inode allocation, etc. > > Rather than foist the complexity of dealing with this field to every bit > of logic that requires inode granular information, create the > xfs_inobt_ialloc_bitmap() helper to convert the holemask to an inode > allocation bitmap. The inode allocation bitmap is inode granularity > similar to the inobt record free mask and indicates which inodes of the > chunk are physically allocated on disk, irrespective of whether the > inode is considered allocated or free by the filesystem. The generic > bitmap type requires the use of generic kernel bitmap APIs. > > The bitmap_to_u64() helper is provided to convert the bitmap back to a > native 64-bit type (for native bitwise operations). This is required > because the generic bitmap code represents a bitmap as an array of > unsigned long in a little endian style (though each array value is cpu > order). To ensure compatibility with various word sizes and endianness', > bitmap_to_u64() exports the bitmap to little endian and swaps back to > cpu byte order. > > Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx> > --- > fs/xfs/libxfs/xfs_ialloc.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 69 insertions(+) > > diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c > index 72ade0e..fc001d9 100644 > --- a/fs/xfs/libxfs/xfs_ialloc.c > +++ b/fs/xfs/libxfs/xfs_ialloc.c > @@ -39,6 +39,8 @@ > #include "xfs_icache.h" > #include "xfs_trace.h" > > +STATIC void > +xfs_inobt_ialloc_bitmap(unsigned long *, struct xfs_inobt_rec_incore *); xfs_inobt_irec_to_allocmap() seems more appropriate for what it does. I'd also suggest it should be in xfs_ialloc_btree.c, too. > /* > * Allocation group level functions. > @@ -739,6 +741,73 @@ xfs_ialloc_get_rec( > return 0; > } > > +static inline uint64_t > +bitmap_to_u64( > + const unsigned long *bitmap, > + int nbits) > +{ > + __le64 lebitmap; > + > + ASSERT(nbits <= 64); > + > + /* > + * The bitmap format depends on the native word size. E.g., bit 0 could > + * land in the middle of the 64 bits on a big endian 32-bit arch (see > + * arch/powerpc/include/asm/bitops.h). To handle this, export the bitmap > + * as 64-bit little endian and convert back to native byte order. > + */ > + bitmap_copy_le(&lebitmap, bitmap, nbits); > + return le64_to_cpu(lebitmap); > +} Ok, so this is for doing logic operations on the bitmap, such as inverting or masking out a bunch of bits? The first use of this function is xfs_inobt_first_free_inode(), and the other use is in xfs_difree_inobt() to build the mask of inodes in the chunk that need to be freed, and in both those cases they do xfs_inobt_ialloc_bitmap(alloc, rec) allocmask = bitmap_to_u64(alloc, 64); and the local stack bitmap is otherwise unused. So, wouldn't a helper like: /* * Return a host format mask of all the allocated inodes in the * chunk. The bitmap format depends on the native word size (e.g. * see arch/powerpc/include/asm/bitops.h) and so we have to marshall * the bitmap through a defined endian conversion so that we can * perform host native 64 bit logic operations on the resultant * allocation mask. * * A bit value of 1 means the inode is allocated, a value of 0 means it * is free. */ u64 xfs_inobt_irec_to_allocmask( struct xfs_inobt_rec_incore *irec) { DECLARE_BITMAP(allocmap, 64), __le64 lebitmap; xfs_inobt_rec_to_allocmap(&allocmap, irec); bitmap_copy_le(&lebitmap, allocmap, 64); return le64_to_cpu(lebitmap); } > + > +/* > + * Convert the inode record holemask to an inode allocation bitmap. The inode > + * allocation bitmap is inode granularity and specifies whether an inode is > + * physically allocated on disk (not whether the inode is considered allocated > + * or free by the fs). > + * > + * We have to be careful with regard to byte order and word size since the > + * generic bitmap code uses primitive types. " a bit value of 1 means the inode is allocated, a value of 0 means it is free" > + */ > +STATIC void > +xfs_inobt_ialloc_bitmap( > + unsigned long *allocbmap, > + struct xfs_inobt_rec_incore *rec) > +{ > + int nextbit; > + DECLARE_BITMAP(holemask, 16); > + > + bitmap_zero(allocbmap, 64); > + > + /* > + * bitmaps are represented as an array of unsigned long (each in cpu > + * byte order). ir_holemask is only 16 bits, so direct assignment is > + * safe. > + */ > + ASSERT(sizeof(rec->ir_holemask) <= sizeof(holemask[0])); BUILD_BUG_ON(sizeof(rec->ir_holemask) <= sizeof(holemask[0])); i.e. if we come across a platform where this fails, break the build rather than waiting for the unlikely event of someone running a debug kernel on that platform... Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs