* [PATCH 0/6] kexec: enable kexec_crash_size to support two crash kernel regions
@ 2023-05-27 12:34 Zhen Lei
2023-05-27 12:34 ` [PATCH 1/6] kexec: fix a memory leak in crash_shrink_memory() Zhen Lei
` (5 more replies)
0 siblings, 6 replies; 23+ messages in thread
From: Zhen Lei @ 2023-05-27 12:34 UTC (permalink / raw)
To: Eric Biederman, Baoquan He, kexec, linux-kernel
Cc: Zhen Lei, Michael Holzheu, Andrew Morton, Amerigo Wang
When crashkernel=X fails to reserve region under 4G, it will fall back to
reserve region above 4G and a region of the default size will also be reserved
under 4G. Unfortunately, /sys/kernel/kexec_crash_size only supports one crash
kernel region now, the user cannot sense the low memory reserved by reading
/sys/kernel/kexec_crash_size. Also, low memory cannot be freed by writing this
file.
For example:
resource_size(crashk_res) = 512M
resource_size(crashk_low_res) = 256M
The result of 'cat /sys/kernel/kexec_crash_size' is 512M, but it should be 768M.
When we execute 'echo 0 > /sys/kernel/kexec_crash_size', the size of crashk_res
becomes 0 and resource_size(crashk_low_res) is still 256 MB, which is incorrect.
Since crashk_res manages the memory with high address and crashk_low_res manages
the memory with low address, crashk_low_res is shrunken only when all crashk_res
is shrunken. And because when there is only one crash kernel region, crashk_res
is always used. Therefore, if all crashk_res is shrunken and crashk_low_res still
exists, swap them.
Zhen Lei (6):
kexec: fix a memory leak in crash_shrink_memory()
kexec: delete a useless check in crash_shrink_memory()
kexec: clear crashk_res if all its memory has been released
kexec: improve the readability of crash_shrink_memory()
kexec: add helper __crash_shrink_memory()
kexec: enable kexec_crash_size to support two crash kernel regions
kernel/kexec_core.c | 92 +++++++++++++++++++++++++++++++--------------
1 file changed, 64 insertions(+), 28 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 1/6] kexec: fix a memory leak in crash_shrink_memory()
2023-05-27 12:34 [PATCH 0/6] kexec: enable kexec_crash_size to support two crash kernel regions Zhen Lei
@ 2023-05-27 12:34 ` Zhen Lei
2023-05-31 0:13 ` Baoquan He
2023-05-27 12:34 ` [PATCH 2/6] kexec: delete a useless check " Zhen Lei
` (4 subsequent siblings)
5 siblings, 1 reply; 23+ messages in thread
From: Zhen Lei @ 2023-05-27 12:34 UTC (permalink / raw)
To: Eric Biederman, Baoquan He, kexec, linux-kernel
Cc: Zhen Lei, Michael Holzheu, Andrew Morton, Amerigo Wang
If the value of parameter 'new_size' is in the semi-open and semi-closed
interval (crashk_res.end - KEXEC_CRASH_MEM_ALIGN + 1, crashk_res.end], the
calculation result of ram_res is:
ram_res->start = crashk_res.end + 1
ram_res->end = crashk_res.end
The operation of function insert_resource() fails, and ram_res is not
added to iomem_resource. As a result, the memory of the control block
ram_res is leaked.
In fact, on all architectures, the start address and size of crashk_res
are already aligned by KEXEC_CRASH_MEM_ALIGN. Therefore, we do not need to
round up crashk_res.start again. Instead, we should round up 'new_size'
in advance.
Fixes: 6480e5a09237 ("kdump: add missing RAM resource in crash_shrink_memory()")
Fixes: 06a7f711246b ("kexec: premit reduction of the reserved memory size")
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
kernel/kexec_core.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 3d578c6fefee385..22acee18195a591 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -1122,6 +1122,7 @@ int crash_shrink_memory(unsigned long new_size)
start = crashk_res.start;
end = crashk_res.end;
old_size = (end == 0) ? 0 : end - start + 1;
+ new_size = roundup(new_size, KEXEC_CRASH_MEM_ALIGN);
if (new_size >= old_size) {
ret = (new_size == old_size) ? 0 : -EINVAL;
goto unlock;
@@ -1133,9 +1134,7 @@ int crash_shrink_memory(unsigned long new_size)
goto unlock;
}
- start = roundup(start, KEXEC_CRASH_MEM_ALIGN);
- end = roundup(start + new_size, KEXEC_CRASH_MEM_ALIGN);
-
+ end = start + new_size;
crash_free_reserved_phys_range(end, crashk_res.end);
if ((start == end) && (crashk_res.parent != NULL))
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 2/6] kexec: delete a useless check in crash_shrink_memory()
2023-05-27 12:34 [PATCH 0/6] kexec: enable kexec_crash_size to support two crash kernel regions Zhen Lei
2023-05-27 12:34 ` [PATCH 1/6] kexec: fix a memory leak in crash_shrink_memory() Zhen Lei
@ 2023-05-27 12:34 ` Zhen Lei
2023-05-31 0:17 ` Baoquan He
2023-05-27 12:34 ` [PATCH 3/6] kexec: clear crashk_res if all its memory has been released Zhen Lei
` (3 subsequent siblings)
5 siblings, 1 reply; 23+ messages in thread
From: Zhen Lei @ 2023-05-27 12:34 UTC (permalink / raw)
To: Eric Biederman, Baoquan He, kexec, linux-kernel
Cc: Zhen Lei, Michael Holzheu, Andrew Morton, Amerigo Wang
The check '(crashk_res.parent != NULL)' is added by
commit e05bd3367bd3 ("kexec: fix Oops in crash_shrink_memory()"), but it's
stale now. Because if 'crashk_res' is not reserved, it will be zero in
size and will be intercepted by the above 'if (new_size >= old_size)'.
Ago:
if (new_size >= end - start + 1)
Now:
old_size = (end == 0) ? 0 : end - start + 1;
if (new_size >= old_size)
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
kernel/kexec_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 22acee18195a591..d1ab139dd49035e 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -1137,7 +1137,7 @@ int crash_shrink_memory(unsigned long new_size)
end = start + new_size;
crash_free_reserved_phys_range(end, crashk_res.end);
- if ((start == end) && (crashk_res.parent != NULL))
+ if (start == end)
release_resource(&crashk_res);
ram_res->start = end;
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 3/6] kexec: clear crashk_res if all its memory has been released
2023-05-27 12:34 [PATCH 0/6] kexec: enable kexec_crash_size to support two crash kernel regions Zhen Lei
2023-05-27 12:34 ` [PATCH 1/6] kexec: fix a memory leak in crash_shrink_memory() Zhen Lei
2023-05-27 12:34 ` [PATCH 2/6] kexec: delete a useless check " Zhen Lei
@ 2023-05-27 12:34 ` Zhen Lei
2023-05-31 0:33 ` Baoquan He
2023-05-27 12:34 ` [PATCH 4/6] kexec: improve the readability of crash_shrink_memory() Zhen Lei
` (2 subsequent siblings)
5 siblings, 1 reply; 23+ messages in thread
From: Zhen Lei @ 2023-05-27 12:34 UTC (permalink / raw)
To: Eric Biederman, Baoquan He, kexec, linux-kernel
Cc: Zhen Lei, Michael Holzheu, Andrew Morton, Amerigo Wang
If the resource of crashk_res has been released, it is better to clear
crashk_res.start and crashk_res.end. Because 'end = start - 1' is not
reasonable, and in some places the test is based on crashk_res.end, not
resource_size(&crashk_res).
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
kernel/kexec_core.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index d1ab139dd49035e..bcc86a250ab3bf9 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -1137,15 +1137,18 @@ int crash_shrink_memory(unsigned long new_size)
end = start + new_size;
crash_free_reserved_phys_range(end, crashk_res.end);
- if (start == end)
- release_resource(&crashk_res);
-
ram_res->start = end;
ram_res->end = crashk_res.end;
ram_res->flags = IORESOURCE_BUSY | IORESOURCE_SYSTEM_RAM;
ram_res->name = "System RAM";
- crashk_res.end = end - 1;
+ if (start == end) {
+ release_resource(&crashk_res);
+ crashk_res.start = 0;
+ crashk_res.end = 0;
+ } else {
+ crashk_res.end = end - 1;
+ }
insert_resource(&iomem_resource, ram_res);
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 4/6] kexec: improve the readability of crash_shrink_memory()
2023-05-27 12:34 [PATCH 0/6] kexec: enable kexec_crash_size to support two crash kernel regions Zhen Lei
` (2 preceding siblings ...)
2023-05-27 12:34 ` [PATCH 3/6] kexec: clear crashk_res if all its memory has been released Zhen Lei
@ 2023-05-27 12:34 ` Zhen Lei
2023-05-31 7:48 ` Baoquan He
2023-05-27 12:34 ` [PATCH 5/6] kexec: add helper __crash_shrink_memory() Zhen Lei
2023-05-27 12:34 ` [PATCH 6/6] kexec: enable kexec_crash_size to support two crash kernel regions Zhen Lei
5 siblings, 1 reply; 23+ messages in thread
From: Zhen Lei @ 2023-05-27 12:34 UTC (permalink / raw)
To: Eric Biederman, Baoquan He, kexec, linux-kernel
Cc: Zhen Lei, Michael Holzheu, Andrew Morton, Amerigo Wang
The major adjustments are:
1. end = start + new_size.
The 'end' here is not an accurate representation, because it is not the
new end of crashk_res, but the start of ram_res, difference 1. So
eliminate it and replace it with ram_res->start.
2. Use 'ram_res->start' and 'ram_res->end' as arguments to
crash_free_reserved_phys_range() to indicate that the memory covered by
'ram_res' is released from the crashk. And keep it close to
insert_resource().
3. Replace 'if (start == end)' with 'if (!new_size)', clear indication that
all crashk memory will be shrunken.
No functional change.
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
kernel/kexec_core.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index bcc86a250ab3bf9..69fe92141b0b62d 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -1108,7 +1108,6 @@ ssize_t crash_get_memory_size(void)
int crash_shrink_memory(unsigned long new_size)
{
int ret = 0;
- unsigned long start, end;
unsigned long old_size;
struct resource *ram_res;
@@ -1119,9 +1118,7 @@ int crash_shrink_memory(unsigned long new_size)
ret = -ENOENT;
goto unlock;
}
- start = crashk_res.start;
- end = crashk_res.end;
- old_size = (end == 0) ? 0 : end - start + 1;
+ old_size = !crashk_res.end ? 0 : resource_size(&crashk_res);
new_size = roundup(new_size, KEXEC_CRASH_MEM_ALIGN);
if (new_size >= old_size) {
ret = (new_size == old_size) ? 0 : -EINVAL;
@@ -1134,22 +1131,20 @@ int crash_shrink_memory(unsigned long new_size)
goto unlock;
}
- end = start + new_size;
- crash_free_reserved_phys_range(end, crashk_res.end);
-
- ram_res->start = end;
+ ram_res->start = crashk_res.start + new_size;
ram_res->end = crashk_res.end;
ram_res->flags = IORESOURCE_BUSY | IORESOURCE_SYSTEM_RAM;
ram_res->name = "System RAM";
- if (start == end) {
+ if (!new_size) {
release_resource(&crashk_res);
crashk_res.start = 0;
crashk_res.end = 0;
} else {
- crashk_res.end = end - 1;
+ crashk_res.end = ram_res->start - 1;
}
+ crash_free_reserved_phys_range(ram_res->start, ram_res->end);
insert_resource(&iomem_resource, ram_res);
unlock:
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 5/6] kexec: add helper __crash_shrink_memory()
2023-05-27 12:34 [PATCH 0/6] kexec: enable kexec_crash_size to support two crash kernel regions Zhen Lei
` (3 preceding siblings ...)
2023-05-27 12:34 ` [PATCH 4/6] kexec: improve the readability of crash_shrink_memory() Zhen Lei
@ 2023-05-27 12:34 ` Zhen Lei
2023-05-28 0:08 ` kernel test robot
` (3 more replies)
2023-05-27 12:34 ` [PATCH 6/6] kexec: enable kexec_crash_size to support two crash kernel regions Zhen Lei
5 siblings, 4 replies; 23+ messages in thread
From: Zhen Lei @ 2023-05-27 12:34 UTC (permalink / raw)
To: Eric Biederman, Baoquan He, kexec, linux-kernel
Cc: Zhen Lei, Michael Holzheu, Andrew Morton, Amerigo Wang
No functional change, in preparation for the next patch so that it is
easier to review.
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
kernel/kexec_core.c | 50 +++++++++++++++++++++++++--------------------
1 file changed, 28 insertions(+), 22 deletions(-)
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 69fe92141b0b62d..e82bc6d6634136a 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -1105,11 +1105,37 @@ ssize_t crash_get_memory_size(void)
return size;
}
+int __crash_shrink_memory(struct resource *old_res, unsigned long new_size)
+{
+ struct resource *ram_res;
+
+ ram_res = kzalloc(sizeof(*ram_res), GFP_KERNEL);
+ if (!ram_res)
+ return -ENOMEM;
+
+ ram_res->start = old_res->start + new_size;
+ ram_res->end = old_res->end;
+ ram_res->flags = IORESOURCE_BUSY | IORESOURCE_SYSTEM_RAM;
+ ram_res->name = "System RAM";
+
+ if (!new_size) {
+ release_resource(old_res);
+ old_res->start = 0;
+ old_res->end = 0;
+ } else {
+ crashk_res.end = ram_res->start - 1;
+ }
+
+ crash_free_reserved_phys_range(ram_res->start, ram_res->end);
+ insert_resource(&iomem_resource, ram_res);
+
+ return 0;
+}
+
int crash_shrink_memory(unsigned long new_size)
{
int ret = 0;
unsigned long old_size;
- struct resource *ram_res;
if (!kexec_trylock())
return -EBUSY;
@@ -1125,27 +1151,7 @@ int crash_shrink_memory(unsigned long new_size)
goto unlock;
}
- ram_res = kzalloc(sizeof(*ram_res), GFP_KERNEL);
- if (!ram_res) {
- ret = -ENOMEM;
- goto unlock;
- }
-
- ram_res->start = crashk_res.start + new_size;
- ram_res->end = crashk_res.end;
- ram_res->flags = IORESOURCE_BUSY | IORESOURCE_SYSTEM_RAM;
- ram_res->name = "System RAM";
-
- if (!new_size) {
- release_resource(&crashk_res);
- crashk_res.start = 0;
- crashk_res.end = 0;
- } else {
- crashk_res.end = ram_res->start - 1;
- }
-
- crash_free_reserved_phys_range(ram_res->start, ram_res->end);
- insert_resource(&iomem_resource, ram_res);
+ ret = __crash_shrink_memory(&crashk_res, new_size);
unlock:
kexec_unlock();
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 6/6] kexec: enable kexec_crash_size to support two crash kernel regions
2023-05-27 12:34 [PATCH 0/6] kexec: enable kexec_crash_size to support two crash kernel regions Zhen Lei
` (4 preceding siblings ...)
2023-05-27 12:34 ` [PATCH 5/6] kexec: add helper __crash_shrink_memory() Zhen Lei
@ 2023-05-27 12:34 ` Zhen Lei
2023-05-31 9:53 ` Baoquan He
5 siblings, 1 reply; 23+ messages in thread
From: Zhen Lei @ 2023-05-27 12:34 UTC (permalink / raw)
To: Eric Biederman, Baoquan He, kexec, linux-kernel
Cc: Zhen Lei, Michael Holzheu, Andrew Morton, Amerigo Wang
The crashk_low_res should be considered by /sys/kernel/kexec_crash_size
to support two crash kernel regions. Since crashk_res manages the memory
with high address and crashk_low_res manages the memory with low address,
crashk_low_res is shrunken only when all crashk_res is shrunken. And
because when there is only one crash kernel region, crashk_res is always
used. Therefore, if all crashk_res is shrunken and crashk_low_res still
exists, swap them.
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
kernel/kexec_core.c | 43 ++++++++++++++++++++++++++++++++++++++-----
1 file changed, 38 insertions(+), 5 deletions(-)
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index e82bc6d6634136a..c1d50f6566300d9 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -1091,6 +1091,11 @@ __bpf_kfunc void crash_kexec(struct pt_regs *regs)
}
}
+static inline resource_size_t crash_resource_size(const struct resource *res)
+{
+ return !res->end ? 0 : resource_size(res);
+}
+
ssize_t crash_get_memory_size(void)
{
ssize_t size = 0;
@@ -1098,8 +1103,8 @@ ssize_t crash_get_memory_size(void)
if (!kexec_trylock())
return -EBUSY;
- if (crashk_res.end != crashk_res.start)
- size = resource_size(&crashk_res);
+ size += crash_resource_size(&crashk_res);
+ size += crash_resource_size(&crashk_low_res);
kexec_unlock();
return size;
@@ -1135,7 +1140,7 @@ int __crash_shrink_memory(struct resource *old_res, unsigned long new_size)
int crash_shrink_memory(unsigned long new_size)
{
int ret = 0;
- unsigned long old_size;
+ unsigned long old_size, low_size;
if (!kexec_trylock())
return -EBUSY;
@@ -1144,14 +1149,42 @@ int crash_shrink_memory(unsigned long new_size)
ret = -ENOENT;
goto unlock;
}
- old_size = !crashk_res.end ? 0 : resource_size(&crashk_res);
+
+ low_size = crash_resource_size(&crashk_low_res);
+ old_size = crash_resource_size(&crashk_res) + low_size;
new_size = roundup(new_size, KEXEC_CRASH_MEM_ALIGN);
if (new_size >= old_size) {
ret = (new_size == old_size) ? 0 : -EINVAL;
goto unlock;
}
- ret = __crash_shrink_memory(&crashk_res, new_size);
+ /*
+ * (low_size > new_size) implies that low_size is greater than zero.
+ * This also means that if low_size is zero, the else branch is taken.
+ *
+ * If low_size is greater than 0, (low_size > new_size) indicates that
+ * crashk_low_res also needs to be shrunken. Otherwise, only crashk_res
+ * needs to be shrunken.
+ */
+ if (low_size > new_size) {
+ ret = __crash_shrink_memory(&crashk_res, 0);
+ if (ret)
+ goto unlock;
+
+ ret = __crash_shrink_memory(&crashk_low_res, new_size);
+ } else {
+ ret = __crash_shrink_memory(&crashk_res, new_size - low_size);
+ }
+
+ /* Swap crashk_res and crashk_low_res if needed */
+ if (!crashk_res.end && crashk_low_res.end) {
+ crashk_res.start = crashk_low_res.start;
+ crashk_res.end = crashk_low_res.end;
+ release_resource(&crashk_low_res);
+ crashk_low_res.start = 0;
+ crashk_low_res.end = 0;
+ insert_resource(&iomem_resource, &crashk_res);
+ }
unlock:
kexec_unlock();
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 5/6] kexec: add helper __crash_shrink_memory()
2023-05-27 12:34 ` [PATCH 5/6] kexec: add helper __crash_shrink_memory() Zhen Lei
@ 2023-05-28 0:08 ` kernel test robot
2023-05-29 0:37 ` Leizhen (ThunderTown)
2023-05-28 1:44 ` kernel test robot
` (2 subsequent siblings)
3 siblings, 1 reply; 23+ messages in thread
From: kernel test robot @ 2023-05-28 0:08 UTC (permalink / raw)
To: Zhen Lei, Eric Biederman, Baoquan He, kexec, linux-kernel
Cc: llvm, oe-kbuild-all, Zhen Lei, Michael Holzheu, Andrew Morton,
Linux Memory Management List, Amerigo Wang
Hi Zhen,
kernel test robot noticed the following build warnings:
[auto build test WARNING on akpm-mm/mm-everything]
[also build test WARNING on linus/master v6.4-rc3 next-20230525]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Zhen-Lei/kexec-fix-a-memory-leak-in-crash_shrink_memory/20230527-203821
base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link: https://lore.kernel.org/r/20230527123439.772-6-thunder.leizhen%40huawei.com
patch subject: [PATCH 5/6] kexec: add helper __crash_shrink_memory()
config: riscv-randconfig-r042-20230526 (https://download.01.org/0day-ci/archive/20230528/202305280717.Pw06aLkz-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 4faf3aaf28226a4e950c103a14f6fc1d1fdabb1b)
reproduce (this is a W=1 build):
mkdir -p ~/bin
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/dea97cef503d26e05d0e11818ae44176056ddf64
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Zhen-Lei/kexec-fix-a-memory-leak-in-crash_shrink_memory/20230527-203821
git checkout dea97cef503d26e05d0e11818ae44176056ddf64
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=riscv olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202305280717.Pw06aLkz-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> kernel/kexec_core.c:1108:5: warning: no previous prototype for function '__crash_shrink_memory' [-Wmissing-prototypes]
int __crash_shrink_memory(struct resource *old_res, unsigned long new_size)
^
kernel/kexec_core.c:1108:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
int __crash_shrink_memory(struct resource *old_res, unsigned long new_size)
^
static
1 warning generated.
vim +/__crash_shrink_memory +1108 kernel/kexec_core.c
1107
> 1108 int __crash_shrink_memory(struct resource *old_res, unsigned long new_size)
1109 {
1110 struct resource *ram_res;
1111
1112 ram_res = kzalloc(sizeof(*ram_res), GFP_KERNEL);
1113 if (!ram_res)
1114 return -ENOMEM;
1115
1116 ram_res->start = old_res->start + new_size;
1117 ram_res->end = old_res->end;
1118 ram_res->flags = IORESOURCE_BUSY | IORESOURCE_SYSTEM_RAM;
1119 ram_res->name = "System RAM";
1120
1121 if (!new_size) {
1122 release_resource(old_res);
1123 old_res->start = 0;
1124 old_res->end = 0;
1125 } else {
1126 crashk_res.end = ram_res->start - 1;
1127 }
1128
1129 crash_free_reserved_phys_range(ram_res->start, ram_res->end);
1130 insert_resource(&iomem_resource, ram_res);
1131
1132 return 0;
1133 }
1134
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 5/6] kexec: add helper __crash_shrink_memory()
2023-05-27 12:34 ` [PATCH 5/6] kexec: add helper __crash_shrink_memory() Zhen Lei
2023-05-28 0:08 ` kernel test robot
@ 2023-05-28 1:44 ` kernel test robot
2023-05-28 6:26 ` kernel test robot
2023-05-31 7:50 ` Baoquan He
3 siblings, 0 replies; 23+ messages in thread
From: kernel test robot @ 2023-05-28 1:44 UTC (permalink / raw)
To: Zhen Lei, Eric Biederman, Baoquan He, kexec, linux-kernel
Cc: oe-kbuild-all, Zhen Lei, Michael Holzheu, Andrew Morton,
Linux Memory Management List, Amerigo Wang
Hi Zhen,
kernel test robot noticed the following build warnings:
[auto build test WARNING on akpm-mm/mm-everything]
[also build test WARNING on linus/master v6.4-rc3 next-20230525]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Zhen-Lei/kexec-fix-a-memory-leak-in-crash_shrink_memory/20230527-203821
base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link: https://lore.kernel.org/r/20230527123439.772-6-thunder.leizhen%40huawei.com
patch subject: [PATCH 5/6] kexec: add helper __crash_shrink_memory()
config: mips-randconfig-r012-20230528 (https://download.01.org/0day-ci/archive/20230528/202305280949.yYtbjy5B-lkp@intel.com/config)
compiler: mipsel-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
mkdir -p ~/bin
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/dea97cef503d26e05d0e11818ae44176056ddf64
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Zhen-Lei/kexec-fix-a-memory-leak-in-crash_shrink_memory/20230527-203821
git checkout dea97cef503d26e05d0e11818ae44176056ddf64
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 ~/bin/make.cross W=1 O=build_dir ARCH=mips olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 ~/bin/make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202305280949.yYtbjy5B-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> kernel/kexec_core.c:1108:5: warning: no previous prototype for '__crash_shrink_memory' [-Wmissing-prototypes]
1108 | int __crash_shrink_memory(struct resource *old_res, unsigned long new_size)
| ^~~~~~~~~~~~~~~~~~~~~
vim +/__crash_shrink_memory +1108 kernel/kexec_core.c
1107
> 1108 int __crash_shrink_memory(struct resource *old_res, unsigned long new_size)
1109 {
1110 struct resource *ram_res;
1111
1112 ram_res = kzalloc(sizeof(*ram_res), GFP_KERNEL);
1113 if (!ram_res)
1114 return -ENOMEM;
1115
1116 ram_res->start = old_res->start + new_size;
1117 ram_res->end = old_res->end;
1118 ram_res->flags = IORESOURCE_BUSY | IORESOURCE_SYSTEM_RAM;
1119 ram_res->name = "System RAM";
1120
1121 if (!new_size) {
1122 release_resource(old_res);
1123 old_res->start = 0;
1124 old_res->end = 0;
1125 } else {
1126 crashk_res.end = ram_res->start - 1;
1127 }
1128
1129 crash_free_reserved_phys_range(ram_res->start, ram_res->end);
1130 insert_resource(&iomem_resource, ram_res);
1131
1132 return 0;
1133 }
1134
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 5/6] kexec: add helper __crash_shrink_memory()
2023-05-27 12:34 ` [PATCH 5/6] kexec: add helper __crash_shrink_memory() Zhen Lei
2023-05-28 0:08 ` kernel test robot
2023-05-28 1:44 ` kernel test robot
@ 2023-05-28 6:26 ` kernel test robot
2023-05-31 7:50 ` Baoquan He
3 siblings, 0 replies; 23+ messages in thread
From: kernel test robot @ 2023-05-28 6:26 UTC (permalink / raw)
To: Zhen Lei, Eric Biederman, Baoquan He, kexec, linux-kernel
Cc: oe-kbuild-all, Zhen Lei, Michael Holzheu, Andrew Morton,
Linux Memory Management List, Amerigo Wang
Hi Zhen,
kernel test robot noticed the following build warnings:
[auto build test WARNING on akpm-mm/mm-everything]
[also build test WARNING on linus/master v6.4-rc3 next-20230525]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Zhen-Lei/kexec-fix-a-memory-leak-in-crash_shrink_memory/20230527-203821
base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link: https://lore.kernel.org/r/20230527123439.772-6-thunder.leizhen%40huawei.com
patch subject: [PATCH 5/6] kexec: add helper __crash_shrink_memory()
config: x86_64-randconfig-s033-20230528 (https://download.01.org/0day-ci/archive/20230528/202305281410.FdGZvhsb-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.4-39-gce1a6720-dirty
# https://github.com/intel-lab-lkp/linux/commit/dea97cef503d26e05d0e11818ae44176056ddf64
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Zhen-Lei/kexec-fix-a-memory-leak-in-crash_shrink_memory/20230527-203821
git checkout dea97cef503d26e05d0e11818ae44176056ddf64
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=x86_64 olddefconfig
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=x86_64 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202305281410.FdGZvhsb-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
>> kernel/kexec_core.c:1108:5: sparse: sparse: symbol '__crash_shrink_memory' was not declared. Should it be static?
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 5/6] kexec: add helper __crash_shrink_memory()
2023-05-28 0:08 ` kernel test robot
@ 2023-05-29 0:37 ` Leizhen (ThunderTown)
0 siblings, 0 replies; 23+ messages in thread
From: Leizhen (ThunderTown) @ 2023-05-29 0:37 UTC (permalink / raw)
To: kernel test robot, Eric Biederman, Baoquan He, kexec, linux-kernel
Cc: llvm, oe-kbuild-all, Michael Holzheu, Andrew Morton,
Linux Memory Management List, Amerigo Wang
On 2023/5/28 8:08, kernel test robot wrote:
> Hi Zhen,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on akpm-mm/mm-everything]
> [also build test WARNING on linus/master v6.4-rc3 next-20230525]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Zhen-Lei/kexec-fix-a-memory-leak-in-crash_shrink_memory/20230527-203821
> base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
> patch link: https://lore.kernel.org/r/20230527123439.772-6-thunder.leizhen%40huawei.com
> patch subject: [PATCH 5/6] kexec: add helper __crash_shrink_memory()
> config: riscv-randconfig-r042-20230526 (https://download.01.org/0day-ci/archive/20230528/202305280717.Pw06aLkz-lkp@intel.com/config)
> compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 4faf3aaf28226a4e950c103a14f6fc1d1fdabb1b)
> reproduce (this is a W=1 build):
> mkdir -p ~/bin
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # install riscv cross compiling tool for clang build
> # apt-get install binutils-riscv64-linux-gnu
> # https://github.com/intel-lab-lkp/linux/commit/dea97cef503d26e05d0e11818ae44176056ddf64
> git remote add linux-review https://github.com/intel-lab-lkp/linux
> git fetch --no-tags linux-review Zhen-Lei/kexec-fix-a-memory-leak-in-crash_shrink_memory/20230527-203821
> git checkout dea97cef503d26e05d0e11818ae44176056ddf64
> # save the config file
> mkdir build_dir && cp config build_dir/.config
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=riscv olddefconfig
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202305280717.Pw06aLkz-lkp@intel.com/
>
> All warnings (new ones prefixed by >>):
>
>>> kernel/kexec_core.c:1108:5: warning: no previous prototype for function '__crash_shrink_memory' [-Wmissing-prototypes]
> int __crash_shrink_memory(struct resource *old_res, unsigned long new_size)
> ^
> kernel/kexec_core.c:1108:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
> int __crash_shrink_memory(struct resource *old_res, unsigned long new_size)
> ^
> static
> 1 warning generated.
Yes, thanks, a 'static' should be added.
>
>
> vim +/__crash_shrink_memory +1108 kernel/kexec_core.c
>
> 1107
>> 1108 int __crash_shrink_memory(struct resource *old_res, unsigned long new_size)
> 1109 {
> 1110 struct resource *ram_res;
> 1111
> 1112 ram_res = kzalloc(sizeof(*ram_res), GFP_KERNEL);
> 1113 if (!ram_res)
> 1114 return -ENOMEM;
> 1115
> 1116 ram_res->start = old_res->start + new_size;
> 1117 ram_res->end = old_res->end;
> 1118 ram_res->flags = IORESOURCE_BUSY | IORESOURCE_SYSTEM_RAM;
> 1119 ram_res->name = "System RAM";
> 1120
> 1121 if (!new_size) {
> 1122 release_resource(old_res);
> 1123 old_res->start = 0;
> 1124 old_res->end = 0;
> 1125 } else {
> 1126 crashk_res.end = ram_res->start - 1;
> 1127 }
> 1128
> 1129 crash_free_reserved_phys_range(ram_res->start, ram_res->end);
> 1130 insert_resource(&iomem_resource, ram_res);
> 1131
> 1132 return 0;
> 1133 }
> 1134
>
--
Regards,
Zhen Lei
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/6] kexec: fix a memory leak in crash_shrink_memory()
2023-05-27 12:34 ` [PATCH 1/6] kexec: fix a memory leak in crash_shrink_memory() Zhen Lei
@ 2023-05-31 0:13 ` Baoquan He
2023-05-31 1:16 ` Leizhen (ThunderTown)
0 siblings, 1 reply; 23+ messages in thread
From: Baoquan He @ 2023-05-31 0:13 UTC (permalink / raw)
To: Zhen Lei
Cc: Eric Biederman, kexec, linux-kernel, Michael Holzheu, Andrew Morton
On 05/27/23 at 08:34pm, Zhen Lei wrote:
> If the value of parameter 'new_size' is in the semi-open and semi-closed
> interval (crashk_res.end - KEXEC_CRASH_MEM_ALIGN + 1, crashk_res.end], the
> calculation result of ram_res is:
> ram_res->start = crashk_res.end + 1
> ram_res->end = crashk_res.end
If the new_size is smaller than KEXEC_CRASH_MEM_ALIGN, does it make
any sense except of testing purpose? Do we need to fail this kind of
shrinking, or just shrink all the left crash memory?
> The operation of function insert_resource() fails, and ram_res is not
> added to iomem_resource. As a result, the memory of the control block
> ram_res is leaked.
>
> In fact, on all architectures, the start address and size of crashk_res
> are already aligned by KEXEC_CRASH_MEM_ALIGN. Therefore, we do not need to
> round up crashk_res.start again. Instead, we should round up 'new_size'
> in advance.
>
> Fixes: 6480e5a09237 ("kdump: add missing RAM resource in crash_shrink_memory()")
> Fixes: 06a7f711246b ("kexec: premit reduction of the reserved memory size")
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> ---
> kernel/kexec_core.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> index 3d578c6fefee385..22acee18195a591 100644
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -1122,6 +1122,7 @@ int crash_shrink_memory(unsigned long new_size)
> start = crashk_res.start;
> end = crashk_res.end;
> old_size = (end == 0) ? 0 : end - start + 1;
> + new_size = roundup(new_size, KEXEC_CRASH_MEM_ALIGN);
> if (new_size >= old_size) {
> ret = (new_size == old_size) ? 0 : -EINVAL;
> goto unlock;
> @@ -1133,9 +1134,7 @@ int crash_shrink_memory(unsigned long new_size)
> goto unlock;
> }
>
> - start = roundup(start, KEXEC_CRASH_MEM_ALIGN);
> - end = roundup(start + new_size, KEXEC_CRASH_MEM_ALIGN);
> -
> + end = start + new_size;
> crash_free_reserved_phys_range(end, crashk_res.end);
>
> if ((start == end) && (crashk_res.parent != NULL))
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/6] kexec: delete a useless check in crash_shrink_memory()
2023-05-27 12:34 ` [PATCH 2/6] kexec: delete a useless check " Zhen Lei
@ 2023-05-31 0:17 ` Baoquan He
2023-05-31 2:19 ` Leizhen (ThunderTown)
0 siblings, 1 reply; 23+ messages in thread
From: Baoquan He @ 2023-05-31 0:17 UTC (permalink / raw)
To: Zhen Lei
Cc: Eric Biederman, kexec, linux-kernel, Michael Holzheu, Andrew Morton
On 05/27/23 at 08:34pm, Zhen Lei wrote:
> The check '(crashk_res.parent != NULL)' is added by
> commit e05bd3367bd3 ("kexec: fix Oops in crash_shrink_memory()"), but it's
> stale now. Because if 'crashk_res' is not reserved, it will be zero in
> size and will be intercepted by the above 'if (new_size >= old_size)'.
>
> Ago:
> if (new_size >= end - start + 1)
>
> Now:
> old_size = (end == 0) ? 0 : end - start + 1;
> if (new_size >= old_size)
Hmm, I would strongly suggest we keep that check. Even though the
current code like above can do the acutal checking, but its actual usage
is not obvious for checking of crashk_res existence. In the future,
someone may change above calculation and don't notice the hidden
functionality at all behind the calculation. The cost of the check is
almost zero, right?
>
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> ---
> kernel/kexec_core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> index 22acee18195a591..d1ab139dd49035e 100644
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -1137,7 +1137,7 @@ int crash_shrink_memory(unsigned long new_size)
> end = start + new_size;
> crash_free_reserved_phys_range(end, crashk_res.end);
>
> - if ((start == end) && (crashk_res.parent != NULL))
> + if (start == end)
> release_resource(&crashk_res);
>
> ram_res->start = end;
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 3/6] kexec: clear crashk_res if all its memory has been released
2023-05-27 12:34 ` [PATCH 3/6] kexec: clear crashk_res if all its memory has been released Zhen Lei
@ 2023-05-31 0:33 ` Baoquan He
0 siblings, 0 replies; 23+ messages in thread
From: Baoquan He @ 2023-05-31 0:33 UTC (permalink / raw)
To: Zhen Lei
Cc: Eric Biederman, kexec, linux-kernel, Michael Holzheu, Andrew Morton
On 05/27/23 at 08:34pm, Zhen Lei wrote:
> If the resource of crashk_res has been released, it is better to clear
> crashk_res.start and crashk_res.end. Because 'end = start - 1' is not
> reasonable, and in some places the test is based on crashk_res.end, not
> resource_size(&crashk_res).
This looks reasonable, at least I haven't think of any risk it could
bring. Thanks.
Acked-by: Baoquan He <bhe@redhat.com>
>
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> ---
> kernel/kexec_core.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> index d1ab139dd49035e..bcc86a250ab3bf9 100644
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -1137,15 +1137,18 @@ int crash_shrink_memory(unsigned long new_size)
> end = start + new_size;
> crash_free_reserved_phys_range(end, crashk_res.end);
>
> - if (start == end)
> - release_resource(&crashk_res);
> -
> ram_res->start = end;
> ram_res->end = crashk_res.end;
> ram_res->flags = IORESOURCE_BUSY | IORESOURCE_SYSTEM_RAM;
> ram_res->name = "System RAM";
>
> - crashk_res.end = end - 1;
> + if (start == end) {
> + release_resource(&crashk_res);
> + crashk_res.start = 0;
> + crashk_res.end = 0;
> + } else {
> + crashk_res.end = end - 1;
> + }
>
> insert_resource(&iomem_resource, ram_res);
>
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/6] kexec: fix a memory leak in crash_shrink_memory()
2023-05-31 0:13 ` Baoquan He
@ 2023-05-31 1:16 ` Leizhen (ThunderTown)
2023-05-31 7:31 ` Baoquan He
0 siblings, 1 reply; 23+ messages in thread
From: Leizhen (ThunderTown) @ 2023-05-31 1:16 UTC (permalink / raw)
To: Baoquan He
Cc: Eric Biederman, kexec, linux-kernel, Michael Holzheu, Andrew Morton
On 2023/5/31 8:13, Baoquan He wrote:
> On 05/27/23 at 08:34pm, Zhen Lei wrote:
>> If the value of parameter 'new_size' is in the semi-open and semi-closed
>> interval (crashk_res.end - KEXEC_CRASH_MEM_ALIGN + 1, crashk_res.end], the
>> calculation result of ram_res is:
>> ram_res->start = crashk_res.end + 1
>> ram_res->end = crashk_res.end
>
> If the new_size is smaller than KEXEC_CRASH_MEM_ALIGN, does it make
> any sense except of testing purpose? Do we need to fail this kind of
> shrinking, or just shrink all the left crash memory?
We can't give a fixed value, that is, how much crash memory is reserved to
ensure that the capture kernel runs. The size of KEXEC_CRASH_MEM_ALIGN is
only one page on non-s390 platforms. So, it's better to keep the code simple,
and let the user(administrator) shrink the crash memory reasonably.
include/linux/kexec.h
#define KEXEC_CRASH_MEM_ALIGN PAGE_SIZE
>
>> The operation of function insert_resource() fails, and ram_res is not
>> added to iomem_resource. As a result, the memory of the control block
>> ram_res is leaked.
>>
>> In fact, on all architectures, the start address and size of crashk_res
>> are already aligned by KEXEC_CRASH_MEM_ALIGN. Therefore, we do not need to
>> round up crashk_res.start again. Instead, we should round up 'new_size'
>> in advance.
>>
>> Fixes: 6480e5a09237 ("kdump: add missing RAM resource in crash_shrink_memory()")
>> Fixes: 06a7f711246b ("kexec: premit reduction of the reserved memory size")
>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>> ---
>> kernel/kexec_core.c | 5 ++---
>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
>> index 3d578c6fefee385..22acee18195a591 100644
>> --- a/kernel/kexec_core.c
>> +++ b/kernel/kexec_core.c
>> @@ -1122,6 +1122,7 @@ int crash_shrink_memory(unsigned long new_size)
>> start = crashk_res.start;
>> end = crashk_res.end;
>> old_size = (end == 0) ? 0 : end - start + 1;
>> + new_size = roundup(new_size, KEXEC_CRASH_MEM_ALIGN);
>> if (new_size >= old_size) {
>> ret = (new_size == old_size) ? 0 : -EINVAL;
>> goto unlock;
>> @@ -1133,9 +1134,7 @@ int crash_shrink_memory(unsigned long new_size)
>> goto unlock;
>> }
>>
>> - start = roundup(start, KEXEC_CRASH_MEM_ALIGN);
>> - end = roundup(start + new_size, KEXEC_CRASH_MEM_ALIGN);
>> -
>> + end = start + new_size;
>> crash_free_reserved_phys_range(end, crashk_res.end);
>>
>> if ((start == end) && (crashk_res.parent != NULL))
>> --
>> 2.25.1
>>
>
> .
>
--
Regards,
Zhen Lei
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/6] kexec: delete a useless check in crash_shrink_memory()
2023-05-31 0:17 ` Baoquan He
@ 2023-05-31 2:19 ` Leizhen (ThunderTown)
2023-05-31 7:41 ` Baoquan He
0 siblings, 1 reply; 23+ messages in thread
From: Leizhen (ThunderTown) @ 2023-05-31 2:19 UTC (permalink / raw)
To: Baoquan He
Cc: Eric Biederman, kexec, linux-kernel, Michael Holzheu, Andrew Morton
On 2023/5/31 8:17, Baoquan He wrote:
> On 05/27/23 at 08:34pm, Zhen Lei wrote:
>> The check '(crashk_res.parent != NULL)' is added by
>> commit e05bd3367bd3 ("kexec: fix Oops in crash_shrink_memory()"), but it's
>> stale now. Because if 'crashk_res' is not reserved, it will be zero in
>> size and will be intercepted by the above 'if (new_size >= old_size)'.
>>
>> Ago:
>> if (new_size >= end - start + 1)
>>
>> Now:
>> old_size = (end == 0) ? 0 : end - start + 1;
>> if (new_size >= old_size)
>
> Hmm, I would strongly suggest we keep that check. Even though the
> current code like above can do the acutal checking, but its actual usage
> is not obvious for checking of crashk_res existence. In the future,
> someone may change above calculation and don't notice the hidden
> functionality at all behind the calculation. The cost of the check is
> almost zero, right?
The cost of the check is negligible. The only downside is that it's hard to
understand why it's added, and I only found out why by looking at the history
log. In my opinion, the above 'Now:' is the right fix.
>
>>
>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>> ---
>> kernel/kexec_core.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
>> index 22acee18195a591..d1ab139dd49035e 100644
>> --- a/kernel/kexec_core.c
>> +++ b/kernel/kexec_core.c
>> @@ -1137,7 +1137,7 @@ int crash_shrink_memory(unsigned long new_size)
>> end = start + new_size;
>> crash_free_reserved_phys_range(end, crashk_res.end);
>>
>> - if ((start == end) && (crashk_res.parent != NULL))
>> + if (start == end)
>> release_resource(&crashk_res);
>>
>> ram_res->start = end;
>> --
>> 2.25.1
>>
>
> .
>
--
Regards,
Zhen Lei
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/6] kexec: fix a memory leak in crash_shrink_memory()
2023-05-31 1:16 ` Leizhen (ThunderTown)
@ 2023-05-31 7:31 ` Baoquan He
0 siblings, 0 replies; 23+ messages in thread
From: Baoquan He @ 2023-05-31 7:31 UTC (permalink / raw)
To: Leizhen (ThunderTown)
Cc: Eric Biederman, kexec, linux-kernel, Michael Holzheu, Andrew Morton
On 05/31/23 at 09:16am, Leizhen (ThunderTown) wrote:
>
>
> On 2023/5/31 8:13, Baoquan He wrote:
> > On 05/27/23 at 08:34pm, Zhen Lei wrote:
> >> If the value of parameter 'new_size' is in the semi-open and semi-closed
> >> interval (crashk_res.end - KEXEC_CRASH_MEM_ALIGN + 1, crashk_res.end], the
> >> calculation result of ram_res is:
> >> ram_res->start = crashk_res.end + 1
> >> ram_res->end = crashk_res.end
> >
> > If the new_size is smaller than KEXEC_CRASH_MEM_ALIGN, does it make
> > any sense except of testing purpose? Do we need to fail this kind of
> > shrinking, or just shrink all the left crash memory?
OK, I misread your log. You are saying the new_size is close to
crashk_res.end but has a tiny difference in your example, I
thought the new_size is smaller than KEXEC_CRASH_MEM_ALIGN which is just
in the opposite direction.
Yea, it does have the possibility to waste a ram_res but does nothing
even though the chance is very small.
Acked-by: Baoquan He <bhe@redhat.com>
>
> We can't give a fixed value, that is, how much crash memory is reserved to
> ensure that the capture kernel runs. The size of KEXEC_CRASH_MEM_ALIGN is
> only one page on non-s390 platforms. So, it's better to keep the code simple,
> and let the user(administrator) shrink the crash memory reasonably.
>
> include/linux/kexec.h
> #define KEXEC_CRASH_MEM_ALIGN PAGE_SIZE
>
> >
> >> The operation of function insert_resource() fails, and ram_res is not
> >> added to iomem_resource. As a result, the memory of the control block
> >> ram_res is leaked.
> >>
> >> In fact, on all architectures, the start address and size of crashk_res
> >> are already aligned by KEXEC_CRASH_MEM_ALIGN. Therefore, we do not need to
> >> round up crashk_res.start again. Instead, we should round up 'new_size'
> >> in advance.
> >>
> >> Fixes: 6480e5a09237 ("kdump: add missing RAM resource in crash_shrink_memory()")
> >> Fixes: 06a7f711246b ("kexec: premit reduction of the reserved memory size")
> >> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> >> ---
> >> kernel/kexec_core.c | 5 ++---
> >> 1 file changed, 2 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> >> index 3d578c6fefee385..22acee18195a591 100644
> >> --- a/kernel/kexec_core.c
> >> +++ b/kernel/kexec_core.c
> >> @@ -1122,6 +1122,7 @@ int crash_shrink_memory(unsigned long new_size)
> >> start = crashk_res.start;
> >> end = crashk_res.end;
> >> old_size = (end == 0) ? 0 : end - start + 1;
> >> + new_size = roundup(new_size, KEXEC_CRASH_MEM_ALIGN);
> >> if (new_size >= old_size) {
> >> ret = (new_size == old_size) ? 0 : -EINVAL;
> >> goto unlock;
> >> @@ -1133,9 +1134,7 @@ int crash_shrink_memory(unsigned long new_size)
> >> goto unlock;
> >> }
> >>
> >> - start = roundup(start, KEXEC_CRASH_MEM_ALIGN);
> >> - end = roundup(start + new_size, KEXEC_CRASH_MEM_ALIGN);
> >> -
> >> + end = start + new_size;
> >> crash_free_reserved_phys_range(end, crashk_res.end);
> >>
> >> if ((start == end) && (crashk_res.parent != NULL))
> >> --
> >> 2.25.1
> >>
> >
> > .
> >
>
> --
> Regards,
> Zhen Lei
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/6] kexec: delete a useless check in crash_shrink_memory()
2023-05-31 2:19 ` Leizhen (ThunderTown)
@ 2023-05-31 7:41 ` Baoquan He
2023-05-31 8:26 ` Leizhen (ThunderTown)
0 siblings, 1 reply; 23+ messages in thread
From: Baoquan He @ 2023-05-31 7:41 UTC (permalink / raw)
To: Leizhen (ThunderTown)
Cc: Eric Biederman, kexec, linux-kernel, Michael Holzheu, Andrew Morton
On 05/31/23 at 10:19am, Leizhen (ThunderTown) wrote:
>
>
> On 2023/5/31 8:17, Baoquan He wrote:
> > On 05/27/23 at 08:34pm, Zhen Lei wrote:
> >> The check '(crashk_res.parent != NULL)' is added by
> >> commit e05bd3367bd3 ("kexec: fix Oops in crash_shrink_memory()"), but it's
> >> stale now. Because if 'crashk_res' is not reserved, it will be zero in
> >> size and will be intercepted by the above 'if (new_size >= old_size)'.
> >>
> >> Ago:
> >> if (new_size >= end - start + 1)
> >>
> >> Now:
> >> old_size = (end == 0) ? 0 : end - start + 1;
> >> if (new_size >= old_size)
> >
> > Hmm, I would strongly suggest we keep that check. Even though the
> > current code like above can do the acutal checking, but its actual usage
> > is not obvious for checking of crashk_res existence. In the future,
> > someone may change above calculation and don't notice the hidden
> > functionality at all behind the calculation. The cost of the check is
> > almost zero, right?
>
> The cost of the check is negligible. The only downside is that it's hard to
> understand why it's added, and I only found out why by looking at the history
> log. In my opinion, the above 'Now:' is the right fix.
It checks if the resource exists before releasing, just a normal
checking?
>
> >
> >>
> >> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> >> ---
> >> kernel/kexec_core.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> >> index 22acee18195a591..d1ab139dd49035e 100644
> >> --- a/kernel/kexec_core.c
> >> +++ b/kernel/kexec_core.c
> >> @@ -1137,7 +1137,7 @@ int crash_shrink_memory(unsigned long new_size)
> >> end = start + new_size;
> >> crash_free_reserved_phys_range(end, crashk_res.end);
> >>
> >> - if ((start == end) && (crashk_res.parent != NULL))
> >> + if (start == end)
> >> release_resource(&crashk_res);
> >>
> >> ram_res->start = end;
> >> --
> >> 2.25.1
> >>
> >
> > .
> >
>
> --
> Regards,
> Zhen Lei
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/6] kexec: improve the readability of crash_shrink_memory()
2023-05-27 12:34 ` [PATCH 4/6] kexec: improve the readability of crash_shrink_memory() Zhen Lei
@ 2023-05-31 7:48 ` Baoquan He
0 siblings, 0 replies; 23+ messages in thread
From: Baoquan He @ 2023-05-31 7:48 UTC (permalink / raw)
To: Zhen Lei
Cc: Eric Biederman, kexec, linux-kernel, Michael Holzheu, Andrew Morton
On 05/27/23 at 08:34pm, Zhen Lei wrote:
> The major adjustments are:
> 1. end = start + new_size.
> The 'end' here is not an accurate representation, because it is not the
> new end of crashk_res, but the start of ram_res, difference 1. So
> eliminate it and replace it with ram_res->start.
> 2. Use 'ram_res->start' and 'ram_res->end' as arguments to
> crash_free_reserved_phys_range() to indicate that the memory covered by
> 'ram_res' is released from the crashk. And keep it close to
> insert_resource().
> 3. Replace 'if (start == end)' with 'if (!new_size)', clear indication that
> all crashk memory will be shrunken.
>
> No functional change.
>
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
LGTM,
Acked-by: Baoquan He <bhe@redhat.com>
> ---
> kernel/kexec_core.c | 15 +++++----------
> 1 file changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> index bcc86a250ab3bf9..69fe92141b0b62d 100644
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -1108,7 +1108,6 @@ ssize_t crash_get_memory_size(void)
> int crash_shrink_memory(unsigned long new_size)
> {
> int ret = 0;
> - unsigned long start, end;
> unsigned long old_size;
> struct resource *ram_res;
>
> @@ -1119,9 +1118,7 @@ int crash_shrink_memory(unsigned long new_size)
> ret = -ENOENT;
> goto unlock;
> }
> - start = crashk_res.start;
> - end = crashk_res.end;
> - old_size = (end == 0) ? 0 : end - start + 1;
> + old_size = !crashk_res.end ? 0 : resource_size(&crashk_res);
> new_size = roundup(new_size, KEXEC_CRASH_MEM_ALIGN);
> if (new_size >= old_size) {
> ret = (new_size == old_size) ? 0 : -EINVAL;
> @@ -1134,22 +1131,20 @@ int crash_shrink_memory(unsigned long new_size)
> goto unlock;
> }
>
> - end = start + new_size;
> - crash_free_reserved_phys_range(end, crashk_res.end);
> -
> - ram_res->start = end;
> + ram_res->start = crashk_res.start + new_size;
> ram_res->end = crashk_res.end;
> ram_res->flags = IORESOURCE_BUSY | IORESOURCE_SYSTEM_RAM;
> ram_res->name = "System RAM";
>
> - if (start == end) {
> + if (!new_size) {
> release_resource(&crashk_res);
> crashk_res.start = 0;
> crashk_res.end = 0;
> } else {
> - crashk_res.end = end - 1;
> + crashk_res.end = ram_res->start - 1;
> }
>
> + crash_free_reserved_phys_range(ram_res->start, ram_res->end);
> insert_resource(&iomem_resource, ram_res);
>
> unlock:
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 5/6] kexec: add helper __crash_shrink_memory()
2023-05-27 12:34 ` [PATCH 5/6] kexec: add helper __crash_shrink_memory() Zhen Lei
` (2 preceding siblings ...)
2023-05-28 6:26 ` kernel test robot
@ 2023-05-31 7:50 ` Baoquan He
3 siblings, 0 replies; 23+ messages in thread
From: Baoquan He @ 2023-05-31 7:50 UTC (permalink / raw)
To: Zhen Lei
Cc: Eric Biederman, kexec, linux-kernel, Michael Holzheu, Andrew Morton
On 05/27/23 at 08:34pm, Zhen Lei wrote:
> No functional change, in preparation for the next patch so that it is
> easier to review.
>
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> ---
> kernel/kexec_core.c | 50 +++++++++++++++++++++++++--------------------
> 1 file changed, 28 insertions(+), 22 deletions(-)
>
> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> index 69fe92141b0b62d..e82bc6d6634136a 100644
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -1105,11 +1105,37 @@ ssize_t crash_get_memory_size(void)
> return size;
> }
>
> +int __crash_shrink_memory(struct resource *old_res, unsigned long new_size)
Other than the missing static, looks good,
Acked-by: Baoquan He <bhe@redhat.com>
> +{
> + struct resource *ram_res;
> +
> + ram_res = kzalloc(sizeof(*ram_res), GFP_KERNEL);
> + if (!ram_res)
> + return -ENOMEM;
> +
> + ram_res->start = old_res->start + new_size;
> + ram_res->end = old_res->end;
> + ram_res->flags = IORESOURCE_BUSY | IORESOURCE_SYSTEM_RAM;
> + ram_res->name = "System RAM";
> +
> + if (!new_size) {
> + release_resource(old_res);
> + old_res->start = 0;
> + old_res->end = 0;
> + } else {
> + crashk_res.end = ram_res->start - 1;
> + }
> +
> + crash_free_reserved_phys_range(ram_res->start, ram_res->end);
> + insert_resource(&iomem_resource, ram_res);
> +
> + return 0;
> +}
> +
> int crash_shrink_memory(unsigned long new_size)
> {
> int ret = 0;
> unsigned long old_size;
> - struct resource *ram_res;
>
> if (!kexec_trylock())
> return -EBUSY;
> @@ -1125,27 +1151,7 @@ int crash_shrink_memory(unsigned long new_size)
> goto unlock;
> }
>
> - ram_res = kzalloc(sizeof(*ram_res), GFP_KERNEL);
> - if (!ram_res) {
> - ret = -ENOMEM;
> - goto unlock;
> - }
> -
> - ram_res->start = crashk_res.start + new_size;
> - ram_res->end = crashk_res.end;
> - ram_res->flags = IORESOURCE_BUSY | IORESOURCE_SYSTEM_RAM;
> - ram_res->name = "System RAM";
> -
> - if (!new_size) {
> - release_resource(&crashk_res);
> - crashk_res.start = 0;
> - crashk_res.end = 0;
> - } else {
> - crashk_res.end = ram_res->start - 1;
> - }
> -
> - crash_free_reserved_phys_range(ram_res->start, ram_res->end);
> - insert_resource(&iomem_resource, ram_res);
> + ret = __crash_shrink_memory(&crashk_res, new_size);
>
> unlock:
> kexec_unlock();
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/6] kexec: delete a useless check in crash_shrink_memory()
2023-05-31 7:41 ` Baoquan He
@ 2023-05-31 8:26 ` Leizhen (ThunderTown)
0 siblings, 0 replies; 23+ messages in thread
From: Leizhen (ThunderTown) @ 2023-05-31 8:26 UTC (permalink / raw)
To: Baoquan He
Cc: Eric Biederman, kexec, linux-kernel, Michael Holzheu, Andrew Morton
On 2023/5/31 15:41, Baoquan He wrote:
> On 05/31/23 at 10:19am, Leizhen (ThunderTown) wrote:
>>
>>
>> On 2023/5/31 8:17, Baoquan He wrote:
>>> On 05/27/23 at 08:34pm, Zhen Lei wrote:
>>>> The check '(crashk_res.parent != NULL)' is added by
>>>> commit e05bd3367bd3 ("kexec: fix Oops in crash_shrink_memory()"), but it's
>>>> stale now. Because if 'crashk_res' is not reserved, it will be zero in
>>>> size and will be intercepted by the above 'if (new_size >= old_size)'.
>>>>
>>>> Ago:
>>>> if (new_size >= end - start + 1)
>>>>
>>>> Now:
>>>> old_size = (end == 0) ? 0 : end - start + 1;
>>>> if (new_size >= old_size)
>>>
>>> Hmm, I would strongly suggest we keep that check. Even though the
>>> current code like above can do the acutal checking, but its actual usage
>>> is not obvious for checking of crashk_res existence. In the future,
>>> someone may change above calculation and don't notice the hidden
>>> functionality at all behind the calculation. The cost of the check is
>>> almost zero, right?
>>
>> The cost of the check is negligible. The only downside is that it's hard to
>> understand why it's added, and I only found out why by looking at the history
>> log. In my opinion, the above 'Now:' is the right fix.
>
> It checks if the resource exists before releasing, just a normal
> checking?
If resource_size(&crashk_res) is zero, it means that crashk_res has not been
added(insert_resource) or has been deleted(release_resource). I've tested it. It's okay.
>>
>>>
>>>>
>>>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>>>> ---
>>>> kernel/kexec_core.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
>>>> index 22acee18195a591..d1ab139dd49035e 100644
>>>> --- a/kernel/kexec_core.c
>>>> +++ b/kernel/kexec_core.c
>>>> @@ -1137,7 +1137,7 @@ int crash_shrink_memory(unsigned long new_size)
>>>> end = start + new_size;
>>>> crash_free_reserved_phys_range(end, crashk_res.end);
>>>>
>>>> - if ((start == end) && (crashk_res.parent != NULL))
>>>> + if (start == end)
>>>> release_resource(&crashk_res);
>>>>
>>>> ram_res->start = end;
>>>> --
>>>> 2.25.1
>>>>
>>>
>>> .
>>>
>>
>> --
>> Regards,
>> Zhen Lei
>>
>
>
> .
>
--
Regards,
Zhen Lei
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 6/6] kexec: enable kexec_crash_size to support two crash kernel regions
2023-05-27 12:34 ` [PATCH 6/6] kexec: enable kexec_crash_size to support two crash kernel regions Zhen Lei
@ 2023-05-31 9:53 ` Baoquan He
2023-05-31 14:25 ` Leizhen (ThunderTown)
0 siblings, 1 reply; 23+ messages in thread
From: Baoquan He @ 2023-05-31 9:53 UTC (permalink / raw)
To: Zhen Lei
Cc: Eric Biederman, kexec, linux-kernel, Michael Holzheu, Andrew Morton
On 05/27/23 at 08:34pm, Zhen Lei wrote:
> The crashk_low_res should be considered by /sys/kernel/kexec_crash_size
> to support two crash kernel regions. Since crashk_res manages the memory
> with high address and crashk_low_res manages the memory with low address,
> crashk_low_res is shrunken only when all crashk_res is shrunken. And
> because when there is only one crash kernel region, crashk_res is always
> used. Therefore, if all crashk_res is shrunken and crashk_low_res still
> exists, swap them.
This looks good, otherwise someone else won't stop attempting to add
support of crashk_low_res shrinking. Not sure if this will bring corner
case issue in testing, let's see. For the patch log, I tried to
rephrase, feel free to refer to.
=====
The crashk_low_res should be considered by /sys/kernel/kexec_crash_size
to support two crash kernel regions shrinking if existing.
While doing it, crashk_low_res will only be shrunk when the entire
crashk_res is empty; and if the crashk_res is empty and crahk_low_res
is not, change crashk_low_res to be crashk_res.
=====
With the log updated, you can add:
Acked-by: Baoquan He <bhe@redhat.com>
>
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> ---
> kernel/kexec_core.c | 43 ++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 38 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> index e82bc6d6634136a..c1d50f6566300d9 100644
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -1091,6 +1091,11 @@ __bpf_kfunc void crash_kexec(struct pt_regs *regs)
> }
> }
>
> +static inline resource_size_t crash_resource_size(const struct resource *res)
> +{
> + return !res->end ? 0 : resource_size(res);
> +}
> +
> ssize_t crash_get_memory_size(void)
> {
> ssize_t size = 0;
> @@ -1098,8 +1103,8 @@ ssize_t crash_get_memory_size(void)
> if (!kexec_trylock())
> return -EBUSY;
>
> - if (crashk_res.end != crashk_res.start)
> - size = resource_size(&crashk_res);
> + size += crash_resource_size(&crashk_res);
> + size += crash_resource_size(&crashk_low_res);
>
> kexec_unlock();
> return size;
> @@ -1135,7 +1140,7 @@ int __crash_shrink_memory(struct resource *old_res, unsigned long new_size)
> int crash_shrink_memory(unsigned long new_size)
> {
> int ret = 0;
> - unsigned long old_size;
> + unsigned long old_size, low_size;
>
> if (!kexec_trylock())
> return -EBUSY;
> @@ -1144,14 +1149,42 @@ int crash_shrink_memory(unsigned long new_size)
> ret = -ENOENT;
> goto unlock;
> }
> - old_size = !crashk_res.end ? 0 : resource_size(&crashk_res);
> +
> + low_size = crash_resource_size(&crashk_low_res);
> + old_size = crash_resource_size(&crashk_res) + low_size;
> new_size = roundup(new_size, KEXEC_CRASH_MEM_ALIGN);
> if (new_size >= old_size) {
> ret = (new_size == old_size) ? 0 : -EINVAL;
> goto unlock;
> }
>
> - ret = __crash_shrink_memory(&crashk_res, new_size);
> + /*
> + * (low_size > new_size) implies that low_size is greater than zero.
> + * This also means that if low_size is zero, the else branch is taken.
> + *
> + * If low_size is greater than 0, (low_size > new_size) indicates that
> + * crashk_low_res also needs to be shrunken. Otherwise, only crashk_res
> + * needs to be shrunken.
> + */
> + if (low_size > new_size) {
> + ret = __crash_shrink_memory(&crashk_res, 0);
> + if (ret)
> + goto unlock;
> +
> + ret = __crash_shrink_memory(&crashk_low_res, new_size);
> + } else {
> + ret = __crash_shrink_memory(&crashk_res, new_size - low_size);
> + }
> +
> + /* Swap crashk_res and crashk_low_res if needed */
> + if (!crashk_res.end && crashk_low_res.end) {
> + crashk_res.start = crashk_low_res.start;
> + crashk_res.end = crashk_low_res.end;
> + release_resource(&crashk_low_res);
> + crashk_low_res.start = 0;
> + crashk_low_res.end = 0;
> + insert_resource(&iomem_resource, &crashk_res);
> + }
>
> unlock:
> kexec_unlock();
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 6/6] kexec: enable kexec_crash_size to support two crash kernel regions
2023-05-31 9:53 ` Baoquan He
@ 2023-05-31 14:25 ` Leizhen (ThunderTown)
0 siblings, 0 replies; 23+ messages in thread
From: Leizhen (ThunderTown) @ 2023-05-31 14:25 UTC (permalink / raw)
To: Baoquan He
Cc: Eric Biederman, kexec, linux-kernel, Michael Holzheu, Andrew Morton
On 2023/5/31 17:53, Baoquan He wrote:
> On 05/27/23 at 08:34pm, Zhen Lei wrote:
>> The crashk_low_res should be considered by /sys/kernel/kexec_crash_size
>> to support two crash kernel regions. Since crashk_res manages the memory
>> with high address and crashk_low_res manages the memory with low address,
>> crashk_low_res is shrunken only when all crashk_res is shrunken. And
>> because when there is only one crash kernel region, crashk_res is always
>> used. Therefore, if all crashk_res is shrunken and crashk_low_res still
>> exists, swap them.
>
> This looks good, otherwise someone else won't stop attempting to add
> support of crashk_low_res shrinking. Not sure if this will bring corner
> case issue in testing, let's see. For the patch log, I tried to
> rephrase, feel free to refer to.
>
> =====
> The crashk_low_res should be considered by /sys/kernel/kexec_crash_size
> to support two crash kernel regions shrinking if existing.
>
> While doing it, crashk_low_res will only be shrunk when the entire
> crashk_res is empty; and if the crashk_res is empty and crahk_low_res
> is not, change crashk_low_res to be crashk_res.
> =====
>
> With the log updated, you can add:
>
> Acked-by: Baoquan He <bhe@redhat.com>
OK, I will update the log and add Acked-by in v2.
>
>>
>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>> ---
>> kernel/kexec_core.c | 43 ++++++++++++++++++++++++++++++++++++++-----
>> 1 file changed, 38 insertions(+), 5 deletions(-)
>>
>> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
>> index e82bc6d6634136a..c1d50f6566300d9 100644
>> --- a/kernel/kexec_core.c
>> +++ b/kernel/kexec_core.c
>> @@ -1091,6 +1091,11 @@ __bpf_kfunc void crash_kexec(struct pt_regs *regs)
>> }
>> }
>>
>> +static inline resource_size_t crash_resource_size(const struct resource *res)
>> +{
>> + return !res->end ? 0 : resource_size(res);
>> +}
>> +
>> ssize_t crash_get_memory_size(void)
>> {
>> ssize_t size = 0;
>> @@ -1098,8 +1103,8 @@ ssize_t crash_get_memory_size(void)
>> if (!kexec_trylock())
>> return -EBUSY;
>>
>> - if (crashk_res.end != crashk_res.start)
>> - size = resource_size(&crashk_res);
>> + size += crash_resource_size(&crashk_res);
>> + size += crash_resource_size(&crashk_low_res);
>>
>> kexec_unlock();
>> return size;
>> @@ -1135,7 +1140,7 @@ int __crash_shrink_memory(struct resource *old_res, unsigned long new_size)
>> int crash_shrink_memory(unsigned long new_size)
>> {
>> int ret = 0;
>> - unsigned long old_size;
>> + unsigned long old_size, low_size;
>>
>> if (!kexec_trylock())
>> return -EBUSY;
>> @@ -1144,14 +1149,42 @@ int crash_shrink_memory(unsigned long new_size)
>> ret = -ENOENT;
>> goto unlock;
>> }
>> - old_size = !crashk_res.end ? 0 : resource_size(&crashk_res);
>> +
>> + low_size = crash_resource_size(&crashk_low_res);
>> + old_size = crash_resource_size(&crashk_res) + low_size;
>> new_size = roundup(new_size, KEXEC_CRASH_MEM_ALIGN);
>> if (new_size >= old_size) {
>> ret = (new_size == old_size) ? 0 : -EINVAL;
>> goto unlock;
>> }
>>
>> - ret = __crash_shrink_memory(&crashk_res, new_size);
>> + /*
>> + * (low_size > new_size) implies that low_size is greater than zero.
>> + * This also means that if low_size is zero, the else branch is taken.
>> + *
>> + * If low_size is greater than 0, (low_size > new_size) indicates that
>> + * crashk_low_res also needs to be shrunken. Otherwise, only crashk_res
>> + * needs to be shrunken.
>> + */
>> + if (low_size > new_size) {
>> + ret = __crash_shrink_memory(&crashk_res, 0);
>> + if (ret)
>> + goto unlock;
>> +
>> + ret = __crash_shrink_memory(&crashk_low_res, new_size);
>> + } else {
>> + ret = __crash_shrink_memory(&crashk_res, new_size - low_size);
>> + }
>> +
>> + /* Swap crashk_res and crashk_low_res if needed */
>> + if (!crashk_res.end && crashk_low_res.end) {
>> + crashk_res.start = crashk_low_res.start;
>> + crashk_res.end = crashk_low_res.end;
>> + release_resource(&crashk_low_res);
>> + crashk_low_res.start = 0;
>> + crashk_low_res.end = 0;
>> + insert_resource(&iomem_resource, &crashk_res);
>> + }
>>
>> unlock:
>> kexec_unlock();
>> --
>> 2.25.1
>>
>
> .
>
--
Regards,
Zhen Lei
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2023-05-31 14:25 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-27 12:34 [PATCH 0/6] kexec: enable kexec_crash_size to support two crash kernel regions Zhen Lei
2023-05-27 12:34 ` [PATCH 1/6] kexec: fix a memory leak in crash_shrink_memory() Zhen Lei
2023-05-31 0:13 ` Baoquan He
2023-05-31 1:16 ` Leizhen (ThunderTown)
2023-05-31 7:31 ` Baoquan He
2023-05-27 12:34 ` [PATCH 2/6] kexec: delete a useless check " Zhen Lei
2023-05-31 0:17 ` Baoquan He
2023-05-31 2:19 ` Leizhen (ThunderTown)
2023-05-31 7:41 ` Baoquan He
2023-05-31 8:26 ` Leizhen (ThunderTown)
2023-05-27 12:34 ` [PATCH 3/6] kexec: clear crashk_res if all its memory has been released Zhen Lei
2023-05-31 0:33 ` Baoquan He
2023-05-27 12:34 ` [PATCH 4/6] kexec: improve the readability of crash_shrink_memory() Zhen Lei
2023-05-31 7:48 ` Baoquan He
2023-05-27 12:34 ` [PATCH 5/6] kexec: add helper __crash_shrink_memory() Zhen Lei
2023-05-28 0:08 ` kernel test robot
2023-05-29 0:37 ` Leizhen (ThunderTown)
2023-05-28 1:44 ` kernel test robot
2023-05-28 6:26 ` kernel test robot
2023-05-31 7:50 ` Baoquan He
2023-05-27 12:34 ` [PATCH 6/6] kexec: enable kexec_crash_size to support two crash kernel regions Zhen Lei
2023-05-31 9:53 ` Baoquan He
2023-05-31 14:25 ` Leizhen (ThunderTown)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).