Re: [Xen-devel] [PATCH 2/4] build: Alloc space for sched list in the link file
On 18/12/15 17:11, Jonathan Creekmore wrote: >> On Dec 18, 2015, at 11:07 AM, Jan Beulich wrote: >> > On 18.12.15 at 17:48, wrote: >>> On 18/12/15 16:40, Jonathan Creekmore wrote: > On Dec 18, 2015, at 3:01 AM, Andrew Cooper > wrote: > > On 17/12/2015 20:59, Jonathan Creekmore wrote: >> Creates a section to contain scheduler entry pointers that are gathered >> together into an array. This will allow, in a follow-on patch, scheduler >> entries to be automatically gathered together into the array for >> automatic parsing. >> >> CC: Ian Campbell >> CC: Stefano Stabellini >> CC: Keir Fraser >> CC: Jan Beulich >> CC: Andrew Cooper >> Signed-off-by: Jonathan Creekmore >> --- >> xen/arch/arm/xen.lds.S | 4 >> xen/arch/x86/xen.lds.S | 4 >> 2 files changed, 8 insertions(+) >> >> diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S >> index 0488f37..39a4c86 100644 >> --- a/xen/arch/arm/xen.lds.S >> +++ b/xen/arch/arm/xen.lds.S >> @@ -57,6 +57,10 @@ SECTIONS >> . = ALIGN(PAGE_SIZE); >> *(.data.page_aligned) >> *(.data) >> + . = ALIGN(8); >> + __schedulers_start = .; >> + *(.data.schedulers) >> + __schedulers_end = .; > These arrays are only ever used in __init scheduler_init(). They should > be in .init.data rather than .data, which allows their memory to be > reclaimed after boot. > > With that, Reviewed-by: Andrew Cooper So, they are used in scheduler_init() which is marked __init, but >>> scheduler_alloc also uses that array (and did before my patch) and it is not marked __init. >>> Ah yes - so they are. Apologies for the noise. This should be in .data >>> and my R-b stands. >> In .rodata perhaps? > I initially put it in .rodata, but the current algorithm for choosing the > scheduler NULLs out > the pointers in the array if the global_init() function fails. To emulate > that behavior, I had to > move the section to .data instead of .rodata. Hmm. That is unfortunate. Best to leave it in .data, and leave fixing the scheduler initialisation to the other scheduler clean-up work; there is plenty of other cleanup work to do, and conflating the two issues is best avoided. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 2/4] build: Alloc space for sched list in the link file
> On Dec 18, 2015, at 11:07 AM, Jan Beulich wrote: > On 18.12.15 at 17:48, wrote: >> On 18/12/15 16:40, Jonathan Creekmore wrote: On Dec 18, 2015, at 3:01 AM, Andrew Cooper wrote: On 17/12/2015 20:59, Jonathan Creekmore wrote: > Creates a section to contain scheduler entry pointers that are gathered > together into an array. This will allow, in a follow-on patch, scheduler > entries to be automatically gathered together into the array for > automatic parsing. > > CC: Ian Campbell > CC: Stefano Stabellini > CC: Keir Fraser > CC: Jan Beulich > CC: Andrew Cooper > Signed-off-by: Jonathan Creekmore > --- > xen/arch/arm/xen.lds.S | 4 > xen/arch/x86/xen.lds.S | 4 > 2 files changed, 8 insertions(+) > > diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S > index 0488f37..39a4c86 100644 > --- a/xen/arch/arm/xen.lds.S > +++ b/xen/arch/arm/xen.lds.S > @@ -57,6 +57,10 @@ SECTIONS > . = ALIGN(PAGE_SIZE); > *(.data.page_aligned) > *(.data) > + . = ALIGN(8); > + __schedulers_start = .; > + *(.data.schedulers) > + __schedulers_end = .; These arrays are only ever used in __init scheduler_init(). They should be in .init.data rather than .data, which allows their memory to be reclaimed after boot. With that, Reviewed-by: Andrew Cooper >>> So, they are used in scheduler_init() which is marked __init, but >> scheduler_alloc >>> also uses that array (and did before my patch) and it is not marked __init. >> >> Ah yes - so they are. Apologies for the noise. This should be in .data >> and my R-b stands. > > In .rodata perhaps? I initially put it in .rodata, but the current algorithm for choosing the scheduler NULLs out the pointers in the array if the global_init() function fails. To emulate that behavior, I had to move the section to .data instead of .rodata. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 2/4] build: Alloc space for sched list in the link file
On 18/12/15 17:07, Jan Beulich wrote: On 18.12.15 at 17:48, wrote: >> On 18/12/15 16:40, Jonathan Creekmore wrote: On Dec 18, 2015, at 3:01 AM, Andrew Cooper wrote: On 17/12/2015 20:59, Jonathan Creekmore wrote: > Creates a section to contain scheduler entry pointers that are gathered > together into an array. This will allow, in a follow-on patch, scheduler > entries to be automatically gathered together into the array for > automatic parsing. > > CC: Ian Campbell > CC: Stefano Stabellini > CC: Keir Fraser > CC: Jan Beulich > CC: Andrew Cooper > Signed-off-by: Jonathan Creekmore > --- > xen/arch/arm/xen.lds.S | 4 > xen/arch/x86/xen.lds.S | 4 > 2 files changed, 8 insertions(+) > > diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S > index 0488f37..39a4c86 100644 > --- a/xen/arch/arm/xen.lds.S > +++ b/xen/arch/arm/xen.lds.S > @@ -57,6 +57,10 @@ SECTIONS >. = ALIGN(PAGE_SIZE); >*(.data.page_aligned) >*(.data) > + . = ALIGN(8); > + __schedulers_start = .; > + *(.data.schedulers) > + __schedulers_end = .; These arrays are only ever used in __init scheduler_init(). They should be in .init.data rather than .data, which allows their memory to be reclaimed after boot. With that, Reviewed-by: Andrew Cooper >>> So, they are used in scheduler_init() which is marked __init, but >> scheduler_alloc >>> also uses that array (and did before my patch) and it is not marked __init. >> Ah yes - so they are. Apologies for the noise. This should be in .data >> and my R-b stands. > In .rodata perhaps? Ah yes - they don't need modifying at all. They are just pointers to each of the struct scheduler ops. .rodata it is. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 2/4] build: Alloc space for sched list in the link file
>>> On 18.12.15 at 17:48, wrote: > On 18/12/15 16:40, Jonathan Creekmore wrote: >>> On Dec 18, 2015, at 3:01 AM, Andrew Cooper >>> wrote: >>> >>> On 17/12/2015 20:59, Jonathan Creekmore wrote: Creates a section to contain scheduler entry pointers that are gathered together into an array. This will allow, in a follow-on patch, scheduler entries to be automatically gathered together into the array for automatic parsing. CC: Ian Campbell CC: Stefano Stabellini CC: Keir Fraser CC: Jan Beulich CC: Andrew Cooper Signed-off-by: Jonathan Creekmore --- xen/arch/arm/xen.lds.S | 4 xen/arch/x86/xen.lds.S | 4 2 files changed, 8 insertions(+) diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S index 0488f37..39a4c86 100644 --- a/xen/arch/arm/xen.lds.S +++ b/xen/arch/arm/xen.lds.S @@ -57,6 +57,10 @@ SECTIONS . = ALIGN(PAGE_SIZE); *(.data.page_aligned) *(.data) + . = ALIGN(8); + __schedulers_start = .; + *(.data.schedulers) + __schedulers_end = .; >>> These arrays are only ever used in __init scheduler_init(). They should >>> be in .init.data rather than .data, which allows their memory to be >>> reclaimed after boot. >>> >>> With that, Reviewed-by: Andrew Cooper >> So, they are used in scheduler_init() which is marked __init, but > scheduler_alloc >> also uses that array (and did before my patch) and it is not marked __init. > > Ah yes - so they are. Apologies for the noise. This should be in .data > and my R-b stands. In .rodata perhaps? Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 2/4] build: Alloc space for sched list in the link file
On 18/12/15 16:40, Jonathan Creekmore wrote: >> On Dec 18, 2015, at 3:01 AM, Andrew Cooper wrote: >> >> On 17/12/2015 20:59, Jonathan Creekmore wrote: >>> Creates a section to contain scheduler entry pointers that are gathered >>> together into an array. This will allow, in a follow-on patch, scheduler >>> entries to be automatically gathered together into the array for >>> automatic parsing. >>> >>> CC: Ian Campbell >>> CC: Stefano Stabellini >>> CC: Keir Fraser >>> CC: Jan Beulich >>> CC: Andrew Cooper >>> Signed-off-by: Jonathan Creekmore >>> --- >>> xen/arch/arm/xen.lds.S | 4 >>> xen/arch/x86/xen.lds.S | 4 >>> 2 files changed, 8 insertions(+) >>> >>> diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S >>> index 0488f37..39a4c86 100644 >>> --- a/xen/arch/arm/xen.lds.S >>> +++ b/xen/arch/arm/xen.lds.S >>> @@ -57,6 +57,10 @@ SECTIONS >>>. = ALIGN(PAGE_SIZE); >>>*(.data.page_aligned) >>>*(.data) >>> + . = ALIGN(8); >>> + __schedulers_start = .; >>> + *(.data.schedulers) >>> + __schedulers_end = .; >> These arrays are only ever used in __init scheduler_init(). They should >> be in .init.data rather than .data, which allows their memory to be >> reclaimed after boot. >> >> With that, Reviewed-by: Andrew Cooper > So, they are used in scheduler_init() which is marked __init, but > scheduler_alloc > also uses that array (and did before my patch) and it is not marked __init. > Ah yes - so they are. Apologies for the noise. This should be in .data and my R-b stands. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 2/4] build: Alloc space for sched list in the link file
> On Dec 18, 2015, at 3:01 AM, Andrew Cooper wrote: > > On 17/12/2015 20:59, Jonathan Creekmore wrote: >> Creates a section to contain scheduler entry pointers that are gathered >> together into an array. This will allow, in a follow-on patch, scheduler >> entries to be automatically gathered together into the array for >> automatic parsing. >> >> CC: Ian Campbell >> CC: Stefano Stabellini >> CC: Keir Fraser >> CC: Jan Beulich >> CC: Andrew Cooper >> Signed-off-by: Jonathan Creekmore >> --- >> xen/arch/arm/xen.lds.S | 4 >> xen/arch/x86/xen.lds.S | 4 >> 2 files changed, 8 insertions(+) >> >> diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S >> index 0488f37..39a4c86 100644 >> --- a/xen/arch/arm/xen.lds.S >> +++ b/xen/arch/arm/xen.lds.S >> @@ -57,6 +57,10 @@ SECTIONS >>. = ALIGN(PAGE_SIZE); >>*(.data.page_aligned) >>*(.data) >> + . = ALIGN(8); >> + __schedulers_start = .; >> + *(.data.schedulers) >> + __schedulers_end = .; > > These arrays are only ever used in __init scheduler_init(). They should > be in .init.data rather than .data, which allows their memory to be > reclaimed after boot. > > With that, Reviewed-by: Andrew Cooper So, they are used in scheduler_init() which is marked __init, but scheduler_alloc also uses that array (and did before my patch) and it is not marked __init. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 2/4] build: Alloc space for sched list in the link file
On 17/12/2015 20:59, Jonathan Creekmore wrote: > Creates a section to contain scheduler entry pointers that are gathered > together into an array. This will allow, in a follow-on patch, scheduler > entries to be automatically gathered together into the array for > automatic parsing. > > CC: Ian Campbell > CC: Stefano Stabellini > CC: Keir Fraser > CC: Jan Beulich > CC: Andrew Cooper > Signed-off-by: Jonathan Creekmore > --- > xen/arch/arm/xen.lds.S | 4 > xen/arch/x86/xen.lds.S | 4 > 2 files changed, 8 insertions(+) > > diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S > index 0488f37..39a4c86 100644 > --- a/xen/arch/arm/xen.lds.S > +++ b/xen/arch/arm/xen.lds.S > @@ -57,6 +57,10 @@ SECTIONS > . = ALIGN(PAGE_SIZE); > *(.data.page_aligned) > *(.data) > + . = ALIGN(8); > + __schedulers_start = .; > + *(.data.schedulers) > + __schedulers_end = .; These arrays are only ever used in __init scheduler_init(). They should be in .init.data rather than .data, which allows their memory to be reclaimed after boot. With that, Reviewed-by: Andrew Cooper ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH 2/4] build: Alloc space for sched list in the link file
Creates a section to contain scheduler entry pointers that are gathered together into an array. This will allow, in a follow-on patch, scheduler entries to be automatically gathered together into the array for automatic parsing. CC: Ian Campbell CC: Stefano Stabellini CC: Keir Fraser CC: Jan Beulich CC: Andrew Cooper Signed-off-by: Jonathan Creekmore --- xen/arch/arm/xen.lds.S | 4 xen/arch/x86/xen.lds.S | 4 2 files changed, 8 insertions(+) diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S index 0488f37..39a4c86 100644 --- a/xen/arch/arm/xen.lds.S +++ b/xen/arch/arm/xen.lds.S @@ -57,6 +57,10 @@ SECTIONS . = ALIGN(PAGE_SIZE); *(.data.page_aligned) *(.data) + . = ALIGN(8); + __schedulers_start = .; + *(.data.schedulers) + __schedulers_end = .; *(.data.rel) *(.data.rel.*) CONSTRUCTORS diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S index e18e08f..70caf85 100644 --- a/xen/arch/x86/xen.lds.S +++ b/xen/arch/x86/xen.lds.S @@ -88,6 +88,10 @@ SECTIONS . = ALIGN(PAGE_SIZE); *(.data.page_aligned) *(.data) + . = ALIGN(8); + __schedulers_start = .; + *(.data.schedulers) + __schedulers_end = .; *(.data.rel) *(.data.rel.*) CONSTRUCTORS -- 2.6.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
