From: Baoquan He <bhe@redhat.com>
To: "Leizhen (ThunderTown)" <thunder.leizhen@huawei.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
linux-kernel@vger.kernel.org, horms@kernel.org,
John.p.donnelly@oracle.com, will@kernel.org,
kexec@lists.infradead.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v4] arm64: kdump: simplify the reservation behaviour of crashkernel=,high
Date: Fri, 24 Mar 2023 22:53:12 +0800 [thread overview]
Message-ID: <ZB25WN02AHOS8RTT@MiWiFi-R3L-srv> (raw)
In-Reply-To: <4d4ecdd6-9716-570d-5595-e47bfbb58cdf@huawei.com>
Hi Leizhen,
On 03/24/23 at 10:47am, Leizhen (ThunderTown) wrote:
......
> >>>> 2) with the fixed CRASH_ADDR_LOW_MAX as 4G, we can easily fix the
> >>>> problem of base page mapping for the whole linear mapping if crsahkernel=
> >>>> is set in kernel parameter shown in [1] at bottom.
> >>>
> >>> That's a different problem ;). I should re-read that thread, forgot most
> >>> of the details but I recall one of the counter arguments was that there
> >>> isn't a strong case to unmap the crashkernel reservation. Now, if we
> >>> place crashdump kernel image goes in the 'high' reservation, can we not
> >>> leave the 'low' reservation mapped? We don't really care about it as it
> >>> wouldn't have any meaningful code/data to be preserved. If the 'high'
> >>> one goes above 4G always, we don't depend on the arm64_dma_phys_limit.
> >>
> >> Yes, this looks ideal. While it only works when crashkernel=,high case and
> >> it succeeds to reserve a memory region for the specified size of crashkernel
> >> high memory. At below, we have 4 cases of crashkernel= syntax:
> >>
> >> crashkernel=size
> >> 1)first attempt: low memory under arm64_dma_phys_limit
> >> 2)fallback: finding memory above 4G
> >
> > (2) should be 'finding memory above arm64_dma_phys_limit' to keep the
> > current behaviour for RPi4.
> >
> >> crashkernel=size,high
> >> 3)first attempt: finding memory above 4G
> >> 4)fallback: low memory under arm64_dma_phys_limit
> >
> > Yes.
> >
> >> case 3) works with your suggestion. However, 1), 2), 4) all need to
> >> defer to bootmem_init(). With these cases and different handling,
> >> reserve_crashkernel() could be too complicated.
> >
> > Ah, because of the fallback below arm64_dma_phys_limit as in (4), we
> > still can't move the full crashkernel reservation early. Well, we could
> > do it in two steps: (a) early attempt at crashkernel reservation above
> > 4G if 'high' was specified and we avoid mapping it if successful and (b)
> > do the late crashkernel reservation below arm64_dma_phys_limit and skip
> > unmapping as being too late. This way most server-like platforms would
> > get a reservation above 4G, unmapped.
> >
> >> I am wondering if we can cancel the protection of crashkernel memory
> >> region on arm64 for now. In earlier discussion, people questioned if the
> >> protection is necessary on arm64. After comparison, I would rather take
> >> away the protection method of crashkernel region since they try to
> >> protect in a chance in one million , while the base page mapping for the
> >> whole linear mapping is mitigating arm64 high end server always.
> >
> > This works for me. We can add the protection later for addresses above
> > 4GB only as mentioned above.
>
> Recently, I've also been rethinking the performance issues when kdump is
> enabled. I have a new idea. For crashkernel=X, we can temporarily search
> for free memory from the low address to the high address. As below:
>
> save_bottom_up = memblock_bottom_up();
> if (!high)
> memblock_set_bottom_up(true);
> crash_base = memblock_phys_alloc_range(crash_size, CRASH_ALIGN, crash_base, crash_max);
> memblock_set_bottom_up(save_bottom_up);
>
> The final code change should be small, and I'll try it today.
I have sent a patchset to remove the crashkernel region protection code
as per Catalin's confirmation. I personally like the code conciseness w/o
protection because kinds of crahskernel reservation has been complex,
the situation on arm64 will makes it worse if we try to keep the
protection and fix the performance issue. While I am glad to see any
attempt to achieve the two goals if it's satisfactory.
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
WARNING: multiple messages have this Message-ID (diff)
From: Baoquan He <bhe@redhat.com>
To: "Leizhen (ThunderTown)" <thunder.leizhen@huawei.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
linux-kernel@vger.kernel.org, horms@kernel.org,
John.p.donnelly@oracle.com, will@kernel.org,
kexec@lists.infradead.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v4] arm64: kdump: simplify the reservation behaviour of crashkernel=,high
Date: Fri, 24 Mar 2023 22:53:12 +0800 [thread overview]
Message-ID: <ZB25WN02AHOS8RTT@MiWiFi-R3L-srv> (raw)
In-Reply-To: <4d4ecdd6-9716-570d-5595-e47bfbb58cdf@huawei.com>
Hi Leizhen,
On 03/24/23 at 10:47am, Leizhen (ThunderTown) wrote:
......
> >>>> 2) with the fixed CRASH_ADDR_LOW_MAX as 4G, we can easily fix the
> >>>> problem of base page mapping for the whole linear mapping if crsahkernel=
> >>>> is set in kernel parameter shown in [1] at bottom.
> >>>
> >>> That's a different problem ;). I should re-read that thread, forgot most
> >>> of the details but I recall one of the counter arguments was that there
> >>> isn't a strong case to unmap the crashkernel reservation. Now, if we
> >>> place crashdump kernel image goes in the 'high' reservation, can we not
> >>> leave the 'low' reservation mapped? We don't really care about it as it
> >>> wouldn't have any meaningful code/data to be preserved. If the 'high'
> >>> one goes above 4G always, we don't depend on the arm64_dma_phys_limit.
> >>
> >> Yes, this looks ideal. While it only works when crashkernel=,high case and
> >> it succeeds to reserve a memory region for the specified size of crashkernel
> >> high memory. At below, we have 4 cases of crashkernel= syntax:
> >>
> >> crashkernel=size
> >> 1)first attempt: low memory under arm64_dma_phys_limit
> >> 2)fallback: finding memory above 4G
> >
> > (2) should be 'finding memory above arm64_dma_phys_limit' to keep the
> > current behaviour for RPi4.
> >
> >> crashkernel=size,high
> >> 3)first attempt: finding memory above 4G
> >> 4)fallback: low memory under arm64_dma_phys_limit
> >
> > Yes.
> >
> >> case 3) works with your suggestion. However, 1), 2), 4) all need to
> >> defer to bootmem_init(). With these cases and different handling,
> >> reserve_crashkernel() could be too complicated.
> >
> > Ah, because of the fallback below arm64_dma_phys_limit as in (4), we
> > still can't move the full crashkernel reservation early. Well, we could
> > do it in two steps: (a) early attempt at crashkernel reservation above
> > 4G if 'high' was specified and we avoid mapping it if successful and (b)
> > do the late crashkernel reservation below arm64_dma_phys_limit and skip
> > unmapping as being too late. This way most server-like platforms would
> > get a reservation above 4G, unmapped.
> >
> >> I am wondering if we can cancel the protection of crashkernel memory
> >> region on arm64 for now. In earlier discussion, people questioned if the
> >> protection is necessary on arm64. After comparison, I would rather take
> >> away the protection method of crashkernel region since they try to
> >> protect in a chance in one million , while the base page mapping for the
> >> whole linear mapping is mitigating arm64 high end server always.
> >
> > This works for me. We can add the protection later for addresses above
> > 4GB only as mentioned above.
>
> Recently, I've also been rethinking the performance issues when kdump is
> enabled. I have a new idea. For crashkernel=X, we can temporarily search
> for free memory from the low address to the high address. As below:
>
> save_bottom_up = memblock_bottom_up();
> if (!high)
> memblock_set_bottom_up(true);
> crash_base = memblock_phys_alloc_range(crash_size, CRASH_ALIGN, crash_base, crash_max);
> memblock_set_bottom_up(save_bottom_up);
>
> The final code change should be small, and I'll try it today.
I have sent a patchset to remove the crashkernel region protection code
as per Catalin's confirmation. I personally like the code conciseness w/o
protection because kinds of crahskernel reservation has been complex,
the situation on arm64 will makes it worse if we try to keep the
protection and fix the performance issue. While I am glad to see any
attempt to achieve the two goals if it's satisfactory.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: Baoquan He <bhe@redhat.com>
To: "Leizhen (ThunderTown)" <thunder.leizhen@huawei.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
linux-kernel@vger.kernel.org, horms@kernel.org,
John.p.donnelly@oracle.com, will@kernel.org,
kexec@lists.infradead.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v4] arm64: kdump: simplify the reservation behaviour of crashkernel=,high
Date: Fri, 24 Mar 2023 22:53:12 +0800 [thread overview]
Message-ID: <ZB25WN02AHOS8RTT@MiWiFi-R3L-srv> (raw)
In-Reply-To: <4d4ecdd6-9716-570d-5595-e47bfbb58cdf@huawei.com>
Hi Leizhen,
On 03/24/23 at 10:47am, Leizhen (ThunderTown) wrote:
......
> >>>> 2) with the fixed CRASH_ADDR_LOW_MAX as 4G, we can easily fix the
> >>>> problem of base page mapping for the whole linear mapping if crsahkernel=
> >>>> is set in kernel parameter shown in [1] at bottom.
> >>>
> >>> That's a different problem ;). I should re-read that thread, forgot most
> >>> of the details but I recall one of the counter arguments was that there
> >>> isn't a strong case to unmap the crashkernel reservation. Now, if we
> >>> place crashdump kernel image goes in the 'high' reservation, can we not
> >>> leave the 'low' reservation mapped? We don't really care about it as it
> >>> wouldn't have any meaningful code/data to be preserved. If the 'high'
> >>> one goes above 4G always, we don't depend on the arm64_dma_phys_limit.
> >>
> >> Yes, this looks ideal. While it only works when crashkernel=,high case and
> >> it succeeds to reserve a memory region for the specified size of crashkernel
> >> high memory. At below, we have 4 cases of crashkernel= syntax:
> >>
> >> crashkernel=size
> >> 1)first attempt: low memory under arm64_dma_phys_limit
> >> 2)fallback: finding memory above 4G
> >
> > (2) should be 'finding memory above arm64_dma_phys_limit' to keep the
> > current behaviour for RPi4.
> >
> >> crashkernel=size,high
> >> 3)first attempt: finding memory above 4G
> >> 4)fallback: low memory under arm64_dma_phys_limit
> >
> > Yes.
> >
> >> case 3) works with your suggestion. However, 1), 2), 4) all need to
> >> defer to bootmem_init(). With these cases and different handling,
> >> reserve_crashkernel() could be too complicated.
> >
> > Ah, because of the fallback below arm64_dma_phys_limit as in (4), we
> > still can't move the full crashkernel reservation early. Well, we could
> > do it in two steps: (a) early attempt at crashkernel reservation above
> > 4G if 'high' was specified and we avoid mapping it if successful and (b)
> > do the late crashkernel reservation below arm64_dma_phys_limit and skip
> > unmapping as being too late. This way most server-like platforms would
> > get a reservation above 4G, unmapped.
> >
> >> I am wondering if we can cancel the protection of crashkernel memory
> >> region on arm64 for now. In earlier discussion, people questioned if the
> >> protection is necessary on arm64. After comparison, I would rather take
> >> away the protection method of crashkernel region since they try to
> >> protect in a chance in one million , while the base page mapping for the
> >> whole linear mapping is mitigating arm64 high end server always.
> >
> > This works for me. We can add the protection later for addresses above
> > 4GB only as mentioned above.
>
> Recently, I've also been rethinking the performance issues when kdump is
> enabled. I have a new idea. For crashkernel=X, we can temporarily search
> for free memory from the low address to the high address. As below:
>
> save_bottom_up = memblock_bottom_up();
> if (!high)
> memblock_set_bottom_up(true);
> crash_base = memblock_phys_alloc_range(crash_size, CRASH_ALIGN, crash_base, crash_max);
> memblock_set_bottom_up(save_bottom_up);
>
> The final code change should be small, and I'll try it today.
I have sent a patchset to remove the crashkernel region protection code
as per Catalin's confirmation. I personally like the code conciseness w/o
protection because kinds of crahskernel reservation has been complex,
the situation on arm64 will makes it worse if we try to keep the
protection and fix the performance issue. While I am glad to see any
attempt to achieve the two goals if it's satisfactory.
next prev parent reply other threads:[~2023-03-24 14:53 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-06 8:41 [PATCH v4] arm64: kdump: simplify the reservation behaviour of crashkernel=,high Baoquan He
2023-03-06 8:41 ` Baoquan He
2023-03-06 8:41 ` Baoquan He
2023-03-06 12:55 ` Leizhen (ThunderTown)
2023-03-06 12:55 ` Leizhen (ThunderTown)
2023-03-06 12:55 ` Leizhen (ThunderTown)
2023-03-08 11:02 ` Simon Horman
2023-03-08 11:02 ` Simon Horman
2023-03-08 11:02 ` Simon Horman
2023-03-15 14:52 ` Catalin Marinas
2023-03-15 14:52 ` Catalin Marinas
2023-03-15 14:52 ` Catalin Marinas
2023-03-16 9:47 ` Baoquan He
2023-03-16 9:47 ` Baoquan He
2023-03-16 9:47 ` Baoquan He
2023-03-16 17:35 ` Catalin Marinas
2023-03-16 17:35 ` Catalin Marinas
2023-03-16 17:35 ` Catalin Marinas
2023-03-17 15:09 ` Baoquan He
2023-03-17 15:09 ` Baoquan He
2023-03-17 15:09 ` Baoquan He
2023-03-17 18:05 ` Catalin Marinas
2023-03-17 18:05 ` Catalin Marinas
2023-03-17 18:05 ` Catalin Marinas
2023-03-20 13:12 ` Baoquan He
2023-03-20 13:12 ` Baoquan He
2023-03-20 13:12 ` Baoquan He
2023-03-23 17:25 ` Catalin Marinas
2023-03-23 17:25 ` Catalin Marinas
2023-03-23 17:25 ` Catalin Marinas
2023-03-24 2:47 ` Leizhen (ThunderTown)
2023-03-24 2:47 ` Leizhen (ThunderTown)
2023-03-24 2:47 ` Leizhen (ThunderTown)
2023-03-24 14:53 ` Baoquan He [this message]
2023-03-24 14:53 ` Baoquan He
2023-03-24 14:53 ` Baoquan He
2023-03-25 1:53 ` Leizhen (ThunderTown)
2023-03-25 1:53 ` Leizhen (ThunderTown)
2023-03-25 1:53 ` Leizhen (ThunderTown)
2023-03-24 14:08 ` Baoquan He
2023-03-24 14:08 ` Baoquan He
2023-03-24 14:08 ` Baoquan He
2023-03-24 17:08 ` Catalin Marinas
2023-03-24 17:08 ` Catalin Marinas
2023-03-24 17:08 ` Catalin Marinas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZB25WN02AHOS8RTT@MiWiFi-R3L-srv \
--to=bhe@redhat.com \
--cc=John.p.donnelly@oracle.com \
--cc=catalin.marinas@arm.com \
--cc=horms@kernel.org \
--cc=kexec@lists.infradead.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=thunder.leizhen@huawei.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.