Re: [RFC please check] KVM: question about the commit "Use Little Endian for Dirty Bitmap"

[Takuya Yoshikawa <yoshikawa.takuya-gVGce1chcLdL9jVzuh4AOg@public.gmane.org>]

>>> So please explain me about the commit:
>>>    1. is this really the thing you intended to do?
>>>
>>
>> I think so.
>>
>>>    2. including<asm-generic/bitops/le.h>   directly is OK?
>>>       -- I made a sample patch to avoid this, see below.
>>>
>>
>> I don't see a problem with it, it is also included from other places.
>>
>> It might be possible to change it to<asm/bitops/le.h>, not sure how the include paths are set out.
>
> That's the great thing about being in-kernel :-).
>
>>
>>>    3. or, I misunderstand something about Alex's comment?
>>>
>>
>> I missed the comment.  What was it?
>
> What comment? The reason was that longs and big endian don't match and I wanted to make it LE. This seemed like the right function to use, as other code in Linux uses it too.
>

This one!

===
 > +static int __mark_page_dirty(unsigned long nr,
 > > +			     unsigned long *dirty_bitmap)
 > > +{
 > > +#ifdef __BIG_ENDIAN
 > > +	nr = nr ^ BITOP_LE_SWIZZLE;
Why an XOR here?

Also is this LE set_bit new? I didn't see it when I did the patch back then :).
===

I confused that "LE set_bit" is about the current generic___set_le_bit().

So I felt that "then, who introduced this set le bit actually?"

Now, it's OK about it, thanks :).


But I still feel that __set_le_bit_user() will be too KVM specific helper.

Then, I have one idea:

  while looking around the bitops headers, I noticed that ppc bitops.h has exact
  copy of the *_le_bit definitions which are in the asm-generic le.h -- just for its
  internal use.

  So I'll check whether we can include le.h in ppc bitops.h for cleanup and during
  that work, define the "(nr) ^ BITOP_LE_SWIZZLE" part as separate macro like
  le_bit_offset.

  I don't know this will be accepted but this sounds little bit more generic: and
  we can reduce chances that we have to ask other maintainers to merge helpers.



> Alex
>