Patchwork resource allocator hole handling (simple and old)

login
register
about
Submitter Myles Watson
Date 2009-10-25 04:01:36
Message ID <81F3F3D229614447B12493A64AF500E7@chimp>
Download mbox | patch
Permalink /patch/485/
State Accepted
Headers show

Comments

Myles Watson - 2009-10-25 04:01:36
continue;

I think fixed resources of size 0 are broken.  I'd rather fix the targets.
It's hard for the allocator to avoid size 0 resources.

Thanks,
Myles
Stefan Reinauer - 2009-10-28 14:35:34
Myles Watson wrote:
> Index: src/devices/device.c
> ===================================================================
> --- src/devices/device.c	(revision 4842)
> +++ src/devices/device.c	(working copy)
> @@ -576,7 +576,7 @@
>  			continue;
>  
>  		/* Is it already outside the limits? */
> -		if (res->size && (((res->base + res->size -1) < lim->base)
> ||
> +		if (!res->size || (((res->base + res->size -1) < lim->base)
> ||
>  				  (res->base > lim->limit)))
>  			continue;
>
> I think fixed resources of size 0 are broken.  I'd rather fix the targets.
> It's hard for the allocator to avoid size 0 resources.
>   

Ok, this seems to be a left-over which came in through intelligent merging.

I checked the code and size 0 resources are skipped already..

                if (!res->size) {
                        /* It makes no sense to have 0-sized, fixed
resources.*/
                        printk_err("skipping %s@%lx fixed resource,
size=0!\n",
                                   dev_path(dev), res->index);
                        continue;
                }

so generally that check for res->size could be dropped completely in
above construct, making it

         /* Is it already outside the limits? */
-        if (res->size && (((res->base + res->size -1) < lim->base) ||
-                  (res->base > lim->limit)))
+        if (((res->base + res->size -1) < lim->base) || (res->base >
lim->limit))
             continue;


Stefan
Myles Watson - 2009-10-28 14:51:46
> I checked the code and size 0 resources are skipped already..
Yes, I should have remembered that.

> so generally that check for res->size could be dropped completely in
> above construct, making it
> 
>          /* Is it already outside the limits? */
> -        if (res->size && (((res->base + res->size -1) < lim->base) ||
> -                  (res->base > lim->limit)))
> +        if (((res->base + res->size -1) < lim->base) || (res->base >
> lim->limit))
>              continue;
That could be done.  There's no functional difference as long as the check
for size 0 resources stays.

Thanks,
Myles

Patch

Index: src/devices/device.c
===================================================================
--- src/devices/device.c	(revision 4842)
+++ src/devices/device.c	(working copy)
@@ -576,7 +576,7 @@ 
 			continue;
 
 		/* Is it already outside the limits? */
-		if (res->size && (((res->base + res->size -1) < lim->base)
||
+		if (!res->size || (((res->base + res->size -1) < lim->base)
||
 				  (res->base > lim->limit)))