Messages in this thread | | | Date | Fri, 29 Aug 2008 09:37:06 -0700 | From | "Yinghai Lu" <> | Subject | Re: [PATCH] x86: split e820 reserved entries record to late v4 - fix |
| |
On Fri, Aug 29, 2008 at 9:05 AM, Linus Torvalds <torvalds@linux-foundation.org> wrote: > > > On Fri, 29 Aug 2008, Yinghai Lu wrote: >> >> 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. >
good, will build one test stub to test it.
YH
| |