Patchwork unavailable resources

login
register
about
Submitter Stefan Reinauer
Date 2010-06-04 16:29:59
Message ID <4C092A07.3070408@coresystems.de>
Download mbox | patch
Permalink /patch/1465/
State Rejected
Headers show

Comments

Stefan Reinauer - 2010-06-04 16:29:59
See patch
Warn if resources could not be assigned.

Signed-off-by: Stefan Reinauer <stepan@coresystems.de>
Myles Watson - 2010-06-04 16:36:24
if (!(res->flags & IORESOURCE_FIXED))
			continue;
...

 		/* Is it already outside the limits? */
-		if (((res->base + res->size -1) < lim->base) || (res->base > lim->limit))
+		if (((res->base + res->size -1) < lim->base) || (res->base > lim->limit)) {
+			printk(BIOS_ERR, "%s: Resources not available.\n", dev_path(dev));
 			continue;
+		}

This code only is executed for fixed resources.  If they are outside
of the limits it's a good thing.  I don't think there should be an
error message.

Thanks,
Myles
Stefan Reinauer - 2010-06-04 16:41:31
On 6/4/10 6:36 PM, Myles Watson wrote:
> 		if (!(res->flags & IORESOURCE_FIXED))
> 			continue;
> ...
>
>  		/* Is it already outside the limits? */
> -		if (((res->base + res->size -1) < lim->base) || (res->base > lim->limit))
> +		if (((res->base + res->size -1) < lim->base) || (res->base > lim->limit)) {
> +			printk(BIOS_ERR, "%s: Resources not available.\n", dev_path(dev));
>  			continue;
> +		}
>
> This code only is executed for fixed resources.  If they are outside
> of the limits it's a good thing.  I don't think there should be an
> error message.
>   
Why is that good? (can we add that explanation to the code as a comment?)
Myles Watson - 2010-06-04 16:49:45
On Fri, Jun 4, 2010 at 10:41 AM, Stefan Reinauer <stepan@coresystems.de> wrote:
> On 6/4/10 6:36 PM, Myles Watson wrote:
>>               if (!(res->flags & IORESOURCE_FIXED))
>>                       continue;
>> ...
>>
>>               /* Is it already outside the limits? */
>> -             if (((res->base + res->size -1) < lim->base) || (res->base > lim->limit))
>> +             if (((res->base + res->size -1) < lim->base) || (res->base > lim->limit)) {
>> +                     printk(BIOS_ERR, "%s: Resources not available.\n", dev_path(dev));
>>                       continue;
>> +             }
>>
>> This code only is executed for fixed resources.  If they are outside
>> of the limits it's a good thing.  I don't think there should be an
>> error message.
>>
> Why is that good? (can we add that explanation to the code as a comment?)
Sure.  I guess since I wrote it I'm not a good judge of clarifying comments.

Here's the big picture:

You start with a full address space
You constrain what the allocator can use by subtracting fixed resources

And a completely contrived example:
 - take I/O 0x0000-0xffff
 - let's imagine that 0x400-420, 0x40-0x80, and 0xf000-0xf800 are
fixed resources (the allocator can't use that space)
First pass, the allocator chooses to use 0x420-0xffff (larger than 0x0-0x400)
Second pass, the allocator does nothing because 0x40-0x80 is already
outside the limits
Third pass, the allocator chooses to use 0x420-0xf800 (larger than
0xf800-0xffff.

I'd love it if you'd add some clarifying comments.

Thanks,
Myles

Patch

Index: src/devices/device.c
===================================================================
--- src/devices/device.c	(revision 5568)
+++ src/devices/device.c	(working copy)
@@ -578,8 +578,10 @@ 
 			continue;
 
 		/* Is it already outside the limits? */
-		if (((res->base + res->size -1) < lim->base) || (res->base > lim->limit))
+		if (((res->base + res->size -1) < lim->base) || (res->base > lim->limit)) {
+			printk(BIOS_ERR, "%s: Resources not available.\n", dev_path(dev));
 			continue;
+		}
 
 		/* Choose to be above or below fixed resources.  This
 		 * check is signed so that "negative" amounts of space