Question of resource_size() implementation

Question of resource_size() implementation

[Felipe Balbi]

Hi all,

I'm wondering whether the +1 in resource_size() is actually necessary.
To illustrate the problem imagine we have two subsequent 4k memory areas
which should be passed to ioremap(). When passing those address spaces
to the drivers we will use struct resource and define something like:

#define MEM_AREA1_BASE	0x480ab000
#define MEM_AREA2_BASE	0x480ac000

struct resource my_device_resources = {
	{
		.start	= MEM_AREA1_BASE,
		.end	= MEM_AREA1_BASE + SZ_4K - 1,
		.flags	= IORESOURCE_MEM,
	},
	{
		.start	= MEM_AREA2_BASE,
		.end	= MEM_AREA2_BASE + SZ_4K - 1,
		.flags	= IORESOURCE_MEM,
	},
};

that -1 would cause us to pass the correct end for each area,
respectively 0x480abfff and 0x480acffff, right ?

But resource_size() is defined as:

static inline resource_size_t resource_size(const struct resource *res)
{
	return res->end - res->start + 1;
}

then we put +1 again and the end addresses would then become
respectively 0x480ac000 and 0x480ad0000. Wouldn't we then fail ioremap()
of the second area ? wouldn't it return -EBUSY ? Are we off-by-one
here ? Or is this all expected ?

I have to say, I didn't test this on real hw yet...

-- 
balbi

Re: Question of resource_size() implementation

[Felipe Balbi]

On Tue, 2009-12-29 at 16:12 -0800, Joe Perches wrote:
> On Wed, 2009-12-30 at 01:43 +0200, Felipe Balbi wrote:
> > I'm wondering whether the +1 in resource_size() is actually necessary.
> > resource_size() is defined as:
> []
> > static inline resource_size_t resource_size(const struct resource *res)
> > {
> > 	return res->end - res->start + 1;
> > }
> > Are we off-by-one
> > here ? Or is this all expected ?
> 
> Imagine you have 1 byte sized resources.
> 
> AREA1 = 0x40000000
> AREA2 = 0x40000001
> 
> area1.start = 0x40000000
> area1.end   = 0x40000000
> 
> area2.start = 0x40000001
> area2.end   = 0x40000001

(adding lkml back to the loop)

in that you wouldn't use any of the SZ_* macros and simply hardcode
start and end, right ? then you would define:

area1.start = 0x40000000
area1.end = 0x40000001

and ioremap 2 bytes due to +1 in resource_size().

-- 
balbi

Re: Question of resource_size() implementation

[Ben Nizette]

On 30/12/2009, at 11:16 AM, Felipe Balbi wrote:

> On Tue, 2009-12-29 at 16:12 -0800, Joe Perches wrote:
>> On Wed, 2009-12-30 at 01:43 +0200, Felipe Balbi wrote:
>>> I'm wondering whether the +1 in resource_size() is actually necessary.
>>> resource_size() is defined as:
>> []
>>> static inline resource_size_t resource_size(const struct resource *res)
>>> {
>>> 	return res->end - res->start + 1;
>>> }
>>> Are we off-by-one
>>> here ? Or is this all expected ?
>> 
>> Imagine you have 1 byte sized resources.
>> 
>> AREA1 = 0x40000000
>> AREA2 = 0x40000001
>> 
>> area1.start = 0x40000000
>> area1.end   = 0x40000000
>> 
>> area2.start = 0x40000001
>> area2.end   = 0x40000001
> 
> (adding lkml back to the loop)
> 
> in that you wouldn't use any of the SZ_* macros and simply hardcode
> start and end, right ? then you would define:
> 
> area1.start = 0x40000000
> area1.end = 0x40000001

No-o, you'd hardcode

area1.start=0x40000000
area1.end=0x40000000

As Joe wrote.  Then the resource_size would return 1 and ioremap would map the 1 byte starting at 0x400000000, i.e. just 0x400000000.  This is correct.

In your original example you had

		.start	= MEM_AREA1_BASE,
		.end	= MEM_AREA1_BASE + SZ_4K - 1

So resource_size would return SZ_4K and ioremap would map the SZ_4K bytes starting at MEM_AREA1_BASE, i.e. MEM_AREA1_BASE to MEM_AREA1_BASE - 1 /inclusive/.  This is also correct.

	--Ben.

Re: Question of resource_size() implementation

[Ben Nizette]

On 30/12/2009, at 12:04 PM, Ben Nizette wrote:
> 
> 
> So resource_size would return SZ_4K and ioremap would map the SZ_4K bytes starting at MEM_AREA1_BASE, i.e. MEM_AREA1_BASE to MEM_AREA1_BASE - 1 /inclusive/.  This is also correct.
> 

IOW it's a matter of convention.  Either we could define a single-byte range to be end=start and put the +1 in the resource_size or we could define a single-byte range to be end=start+1 and leave the +1 out of resource_size.  The in-kernel convention is the former

> 	--Ben.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: Question of resource_size() implementation

[Felipe Balbi]

On Wed, 2009-12-30 at 12:08 +1100, Ben Nizette wrote:
> IOW it's a matter of convention.  Either we could define a single-byte 
> range to be end=start and put the +1 in the resource_size or we could 
> define a single-byte range to be end=start+1 and leave the +1 out of
> resource_size.  The in-kernel convention is the former

ok, thanks for the explanation Ben.

-- 
balbi