On Fri, 18 Dec 2015 15:50:24 +0100 Vitaly Kuznetsov <[email protected]> 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)
_

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to