| Message ID | alpine.DEB.2.00.1007201938100.8728@chino.kir.corp.google.com |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
From: David Rientjes <rientjes@google.com> Date: Tue, 20 Jul 2010 19:44:53 -0700 (PDT) > The kmalloc() in mdesc_kmalloc() is failable, so remove __GFP_NOFAIL from > its mask. > > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Signed-off-by: David Rientjes <rientjes@google.com> The __GFP_NOFAIL is there intentionally. The code above this, in the cases where the machine description is dynamically updated by the hypervisor at run time, long after boot, has no failure handling. We absolutely must accept the machine descriptor update and fetch it from the hypervisor into a new buffer. Please don't remove this. -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 20 Jul 2010, David Miller wrote: > > The kmalloc() in mdesc_kmalloc() is failable, so remove __GFP_NOFAIL from > > its mask. > > > > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> > > Signed-off-by: David Rientjes <rientjes@google.com> > > The __GFP_NOFAIL is there intentionally. > > The code above this, in the cases where the machine description is > dynamically updated by the hypervisor at run time, long after boot, > has no failure handling. > > We absolutely must accept the machine descriptor update and fetch it > from the hypervisor into a new buffer. > > Please don't remove this. > Ok, fair enough. I was convinced by the error handling in both mdesc_update() and mdesc_kmallloc() that this was a failable allocation, but I understand how mdesc_update() must succeed given your description. We can remove those branches from those two functions, though, since __GFP_NOFAIL will always succeed before returning. I'm planning on replacing __GFP_NOFAIL with a __GFP_KILLABLE flag that will use all of the page allocator's capabilities (direct reclaim, memory compaction for order > 0, and the oom killer) before failing. Then, existing __GFP_NOFAIL users can use do { page = alloc_page(GFP_KERNEL | __GFP_KILLABLE); } while (!page); to remove several branches from the page allocator that we'll no longer need. I'll do this in phase two and make sure to convert this instance to do that. Thanks! -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/sparc/kernel/mdesc.c b/arch/sparc/kernel/mdesc.c --- a/arch/sparc/kernel/mdesc.c +++ b/arch/sparc/kernel/mdesc.c @@ -134,7 +134,7 @@ static struct mdesc_handle *mdesc_kmalloc(unsigned int mdesc_size) sizeof(struct mdesc_hdr) + mdesc_size); - base = kmalloc(handle_size + 15, GFP_KERNEL | __GFP_NOFAIL); + base = kmalloc(handle_size + 15, GFP_KERNEL); if (base) { struct mdesc_handle *hp; unsigned long addr;
The kmalloc() in mdesc_kmalloc() is failable, so remove __GFP_NOFAIL from its mask. Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> Signed-off-by: David Rientjes <rientjes@google.com> --- arch/sparc/kernel/mdesc.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html