From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751925AbbLRWuZ (ORCPT ); Fri, 18 Dec 2015 17:50:25 -0500 Received: from mail.linuxfoundation.org ([140.211.169.12]:41564 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751356AbbLRWuX (ORCPT ); Fri, 18 Dec 2015 17:50:23 -0500 Date: Fri, 18 Dec 2015 14:50:22 -0800 From: Andrew Morton To: Vitaly Kuznetsov Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Tang Chen , Naoya Horiguchi , Xishi Qiu , Sheng Yong , David Rientjes , Zhu Guihua , Dan Williams , David Vrabel , Igor Mammedov Subject: Re: [PATCH] memory-hotplug: don't BUG() in register_memory_resource() Message-Id: <20151218145022.eae1e368c82f090900582fcc@linux-foundation.org> In-Reply-To: <1450450224-18515-1-git-send-email-vkuznets@redhat.com> References: <1450450224-18515-1-git-send-email-vkuznets@redhat.com> X-Mailer: Sylpheed 3.4.1 (GTK+ 2.24.23; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 18 Dec 2015 15:50:24 +0100 Vitaly Kuznetsov wrote: > Out of memory condition is not a bug and while we can't add new memory in > such case crashing the system seems wrong. Propagating the return value > from register_memory_resource() requires interface change. > > --- a/mm/memory_hotplug.c > +++ b/mm/memory_hotplug.c > +static int register_memory_resource(u64 start, u64 size, > + struct resource **resource) > { > struct resource *res; > res = kzalloc(sizeof(struct resource), GFP_KERNEL); > - BUG_ON(!res); > + if (!res) > + return -ENOMEM; > > res->name = "System RAM"; > res->start = start; > @@ -140,9 +142,10 @@ static struct resource *register_memory_resource(u64 start, u64 size) > if (request_resource(&iomem_resource, res) < 0) { > pr_debug("System RAM resource %pR cannot be added\n", res); > kfree(res); > - res = NULL; > + return -EEXIST; > } > - return res; > + *resource = res; > + return 0; > } Was there a reason for overwriting the request_resource() return value? Ordinarily it should be propagated back to callers. Please review. --- a/mm/memory_hotplug.c~memory-hotplug-dont-bug-in-register_memory_resource-fix +++ a/mm/memory_hotplug.c @@ -131,7 +131,9 @@ static int register_memory_resource(u64 struct resource **resource) { struct resource *res; + int ret = 0; res = kzalloc(sizeof(struct resource), GFP_KERNEL); + if (!res) return -ENOMEM; @@ -139,13 +141,14 @@ static int register_memory_resource(u64 res->start = start; res->end = start + size - 1; res->flags = IORESOURCE_MEM | IORESOURCE_BUSY; - if (request_resource(&iomem_resource, res) < 0) { + ret = request_resource(&iomem_resource, res); + if (ret < 0) { pr_debug("System RAM resource %pR cannot be added\n", res); kfree(res); - return -EEXIST; + } else { + *resource = res; } - *resource = res; - return 0; + return ret; } static void release_memory_resource(struct resource *res) _