On Sun, Jul 03, 2011 at 02:30:00PM -0700, Linus Torvalds wrote:
> Gaah. I'm still rather uncomfortable about this, and I wonder about
> patch 2 in particular. It seems that that patch could/should be split
> up: the whole change to "find_resource()" etc looks like prime
> material for a separate patch that splits up that function and
> explains why that change is done.
>
> Also, quite frankly, by the time you pass in eight different arguments
> (and pretty complex ones at that, with one being the alignment
> function pointer), I start thinking that you should have passed in a
> pointer to a descriptor structure instead. I get the feeling that the
> "resource requirements" really should be a structure instead of lots
> of individual arguments:
>
> IOW, this part:
>
> + resource_size_t newsize, resource_size_t min,
> + resource_size_t max, resource_size_t align,
> + resource_size_t (*alignf)(void *,
> + const struct resource *,
> + resource_size_t,
> + resource_size_t),
> + void *alignf_data)
>
> really makes me go
>
> struct resource_requirement {
> resource_size_t min, max, align;
> resource_size_t (*alignf)(const struct resource *, struct
> resource_requirement *);
> void *alignf_data);
> };
>
> and I'd really change the function argument to take that kind of
> simplified thing instead.
>
> And that cleanup/re-organization would be prime material for a totally
> independent patch that changes no semantics at all, just prepares for
> the other changes.
>
> That way the final "patch 2" would be smaller and do the semantic
> changes, instead of being a mix of semantic changes and infrastructure
> changes.
>
> And some of the cleanup stuff I could merge for 3.0 just to make things easier.
>
> Hmm?
Here is a cleaned up patch that just adds functionality to kernel/resource.c
It does make a small semantic addition to allocate_resource(), where it reallocates
the resource with a newer size if that resource was already allocated.
Will this be acceptable for 3.0.0?
From: Ram Pai <linuxram@xxxxxxxxxx>
Date: Tue, 5 Jul 2011 23:44:30 -0700
Subject: [PATCH 1/1] resource: ability to resize an allocated resource
Provides the ability to resize a resource that is already allocated.
This functionality is put in place to support reallocation needs of
pci resources.
Signed-off-by: Ram Pai <linuxram@xxxxxxxxxx>
---
kernel/resource.c | 118 ++++++++++++++++++++++++++++++++++++++++++++---------
1 files changed, 99 insertions(+), 19 deletions(-)
diff --git a/kernel/resource.c b/kernel/resource.c
index 798e2fa..ba727b6 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -38,6 +38,14 @@ struct resource iomem_resource = {
};
EXPORT_SYMBOL(iomem_resource);
+/* constraints to be met while allocating resources */
+struct resource_constraint {
+ resource_size_t min, max, align;
+ resource_size_t (*alignf)(void *, const struct resource *,
+ resource_size_t, resource_size_t);
+ void *alignf_data;
+};
+
static DEFINE_RWLOCK(resource_lock);
static void *r_next(struct seq_file *m, void *v, loff_t *pos)
@@ -384,16 +392,13 @@ static bool resource_contains(struct resource *res1, struct resource *res2)
}
/*
- * Find empty slot in the resource tree given range and alignment.
+ * Find empty slot in the resource tree with the given range and
+ * alignment constraints
*/
-static int find_resource(struct resource *root, struct resource *new,
- resource_size_t size, resource_size_t min,
- resource_size_t max, resource_size_t align,
- resource_size_t (*alignf)(void *,
- const struct resource *,
- resource_size_t,
- resource_size_t),
- void *alignf_data)
+static int __find_resource(struct resource *root, struct resource *old,
+ struct resource *new,
+ resource_size_t size,
+ struct resource_constraint *constraint)
{
struct resource *this = root->child;
struct resource tmp = *new, avail, alloc;
@@ -404,25 +409,26 @@ static int find_resource(struct resource *root, struct resource *new,
* Skip past an allocated resource that starts at 0, since the assignment
* of this->start - 1 to tmp->end below would cause an underflow.
*/
- if (this && this->start == 0) {
- tmp.start = this->end + 1;
- this = this->sibling;
+ if (this && this->start == root->start) {
+ tmp.start = (this == old) ? old->start : this->end + 1;
+ this = this->sibling;
}
for(;;) {
if (this)
- tmp.end = this->start - 1;
+ tmp.end = (this == old) ? this->end : this->start - 1;
else
tmp.end = root->end;
- resource_clip(&tmp, min, max);
+ resource_clip(&tmp, constraint->min, constraint->max);
arch_remove_reservations(&tmp);
/* Check for overflow after ALIGN() */
avail = *new;
- avail.start = ALIGN(tmp.start, align);
+ avail.start = ALIGN(tmp.start, constraint->align);
avail.end = tmp.end;
if (avail.start >= tmp.start) {
- alloc.start = alignf(alignf_data, &avail, size, align);
+ alloc.start = constraint->alignf(constraint->alignf_data, &avail,
+ size, constraint->align);
alloc.end = alloc.start + size - 1;
if (resource_contains(&avail, &alloc)) {
new->start = alloc.start;
@@ -432,14 +438,75 @@ static int find_resource(struct resource *root, struct resource *new,
}
if (!this)
break;
- tmp.start = this->end + 1;
+ if (this != old)
+ tmp.start = this->end + 1;
this = this->sibling;
}
return -EBUSY;
}
+/*
+ * Find empty slot in the resource tree given range and alignment.
+ */
+static int find_resource(struct resource *root, struct resource *new,
+ resource_size_t size,
+ struct resource_constraint *constraint)
+{
+ return __find_resource(root, NULL, new, size, constraint);
+}
+
+/**
+ * reallocate_resource - allocate a slot in the resource tree given range & alignment.
+ * The resource will be relocated if the new size cannot be reallocated in the
+ * current location.
+ *
+ * @root: root resource descriptor
+ * @old: resource descriptor desired by caller
+ * @newsize: new size of the resource descriptor
+ * @constraint: the size and alignment constraints to be met.
+ */
+int reallocate_resource(struct resource *root, struct resource *old,
+ resource_size_t newsize,
+ struct resource_constraint *constraint)
+{
+ int err=0;
+ struct resource new = *old;
+ struct resource *conflict;
+
+ write_lock(&resource_lock);
+
+ if ((err = __find_resource(root, old, &new, newsize, constraint)))
+ goto out;
+
+ if (resource_contains(&new, old)) {
+ old->start = new.start;
+ old->end = new.end;
+ goto out;
+ }
+
+ if (old->child) {
+ err = -EBUSY;
+ goto out;
+ }
+
+ if (resource_contains(old, &new)) {
+ old->start = new.start;
+ old->end = new.end;
+ } else {
+ __release_resource(old);
+ *old = new;
+ conflict = __request_resource(root, old);
+ BUG_ON(conflict);
+ }
+out:
+ write_unlock(&resource_lock);
+ return err;
+}
+
+
/**
- * allocate_resource - allocate empty slot in the resource tree given range & alignment
+ * allocate_resource - allocate empty slot in the resource tree given range & alignment.
+ * The resource will be reallocated with a new size if it was already allocated
* @root: root resource descriptor
* @new: resource descriptor desired by caller
* @size: requested resource region size
@@ -459,12 +526,25 @@ int allocate_resource(struct resource *root, struct resource *new,
void *alignf_data)
{
int err;
+ struct resource_constraint constraint;
if (!alignf)
alignf = simple_align_resource;
+ constraint.min = min;
+ constraint.max = max;
+ constraint.align = align;
+ constraint.alignf = alignf;
+ constraint.alignf_data = alignf_data;
+
+ if ( new->parent ) {
+ /* resource is already allocated, try reallocating with
+ the new constraints */
+ return reallocate_resource(root, new, size, &constraint);
+ }
+
write_lock(&resource_lock);
- err = find_resource(root, new, size, min, max, align, alignf, alignf_data);
+ err = find_resource(root, new, size, &constraint);
if (err >= 0 && __request_resource(root, new))
err = -EBUSY;
write_unlock(&resource_lock);
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
