Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1725623 > unrolled thread

[PATCH 1/1] x86/xen: fix section of xen_init_time_ops() in header

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/1] x86/xen: fix section of xen_init_time_ops() in header Nicolas Iooss <nicolas.iooss_linux@m4x.org> - 2017-09-03 10:40 +0200
    Re: [PATCH 1/1] x86/xen: fix section of xen_init_time_ops() in header Juergen Gross <jgross@suse.com> - 2017-09-04 10:20 +0200
      Re: [Xen-devel] [PATCH 1/1] x86/xen: fix section of  xen_init_time_ops() in header "Jan Beulich" <JBeulich@suse.com> - 2017-09-04 12:20 +0200
        Re: [Xen-devel] [PATCH 1/1] x86/xen: fix section of  xen_init_time_ops() in header Andrew Cooper <andrew.cooper3@citrix.com> - 2017-09-04 12:40 +0200
          Re: [Xen-devel] [PATCH 1/1] x86/xen: fix section of  xen_init_time_ops() in header Andrew Cooper <andrew.cooper3@citrix.com> - 2017-09-04 14:50 +0200
            Re: [Xen-devel] [PATCH 1/1] x86/xen: fix section of  xen_init_time_ops() in header "Jan Beulich" <JBeulich@suse.com> - 2017-09-04 15:00 +0200
          Re: [Xen-devel] [PATCH 1/1] x86/xen: fix section of  xen_init_time_ops() in header "Jan Beulich" <JBeulich@suse.com> - 2017-09-04 14:50 +0200

#1725623 — [PATCH 1/1] x86/xen: fix section of xen_init_time_ops() in header

Commit d162809f85b4 ("xen/x86: Do not call xen_init_time_ops() until
shared_info is initialized") moved xen_init_time_ops() from __init to
__ref without updating xen-ops.h accordingly. Fix this.

Fixes: d162809f85b4 ("xen/x86: Do not call xen_init_time_ops() until
shared_info is initialized")
Signed-off-by: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
---
 arch/x86/xen/xen-ops.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
index 0d5004477db6..b2a5d48a2c2a 100644
--- a/arch/x86/xen/xen-ops.h
+++ b/arch/x86/xen/xen-ops.h
@@ -69,7 +69,7 @@ void xen_setup_runstate_info(int cpu);
 void xen_teardown_timer(int cpu);
 u64 xen_clocksource_read(void);
 void xen_setup_cpu_clockevents(void);
-void __init xen_init_time_ops(void);
+void __ref xen_init_time_ops(void);
 void __init xen_hvm_init_time_ops(void);
 
 irqreturn_t xen_debug_interrupt(int irq, void *dev_id);
-- 
2.14.1

[toc] | [next] | [standalone]

#1725923

On 03/09/17 10:38, Nicolas Iooss wrote:
> Commit d162809f85b4 ("xen/x86: Do not call xen_init_time_ops() until
> shared_info is initialized") moved xen_init_time_ops() from __init to
> __ref without updating xen-ops.h accordingly. Fix this.
> 
> Fixes: d162809f85b4 ("xen/x86: Do not call xen_init_time_ops() until
> shared_info is initialized")
> Signed-off-by: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
> ---
>  arch/x86/xen/xen-ops.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
> index 0d5004477db6..b2a5d48a2c2a 100644
> --- a/arch/x86/xen/xen-ops.h
> +++ b/arch/x86/xen/xen-ops.h
> @@ -69,7 +69,7 @@ void xen_setup_runstate_info(int cpu);
>  void xen_teardown_timer(int cpu);
>  u64 xen_clocksource_read(void);
>  void xen_setup_cpu_clockevents(void);
> -void __init xen_init_time_ops(void);
> +void __ref xen_init_time_ops(void);
>  void __init xen_hvm_init_time_ops(void);

When correcting this could you please modify the prototypes to comply to
the intended form as noted in include/linux/init.h (the __ref or __init
annotations should be just before the ending semicolon)?


Juergen

[toc] | [prev] | [next] | [standalone]

#1725976 — Re: [Xen-devel] [PATCH 1/1] x86/xen: fix section of xen_init_time_ops() in header

>>> On 04.09.17 at 10:17, <jgross@suse.com> wrote:
> On 03/09/17 10:38, Nicolas Iooss wrote:
>> Commit d162809f85b4 ("xen/x86: Do not call xen_init_time_ops() until
>> shared_info is initialized") moved xen_init_time_ops() from __init to
>> __ref without updating xen-ops.h accordingly. Fix this.
>> 
>> Fixes: d162809f85b4 ("xen/x86: Do not call xen_init_time_ops() until
>> shared_info is initialized")
>> Signed-off-by: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
>> ---
>>  arch/x86/xen/xen-ops.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
>> index 0d5004477db6..b2a5d48a2c2a 100644
>> --- a/arch/x86/xen/xen-ops.h
>> +++ b/arch/x86/xen/xen-ops.h
>> @@ -69,7 +69,7 @@ void xen_setup_runstate_info(int cpu);
>>  void xen_teardown_timer(int cpu);
>>  u64 xen_clocksource_read(void);
>>  void xen_setup_cpu_clockevents(void);
>> -void __init xen_init_time_ops(void);
>> +void __ref xen_init_time_ops(void);
>>  void __init xen_hvm_init_time_ops(void);
> 
> When correcting this could you please modify the prototypes to comply to
> the intended form as noted in include/linux/init.h (the __ref or __init
> annotations should be just before the ending semicolon)?

Why would these annotations be kept on the declarations anyway?
Attributes affecting code/data placement generally belong on the
definitions only.

I also question the suggested placement, despite init.h saying so.

Jan

[toc] | [prev] | [next] | [standalone]

#1725988 — Re: [Xen-devel] [PATCH 1/1] x86/xen: fix section of xen_init_time_ops() in header

On 04/09/17 11:15, Jan Beulich wrote:
>>>> On 04.09.17 at 10:17, <jgross@suse.com> wrote:
>> On 03/09/17 10:38, Nicolas Iooss wrote:
>>> Commit d162809f85b4 ("xen/x86: Do not call xen_init_time_ops() until
>>> shared_info is initialized") moved xen_init_time_ops() from __init to
>>> __ref without updating xen-ops.h accordingly. Fix this.
>>>
>>> Fixes: d162809f85b4 ("xen/x86: Do not call xen_init_time_ops() until
>>> shared_info is initialized")
>>> Signed-off-by: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
>>> ---
>>>  arch/x86/xen/xen-ops.h | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
>>> index 0d5004477db6..b2a5d48a2c2a 100644
>>> --- a/arch/x86/xen/xen-ops.h
>>> +++ b/arch/x86/xen/xen-ops.h
>>> @@ -69,7 +69,7 @@ void xen_setup_runstate_info(int cpu);
>>>  void xen_teardown_timer(int cpu);
>>>  u64 xen_clocksource_read(void);
>>>  void xen_setup_cpu_clockevents(void);
>>> -void __init xen_init_time_ops(void);
>>> +void __ref xen_init_time_ops(void);
>>>  void __init xen_hvm_init_time_ops(void);
>> When correcting this could you please modify the prototypes to comply to
>> the intended form as noted in include/linux/init.h (the __ref or __init
>> annotations should be just before the ending semicolon)?
> Why would these annotations be kept on the declarations anyway?
> Attributes affecting code/data placement generally belong on the
> definitions only.

Because:

a) That’s what the coding style says, and

b) So various static analysis can be done (e.g. sparse) on an individual
translation unit basis.


Your objection to having annotations on declarations is why I've never
got around to adding sparse to the hypervisor build.

~Andrew

[toc] | [prev] | [next] | [standalone]

#1726037 — Re: [Xen-devel] [PATCH 1/1] x86/xen: fix section of xen_init_time_ops() in header

On 04/09/17 13:41, Jan Beulich wrote:
>>>> On 04.09.17 at 12:35, <andrew.cooper3@citrix.com> wrote:
>> On 04/09/17 11:15, Jan Beulich wrote:
>>>>>> On 04.09.17 at 10:17, <jgross@suse.com> wrote:
>>>> On 03/09/17 10:38, Nicolas Iooss wrote:
>>>>> Commit d162809f85b4 ("xen/x86: Do not call xen_init_time_ops() until
>>>>> shared_info is initialized") moved xen_init_time_ops() from __init to
>>>>> __ref without updating xen-ops.h accordingly. Fix this.
>>>>>
>>>>> Fixes: d162809f85b4 ("xen/x86: Do not call xen_init_time_ops() until
>>>>> shared_info is initialized")
>>>>> Signed-off-by: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
>>>>> ---
>>>>>  arch/x86/xen/xen-ops.h | 2 +-
>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
>>>>> index 0d5004477db6..b2a5d48a2c2a 100644
>>>>> --- a/arch/x86/xen/xen-ops.h
>>>>> +++ b/arch/x86/xen/xen-ops.h
>>>>> @@ -69,7 +69,7 @@ void xen_setup_runstate_info(int cpu);
>>>>>  void xen_teardown_timer(int cpu);
>>>>>  u64 xen_clocksource_read(void);
>>>>>  void xen_setup_cpu_clockevents(void);
>>>>> -void __init xen_init_time_ops(void);
>>>>> +void __ref xen_init_time_ops(void);
>>>>>  void __init xen_hvm_init_time_ops(void);
>>>> When correcting this could you please modify the prototypes to comply to
>>>> the intended form as noted in include/linux/init.h (the __ref or __init
>>>> annotations should be just before the ending semicolon)?
>>> Why would these annotations be kept on the declarations anyway?
>>> Attributes affecting code/data placement generally belong on the
>>> definitions only.
>> Because:
>>
>> a) That’s what the coding style says, and
>>
>> b) So various static analysis can be done (e.g. sparse) on an individual
>> translation unit basis.
>>
>>
>> Your objection to having annotations on declarations is why I've never
>> got around to adding sparse to the hypervisor build.
> Would you mind educating me what use these annotations can be
> for static analysis? If they're useful in headers, I would stop
> objecting to them being added there, but I'd then demand for them
> to never be present on non-static definitions (unless there are
> attributes where the compiler requires them to be repeated, but I
> think all attributes are cumulative).

For one, finding calls to __init functions from non __init functions.

~Andrew

[toc] | [prev] | [next] | [standalone]

#1726054 — Re: [Xen-devel] [PATCH 1/1] x86/xen: fix section of xen_init_time_ops() in header

>>> On 04.09.17 at 14:43, <andrew.cooper3@citrix.com> wrote:
> On 04/09/17 13:41, Jan Beulich wrote:
>>>>> On 04.09.17 at 12:35, <andrew.cooper3@citrix.com> wrote:
>>> On 04/09/17 11:15, Jan Beulich wrote:
>>>>>>> On 04.09.17 at 10:17, <jgross@suse.com> wrote:
>>>>> On 03/09/17 10:38, Nicolas Iooss wrote:
>>>>>> Commit d162809f85b4 ("xen/x86: Do not call xen_init_time_ops() until
>>>>>> shared_info is initialized") moved xen_init_time_ops() from __init to
>>>>>> __ref without updating xen-ops.h accordingly. Fix this.
>>>>>>
>>>>>> Fixes: d162809f85b4 ("xen/x86: Do not call xen_init_time_ops() until
>>>>>> shared_info is initialized")
>>>>>> Signed-off-by: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
>>>>>> ---
>>>>>>  arch/x86/xen/xen-ops.h | 2 +-
>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
>>>>>> index 0d5004477db6..b2a5d48a2c2a 100644
>>>>>> --- a/arch/x86/xen/xen-ops.h
>>>>>> +++ b/arch/x86/xen/xen-ops.h
>>>>>> @@ -69,7 +69,7 @@ void xen_setup_runstate_info(int cpu);
>>>>>>  void xen_teardown_timer(int cpu);
>>>>>>  u64 xen_clocksource_read(void);
>>>>>>  void xen_setup_cpu_clockevents(void);
>>>>>> -void __init xen_init_time_ops(void);
>>>>>> +void __ref xen_init_time_ops(void);
>>>>>>  void __init xen_hvm_init_time_ops(void);
>>>>> When correcting this could you please modify the prototypes to comply to
>>>>> the intended form as noted in include/linux/init.h (the __ref or __init
>>>>> annotations should be just before the ending semicolon)?
>>>> Why would these annotations be kept on the declarations anyway?
>>>> Attributes affecting code/data placement generally belong on the
>>>> definitions only.
>>> Because:
>>>
>>> a) That’s what the coding style says, and
>>>
>>> b) So various static analysis can be done (e.g. sparse) on an individual
>>> translation unit basis.
>>>
>>>
>>> Your objection to having annotations on declarations is why I've never
>>> got around to adding sparse to the hypervisor build.
>> Would you mind educating me what use these annotations can be
>> for static analysis? If they're useful in headers, I would stop
>> objecting to them being added there, but I'd then demand for them
>> to never be present on non-static definitions (unless there are
>> attributes where the compiler requires them to be repeated, but I
>> think all attributes are cumulative).
> 
> For one, finding calls to __init functions from non __init functions.

Hmm, that's been solved differently on Linux, so I wouldn't have
assumed this is something to use sparse for.

Jan

[toc] | [prev] | [next] | [standalone]

#1726039 — Re: [Xen-devel] [PATCH 1/1] x86/xen: fix section of xen_init_time_ops() in header

>>> On 04.09.17 at 12:35, <andrew.cooper3@citrix.com> wrote:
> On 04/09/17 11:15, Jan Beulich wrote:
>>>>> On 04.09.17 at 10:17, <jgross@suse.com> wrote:
>>> On 03/09/17 10:38, Nicolas Iooss wrote:
>>>> Commit d162809f85b4 ("xen/x86: Do not call xen_init_time_ops() until
>>>> shared_info is initialized") moved xen_init_time_ops() from __init to
>>>> __ref without updating xen-ops.h accordingly. Fix this.
>>>>
>>>> Fixes: d162809f85b4 ("xen/x86: Do not call xen_init_time_ops() until
>>>> shared_info is initialized")
>>>> Signed-off-by: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
>>>> ---
>>>>  arch/x86/xen/xen-ops.h | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
>>>> index 0d5004477db6..b2a5d48a2c2a 100644
>>>> --- a/arch/x86/xen/xen-ops.h
>>>> +++ b/arch/x86/xen/xen-ops.h
>>>> @@ -69,7 +69,7 @@ void xen_setup_runstate_info(int cpu);
>>>>  void xen_teardown_timer(int cpu);
>>>>  u64 xen_clocksource_read(void);
>>>>  void xen_setup_cpu_clockevents(void);
>>>> -void __init xen_init_time_ops(void);
>>>> +void __ref xen_init_time_ops(void);
>>>>  void __init xen_hvm_init_time_ops(void);
>>> When correcting this could you please modify the prototypes to comply to
>>> the intended form as noted in include/linux/init.h (the __ref or __init
>>> annotations should be just before the ending semicolon)?
>> Why would these annotations be kept on the declarations anyway?
>> Attributes affecting code/data placement generally belong on the
>> definitions only.
> 
> Because:
> 
> a) That’s what the coding style says, and
> 
> b) So various static analysis can be done (e.g. sparse) on an individual
> translation unit basis.
> 
> 
> Your objection to having annotations on declarations is why I've never
> got around to adding sparse to the hypervisor build.

Would you mind educating me what use these annotations can be
for static analysis? If they're useful in headers, I would stop
objecting to them being added there, but I'd then demand for them
to never be present on non-static definitions (unless there are
attributes where the compiler requires them to be repeated, but I
think all attributes are cumulative).

Jan

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web