x86: split e820 reserved entries record to late v4 - fix

try to insert_resource second time, by expand the resource...

for case: e820 reserved entry is partially overlapped with bar res...

hope it will never happen

Signed-off-by Yinghai Lu <[email protected]>

---
 arch/x86/kernel/e820.c |   32 ++++++++++++++++++++++++++++++--
 include/linux/ioport.h |    1 +
 kernel/resource.c      |   35 ++++++++++++++++++++++++++---------
 3 files changed, 57 insertions(+), 11 deletions(-)

Index: linux-2.6/arch/x86/kernel/e820.c
===================================================================

arch/x86/kernel/e820.c +7 -2

@@ -1319,8 +1319,36 @@ void __init e820_reserve_resources_late(
 
       res = e820_res;
       for (i = 0; i < e820.nr_map; i++) {
-               if (!res->parent && res->end)
-                       insert_resource(&iomem_resource, res);
+               if (!res->parent && res->end) {
+                       struct resource *ret;
+
+                       ret = __insert_resource(&iomem_resource, res);
+                       if (ret) {
+                               int updated = 0;
+

arch/x86/kernel/e820.c +13 -0

+                                               res->end = ret->end;
+                                               updated = 1;
+                                       }
+                               }
+                               if (updated) {
+                                       ret = __insert_resource(&iomem_resource, res);
+                                       if (ret)
+                                               printk(KERN_WARNING "second try found conflict: %s [%08llx, %08llx] with %s [%08llx, %08llx]\n",
+                                                       res->name, res->start, res->end,
+                                                       ret->name, ret->start, ret->end);
+                               }
+                       }
+               }
               res++;
       }
 }

Index: linux-2.6/include/linux/ioport.h
===================================================================

include/linux/ioport.h +1 -0

@@ -108,6 +108,7 @@ extern struct resource iomem_resource;
 
 extern int request_resource(struct resource *root, struct resource *new);
 extern int release_resource(struct resource *new);
+extern struct resource *__insert_resource(struct resource *parent, struct resource *new);
 extern int insert_resource(struct resource *parent, struct resource *new);
 extern int allocate_resource(struct resource *root, struct resource *new,
                            resource_size_t size, resource_size_t min,

Index: linux-2.6/kernel/resource.c
===================================================================

kernel/resource.c +26 -9

@@ -363,11 +363,11 @@ int allocate_resource(struct resource *r
 EXPORT_SYMBOL(allocate_resource);
 
 /**
- * insert_resource - Inserts a resource in the resource tree
+ * __insert_resource - Inserts a resource in the resource tree
 * @parent: parent of the new resource
 * @new: new resource to insert
 *
- * Returns 0 on success, -EBUSY if the resource can't be inserted.
+ * Returns NULL on success, or first conflict resource.
 *
 * This function is equivalent to request_resource when no conflict
 * happens. If a conflict happens, and the conflicting resources

@@ -375,20 +375,20 @@ EXPORT_SYMBOL(allocate_resource);
 * resource is inserted and the conflicting resources become children of
 * the new resource.
 */
-int insert_resource(struct resource *parent, struct resource *new)
+struct resource *__insert_resource(struct resource *parent, struct resource *new)
 {
-       int result;
+       struct resource *ret_res;
       struct resource *first, *next;
 
       write_lock(&resource_lock);
 
       for (;; parent = first) {
-               result = 0;
+               ret_res = NULL;
               first = __request_resource(parent, new);
               if (!first)
                       goto out;
 
-               result = -EBUSY;
+               ret_res = first;
               if (first == parent)
                       goto out;
 

@@ -400,15 +400,17 @@ int insert_resource(struct resource *par
 
       for (next = first; ; next = next->sibling) {
               /* Partial overlap? Bad, and unfixable */
-               if (next->start < new->start || next->end > new->end)
+               if (next->start < new->start || next->end > new->end) {
+                       ret_res = next;
                       goto out;
+               }
               if (!next->sibling)
                       break;
               if (next->sibling->start > new->end)
                       break;
       }
 
-       result = 0;
+       ret_res = NULL;
 
       new->parent = parent;
       new->sibling = next->sibling;

@@ -429,10 +431,25 @@ int insert_resource(struct resource *par
 
 out:
       write_unlock(&resource_lock);
-       return result;
+       return ret_res;
 }
 
 /**
+ * insert_resource - Inserts a resource in the resource tree
+ * @parent: parent of the new resource
+ * @new: new resource to insert
+ *
+ * Returns 0 on success, -EBUSY if the resource can't be inserted.
+ */
+int insert_resource(struct resource *parent, struct resource *new)
+{
+       struct resource *res_conflict;
+
+       res_conflict = __insert_resource(parent, new);
+
+       return res_conflict ? -EBUSY : 0;
+}
+/**
 * adjust_resource - modify a resource's start and size
 * @res: resource to modify
 * @start: new start value

try to insert_resource second time, by expand the resource...

Quoted 2 lines · Yinghai Lu

try to insert_resource second time, by expand the resource...

I would hold off on this unless it's shown to actually be needed. And _if_ it is needed, I would just make a new function for doing this all:
"insert_resource_expand_to_fit()"

That said, I think the insert_resource()/__insert_resource() change is pretty ok. However, it doesn't follow the rules, and is racy. The rules for resources are:

- the "internal" version (with the "__" prepended) is static to resource.c, because it must not be called from outside, which is in turn because:

- it must be called with the lock taken by the caller, because otherwise returning a "struct resource *" is racy - the resource is not protected by anything!

So the "insert_resource_expand_to_fit()" thing would look something like this:

       void insert_resource_expand_to_fit(struct resource *root, struct resource *new)
       {
               write_lock(&resource_lock);
               while (new->start && new->parent) {
                       struct resource *conflict;

                       conflict = __insert_resource(root, new);
                       if (!conflict)
                               break;
                       if (conflict->start < new->start)
                               new->start = conflict->start;
                       if (conflict->end > new->end)
                               new->end = conflict->end;
               }
               write_unlock(&resource_lock);
       }

but the above is obviously _totally_ untested.

Quoted 5 lines

       void insert_resource_expand_to_fit(struct resource *root, struct resource *new)
       {
               write_lock(&resource_lock);
               while (new->start && new->parent) {