Re: ext4 build errors

[Joakim Tjernlund <Joakim.Tjernlund@infinera.com>]

On Mon, 2017-10-02 at 10:55 -0400, Theodore Ts'o wrote:
> On Mon, Oct 02, 2017 at 02:23:02PM +0000, Joakim Tjernlund wrote:
> > Hi ext4 devs
> > 
> > Adding the patch last in this mail cause lots of build errors in ext4, here is a few:
> 
> Why did you need this patch to fix problems in VirtualBox?

Earlier VirtualBox had some c++ code that included kernel header and g++ is
much pickier about these things so I had to do some cleanups to build
some of the VirtualBox modules. I think it is better now but I haven't really checked.

> 
> Cleaning this up is going to be a little tricky, because one of the
> implications the void * declaration in the __set_bit_le() declaration
> is that there isn't any particular alignment requirement with the __le
> functions.  But the long * declaration implies that the bitmaps have
> to be aligned to sizeof(long).
> 
> For the ext4 bitmap, we use it on bh->b_data, for which we can safely
> assume is long-aligned.  But the mballoc buddy bitmaps use
> mb_set_bit() in ways that are _not_ guaranteed to be long aligned.
> 
> So fixing this is going to be a bit painful, and will likely result in
> a performance regression for ext4.  We can make our own version that
> open codes it as C functions --- but then we lose all of the
> architecture optimized bitop functions.
> 
> I believe the reason why the standard bitop functions are made long *
> aligned is that on some BE architectures --- I suspect it was PowerPC
> but I'm not 100% sure about that --- the native bitop functions
> required a long * alignment.  Fortunately all of the little endian
> architectures didn't have these alignment restrictions, so we could
> keep the __set_bit_le functions to not have any long alignment
> restrictions.

If this is a special case for ext4, can you not just do an explicit
type cast in ext4 code? 

> 
> The fact that bitop and the bitop_le functions are not the same
> is... inelegant, but if it represents a practical optimization that is
> possible on LE systems but not on BE systems (where bitop_le gets open
> coded in C, in an inefficient way, but oh, well, BE systems aren't for
> the cool kids anyway :-), I have to ask whether it's really worth it
> to do the cleanup.

I see, but by using void * you also loose type checking w.r.t size so
if you by mistake use an u32, you will not notice.

 Jocke