* Re: [PATCH] sched_ext: Use kvzalloc for large exit_dump allocation
2025-04-07 19:50 [PATCH] sched_ext: Use kvzalloc for large exit_dump allocation Breno Leitao
@ 2025-04-07 20:12 ` Christophe JAILLET
2025-04-07 20:23 ` Andrea Righi
2025-04-08 1:19 ` Changwoo Min
2 siblings, 0 replies; 4+ messages in thread
From: Christophe JAILLET @ 2025-04-07 20:12 UTC (permalink / raw)
To: Breno Leitao, Tejun Heo, David Vernet, Andrea Righi, Changwoo Min,
Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
Valentin Schneider
Cc: linux-kernel, kernel-team, stable, Rik van Riel
Le 07/04/2025 à 21:50, Breno Leitao a écrit :
> Replace kzalloc with kvzalloc for the exit_dump buffer allocation, which
> can require large contiguous memory (up to order=9) depending on the
> implementation. This change prevents allocation failures by allowing the
> system to fall back to vmalloc when contiguous memory allocation fails.
>
> Since this buffer is only used for debugging purposes, physical memory
> contiguity is not required, making vmalloc a suitable alternative.
>
> Cc: stable@vger.kernel.org
> Fixes: 07814a9439a3b0 ("sched_ext: Print debug dump after an error exit")
> Suggested-by: Rik van Riel <riel@surriel.com>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
> kernel/sched/ext.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
> index 66bcd40a28ca1..c82725f9b0559 100644
> --- a/kernel/sched/ext.c
> +++ b/kernel/sched/ext.c
> @@ -4639,7 +4639,7 @@ static struct scx_exit_info *alloc_exit_info(size_t exit_dump_len)
>
> ei->bt = kcalloc(SCX_EXIT_BT_LEN, sizeof(ei->bt[0]), GFP_KERNEL);
> ei->msg = kzalloc(SCX_EXIT_MSG_LEN, GFP_KERNEL);
> - ei->dump = kzalloc(exit_dump_len, GFP_KERNEL);
> + ei->dump = kvzalloc(exit_dump_len, GFP_KERNEL);
Needs kvfree() in free_exit_info()?
CJ
>
> if (!ei->bt || !ei->msg || !ei->dump) {
> free_exit_info(ei);
>
> ---
> base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
> change-id: 20250407-scx-11dbf94803c3
>
> Best regards,
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] sched_ext: Use kvzalloc for large exit_dump allocation
2025-04-07 19:50 [PATCH] sched_ext: Use kvzalloc for large exit_dump allocation Breno Leitao
2025-04-07 20:12 ` Christophe JAILLET
@ 2025-04-07 20:23 ` Andrea Righi
2025-04-08 1:19 ` Changwoo Min
2 siblings, 0 replies; 4+ messages in thread
From: Andrea Righi @ 2025-04-07 20:23 UTC (permalink / raw)
To: Breno Leitao
Cc: Tejun Heo, David Vernet, Changwoo Min, Ingo Molnar,
Peter Zijlstra, Juri Lelli, Vincent Guittot, Dietmar Eggemann,
Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider,
linux-kernel, kernel-team, stable, Rik van Riel
Hi Breno,
On Mon, Apr 07, 2025 at 12:50:29PM -0700, Breno Leitao wrote:
> Replace kzalloc with kvzalloc for the exit_dump buffer allocation, which
> can require large contiguous memory (up to order=9) depending on the
> implementation. This change prevents allocation failures by allowing the
> system to fall back to vmalloc when contiguous memory allocation fails.
>
> Since this buffer is only used for debugging purposes, physical memory
> contiguity is not required, making vmalloc a suitable alternative.
>
> Cc: stable@vger.kernel.org
> Fixes: 07814a9439a3b0 ("sched_ext: Print debug dump after an error exit")
> Suggested-by: Rik van Riel <riel@surriel.com>
> Signed-off-by: Breno Leitao <leitao@debian.org>
Makes sense to me.
Acked-by: Andrea Righi <arighi@nvidia.com>
Thanks,
-Andrea
> ---
> kernel/sched/ext.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
> index 66bcd40a28ca1..c82725f9b0559 100644
> --- a/kernel/sched/ext.c
> +++ b/kernel/sched/ext.c
> @@ -4639,7 +4639,7 @@ static struct scx_exit_info *alloc_exit_info(size_t exit_dump_len)
>
> ei->bt = kcalloc(SCX_EXIT_BT_LEN, sizeof(ei->bt[0]), GFP_KERNEL);
> ei->msg = kzalloc(SCX_EXIT_MSG_LEN, GFP_KERNEL);
> - ei->dump = kzalloc(exit_dump_len, GFP_KERNEL);
> + ei->dump = kvzalloc(exit_dump_len, GFP_KERNEL);
>
> if (!ei->bt || !ei->msg || !ei->dump) {
> free_exit_info(ei);
>
> ---
> base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
> change-id: 20250407-scx-11dbf94803c3
>
> Best regards,
> --
> Breno Leitao <leitao@debian.org>
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] sched_ext: Use kvzalloc for large exit_dump allocation
2025-04-07 19:50 [PATCH] sched_ext: Use kvzalloc for large exit_dump allocation Breno Leitao
2025-04-07 20:12 ` Christophe JAILLET
2025-04-07 20:23 ` Andrea Righi
@ 2025-04-08 1:19 ` Changwoo Min
2 siblings, 0 replies; 4+ messages in thread
From: Changwoo Min @ 2025-04-08 1:19 UTC (permalink / raw)
To: Breno Leitao, Tejun Heo, David Vernet, Andrea Righi, Ingo Molnar,
Peter Zijlstra, Juri Lelli, Vincent Guittot, Dietmar Eggemann,
Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider
Cc: linux-kernel, kernel-team, stable, Rik van Riel
Hi,
On 4/8/25 04:50, Breno Leitao wrote:
> Replace kzalloc with kvzalloc for the exit_dump buffer allocation, which
> can require large contiguous memory (up to order=9) depending on the
> implementation. This change prevents allocation failures by allowing the
> system to fall back to vmalloc when contiguous memory allocation fails.
>
> Since this buffer is only used for debugging purposes, physical memory
> contiguity is not required, making vmalloc a suitable alternative.
>
> Cc: stable@vger.kernel.org
> Fixes: 07814a9439a3b0 ("sched_ext: Print debug dump after an error exit")
> Suggested-by: Rik van Riel <riel@surriel.com>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
> kernel/sched/ext.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
> index 66bcd40a28ca1..c82725f9b0559 100644
> --- a/kernel/sched/ext.c
> +++ b/kernel/sched/ext.c
> @@ -4639,7 +4639,7 @@ static struct scx_exit_info *alloc_exit_info(size_t exit_dump_len)
>
> ei->bt = kcalloc(SCX_EXIT_BT_LEN, sizeof(ei->bt[0]), GFP_KERNEL);
> ei->msg = kzalloc(SCX_EXIT_MSG_LEN, GFP_KERNEL);
> - ei->dump = kzalloc(exit_dump_len, GFP_KERNEL);
> + ei->dump = kvzalloc(exit_dump_len, GFP_KERNEL);
>
> if (!ei->bt || !ei->msg || !ei->dump) {
> free_exit_info(ei);
>
The change makes sense to me. But the kfree(ei->dump) in
free_exit_info() also should be replaced with kvfree(ei->dump).
Regards,
Changwoo Min
> ---
> base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
> change-id: 20250407-scx-11dbf94803c3
>
> Best regards,
^ permalink raw reply [flat|nested] 4+ messages in thread