[PATCH 2/8] arch/x86/xen/irq: Cleanup code/data sections definitions

[PATCH 2/8] arch/x86/xen/irq: Cleanup code/data sections definitions

[Daniel Kiper]

Cleanup code/data sections definitions
accordingly to include/linux/init.h.

Signed-off-by: Daniel Kiper <dkiper@net-space.pl>
---
 arch/x86/xen/irq.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/xen/irq.c b/arch/x86/xen/irq.c
index 6a6fe89..8bbb465 100644
--- a/arch/x86/xen/irq.c
+++ b/arch/x86/xen/irq.c
@@ -113,7 +113,7 @@ static void xen_halt(void)
 		xen_safe_halt();
 }
 
-static const struct pv_irq_ops xen_irq_ops __initdata = {
+static const struct pv_irq_ops xen_irq_ops __initconst = {
 	.save_fl = PV_CALLEE_SAVE(xen_save_fl),
 	.restore_fl = PV_CALLEE_SAVE(xen_restore_fl),
 	.irq_disable = PV_CALLEE_SAVE(xen_irq_disable),
-- 
1.5.6.5

Re: [Xen-devel] [PATCH 2/8] arch/x86/xen/irq: Cleanup code/data sections definitions

[Jan Beulich]

>>> On 04.05.11 at 20:16, Daniel Kiper <dkiper@net-space.pl> wrote:
> Cleanup code/data sections definitions
> accordingly to include/linux/init.h.

I'm not clear what the motivation for this patch series is in the first
place, but I'm particularly unhappy with following inconsistent
guidelines: The placement suggestion for function and data
definitions are not in sync - the annotation is said to go between
type and name for functions, but after the name for data, yet
there's no reason known to me why the rule for data can't be the
same as that for functions (the other way around doesn't work,
as gcc's documentation says that for function definitions (other
than for their declarations) attributes cannot currently follow the
declarator.

So I'd rather think the guidelines should first be made consistent
(or it should be explained why they must be different), and then
the users of those annotations should get updated.

Jan

> Signed-off-by: Daniel Kiper <dkiper@net-space.pl>
> ---
>  arch/x86/xen/irq.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/x86/xen/irq.c b/arch/x86/xen/irq.c
> index 6a6fe89..8bbb465 100644
> --- a/arch/x86/xen/irq.c
> +++ b/arch/x86/xen/irq.c
> @@ -113,7 +113,7 @@ static void xen_halt(void)
>  		xen_safe_halt();
>  }
>  
> -static const struct pv_irq_ops xen_irq_ops __initdata = {
> +static const struct pv_irq_ops xen_irq_ops __initconst = {
>  	.save_fl = PV_CALLEE_SAVE(xen_save_fl),
>  	.restore_fl = PV_CALLEE_SAVE(xen_restore_fl),
>  	.irq_disable = PV_CALLEE_SAVE(xen_irq_disable),

Re: [PATCH 2/8] arch/x86/xen/irq: Cleanup code/data sections definitions

[Daniel Kiper]

On Thu, May 05, 2011 at 08:31:46AM +0100, Jan Beulich wrote:
> >>> On 04.05.11 at 20:16, Daniel Kiper <dkiper@net-space.pl> wrote:
> > Cleanup code/data sections definitions
> > accordingly to include/linux/init.h.
>
> I'm not clear what the motivation for this patch series is in the first
> place, but I'm particularly unhappy with following inconsistent

The main reason was introduction of git commit a38647837a411f7df79623128421eef2118b5884
(xen/mmu: Add workaround "x86-64, mm: Put early page table high") which
revealed section conflict in arch/x86/xen/mmu.c. Section for
static const struct pv_mmu_ops xen_mmu_ops was set incorrectly to
__initdata. When

static __initdata u64 __last_pgt_set_rw = 0;

was added then it conflicted with xen_mmu_ops definition.
After some investigation I found that Xen kernel code contains
more those type of sections mismatches (const data moved to
__initdata) and inconsistency with include/linux/init.h.
That is why I thought it is worth to clean up that problems.

It should be mentioned that git commit a38647837a411f7df79623128421eef2118b5884
will be removed from next rc, however, it does not invalidate my cleanup
patches.

> guidelines: The placement suggestion for function and data
> definitions are not in sync - the annotation is said to go between
> type and name for functions, but after the name for data, yet
> there's no reason known to me why the rule for data can't be the
> same as that for functions (the other way around doesn't work,
> as gcc's documentation says that for function definitions (other
> than for their declarations) attributes cannot currently follow the
> declarator.
>
> So I'd rather think the guidelines should first be made consistent
> (or it should be explained why they must be different), and then
> the users of those annotations should get updated.

I looked for any explanation, however, I did not find anything strong.
Probably it was done to be in sync with GCC documentation (despite that
__attribute__ in variable definition could be used virtually anywhere).
Please look into:

http://gcc.gnu.org/onlinedocs/gcc-2.95.3/gcc_4.html#SEC90
http://gcc.gnu.org/onlinedocs/gcc-3.3.1/gcc/Variable-Attributes.html
http://gcc.gnu.org/onlinedocs/gcc/Variable-Attributes.html

Daniel