[v1] xen: cpu: change 'cpu_hotplug_[begin|done]' to inline function

[Xen-devel] [PATCH] xen: cpu: change 'cpu_hotplug_[begin|done]' to inline function

Posted by Baodong Chen 4 years, 11 months ago

Signed-off-by: Baodong Chen <chenbaodong@mxnavi.com>
---
 xen/common/cpu.c      | 10 ----------
 xen/include/xen/cpu.h |  4 ++--
 2 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/xen/common/cpu.c b/xen/common/cpu.c
index f388d89..a526b55 100644
--- a/xen/common/cpu.c
+++ b/xen/common/cpu.c
@@ -51,16 +51,6 @@ void put_cpu_maps(void)
     spin_unlock_recursive(&cpu_add_remove_lock);
 }
 
-bool_t cpu_hotplug_begin(void)
-{
-    return get_cpu_maps();
-}
-
-void cpu_hotplug_done(void)
-{
-    put_cpu_maps();
-}
-
 static NOTIFIER_HEAD(cpu_chain);
 
 void __init register_cpu_notifier(struct notifier_block *nb)
diff --git a/xen/include/xen/cpu.h b/xen/include/xen/cpu.h
index 4638c50..70a2df4 100644
--- a/xen/include/xen/cpu.h
+++ b/xen/include/xen/cpu.h
@@ -10,8 +10,8 @@ bool_t get_cpu_maps(void);
 void put_cpu_maps(void);
 
 /* Safely perform CPU hotplug and update cpu_online_map, etc. */
-bool_t cpu_hotplug_begin(void);
-void cpu_hotplug_done(void);
+static inline bool_t cpu_hotplug_begin(void) { return get_cpu_maps(); }
+static inline void cpu_hotplug_done(void) { put_cpu_maps(); }
 
 /* Receive notification of CPU hotplug events. */
 void register_cpu_notifier(struct notifier_block *nb);
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] xen: cpu: change 'cpu_hotplug_[begin|done]' to inline function

Posted by Julien Grall 4 years, 11 months ago

Hi,

On 5/31/19 3:46 AM, Baodong Chen wrote:
> Signed-off-by: Baodong Chen <chenbaodong@mxnavi.com>
> ---
>   xen/common/cpu.c      | 10 ----------
>   xen/include/xen/cpu.h |  4 ++--
>   2 files changed, 2 insertions(+), 12 deletions(-)
> 
> diff --git a/xen/common/cpu.c b/xen/common/cpu.c
> index f388d89..a526b55 100644
> --- a/xen/common/cpu.c
> +++ b/xen/common/cpu.c
> @@ -51,16 +51,6 @@ void put_cpu_maps(void)
>       spin_unlock_recursive(&cpu_add_remove_lock);
>   }
>   
> -bool_t cpu_hotplug_begin(void)
> -{
> -    return get_cpu_maps();
> -}
> -
> -void cpu_hotplug_done(void)
> -{
> -    put_cpu_maps();
> -}
> -
>   static NOTIFIER_HEAD(cpu_chain);
>   
>   void __init register_cpu_notifier(struct notifier_block *nb)
> diff --git a/xen/include/xen/cpu.h b/xen/include/xen/cpu.h
> index 4638c50..70a2df4 100644
> --- a/xen/include/xen/cpu.h
> +++ b/xen/include/xen/cpu.h
> @@ -10,8 +10,8 @@ bool_t get_cpu_maps(void);
>   void put_cpu_maps(void);
>   
>   /* Safely perform CPU hotplug and update cpu_online_map, etc. */
> -bool_t cpu_hotplug_begin(void);
> -void cpu_hotplug_done(void);
> +static inline bool_t cpu_hotplug_begin(void) { return get_cpu_maps(); }
> +static inline void cpu_hotplug_done(void) { put_cpu_maps(); }

The coding style should be:

static inline....
{
       ...
}

>   
>   /* Receive notification of CPU hotplug events. */
>   void register_cpu_notifier(struct notifier_block *nb);
> 

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] xen: cpu: change 'cpu_hotplug_[begin|done]' to inline function

Posted by Jan Beulich 4 years, 11 months ago

>>> On 31.05.19 at 12:55, <julien.grall@arm.com> wrote:
> On 5/31/19 3:46 AM, Baodong Chen wrote:
>> --- a/xen/include/xen/cpu.h
>> +++ b/xen/include/xen/cpu.h
>> @@ -10,8 +10,8 @@ bool_t get_cpu_maps(void);
>>   void put_cpu_maps(void);
>>   
>>   /* Safely perform CPU hotplug and update cpu_online_map, etc. */
>> -bool_t cpu_hotplug_begin(void);
>> -void cpu_hotplug_done(void);
>> +static inline bool_t cpu_hotplug_begin(void) { return get_cpu_maps(); }
>> +static inline void cpu_hotplug_done(void) { put_cpu_maps(); }

Plus please switch to bool at the same time.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] xen: cpu: change 'cpu_hotplug_[begin|done]' to inline function

Posted by chenbaodong 4 years, 10 months ago

On 5/31/19 19:30, Jan Beulich wrote:
>>>> On 31.05.19 at 12:55, <julien.grall@arm.com> wrote:
>> On 5/31/19 3:46 AM, Baodong Chen wrote:
>>> --- a/xen/include/xen/cpu.h
>>> +++ b/xen/include/xen/cpu.h
>>> @@ -10,8 +10,8 @@ bool_t get_cpu_maps(void);
>>>    void put_cpu_maps(void);
>>>    
>>>    /* Safely perform CPU hotplug and update cpu_online_map, etc. */
>>> -bool_t cpu_hotplug_begin(void);
>>> -void cpu_hotplug_done(void);
>>> +static inline bool_t cpu_hotplug_begin(void) { return get_cpu_maps(); }
>>> +static inline void cpu_hotplug_done(void) { put_cpu_maps(); }
> Plus please switch to bool at the same time.

Yes, types like boot_t or u32 are getting rid of, so should Not be used.

Will be fixed.

>
> Jan
>
>
> .
>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] xen: cpu: change 'cpu_hotplug_[begin|done]' to inline function

Posted by chenbaodong 4 years, 10 months ago

On 5/31/19 18:55, Julien Grall wrote:
> Hi,
>
> On 5/31/19 3:46 AM, Baodong Chen wrote:
>> Signed-off-by: Baodong Chen <chenbaodong@mxnavi.com>
>> ---
>>   xen/common/cpu.c      | 10 ----------
>>   xen/include/xen/cpu.h |  4 ++--
>>   2 files changed, 2 insertions(+), 12 deletions(-)
>>
>> diff --git a/xen/common/cpu.c b/xen/common/cpu.c
>> index f388d89..a526b55 100644
>> --- a/xen/common/cpu.c
>> +++ b/xen/common/cpu.c
>> @@ -51,16 +51,6 @@ void put_cpu_maps(void)
>>       spin_unlock_recursive(&cpu_add_remove_lock);
>>   }
>>   -bool_t cpu_hotplug_begin(void)
>> -{
>> -    return get_cpu_maps();
>> -}
>> -
>> -void cpu_hotplug_done(void)
>> -{
>> -    put_cpu_maps();
>> -}
>> -
>>   static NOTIFIER_HEAD(cpu_chain);
>>     void __init register_cpu_notifier(struct notifier_block *nb)
>> diff --git a/xen/include/xen/cpu.h b/xen/include/xen/cpu.h
>> index 4638c50..70a2df4 100644
>> --- a/xen/include/xen/cpu.h
>> +++ b/xen/include/xen/cpu.h
>> @@ -10,8 +10,8 @@ bool_t get_cpu_maps(void);
>>   void put_cpu_maps(void);
>>     /* Safely perform CPU hotplug and update cpu_online_map, etc. */
>> -bool_t cpu_hotplug_begin(void);
>> -void cpu_hotplug_done(void);
>> +static inline bool_t cpu_hotplug_begin(void) { return get_cpu_maps(); }
>> +static inline void cpu_hotplug_done(void) { put_cpu_maps(); }
>
> The coding style should be:
>
> static inline....
> {
>       ...
> }
>
Yes, clang-format automated format code for me, will be fixed.
>>     /* Receive notification of CPU hotplug events. */
>>   void register_cpu_notifier(struct notifier_block *nb);
>>
>
> Cheers,
>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] xen: cpu: change 'cpu_hotplug_[begin|done]' to inline function

Posted by Julien Grall 4 years, 10 months ago

Hi,

On 03/06/2019 02:52, chenbaodong wrote:
> 
> On 5/31/19 18:55, Julien Grall wrote:
>> Hi,
>>
>> On 5/31/19 3:46 AM, Baodong Chen wrote:
>>> Signed-off-by: Baodong Chen <chenbaodong@mxnavi.com>
>>> ---
>>>   xen/common/cpu.c      | 10 ----------
>>>   xen/include/xen/cpu.h |  4 ++--
>>>   2 files changed, 2 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/xen/common/cpu.c b/xen/common/cpu.c
>>> index f388d89..a526b55 100644
>>> --- a/xen/common/cpu.c
>>> +++ b/xen/common/cpu.c
>>> @@ -51,16 +51,6 @@ void put_cpu_maps(void)
>>>       spin_unlock_recursive(&cpu_add_remove_lock);
>>>   }
>>>   -bool_t cpu_hotplug_begin(void)
>>> -{
>>> -    return get_cpu_maps();
>>> -}
>>> -
>>> -void cpu_hotplug_done(void)
>>> -{
>>> -    put_cpu_maps();
>>> -}
>>> -
>>>   static NOTIFIER_HEAD(cpu_chain);
>>>     void __init register_cpu_notifier(struct notifier_block *nb)
>>> diff --git a/xen/include/xen/cpu.h b/xen/include/xen/cpu.h
>>> index 4638c50..70a2df4 100644
>>> --- a/xen/include/xen/cpu.h
>>> +++ b/xen/include/xen/cpu.h
>>> @@ -10,8 +10,8 @@ bool_t get_cpu_maps(void);
>>>   void put_cpu_maps(void);
>>>     /* Safely perform CPU hotplug and update cpu_online_map, etc. */
>>> -bool_t cpu_hotplug_begin(void);
>>> -void cpu_hotplug_done(void);
>>> +static inline bool_t cpu_hotplug_begin(void) { return get_cpu_maps(); }
>>> +static inline void cpu_hotplug_done(void) { put_cpu_maps(); }
>>
>> The coding style should be:
>>
>> static inline....
>> {
>>       ...
>> }
>>
> Yes, clang-format automated format code for me, will be fixed.

Hmmm, clang-format does not have Xen coding style support yet. Do you have 
patches on top to handle it?

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] xen: cpu: change 'cpu_hotplug_[begin|done]' to inline function

Posted by chenbaodong 4 years, 10 months ago

On 6/3/19 17:37, Julien Grall wrote:
> Hi,
>
> On 03/06/2019 02:52, chenbaodong wrote:
>>
>> On 5/31/19 18:55, Julien Grall wrote:
>>> Hi,
>>>
>>> On 5/31/19 3:46 AM, Baodong Chen wrote:
>>>> Signed-off-by: Baodong Chen <chenbaodong@mxnavi.com>
>>>> ---
>>>>   xen/common/cpu.c      | 10 ----------
>>>>   xen/include/xen/cpu.h |  4 ++--
>>>>   2 files changed, 2 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/xen/common/cpu.c b/xen/common/cpu.c
>>>> index f388d89..a526b55 100644
>>>> --- a/xen/common/cpu.c
>>>> +++ b/xen/common/cpu.c
>>>> @@ -51,16 +51,6 @@ void put_cpu_maps(void)
>>>>       spin_unlock_recursive(&cpu_add_remove_lock);
>>>>   }
>>>>   -bool_t cpu_hotplug_begin(void)
>>>> -{
>>>> -    return get_cpu_maps();
>>>> -}
>>>> -
>>>> -void cpu_hotplug_done(void)
>>>> -{
>>>> -    put_cpu_maps();
>>>> -}
>>>> -
>>>>   static NOTIFIER_HEAD(cpu_chain);
>>>>     void __init register_cpu_notifier(struct notifier_block *nb)
>>>> diff --git a/xen/include/xen/cpu.h b/xen/include/xen/cpu.h
>>>> index 4638c50..70a2df4 100644
>>>> --- a/xen/include/xen/cpu.h
>>>> +++ b/xen/include/xen/cpu.h
>>>> @@ -10,8 +10,8 @@ bool_t get_cpu_maps(void);
>>>>   void put_cpu_maps(void);
>>>>     /* Safely perform CPU hotplug and update cpu_online_map, etc. */
>>>> -bool_t cpu_hotplug_begin(void);
>>>> -void cpu_hotplug_done(void);
>>>> +static inline bool_t cpu_hotplug_begin(void) { return 
>>>> get_cpu_maps(); }
>>>> +static inline void cpu_hotplug_done(void) { put_cpu_maps(); }
>>>
>>> The coding style should be:
>>>
>>> static inline....
>>> {
>>>       ...
>>> }
>>>
>> Yes, clang-format automated format code for me, will be fixed.
>
> Hmmm, clang-format does not have Xen coding style support yet. Do you 
> have patches on top to handle it?

No, But the linux kernel seems already have it's clang-format support. 
Guess can used by xen.

IMO  i don't like the coding style in xen personally.

But it's code base has long years history. can insist on this or make 
some changes.

I prefer clang-format personally,  because no style issue in patch and 
will make review easier.

>
> Cheers,
>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] xen: cpu: change 'cpu_hotplug_[begin|done]' to inline function

Posted by Julien Grall 4 years, 10 months ago

Hi,

On 03/06/2019 11:22, chenbaodong wrote:
> 
> On 6/3/19 17:37, Julien Grall wrote:
>> Hi,
>>
>> On 03/06/2019 02:52, chenbaodong wrote:
>>>
>>> On 5/31/19 18:55, Julien Grall wrote:
>>>> Hi,
>>>>
>>>> On 5/31/19 3:46 AM, Baodong Chen wrote:
>>>>> Signed-off-by: Baodong Chen <chenbaodong@mxnavi.com>
>>>>> ---
>>>>>   xen/common/cpu.c      | 10 ----------
>>>>>   xen/include/xen/cpu.h |  4 ++--
>>>>>   2 files changed, 2 insertions(+), 12 deletions(-)
>>>>>
>>>>> diff --git a/xen/common/cpu.c b/xen/common/cpu.c
>>>>> index f388d89..a526b55 100644
>>>>> --- a/xen/common/cpu.c
>>>>> +++ b/xen/common/cpu.c
>>>>> @@ -51,16 +51,6 @@ void put_cpu_maps(void)
>>>>>       spin_unlock_recursive(&cpu_add_remove_lock);
>>>>>   }
>>>>>   -bool_t cpu_hotplug_begin(void)
>>>>> -{
>>>>> -    return get_cpu_maps();
>>>>> -}
>>>>> -
>>>>> -void cpu_hotplug_done(void)
>>>>> -{
>>>>> -    put_cpu_maps();
>>>>> -}
>>>>> -
>>>>>   static NOTIFIER_HEAD(cpu_chain);
>>>>>     void __init register_cpu_notifier(struct notifier_block *nb)
>>>>> diff --git a/xen/include/xen/cpu.h b/xen/include/xen/cpu.h
>>>>> index 4638c50..70a2df4 100644
>>>>> --- a/xen/include/xen/cpu.h
>>>>> +++ b/xen/include/xen/cpu.h
>>>>> @@ -10,8 +10,8 @@ bool_t get_cpu_maps(void);
>>>>>   void put_cpu_maps(void);
>>>>>     /* Safely perform CPU hotplug and update cpu_online_map, etc. */
>>>>> -bool_t cpu_hotplug_begin(void);
>>>>> -void cpu_hotplug_done(void);
>>>>> +static inline bool_t cpu_hotplug_begin(void) { return get_cpu_maps(); }
>>>>> +static inline void cpu_hotplug_done(void) { put_cpu_maps(); }
>>>>
>>>> The coding style should be:
>>>>
>>>> static inline....
>>>> {
>>>>       ...
>>>> }
>>>>
>>> Yes, clang-format automated format code for me, will be fixed.
>>
>> Hmmm, clang-format does not have Xen coding style support yet. Do you have 
>> patches on top to handle it?
> 
> No, But the linux kernel seems already have it's clang-format support. Guess can 
> used by xen.

Most of Xen code base does not use Linux coding style. The only exception is 
file imported directly from Linux to ease porting bug fixes.

> 
> IMO  i don't like the coding style in xen personally.

I don't necessarily agree with all of it but some of the Linux style are odd too.

> 
> But it's code base has long years history. can insist on this or make some changes.

Improvement to the coding style are always welcomed. Although, you will notice 
that anything around coding style (in any project) is a matter of taste and can 
be difficult to find an agreement.

> 
> I prefer clang-format personally,  because no style issue in patch and will make 
> review easier.

clang-format is not a coding style. It is a tool helping to format in a specific 
coding style. There are effort to extend clang-format for supporting Xen coding 
style.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] xen: cpu: change 'cpu_hotplug_[begin|done]' to inline function

Posted by chenbaodong 4 years, 10 months ago

On 6/3/19 18:40, Julien Grall wrote:
> Hi,
>
> On 03/06/2019 11:22, chenbaodong wrote:
>>
>> On 6/3/19 17:37, Julien Grall wrote:
>>> Hi,
>>>
>>> On 03/06/2019 02:52, chenbaodong wrote:
>>>>
>>>> On 5/31/19 18:55, Julien Grall wrote:
>>>>> Hi,
>>>>>
>>>>> On 5/31/19 3:46 AM, Baodong Chen wrote:
>>>>>> Signed-off-by: Baodong Chen <chenbaodong@mxnavi.com>
>>>>>> ---
>>>>>>   xen/common/cpu.c      | 10 ----------
>>>>>>   xen/include/xen/cpu.h |  4 ++--
>>>>>>   2 files changed, 2 insertions(+), 12 deletions(-)
>>>>>>
>>>>>> diff --git a/xen/common/cpu.c b/xen/common/cpu.c
>>>>>> index f388d89..a526b55 100644
>>>>>> --- a/xen/common/cpu.c
>>>>>> +++ b/xen/common/cpu.c
>>>>>> @@ -51,16 +51,6 @@ void put_cpu_maps(void)
>>>>>>       spin_unlock_recursive(&cpu_add_remove_lock);
>>>>>>   }
>>>>>>   -bool_t cpu_hotplug_begin(void)
>>>>>> -{
>>>>>> -    return get_cpu_maps();
>>>>>> -}
>>>>>> -
>>>>>> -void cpu_hotplug_done(void)
>>>>>> -{
>>>>>> -    put_cpu_maps();
>>>>>> -}
>>>>>> -
>>>>>>   static NOTIFIER_HEAD(cpu_chain);
>>>>>>     void __init register_cpu_notifier(struct notifier_block *nb)
>>>>>> diff --git a/xen/include/xen/cpu.h b/xen/include/xen/cpu.h
>>>>>> index 4638c50..70a2df4 100644
>>>>>> --- a/xen/include/xen/cpu.h
>>>>>> +++ b/xen/include/xen/cpu.h
>>>>>> @@ -10,8 +10,8 @@ bool_t get_cpu_maps(void);
>>>>>>   void put_cpu_maps(void);
>>>>>>     /* Safely perform CPU hotplug and update cpu_online_map, etc. */
>>>>>> -bool_t cpu_hotplug_begin(void);
>>>>>> -void cpu_hotplug_done(void);
>>>>>> +static inline bool_t cpu_hotplug_begin(void) { return 
>>>>>> get_cpu_maps(); }
>>>>>> +static inline void cpu_hotplug_done(void) { put_cpu_maps(); }
>>>>>
>>>>> The coding style should be:
>>>>>
>>>>> static inline....
>>>>> {
>>>>>       ...
>>>>> }
>>>>>
>>>> Yes, clang-format automated format code for me, will be fixed.
>>>
>>> Hmmm, clang-format does not have Xen coding style support yet. Do 
>>> you have patches on top to handle it?
>>
>> No, But the linux kernel seems already have it's clang-format 
>> support. Guess can used by xen.
>
> Most of Xen code base does not use Linux coding style. The only 
> exception is file imported directly from Linux to ease porting bug fixes.

Roger that.


>
>>
>> IMO  i don't like the coding style in xen personally.
>
> I don't necessarily agree with all of it but some of the Linux style 
> are odd too.
yes yes,  Linux style, eg: 'tab', i also dislike.
>
>>
>> But it's code base has long years history. can insist on this or make 
>> some changes.
>
> Improvement to the coding style are always welcomed. Although, you 
> will notice that anything around coding style (in any project) is a 
> matter of taste and can be difficult to find an agreement.

Agree.

>
>>
>> I prefer clang-format personally,  because no style issue in patch 
>> and will make review easier.
>
> clang-format is not a coding style. It is a tool helping to format in 
> a specific coding style. There are effort to extend clang-format for 
> supporting Xen coding style.

Agree, For xen, it's worth to have it's '.clang-format' file according 
to it's style.

>
> Cheers,
>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel