[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



On Thu, May 05, 2011 at 08:31:46AM +0100, Jan Beulich wrote:
> >>> On 04.05.11 at 20:16, Daniel Kiper <dkiper@xxxxxxxxxxxx> 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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.