On Mon, Feb 09, 2015 at 10:54:48AM +1100, Dave Chinner wrote:
> 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.
>
Ok.
> > /*
> > * 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?
>
Yes, generally to convert to inode granularity for whatever operations
require it (e.g., find a "real" free inode from ir_free).
> 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);
> }
>
Yeah, I went back and forth with doing the conversion within the
original helper. We ultimately end up with a couple more callers that do
use the generic bitmap, but this could be helpful to those that don't.
> > +
> > +/*
> > + * 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"
>
Ok.
> > + */
> > +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...
>
I assume the logic has to be inverted, but otherwise that's probably a
better idea. Thanks.
Brian
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
