Re: [PATCH v2 0/2] arm64: Cut rebuild time when changing CONFIG_BLK_DEV_INITRD
On Thu, Oct 25, 2018 at 12:30 PM Mike Rapoport wrote:
>
> On Thu, Oct 25, 2018 at 08:15:15AM -0500, Rob Herring wrote:
> > +Ard
> >
> > On Thu, Oct 25, 2018 at 4:38 AM Mike Rapoport wrote:
> > >
> > > On Wed, Oct 24, 2018 at 02:55:17PM -0500, Rob Herring wrote:
> > > > On Wed, Oct 24, 2018 at 2:33 PM Florian Fainelli
> > > > wrote:
> > > > >
> > > > > Hi all,
> > > > >
> > > > > While investigating why ARM64 required a ton of objects to be rebuilt
> > > > > when toggling CONFIG_DEV_BLK_INITRD, it became clear that this was
> > > > > because we define __early_init_dt_declare_initrd() differently and we
> > > > > do
> > > > > that in arch/arm64/include/asm/memory.h which gets included by a fair
> > > > > amount of other header files, and translation units as well.
> > > >
> > > > I scratch my head sometimes as to why some config options rebuild so
> > > > much stuff. One down, ? to go. :)
> > > >
> > > > > Changing the value of CONFIG_DEV_BLK_INITRD is a common thing with
> > > > > build
> > > > > systems that generate two kernels: one with the initramfs and one
> > > > > without. buildroot is one of these build systems, OpenWrt is also
> > > > > another one that does this.
> > > > >
> > > > > This patch series proposes adding an empty initrd.h to satisfy the
> > > > > need
> > > > > for drivers/of/fdt.c to unconditionally include that file, and moves
> > > > > the
> > > > > custom __early_init_dt_declare_initrd() definition away from
> > > > > asm/memory.h
> > > > >
> > > > > This cuts the number of objects rebuilds from 1920 down to 26, so a
> > > > > factor 73 approximately.
> > > > >
> > > > > Apologies for the long CC list, please let me know how you would go
> > > > > about merging that and if another approach would be preferable, e.g:
> > > > > introducing a CONFIG_ARCH_INITRD_BELOW_START_OK Kconfig option or
> > > > > something like that.
> > > >
> > > > There may be a better way as of 4.20 because bootmem is now gone and
> > > > only memblock is used. This should unify what each arch needs to do
> > > > with initrd early. We need the physical address early for memblock
> > > > reserving. Then later on we need the virtual address to access the
> > > > initrd. Perhaps we should just change initrd_start and initrd_end to
> > > > physical addresses (or add 2 new variables would be less invasive and
> > > > allow for different translation than __va()). The sanity checks and
> > > > memblock reserve could also perhaps be moved to a common location.
> > > >
> > > > Alternatively, given arm64 is the only oddball, I'd be fine with an
> > > > "if (IS_ENABLED(CONFIG_ARM64))" condition in the default
> > > > __early_init_dt_declare_initrd as long as we have a path to removing
> > > > it like the above option.
> > >
> > > I think arm64 does not have to redefine __early_init_dt_declare_initrd().
> > > Something like this might be just all we need (completely untested,
> > > probably it won't even compile):
> > >
> > > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> > > index 9d9582c..e9ca238 100644
> > > --- a/arch/arm64/mm/init.c
> > > +++ b/arch/arm64/mm/init.c
> > > @@ -62,6 +62,9 @@ s64 memstart_addr __ro_after_init = -1;
> > > phys_addr_t arm64_dma_phys_limit __ro_after_init;
> > >
> > > #ifdef CONFIG_BLK_DEV_INITRD
> > > +
> > > +static phys_addr_t initrd_start_phys, initrd_end_phys;
> > > +
> > > static int __init early_initrd(char *p)
> > > {
> > > unsigned long start, size;
> > > @@ -71,8 +74,8 @@ static int __init early_initrd(char *p)
> > > if (*endp == ',') {
> > > size = memparse(endp + 1, NULL);
> > >
> > > - initrd_start = start;
> > > - initrd_end = start + size;
> > > + initrd_start_phys = start;
> > > + initrd_end_phys = end;
> > > }
> > > return 0;
> > > }
> > > @@ -407,14 +410,27 @@ void __init arm64_memblock_init(void)
> > > memblock_add(__pa_symbol(_text), (u64)(_end - _text));
> > > }
> > >
> > > - if (IS_ENABLED(CONFIG_BLK_DEV_INITRD) && initrd_start) {
> > > + if (IS_ENABLED(CONFIG_BLK_DEV_INITRD) &&
> > > + (initrd_start || initrd_start_phys)) {
> > > + /*
> > > +* FIXME: ensure proper precendence between
> > > +* early_initrd and DT when both are present
> >
> > Command line takes precedence, so just reverse the order.
> >
> > > +*/
> > > + if (initrd_start) {
> > > + initrd_start_phys = __phys_to_virt(initrd_start);
> > > + initrd_end_phys = __phys_to_virt(initrd_end);
BTW, I think you meant virt_to_phys() here?
> >
> > AIUI, the original issue was doing the P2V translation was happening
> > too early and the VA could be wrong if the linear range is adjusted.
> > So I don't think this would work.
>
> Probably things have changed since then, but in the current code there is
>
>
Re: [PATCH v2 0/2] arm64: Cut rebuild time when changing CONFIG_BLK_DEV_INITRD
On Thu, Oct 25, 2018 at 08:15:15AM -0500, Rob Herring wrote:
> +Ard
>
> On Thu, Oct 25, 2018 at 4:38 AM Mike Rapoport wrote:
> >
> > On Wed, Oct 24, 2018 at 02:55:17PM -0500, Rob Herring wrote:
> > > On Wed, Oct 24, 2018 at 2:33 PM Florian Fainelli
> > > wrote:
> > > >
> > > > Hi all,
> > > >
> > > > While investigating why ARM64 required a ton of objects to be rebuilt
> > > > when toggling CONFIG_DEV_BLK_INITRD, it became clear that this was
> > > > because we define __early_init_dt_declare_initrd() differently and we do
> > > > that in arch/arm64/include/asm/memory.h which gets included by a fair
> > > > amount of other header files, and translation units as well.
> > >
> > > I scratch my head sometimes as to why some config options rebuild so
> > > much stuff. One down, ? to go. :)
> > >
> > > > Changing the value of CONFIG_DEV_BLK_INITRD is a common thing with build
> > > > systems that generate two kernels: one with the initramfs and one
> > > > without. buildroot is one of these build systems, OpenWrt is also
> > > > another one that does this.
> > > >
> > > > This patch series proposes adding an empty initrd.h to satisfy the need
> > > > for drivers/of/fdt.c to unconditionally include that file, and moves the
> > > > custom __early_init_dt_declare_initrd() definition away from
> > > > asm/memory.h
> > > >
> > > > This cuts the number of objects rebuilds from 1920 down to 26, so a
> > > > factor 73 approximately.
> > > >
> > > > Apologies for the long CC list, please let me know how you would go
> > > > about merging that and if another approach would be preferable, e.g:
> > > > introducing a CONFIG_ARCH_INITRD_BELOW_START_OK Kconfig option or
> > > > something like that.
> > >
> > > There may be a better way as of 4.20 because bootmem is now gone and
> > > only memblock is used. This should unify what each arch needs to do
> > > with initrd early. We need the physical address early for memblock
> > > reserving. Then later on we need the virtual address to access the
> > > initrd. Perhaps we should just change initrd_start and initrd_end to
> > > physical addresses (or add 2 new variables would be less invasive and
> > > allow for different translation than __va()). The sanity checks and
> > > memblock reserve could also perhaps be moved to a common location.
> > >
> > > Alternatively, given arm64 is the only oddball, I'd be fine with an
> > > "if (IS_ENABLED(CONFIG_ARM64))" condition in the default
> > > __early_init_dt_declare_initrd as long as we have a path to removing
> > > it like the above option.
> >
> > I think arm64 does not have to redefine __early_init_dt_declare_initrd().
> > Something like this might be just all we need (completely untested,
> > probably it won't even compile):
> >
> > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> > index 9d9582c..e9ca238 100644
> > --- a/arch/arm64/mm/init.c
> > +++ b/arch/arm64/mm/init.c
> > @@ -62,6 +62,9 @@ s64 memstart_addr __ro_after_init = -1;
> > phys_addr_t arm64_dma_phys_limit __ro_after_init;
> >
> > #ifdef CONFIG_BLK_DEV_INITRD
> > +
> > +static phys_addr_t initrd_start_phys, initrd_end_phys;
> > +
> > static int __init early_initrd(char *p)
> > {
> > unsigned long start, size;
> > @@ -71,8 +74,8 @@ static int __init early_initrd(char *p)
> > if (*endp == ',') {
> > size = memparse(endp + 1, NULL);
> >
> > - initrd_start = start;
> > - initrd_end = start + size;
> > + initrd_start_phys = start;
> > + initrd_end_phys = end;
> > }
> > return 0;
> > }
> > @@ -407,14 +410,27 @@ void __init arm64_memblock_init(void)
> > memblock_add(__pa_symbol(_text), (u64)(_end - _text));
> > }
> >
> > - if (IS_ENABLED(CONFIG_BLK_DEV_INITRD) && initrd_start) {
> > + if (IS_ENABLED(CONFIG_BLK_DEV_INITRD) &&
> > + (initrd_start || initrd_start_phys)) {
> > + /*
> > +* FIXME: ensure proper precendence between
> > +* early_initrd and DT when both are present
>
> Command line takes precedence, so just reverse the order.
>
> > +*/
> > + if (initrd_start) {
> > + initrd_start_phys = __phys_to_virt(initrd_start);
> > + initrd_end_phys = __phys_to_virt(initrd_end);
>
> AIUI, the original issue was doing the P2V translation was happening
> too early and the VA could be wrong if the linear range is adjusted.
> So I don't think this would work.
Probably things have changed since then, but in the current code there is
initrd_start = __phys_to_virt(initrd_start);
and in between only the code related to CONFIG_RANDOMIZE_BASE, so I believe
it's safe to use __phys_to_virt() here as well.
> I suppose you could convert the VA back to a PA before any adjustments
> and then back to a VA again after. But that's kind of hacky. 2 wrongs
> making a right.
>
> > +
Re: [PATCH v2 0/2] arm64: Cut rebuild time when changing CONFIG_BLK_DEV_INITRD
+Ard
On Thu, Oct 25, 2018 at 4:38 AM Mike Rapoport wrote:
>
> On Wed, Oct 24, 2018 at 02:55:17PM -0500, Rob Herring wrote:
> > On Wed, Oct 24, 2018 at 2:33 PM Florian Fainelli
> > wrote:
> > >
> > > Hi all,
> > >
> > > While investigating why ARM64 required a ton of objects to be rebuilt
> > > when toggling CONFIG_DEV_BLK_INITRD, it became clear that this was
> > > because we define __early_init_dt_declare_initrd() differently and we do
> > > that in arch/arm64/include/asm/memory.h which gets included by a fair
> > > amount of other header files, and translation units as well.
> >
> > I scratch my head sometimes as to why some config options rebuild so
> > much stuff. One down, ? to go. :)
> >
> > > Changing the value of CONFIG_DEV_BLK_INITRD is a common thing with build
> > > systems that generate two kernels: one with the initramfs and one
> > > without. buildroot is one of these build systems, OpenWrt is also
> > > another one that does this.
> > >
> > > This patch series proposes adding an empty initrd.h to satisfy the need
> > > for drivers/of/fdt.c to unconditionally include that file, and moves the
> > > custom __early_init_dt_declare_initrd() definition away from
> > > asm/memory.h
> > >
> > > This cuts the number of objects rebuilds from 1920 down to 26, so a
> > > factor 73 approximately.
> > >
> > > Apologies for the long CC list, please let me know how you would go
> > > about merging that and if another approach would be preferable, e.g:
> > > introducing a CONFIG_ARCH_INITRD_BELOW_START_OK Kconfig option or
> > > something like that.
> >
> > There may be a better way as of 4.20 because bootmem is now gone and
> > only memblock is used. This should unify what each arch needs to do
> > with initrd early. We need the physical address early for memblock
> > reserving. Then later on we need the virtual address to access the
> > initrd. Perhaps we should just change initrd_start and initrd_end to
> > physical addresses (or add 2 new variables would be less invasive and
> > allow for different translation than __va()). The sanity checks and
> > memblock reserve could also perhaps be moved to a common location.
> >
> > Alternatively, given arm64 is the only oddball, I'd be fine with an
> > "if (IS_ENABLED(CONFIG_ARM64))" condition in the default
> > __early_init_dt_declare_initrd as long as we have a path to removing
> > it like the above option.
>
> I think arm64 does not have to redefine __early_init_dt_declare_initrd().
> Something like this might be just all we need (completely untested,
> probably it won't even compile):
>
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 9d9582c..e9ca238 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -62,6 +62,9 @@ s64 memstart_addr __ro_after_init = -1;
> phys_addr_t arm64_dma_phys_limit __ro_after_init;
>
> #ifdef CONFIG_BLK_DEV_INITRD
> +
> +static phys_addr_t initrd_start_phys, initrd_end_phys;
> +
> static int __init early_initrd(char *p)
> {
> unsigned long start, size;
> @@ -71,8 +74,8 @@ static int __init early_initrd(char *p)
> if (*endp == ',') {
> size = memparse(endp + 1, NULL);
>
> - initrd_start = start;
> - initrd_end = start + size;
> + initrd_start_phys = start;
> + initrd_end_phys = end;
> }
> return 0;
> }
> @@ -407,14 +410,27 @@ void __init arm64_memblock_init(void)
> memblock_add(__pa_symbol(_text), (u64)(_end - _text));
> }
>
> - if (IS_ENABLED(CONFIG_BLK_DEV_INITRD) && initrd_start) {
> + if (IS_ENABLED(CONFIG_BLK_DEV_INITRD) &&
> + (initrd_start || initrd_start_phys)) {
> + /*
> +* FIXME: ensure proper precendence between
> +* early_initrd and DT when both are present
Command line takes precedence, so just reverse the order.
> +*/
> + if (initrd_start) {
> + initrd_start_phys = __phys_to_virt(initrd_start);
> + initrd_end_phys = __phys_to_virt(initrd_end);
AIUI, the original issue was doing the P2V translation was happening
too early and the VA could be wrong if the linear range is adjusted.
So I don't think this would work.
I suppose you could convert the VA back to a PA before any adjustments
and then back to a VA again after. But that's kind of hacky. 2 wrongs
making a right.
> + } else if (initrd_start_phys) {
> + initrd_start = __va(initrd_start_phys);
> + initrd_end = __va(initrd_start_phys);
> + }
> +
> /*
> * Add back the memory we just removed if it results in the
> * initrd to become inaccessible via the linear mapping.
> * Otherwise, this is a no-op
> */
> - u64 base = initrd_start & PAGE_MASK;
> - u64 size =
Re: [PATCH 1/4] treewide: remove unused address argument from pte_alloc functions (v2)
On Wed, Oct 24, 2018 at 10:37:16AM +0200, Peter Zijlstra wrote:
> On Fri, Oct 12, 2018 at 06:31:57PM -0700, Joel Fernandes (Google) wrote:
> > This series speeds up mremap(2) syscall by copying page tables at the
> > PMD level even for non-THP systems. There is concern that the extra
> > 'address' argument that mremap passes to pte_alloc may do something
> > subtle architecture related in the future that may make the scheme not
> > work. Also we find that there is no point in passing the 'address' to
> > pte_alloc since its unused. So this patch therefore removes this
> > argument tree-wide resulting in a nice negative diff as well. Also
> > ensuring along the way that the enabled architectures do not do anything
> > funky with 'address' argument that goes unnoticed by the optimization.
>
> Did you happen to look at the history of where that address argument
> came from? -- just being curious here. ISTR something vague about
> architectures having different paging structure for different memory
> ranges.
I see some archicetures (i.e. sparc and, I believe power) used the address
for coloring. It's not needed anymore. Page allocator and SL?B are good
enough now.
See 3c936465249f ("[SPARC64]: Kill pgtable quicklists and use SLAB.")
--
Kirill A. Shutemov
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v2 0/2] arm64: Cut rebuild time when changing CONFIG_BLK_DEV_INITRD
On Thu, Oct 25, 2018 at 10:38:34AM +0100, Mike Rapoport wrote: > On Wed, Oct 24, 2018 at 02:55:17PM -0500, Rob Herring wrote: > > On Wed, Oct 24, 2018 at 2:33 PM Florian Fainelli > > wrote: > > > > > > Hi all, > > > > > > While investigating why ARM64 required a ton of objects to be rebuilt > > > when toggling CONFIG_DEV_BLK_INITRD, it became clear that this was > > > because we define __early_init_dt_declare_initrd() differently and we do > > > that in arch/arm64/include/asm/memory.h which gets included by a fair > > > amount of other header files, and translation units as well. > > > > I scratch my head sometimes as to why some config options rebuild so > > much stuff. One down, ? to go. :) > > > > > Changing the value of CONFIG_DEV_BLK_INITRD is a common thing with build > > > systems that generate two kernels: one with the initramfs and one > > > without. buildroot is one of these build systems, OpenWrt is also > > > another one that does this. > > > > > > This patch series proposes adding an empty initrd.h to satisfy the need > > > for drivers/of/fdt.c to unconditionally include that file, and moves the > > > custom __early_init_dt_declare_initrd() definition away from > > > asm/memory.h > > > > > > This cuts the number of objects rebuilds from 1920 down to 26, so a > > > factor 73 approximately. > > > > > > Apologies for the long CC list, please let me know how you would go > > > about merging that and if another approach would be preferable, e.g: > > > introducing a CONFIG_ARCH_INITRD_BELOW_START_OK Kconfig option or > > > something like that. > > > > There may be a better way as of 4.20 because bootmem is now gone and > > only memblock is used. This should unify what each arch needs to do > > with initrd early. We need the physical address early for memblock > > reserving. Then later on we need the virtual address to access the > > initrd. Perhaps we should just change initrd_start and initrd_end to > > physical addresses (or add 2 new variables would be less invasive and > > allow for different translation than __va()). The sanity checks and > > memblock reserve could also perhaps be moved to a common location. > > > > Alternatively, given arm64 is the only oddball, I'd be fine with an > > "if (IS_ENABLED(CONFIG_ARM64))" condition in the default > > __early_init_dt_declare_initrd as long as we have a path to removing > > it like the above option. > > I think arm64 does not have to redefine __early_init_dt_declare_initrd(). > Something like this might be just all we need (completely untested, > probably it won't even compile): The alternative solution would be to replace initrd_start/initrd_end with physical address versions of these everywhere - that's what we're passed from DT, it's what 32-bit ARM would prefer, and seemingly what 64-bit ARM would also like as well. Grepping for initrd_start in arch/*/mm shows that there's lots of architectures that have virtual/physical conversions on these, and a number that have obviously been derived from 32-bit ARM's approach (with maintaining a phys_initrd_start variable to simplify things). -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 2/4] mm: speed up mremap by 500x on large regions (v2)
On Wed, Oct 24, 2018 at 07:09:07PM -0700, Joel Fernandes wrote:
> On Wed, Oct 24, 2018 at 03:57:24PM +0300, Kirill A. Shutemov wrote:
> > On Wed, Oct 24, 2018 at 10:57:33PM +1100, Balbir Singh wrote:
> > > On Wed, Oct 24, 2018 at 01:12:56PM +0300, Kirill A. Shutemov wrote:
> > > > On Fri, Oct 12, 2018 at 06:31:58PM -0700, Joel Fernandes (Google) wrote:
> > > > > diff --git a/mm/mremap.c b/mm/mremap.c
> > > > > index 9e68a02a52b1..2fd163cff406 100644
> > > > > --- a/mm/mremap.c
> > > > > +++ b/mm/mremap.c
> > > > > @@ -191,6 +191,54 @@ static void move_ptes(struct vm_area_struct
> > > > > *vma, pmd_t *old_pmd,
> > > > > drop_rmap_locks(vma);
> > > > > }
> > > > >
> > > > > +static bool move_normal_pmd(struct vm_area_struct *vma, unsigned
> > > > > long old_addr,
> > > > > + unsigned long new_addr, unsigned long old_end,
> > > > > + pmd_t *old_pmd, pmd_t *new_pmd, bool *need_flush)
> > > > > +{
> > > > > + spinlock_t *old_ptl, *new_ptl;
> > > > > + struct mm_struct *mm = vma->vm_mm;
> > > > > +
> > > > > + if ((old_addr & ~PMD_MASK) || (new_addr & ~PMD_MASK)
> > > > > + || old_end - old_addr < PMD_SIZE)
> > > > > + return false;
> > > > > +
> > > > > + /*
> > > > > + * The destination pmd shouldn't be established, free_pgtables()
> > > > > + * should have release it.
> > > > > + */
> > > > > + if (WARN_ON(!pmd_none(*new_pmd)))
> > > > > + return false;
> > > > > +
> > > > > + /*
> > > > > + * We don't have to worry about the ordering of src and dst
> > > > > + * ptlocks because exclusive mmap_sem prevents deadlock.
> > > > > + */
> > > > > + old_ptl = pmd_lock(vma->vm_mm, old_pmd);
> > > > > + if (old_ptl) {
> > > >
> > > > How can it ever be false?
>
> Kirill,
> It cannot, you are right. I'll remove the test.
>
> By the way, there are new changes upstream by Linus which flush the TLB
> before releasing the ptlock instead of after. I'm guessing that patch came
> about because of reviews of this patch and someone spotted an issue in the
> existing code :)
>
> Anyway the patch in concern is:
> eb66ae030829 ("mremap: properly flush TLB before releasing the page")
>
> I need to rebase on top of that with appropriate modifications, but I worry
> that this patch will slow down performance since we have to flush at every
> PMD/PTE move before releasing the ptlock. Where as with my patch, the
> intention is to flush only at once in the end of move_page_tables. When I
> tried to flush TLB on every PMD move, it was quite slow on my arm64 device
> [2].
>
> Further observation [1] is, it seems like the move_huge_pmds and move_ptes
> code
> is a bit sub optimal in the sense, we are acquiring and releasing the same
> ptlock for a bunch of PMDs if the said PMDs are on the same page-table page
> right? Instead we can do better by acquiring and release the ptlock less
> often.
>
> I think this observation [1] and the frequent TLB flush issue [2] can be
> solved
> by acquiring the ptlock once for a bunch of PMDs, move them all, then flush
> the tlb and then release the ptlock, and then proceed to doing the same thing
> for the PMDs in the next page-table page. What do you think?
Yeah, that's viable optimization.
The tricky part is that one PMD page table can have PMD entires of
different types: THP, page table that you can move as whole and the one
that you cannot (for any reason).
If we cannot move the PMD entry as a whole and must go to PTE page table
we would need to drop PMD ptl and take PTE ptl (it might be the same lock
in some configuations).
Also we don't want to take PMD lock unless it's required.
I expect it to be not very trivial to get everything right. But take a
shot :)
--
Kirill A. Shutemov
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v2 0/2] arm64: Cut rebuild time when changing CONFIG_BLK_DEV_INITRD
On Wed, Oct 24, 2018 at 02:55:17PM -0500, Rob Herring wrote:
> On Wed, Oct 24, 2018 at 2:33 PM Florian Fainelli wrote:
> >
> > Hi all,
> >
> > While investigating why ARM64 required a ton of objects to be rebuilt
> > when toggling CONFIG_DEV_BLK_INITRD, it became clear that this was
> > because we define __early_init_dt_declare_initrd() differently and we do
> > that in arch/arm64/include/asm/memory.h which gets included by a fair
> > amount of other header files, and translation units as well.
>
> I scratch my head sometimes as to why some config options rebuild so
> much stuff. One down, ? to go. :)
>
> > Changing the value of CONFIG_DEV_BLK_INITRD is a common thing with build
> > systems that generate two kernels: one with the initramfs and one
> > without. buildroot is one of these build systems, OpenWrt is also
> > another one that does this.
> >
> > This patch series proposes adding an empty initrd.h to satisfy the need
> > for drivers/of/fdt.c to unconditionally include that file, and moves the
> > custom __early_init_dt_declare_initrd() definition away from
> > asm/memory.h
> >
> > This cuts the number of objects rebuilds from 1920 down to 26, so a
> > factor 73 approximately.
> >
> > Apologies for the long CC list, please let me know how you would go
> > about merging that and if another approach would be preferable, e.g:
> > introducing a CONFIG_ARCH_INITRD_BELOW_START_OK Kconfig option or
> > something like that.
>
> There may be a better way as of 4.20 because bootmem is now gone and
> only memblock is used. This should unify what each arch needs to do
> with initrd early. We need the physical address early for memblock
> reserving. Then later on we need the virtual address to access the
> initrd. Perhaps we should just change initrd_start and initrd_end to
> physical addresses (or add 2 new variables would be less invasive and
> allow for different translation than __va()). The sanity checks and
> memblock reserve could also perhaps be moved to a common location.
>
> Alternatively, given arm64 is the only oddball, I'd be fine with an
> "if (IS_ENABLED(CONFIG_ARM64))" condition in the default
> __early_init_dt_declare_initrd as long as we have a path to removing
> it like the above option.
I think arm64 does not have to redefine __early_init_dt_declare_initrd().
Something like this might be just all we need (completely untested,
probably it won't even compile):
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 9d9582c..e9ca238 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -62,6 +62,9 @@ s64 memstart_addr __ro_after_init = -1;
phys_addr_t arm64_dma_phys_limit __ro_after_init;
#ifdef CONFIG_BLK_DEV_INITRD
+
+static phys_addr_t initrd_start_phys, initrd_end_phys;
+
static int __init early_initrd(char *p)
{
unsigned long start, size;
@@ -71,8 +74,8 @@ static int __init early_initrd(char *p)
if (*endp == ',') {
size = memparse(endp + 1, NULL);
- initrd_start = start;
- initrd_end = start + size;
+ initrd_start_phys = start;
+ initrd_end_phys = end;
}
return 0;
}
@@ -407,14 +410,27 @@ void __init arm64_memblock_init(void)
memblock_add(__pa_symbol(_text), (u64)(_end - _text));
}
- if (IS_ENABLED(CONFIG_BLK_DEV_INITRD) && initrd_start) {
+ if (IS_ENABLED(CONFIG_BLK_DEV_INITRD) &&
+ (initrd_start || initrd_start_phys)) {
+ /*
+* FIXME: ensure proper precendence between
+* early_initrd and DT when both are present
+*/
+ if (initrd_start) {
+ initrd_start_phys = __phys_to_virt(initrd_start);
+ initrd_end_phys = __phys_to_virt(initrd_end);
+ } else if (initrd_start_phys) {
+ initrd_start = __va(initrd_start_phys);
+ initrd_end = __va(initrd_start_phys);
+ }
+
/*
* Add back the memory we just removed if it results in the
* initrd to become inaccessible via the linear mapping.
* Otherwise, this is a no-op
*/
- u64 base = initrd_start & PAGE_MASK;
- u64 size = PAGE_ALIGN(initrd_end) - base;
+ u64 base = initrd_start_phys & PAGE_MASK;
+ u64 size = PAGE_ALIGN(initrd_end_phys) - base;
/*
* We can only add back the initrd memory if we don't end up
@@ -458,7 +474,7 @@ void __init arm64_memblock_init(void)
* pagetables with memblock.
*/
memblock_reserve(__pa_symbol(_text), _end - _text);
-#ifdef CONFIG_BLK_DEV_INITRD
+#if 0
if (initrd_start) {
memblock_reserve(initrd_start, initrd_end - initrd_start);
> Rob
>
--
Sincerely yours,
Mike.
___
linux-snps-arc
