[RFC][PATCH resend 8/12] asm-generic: bitops: introduce le bit

[RFC][PATCH resend 8/12] asm-generic: bitops: introduce le bit

[Takuya Yoshikawa]

Although we can use *_le_bit() helpers to treat bitmaps le arranged,
having le bit offset calculation as a seperate macro gives us more freedom.

For example, KVM has le arranged dirty bitmaps for VGA, live-migration
and they are used in user space too. To avoid bitmap copies between kernel
and user space, we want to update the bitmaps in user space directly.
To achive this, le bit offset with *_user() functions help us a lot.

So let us use the le bit offset calculation part by defining it as a new
macro: generic_le_bit_offset() .

Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
CC: Arnd Bergmann <arnd@arndb.de>
---
 include/asm-generic/bitops/le.h |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/include/asm-generic/bitops/le.h b/include/asm-generic/bitops/le.h
index 80e3bf1..ee445fb 100644
--- a/include/asm-generic/bitops/le.h
+++ b/include/asm-generic/bitops/le.h
@@ -9,6 +9,8 @@
 
 #if defined(__LITTLE_ENDIAN)
 
+#define generic_le_bit_offset(nr)	(nr)
+
 #define generic_test_le_bit(nr, addr) test_bit(nr, addr)
 #define generic___set_le_bit(nr, addr) __set_bit(nr, addr)
 #define generic___clear_le_bit(nr, addr) __clear_bit(nr, addr)
@@ -25,6 +27,8 @@
 
 #elif defined(__BIG_ENDIAN)
 
+#define generic_le_bit_offset(nr)	((nr) ^ BITOP_LE_SWIZZLE)
+
 #define generic_test_le_bit(nr, addr) \
 	test_bit((nr) ^ BITOP_LE_SWIZZLE, (addr))
 #define generic___set_le_bit(nr, addr) \
-- 
1.7.0.4

Re: [RFC][PATCH resend 8/12] asm-generic: bitops: introduce le bit offset macro

[Arnd Bergmann]

On Tuesday 04 May 2010, Takuya Yoshikawa wrote:
> 
> Although we can use *_le_bit() helpers to treat bitmaps le arranged,
> having le bit offset calculation as a seperate macro gives us more freedom.
> 
> For example, KVM has le arranged dirty bitmaps for VGA, live-migration
> and they are used in user space too. To avoid bitmap copies between kernel
> and user space, we want to update the bitmaps in user space directly.
> To achive this, le bit offset with *_user() functions help us a lot.
> 
> So let us use the le bit offset calculation part by defining it as a new
> macro: generic_le_bit_offset() .

Does this work correctly if your user space is 32 bits (i.e. unsigned long
is different size in user space and kernel) in both big- and little-endian
systems?

I'm not sure about all the details, but I think you cannot in general share
bitmaps between user space and kernel because of this.

	Arnd

Re: [RFC][PATCH resend 8/12] asm-generic: bitops: introduce le bit

[Avi Kivity]

On 05/04/2010 06:03 PM, Arnd Bergmann wrote:
> On Tuesday 04 May 2010, Takuya Yoshikawa wrote:
>    
>> Although we can use *_le_bit() helpers to treat bitmaps le arranged,
>> having le bit offset calculation as a seperate macro gives us more freedom.
>>
>> For example, KVM has le arranged dirty bitmaps for VGA, live-migration
>> and they are used in user space too. To avoid bitmap copies between kernel
>> and user space, we want to update the bitmaps in user space directly.
>> To achive this, le bit offset with *_user() functions help us a lot.
>>
>> So let us use the le bit offset calculation part by defining it as a new
>> macro: generic_le_bit_offset() .
>>      
> Does this work correctly if your user space is 32 bits (i.e. unsigned long
> is different size in user space and kernel) in both big- and little-endian
> systems?
>
> I'm not sure about all the details, but I think you cannot in general share
> bitmaps between user space and kernel because of this.
>    

That's why the bitmaps are defined as little endian u64 aligned, even on 
big endian 32-bit systems.  Little endian bitmaps are wordsize agnostic, 
and u64 alignment ensures we can use long-sized bitops on mixed size 
systems.

-- 
error compiling committee.c: too many arguments to function

Re: [RFC][PATCH resend 8/12] asm-generic: bitops: introduce le bit

[Takuya Yoshikawa]

On Tue, 04 May 2010 19:08:23 +0300
Avi Kivity <avi@redhat.com> wrote:

> On 05/04/2010 06:03 PM, Arnd Bergmann wrote:
> > On Tuesday 04 May 2010, Takuya Yoshikawa wrote:
...
> >> So let us use the le bit offset calculation part by defining it as a new
> >> macro: generic_le_bit_offset() .
> >>      
> > Does this work correctly if your user space is 32 bits (i.e. unsigned long
> > is different size in user space and kernel) in both big- and little-endian
> > systems?
> >
> > I'm not sure about all the details, but I think you cannot in general share
> > bitmaps between user space and kernel because of this.
> >    
> 
> That's why the bitmaps are defined as little endian u64 aligned, even on 
> big endian 32-bit systems.  Little endian bitmaps are wordsize agnostic, 
> and u64 alignment ensures we can use long-sized bitops on mixed size 
> systems.

There was a suggestion to propose set_le_bit_user() kind of macros.
But what I thought was these have a constraint you two explained and seemed to be
a little bit specific to some area, like KVM.

So I decided to propose just the offset calculation macro.

  Thanks, Takuya

Re: [RFC][PATCH resend 8/12] asm-generic: bitops: introduce le bit offset macro

[Arnd Bergmann]

On Wednesday 05 May 2010, Takuya Yoshikawa wrote:
> Date: 
> Yesterday 04:59:24
> > That's why the bitmaps are defined as little endian u64 aligned, even on 
> > big endian 32-bit systems.  Little endian bitmaps are wordsize agnostic, 
> > and u64 alignment ensures we can use long-sized bitops on mixed size 
> > systems.

Ok, I see.

> There was a suggestion to propose set_le_bit_user() kind of macros.
> But what I thought was these have a constraint you two explained and seemed to be
> a little bit specific to some area, like KVM.
> 
> So I decided to propose just the offset calculation macro.

I'm not sure I understand how this macro is going to be used though.
If you are just using this in kernel space, that's fine, please go for
it.

However, if the intention is to use the same macro in user space, putting
it into asm-generic/bitops/* is not going to help, because those headers
are not available in user space, and I wouldn't want to change that.

The definition of the macro is not part of the ABI, so just duplicate
it in KVM if you need it there.

	Arnd

Re: [RFC][PATCH resend 8/12] asm-generic: bitops: introduce le bit

[Takuya Yoshikawa]

(2010/05/06 22:38), Arnd Bergmann wrote:
> On Wednesday 05 May 2010, Takuya Yoshikawa wrote:
>> Date:
>> Yesterday 04:59:24
>>> That's why the bitmaps are defined as little endian u64 aligned, even on
>>> big endian 32-bit systems.  Little endian bitmaps are wordsize agnostic,
>>> and u64 alignment ensures we can use long-sized bitops on mixed size
>>> systems.
>
> Ok, I see.
>
>> There was a suggestion to propose set_le_bit_user() kind of macros.
>> But what I thought was these have a constraint you two explained and seemed to be
>> a little bit specific to some area, like KVM.
>>
>> So I decided to propose just the offset calculation macro.
>
> I'm not sure I understand how this macro is going to be used though.
> If you are just using this in kernel space, that's fine, please go for
> it.

Yes, I'm just using in kernel space: qemu has its own endian related helpers.

So if you allow us to place this macro in asm-generic/bitops/* it will help us.

Avi, what do you think? Do you want to place it in kvm.h ?


>
> However, if the intention is to use the same macro in user space, putting
> it into asm-generic/bitops/* is not going to help, because those headers
> are not available in user space, and I wouldn't want to change that.
>
> The definition of the macro is not part of the ABI, so just duplicate
> it in KVM if you need it there.
>
> 	Arnd

Re: [RFC][PATCH resend 8/12] asm-generic: bitops: introduce le bit

[Avi Kivity]

On 05/10/2010 02:46 PM, Takuya Yoshikawa wrote:
> (2010/05/06 22:38), Arnd Bergmann wrote:
>> On Wednesday 05 May 2010, Takuya Yoshikawa wrote:
>>> Date:
>>> Yesterday 04:59:24
>>>> That's why the bitmaps are defined as little endian u64 aligned, 
>>>> even on
>>>> big endian 32-bit systems.  Little endian bitmaps are wordsize 
>>>> agnostic,
>>>> and u64 alignment ensures we can use long-sized bitops on mixed size
>>>> systems.
>>
>> Ok, I see.
>>
>>> There was a suggestion to propose set_le_bit_user() kind of macros.
>>> But what I thought was these have a constraint you two explained and 
>>> seemed to be
>>> a little bit specific to some area, like KVM.
>>>
>>> So I decided to propose just the offset calculation macro.
>>
>> I'm not sure I understand how this macro is going to be used though.
>> If you are just using this in kernel space, that's fine, please go for
>> it.
>
> Yes, I'm just using in kernel space: qemu has its own endian related 
> helpers.
>
> So if you allow us to place this macro in asm-generic/bitops/* it will 
> help us.
>
> Avi, what do you think? Do you want to place it in kvm.h ?

I really prefer anything that is generic to be outside kvm, even if kvm 
is the only user.

-- 
error compiling committee.c: too many arguments to function

Re: [RFC][PATCH resend 8/12] asm-generic: bitops: introduce le bit offset macro

[Arnd Bergmann]

On Monday 10 May 2010, Takuya Yoshikawa wrote:
> (2010/05/06 22:38), Arnd Bergmann wrote:
> > On Wednesday 05 May 2010, Takuya Yoshikawa wrote:
> >> There was a suggestion to propose set_le_bit_user() kind of macros.
> >> But what I thought was these have a constraint you two explained and seemed to be
> >> a little bit specific to some area, like KVM.
> >>
> >> So I decided to propose just the offset calculation macro.
> >
> > I'm not sure I understand how this macro is going to be used though.
> > If you are just using this in kernel space, that's fine, please go for
> > it.
> 
> Yes, I'm just using in kernel space: qemu has its own endian related helpers.
> 
> So if you allow us to place this macro in asm-generic/bitops/* it will help us.

No problem at all then. Thanks for the explanation.

Acked-by: Arnd Bergmann <arnd@arndb.de>

Re: [RFC][PATCH resend 8/12] asm-generic: bitops: introduce le bit

[Takuya Yoshikawa]

>>
>> Yes, I'm just using in kernel space: qemu has its own endian related helpers.
>>
>> So if you allow us to place this macro in asm-generic/bitops/* it will help us.
>
> No problem at all then. Thanks for the explanation.
>
> Acked-by: Arnd Bergmann<arnd@arndb.de>

Thanks you both. I will add your Acked-by from now on!

   Takuya