diff mbox series

zynq: clkc: Add kmalloc allocation flag

Message ID 20230222215834.3507-1-zeming@nfschina.com
State New
Headers
Series zynq: clkc: Add kmalloc allocation flag |

Commit Message

Li zeming Feb. 22, 2023, 9:58 p.m. UTC 
  The kmalloc could crash if allocation fails. Add the __GFP_NOFAIL flag
to ensure that allocation succeeds every time.

Signed-off-by: Li zeming <zeming@nfschina.com>
---
 drivers/clk/zynq/clkc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Stephen Boyd Feb. 21, 2023, 10:50 p.m. UTC | #1 
Quoting Li zeming (2023-02-22 13:58:34)
> The kmalloc could crash if allocation fails. Add the __GFP_NOFAIL flag
> to ensure that allocation succeeds every time.
> 
> Signed-off-by: Li zeming <zeming@nfschina.com>
> ---
>  drivers/clk/zynq/clkc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/zynq/clkc.c b/drivers/clk/zynq/clkc.c
> index 7bdeaff2bfd6..7621c2f00468 100644
> --- a/drivers/clk/zynq/clkc.c
> +++ b/drivers/clk/zynq/clkc.c
> @@ -427,7 +427,7 @@ static void __init zynq_clk_setup(struct device_node *np)
>                         SLCR_GEM1_CLK_CTRL, 0, 0, &gem1clk_lock);
>  
>         tmp = strlen("mio_clk_00x");
> -       clk_name = kmalloc(tmp, GFP_KERNEL);
> +       clk_name = kmalloc(tmp, GFP_KERNEL | __GFP_NOFAIL);

There are so many more allocations happening in this function and they
aren't marked nofail. Why is this one special?

I could see a patch moving mio_clk_00x to the stack and then printing to
it. But it is also fine like this, so I don't see any reason to change
this.
Li zeming Feb. 23, 2023, 6:33 p.m. UTC | #2 
hello senior:
  I observed that some other variable assignments in this function are basically judged by the if statement, while clk_name does not make an if branch statement, and I think clk_name is also relatively important, increasing __GFP_NOFAIL flag ensures that the assignment can succeed under any circumstances.
Michal Simek Feb. 27, 2023, 2:38 p.m. UTC | #3 
Hi,

On 2/23/23 19:33, Li zeming wrote:
> 
> hello senior:
>    I observed that some other variable assignments in this function are basically judged by the if statement, while clk_name does not make an if branch statement, and I think clk_name is also relatively important, increasing __GFP_NOFAIL flag ensures that the assignment can succeed under any circumstances.
> 

I think that solution with array on stack would be better choice.
It will be faster and you can completely skip the whole allocation code for it.

Thanks,
Michal
diff mbox series

Patch

diff --git a/drivers/clk/zynq/clkc.c b/drivers/clk/zynq/clkc.c
index 7bdeaff2bfd6..7621c2f00468 100644
--- a/drivers/clk/zynq/clkc.c
+++ b/drivers/clk/zynq/clkc.c
@@ -427,7 +427,7 @@  static void __init zynq_clk_setup(struct device_node *np)
 			SLCR_GEM1_CLK_CTRL, 0, 0, &gem1clk_lock);
 
 	tmp = strlen("mio_clk_00x");
-	clk_name = kmalloc(tmp, GFP_KERNEL);
+	clk_name = kmalloc(tmp, GFP_KERNEL | __GFP_NOFAIL);
 	for (i = 0; i < NUM_MIO_PINS; i++) {
 		int idx;