Thread (55 messages) 55 messages, 12 authors, 2019-10-02

Re: [PATCH v2 19/21] treewide: add checks for the return value of memblock_alloc*()

From: Christophe Leroy <hidden>
Date: 2019-01-31 06:44:23
Also in: linux-alpha, linux-arm-kernel, linux-devicetree, linux-mips, linux-mm, linux-s390, linux-sh, linux-um, linux-usb, lkml, sparclinux


Le 31/01/2019 à 07:41, Mike Rapoport a écrit :
On Thu, Jan 31, 2019 at 07:07:46AM +0100, Christophe Leroy wrote:
quoted

Le 21/01/2019 à 09:04, Mike Rapoport a écrit :
quoted
Add check for the return value of memblock_alloc*() functions and call
panic() in case of error.
The panic message repeats the one used by panicing memblock allocators with
adjustment of parameters to include only relevant ones.

The replacement was mostly automated with semantic patches like the one
below with manual massaging of format strings.

@@
expression ptr, size, align;
@@
ptr = memblock_alloc(size, align);
+ if (!ptr)
+ 	panic("%s: Failed to allocate %lu bytes align=0x%lx\n", __func__,
size, align);

Signed-off-by: Mike Rapoport <redacted>
Reviewed-by: Guo Ren <redacted>             # c-sky
Acked-by: Paul Burton <redacted>	     # MIPS
Acked-by: Heiko Carstens <redacted> # s390
Reviewed-by: Juergen Gross <jgross@suse.com>         # Xen
---
[...]
quoted
diff --git a/mm/sparse.c b/mm/sparse.c
index 7ea5dc6..ad94242 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
[...]
quoted
@@ -425,6 +436,10 @@ static void __init sparse_buffer_init(unsigned long size, int nid)
  		memblock_alloc_try_nid_raw(size, PAGE_SIZE,
  						__pa(MAX_DMA_ADDRESS),
  						MEMBLOCK_ALLOC_ACCESSIBLE, nid);
+	if (!sparsemap_buf)
+		panic("%s: Failed to allocate %lu bytes align=0x%lx nid=%d from=%lx\n",
+		      __func__, size, PAGE_SIZE, nid, __pa(MAX_DMA_ADDRESS));
+
memblock_alloc_try_nid_raw() does not panic (help explicitly says: Does not
zero allocated memory, does not panic if request cannot be satisfied.).
"Does not panic" does not mean it always succeeds.
I agree, but at least here you are changing the behaviour by making it 
panic explicitly. Are we sure there are not cases where the system could 
just continue functionning ? Maybe a WARN_ON() would be enough there ?

Christophe
  
quoted
Stephen Rothwell reports a boot failure due to this change.
Please see my reply on that thread.
quoted
Christophe
quoted
  	sparsemap_buf_end = sparsemap_buf + size;
  }
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help