Re: [Xen-devel] [PATCH] x86/xen/time: fix section mismatch for xen_init_time_ops()

[Boris Ostrovsky]

On 01/06/2018 03:35 PM, Nick Desaulniers wrote:
> On Tue, Jan 2, 2018 at 7:00 AM, Boris Ostrovsky
>  wrote:
>> On 01/02/2018 09:32 AM, Andrew Cooper wrote:
>>> On 02/01/18 14:24, Juergen Gross wrote:
 On 02/01/18 15:18, Boris Ostrovsky wrote:
> On 12/23/2017 09:50 PM, Nick Desaulniers wrote:
>> The header declares this function as __init but is defined in __ref
>> section.
>>
>> Signed-off-by: Nick Desaulniers 
> AFAIK section attributes in header files are ignored by compiler anyway
> so I'd remove all of them.
 Hmm, I'm not sure all future compilers will ignore the section
 attributes. include/linux/init.h explictily mentions where to put
 the attrubute in a prototype, so I'd rather keep it.
>>> Attributes in the declaration are for static analysis tools such as sparse.
>>>
>>> How else are you going to work out whether a section mismatch has occurred?
>> Isn't this done based on definitions?
>>
>> Tons of __init routines don't have the attribute specified in header
>> files. In fact, even in this file (arch/x86/xen/xen-ops.h) there are
>> some that don't have it.
>>
>> -boris
> What are the next steps for getting this patch merged? This is the
> only function for which I get a compiler warning (with Clang).  Do you
> require a patch instead that changes more function attributes, or can
> that be a follow up patch?

Applied to for-linus-4.15.

-boris

Re: [Xen-devel] [PATCH] x86/xen/time: fix section mismatch for xen_init_time_ops()

[Boris Ostrovsky]

On 01/06/2018 03:35 PM, Nick Desaulniers wrote:
> On Tue, Jan 2, 2018 at 7:00 AM, Boris Ostrovsky
>  wrote:
>> On 01/02/2018 09:32 AM, Andrew Cooper wrote:
>>> On 02/01/18 14:24, Juergen Gross wrote:
 On 02/01/18 15:18, Boris Ostrovsky wrote:
> On 12/23/2017 09:50 PM, Nick Desaulniers wrote:
>> The header declares this function as __init but is defined in __ref
>> section.
>>
>> Signed-off-by: Nick Desaulniers 
> AFAIK section attributes in header files are ignored by compiler anyway
> so I'd remove all of them.
 Hmm, I'm not sure all future compilers will ignore the section
 attributes. include/linux/init.h explictily mentions where to put
 the attrubute in a prototype, so I'd rather keep it.
>>> Attributes in the declaration are for static analysis tools such as sparse.
>>>
>>> How else are you going to work out whether a section mismatch has occurred?
>> Isn't this done based on definitions?
>>
>> Tons of __init routines don't have the attribute specified in header
>> files. In fact, even in this file (arch/x86/xen/xen-ops.h) there are
>> some that don't have it.
>>
>> -boris
> What are the next steps for getting this patch merged? This is the
> only function for which I get a compiler warning (with Clang).  Do you
> require a patch instead that changes more function attributes, or can
> that be a follow up patch?

Applied to for-linus-4.15.

-boris

Re: [Xen-devel] [PATCH] x86/xen/time: fix section mismatch for xen_init_time_ops()

[Nick Desaulniers]

On Tue, Jan 2, 2018 at 7:00 AM, Boris Ostrovsky
 wrote:
> On 01/02/2018 09:32 AM, Andrew Cooper wrote:
>> On 02/01/18 14:24, Juergen Gross wrote:
>>> On 02/01/18 15:18, Boris Ostrovsky wrote:
 On 12/23/2017 09:50 PM, Nick Desaulniers wrote:
> The header declares this function as __init but is defined in __ref
> section.
>
> Signed-off-by: Nick Desaulniers 
 AFAIK section attributes in header files are ignored by compiler anyway
 so I'd remove all of them.
>>> Hmm, I'm not sure all future compilers will ignore the section
>>> attributes. include/linux/init.h explictily mentions where to put
>>> the attrubute in a prototype, so I'd rather keep it.
>> Attributes in the declaration are for static analysis tools such as sparse.
>>
>> How else are you going to work out whether a section mismatch has occurred?
>
> Isn't this done based on definitions?
>
> Tons of __init routines don't have the attribute specified in header
> files. In fact, even in this file (arch/x86/xen/xen-ops.h) there are
> some that don't have it.
>
> -boris

What are the next steps for getting this patch merged? This is the
only function for which I get a compiler warning (with Clang).  Do you
require a patch instead that changes more function attributes, or can
that be a follow up patch?

Re: [Xen-devel] [PATCH] x86/xen/time: fix section mismatch for xen_init_time_ops()

[Nick Desaulniers]

On Tue, Jan 2, 2018 at 7:00 AM, Boris Ostrovsky
 wrote:
> On 01/02/2018 09:32 AM, Andrew Cooper wrote:
>> On 02/01/18 14:24, Juergen Gross wrote:
>>> On 02/01/18 15:18, Boris Ostrovsky wrote:
 On 12/23/2017 09:50 PM, Nick Desaulniers wrote:
> The header declares this function as __init but is defined in __ref
> section.
>
> Signed-off-by: Nick Desaulniers 
 AFAIK section attributes in header files are ignored by compiler anyway
 so I'd remove all of them.
>>> Hmm, I'm not sure all future compilers will ignore the section
>>> attributes. include/linux/init.h explictily mentions where to put
>>> the attrubute in a prototype, so I'd rather keep it.
>> Attributes in the declaration are for static analysis tools such as sparse.
>>
>> How else are you going to work out whether a section mismatch has occurred?
>
> Isn't this done based on definitions?
>
> Tons of __init routines don't have the attribute specified in header
> files. In fact, even in this file (arch/x86/xen/xen-ops.h) there are
> some that don't have it.
>
> -boris

What are the next steps for getting this patch merged? This is the
only function for which I get a compiler warning (with Clang).  Do you
require a patch instead that changes more function attributes, or can
that be a follow up patch?

Re: [Xen-devel] [PATCH] x86/xen/time: fix section mismatch for xen_init_time_ops()

[Boris Ostrovsky]

On 01/02/2018 09:32 AM, Andrew Cooper wrote:
> On 02/01/18 14:24, Juergen Gross wrote:
>> On 02/01/18 15:18, Boris Ostrovsky wrote:
>>> On 12/23/2017 09:50 PM, Nick Desaulniers wrote:
 The header declares this function as __init but is defined in __ref
 section.

 Signed-off-by: Nick Desaulniers 
>>> AFAIK section attributes in header files are ignored by compiler anyway
>>> so I'd remove all of them.
>> Hmm, I'm not sure all future compilers will ignore the section
>> attributes. include/linux/init.h explictily mentions where to put
>> the attrubute in a prototype, so I'd rather keep it.
> Attributes in the declaration are for static analysis tools such as sparse.
>
> How else are you going to work out whether a section mismatch has occurred?

Isn't this done based on definitions?

Tons of __init routines don't have the attribute specified in header
files. In fact, even in this file (arch/x86/xen/xen-ops.h) there are
some that don't have it.

-boris

Re: [Xen-devel] [PATCH] x86/xen/time: fix section mismatch for xen_init_time_ops()

[Boris Ostrovsky]

On 01/02/2018 09:32 AM, Andrew Cooper wrote:
> On 02/01/18 14:24, Juergen Gross wrote:
>> On 02/01/18 15:18, Boris Ostrovsky wrote:
>>> On 12/23/2017 09:50 PM, Nick Desaulniers wrote:
 The header declares this function as __init but is defined in __ref
 section.

 Signed-off-by: Nick Desaulniers 
>>> AFAIK section attributes in header files are ignored by compiler anyway
>>> so I'd remove all of them.
>> Hmm, I'm not sure all future compilers will ignore the section
>> attributes. include/linux/init.h explictily mentions where to put
>> the attrubute in a prototype, so I'd rather keep it.
> Attributes in the declaration are for static analysis tools such as sparse.
>
> How else are you going to work out whether a section mismatch has occurred?

Isn't this done based on definitions?

Tons of __init routines don't have the attribute specified in header
files. In fact, even in this file (arch/x86/xen/xen-ops.h) there are
some that don't have it.

-boris

Re: [Xen-devel] [PATCH] x86/xen/time: fix section mismatch for xen_init_time_ops()

[Andrew Cooper]

On 02/01/18 14:24, Juergen Gross wrote:
> On 02/01/18 15:18, Boris Ostrovsky wrote:
>> On 12/23/2017 09:50 PM, Nick Desaulniers wrote:
>>> The header declares this function as __init but is defined in __ref
>>> section.
>>>
>>> Signed-off-by: Nick Desaulniers 
>> AFAIK section attributes in header files are ignored by compiler anyway
>> so I'd remove all of them.
> Hmm, I'm not sure all future compilers will ignore the section
> attributes. include/linux/init.h explictily mentions where to put
> the attrubute in a prototype, so I'd rather keep it.

Attributes in the declaration are for static analysis tools such as sparse.

How else are you going to work out whether a section mismatch has occurred?

~Andrew

Re: [Xen-devel] [PATCH] x86/xen/time: fix section mismatch for xen_init_time_ops()

[Andrew Cooper]

On 02/01/18 14:24, Juergen Gross wrote:
> On 02/01/18 15:18, Boris Ostrovsky wrote:
>> On 12/23/2017 09:50 PM, Nick Desaulniers wrote:
>>> The header declares this function as __init but is defined in __ref
>>> section.
>>>
>>> Signed-off-by: Nick Desaulniers 
>> AFAIK section attributes in header files are ignored by compiler anyway
>> so I'd remove all of them.
> Hmm, I'm not sure all future compilers will ignore the section
> attributes. include/linux/init.h explictily mentions where to put
> the attrubute in a prototype, so I'd rather keep it.

Attributes in the declaration are for static analysis tools such as sparse.

How else are you going to work out whether a section mismatch has occurred?

~Andrew

Re: [PATCH] x86/xen/time: fix section mismatch for xen_init_time_ops()

[Juergen Gross]

On 02/01/18 15:18, Boris Ostrovsky wrote:
> On 12/23/2017 09:50 PM, Nick Desaulniers wrote:
>> The header declares this function as __init but is defined in __ref
>> section.
>>
>> Signed-off-by: Nick Desaulniers 
> 
> AFAIK section attributes in header files are ignored by compiler anyway
> so I'd remove all of them.

Hmm, I'm not sure all future compilers will ignore the section
attributes. include/linux/init.h explictily mentions where to put
the attrubute in a prototype, so I'd rather keep it.


Juergen

Re: [PATCH] x86/xen/time: fix section mismatch for xen_init_time_ops()

[Juergen Gross]

On 02/01/18 15:18, Boris Ostrovsky wrote:
> On 12/23/2017 09:50 PM, Nick Desaulniers wrote:
>> The header declares this function as __init but is defined in __ref
>> section.
>>
>> Signed-off-by: Nick Desaulniers 
> 
> AFAIK section attributes in header files are ignored by compiler anyway
> so I'd remove all of them.

Hmm, I'm not sure all future compilers will ignore the section
attributes. include/linux/init.h explictily mentions where to put
the attrubute in a prototype, so I'd rather keep it.


Juergen

Re: [PATCH] x86/xen/time: fix section mismatch for xen_init_time_ops()

[Boris Ostrovsky]

On 12/23/2017 09:50 PM, Nick Desaulniers wrote:
> The header declares this function as __init but is defined in __ref
> section.
>
> Signed-off-by: Nick Desaulniers 

AFAIK section attributes in header files are ignored by compiler anyway
so I'd remove all of them.

-boris


> ---
>  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 75011b8..3b34745 100644
> --- a/arch/x86/xen/xen-ops.h
> +++ b/arch/x86/xen/xen-ops.h
> @@ -72,7 +72,7 @@ u64 xen_clocksource_read(void);
>  void xen_setup_cpu_clockevents(void);
>  void xen_save_time_memory_area(void);
>  void xen_restore_time_memory_area(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);

Re: [PATCH] x86/xen/time: fix section mismatch for xen_init_time_ops()

[Boris Ostrovsky]

On 12/23/2017 09:50 PM, Nick Desaulniers wrote:
> The header declares this function as __init but is defined in __ref
> section.
>
> Signed-off-by: Nick Desaulniers 

AFAIK section attributes in header files are ignored by compiler anyway
so I'd remove all of them.

-boris


> ---
>  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 75011b8..3b34745 100644
> --- a/arch/x86/xen/xen-ops.h
> +++ b/arch/x86/xen/xen-ops.h
> @@ -72,7 +72,7 @@ u64 xen_clocksource_read(void);
>  void xen_setup_cpu_clockevents(void);
>  void xen_save_time_memory_area(void);
>  void xen_restore_time_memory_area(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);

Re: [PATCH] x86/xen/time: fix section mismatch for xen_init_time_ops()

[Juergen Gross]

On 24/12/17 03:50, Nick Desaulniers wrote:
> The header declares this function as __init but is defined in __ref
> section.
> 
> Signed-off-by: Nick Desaulniers 

Reviewed-by: Juergen Gross 


Juergen

Re: [PATCH] x86/xen/time: fix section mismatch for xen_init_time_ops()

[Juergen Gross]

On 24/12/17 03:50, Nick Desaulniers wrote:
> The header declares this function as __init but is defined in __ref
> section.
> 
> Signed-off-by: Nick Desaulniers 

Reviewed-by: Juergen Gross 


Juergen

[PATCH] x86/xen/time: fix section mismatch for xen_init_time_ops()

[Nick Desaulniers]

The header declares this function as __init but is defined in __ref
section.

Signed-off-by: Nick Desaulniers 
---
 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 75011b8..3b34745 100644
--- a/arch/x86/xen/xen-ops.h
+++ b/arch/x86/xen/xen-ops.h
@@ -72,7 +72,7 @@ u64 xen_clocksource_read(void);
 void xen_setup_cpu_clockevents(void);
 void xen_save_time_memory_area(void);
 void xen_restore_time_memory_area(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.7.4

[PATCH] x86/xen/time: fix section mismatch for xen_init_time_ops()

[Nick Desaulniers]

The header declares this function as __init but is defined in __ref
section.

Signed-off-by: Nick Desaulniers 
---
 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 75011b8..3b34745 100644
--- a/arch/x86/xen/xen-ops.h
+++ b/arch/x86/xen/xen-ops.h
@@ -72,7 +72,7 @@ u64 xen_clocksource_read(void);
 void xen_setup_cpu_clockevents(void);
 void xen_save_time_memory_area(void);
 void xen_restore_time_memory_area(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.7.4