[PATCH 0/2] x86: Speed up ioremap operations

[PATCH 0/2] x86: Speed up ioremap operations

[Mike Travis]

We have a large university system in the UK that is experiencing
very long delays modprobing the driver for a specific I/O device.
The delay is from 8-10 minutes per device and there are 31 devices
in the system.  This 4 to 5 hour delay in starting up those I/O
devices is very much a burden on the customer.

There are two causes for requiring a restart/reload of the drivers.
First is periodic preventive maintenance (PM) and the second is if
any of the devices experience a fatal error.  Both of these trigger
this excessively long delay in bringing the system back up to full
capability.

The problem was tracked down to a very slow IOREMAP operation and
the excessively long ioresource lookup to insure that the user is
not attempting to ioremap RAM.  These patches provide a speed up
to that function.

-- 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 0/2] x86: Speed up ioremap operations

[Mike Travis]

We have a large university system in the UK that is experiencing
very long delays modprobing the driver for a specific I/O device.
The delay is from 8-10 minutes per device and there are 31 devices
in the system.  This 4 to 5 hour delay in starting up those I/O
devices is very much a burden on the customer.

There are two causes for requiring a restart/reload of the drivers.
First is periodic preventive maintenance (PM) and the second is if
any of the devices experience a fatal error.  Both of these trigger
this excessively long delay in bringing the system back up to full
capability.

The problem was tracked down to a very slow IOREMAP operation and
the excessively long ioresource lookup to insure that the user is
not attempting to ioremap RAM.  These patches provide a speed up
to that function.

-- 

[PATCH 1/2] x86: Optimize resource lookups for ioremap

[Mike Travis]

[-- Attachment #1: add-get-resource-type --]
[-- Type: text/plain, Size: 2872 bytes --]

Since the ioremap operation is verifying that the specified address range
is NOT RAM, it will search the entire ioresource list if the condition
is true.  To make matters worse, it does this one 4k page at a time.
For a 128M BAR region this is 32 passes to determine the entire region
does not contain any RAM addresses.

This patch provides another resource lookup function, region_is_ram,
that searches for the entire region specified, verifying that it is
completely contained within the resource region.  If it is found, then
it is checked to be RAM or not, within a single pass.

The return result reflects if it was found or not (-1), and whether it is
RAM (1) or not (0).  This allows the caller to fallback to the previous
page by page search if it was not found.

Signed-off-by: Mike Travis <travis@sgi.com>
Acked-by: Alex Thorlton <athorlton@sgi.com>
Reviewed-by: Cliff Wickman <cpw@sgi.com>
---
 include/linux/mm.h |    1 +
 kernel/resource.c  |   37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+)

--- linux.orig/include/linux/mm.h
+++ linux/include/linux/mm.h
@@ -346,6 +346,7 @@ static inline int put_page_unless_one(st
 }
 
 extern int page_is_ram(unsigned long pfn);
+extern int region_is_ram(resource_size_t phys_addr, unsigned long size);
 
 /* Support for virtually mapped pages */
 struct page *vmalloc_to_page(const void *addr);
--- linux.orig/kernel/resource.c
+++ linux/kernel/resource.c
@@ -494,6 +494,43 @@ int __weak page_is_ram(unsigned long pfn
 }
 EXPORT_SYMBOL_GPL(page_is_ram);
 
+/*
+ * Search for a resouce entry that fully contains the specified region.
+ * If found, return 1 if it is RAM, 0 if not.
+ * If not found, or region is not fully contained, return -1
+ *
+ * Used by the ioremap functions to insure user not remapping RAM and is as
+ * vast speed up over walking through the resource table page by page.
+ */
+int __weak region_is_ram(resource_size_t start, unsigned long size)
+{
+	struct resource *p;
+	resource_size_t end = start + size - 1;
+	int flags = IORESOURCE_MEM | IORESOURCE_BUSY;
+	const char *name = "System RAM";
+	int ret = -1;
+
+	read_lock(&resource_lock);
+	for (p = iomem_resource.child; p ; p = p->sibling) {
+		if (end < p->start)
+			continue;
+
+		if (p->start <= start && end <= p->end) {
+			/* resource fully contains region */
+			if ((p->flags != flags) || strcmp(p->name, name))
+				ret = 0;
+			else
+				ret = 1;
+			break;
+		}
+		if (p->end < start)
+			break;	/* not found */
+	}
+	read_unlock(&resource_lock);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(region_is_ram);
+
 void __weak arch_remove_reservations(struct resource *avail)
 {
 }

-- 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 1/2] x86: Optimize resource lookups for ioremap

[Mike Travis]

[-- Attachment #1: add-get-resource-type --]
[-- Type: text/plain, Size: 2646 bytes --]

Since the ioremap operation is verifying that the specified address range
is NOT RAM, it will search the entire ioresource list if the condition
is true.  To make matters worse, it does this one 4k page at a time.
For a 128M BAR region this is 32 passes to determine the entire region
does not contain any RAM addresses.

This patch provides another resource lookup function, region_is_ram,
that searches for the entire region specified, verifying that it is
completely contained within the resource region.  If it is found, then
it is checked to be RAM or not, within a single pass.

The return result reflects if it was found or not (-1), and whether it is
RAM (1) or not (0).  This allows the caller to fallback to the previous
page by page search if it was not found.

Signed-off-by: Mike Travis <travis@sgi.com>
Acked-by: Alex Thorlton <athorlton@sgi.com>
Reviewed-by: Cliff Wickman <cpw@sgi.com>
---
 include/linux/mm.h |    1 +
 kernel/resource.c  |   37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+)

--- linux.orig/include/linux/mm.h
+++ linux/include/linux/mm.h
@@ -346,6 +346,7 @@ static inline int put_page_unless_one(st
 }
 
 extern int page_is_ram(unsigned long pfn);
+extern int region_is_ram(resource_size_t phys_addr, unsigned long size);
 
 /* Support for virtually mapped pages */
 struct page *vmalloc_to_page(const void *addr);
--- linux.orig/kernel/resource.c
+++ linux/kernel/resource.c
@@ -494,6 +494,43 @@ int __weak page_is_ram(unsigned long pfn
 }
 EXPORT_SYMBOL_GPL(page_is_ram);
 
+/*
+ * Search for a resouce entry that fully contains the specified region.
+ * If found, return 1 if it is RAM, 0 if not.
+ * If not found, or region is not fully contained, return -1
+ *
+ * Used by the ioremap functions to insure user not remapping RAM and is as
+ * vast speed up over walking through the resource table page by page.
+ */
+int __weak region_is_ram(resource_size_t start, unsigned long size)
+{
+	struct resource *p;
+	resource_size_t end = start + size - 1;
+	int flags = IORESOURCE_MEM | IORESOURCE_BUSY;
+	const char *name = "System RAM";
+	int ret = -1;
+
+	read_lock(&resource_lock);
+	for (p = iomem_resource.child; p ; p = p->sibling) {
+		if (end < p->start)
+			continue;
+
+		if (p->start <= start && end <= p->end) {
+			/* resource fully contains region */
+			if ((p->flags != flags) || strcmp(p->name, name))
+				ret = 0;
+			else
+				ret = 1;
+			break;
+		}
+		if (p->end < start)
+			break;	/* not found */
+	}
+	read_unlock(&resource_lock);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(region_is_ram);
+
 void __weak arch_remove_reservations(struct resource *avail)
 {
 }

-- 

Re: [PATCH 1/2] x86: Optimize resource lookups for ioremap

[Andrew Morton]

On Wed, 27 Aug 2014 17:59:28 -0500 Mike Travis <travis@sgi.com> wrote:

> Since the ioremap operation is verifying that the specified address range
> is NOT RAM, it will search the entire ioresource list if the condition
> is true.  To make matters worse, it does this one 4k page at a time.
> For a 128M BAR region this is 32 passes to determine the entire region
> does not contain any RAM addresses.
> 
> This patch provides another resource lookup function, region_is_ram,
> that searches for the entire region specified, verifying that it is
> completely contained within the resource region.  If it is found, then
> it is checked to be RAM or not, within a single pass.
> 
> The return result reflects if it was found or not (-1), and whether it is
> RAM (1) or not (0).  This allows the caller to fallback to the previous
> page by page search if it was not found.
> 
> ...
>
> --- linux.orig/kernel/resource.c
> +++ linux/kernel/resource.c
> @@ -494,6 +494,43 @@ int __weak page_is_ram(unsigned long pfn
>  }
>  EXPORT_SYMBOL_GPL(page_is_ram);
>  
> +/*
> + * Search for a resouce entry that fully contains the specified region.
> + * If found, return 1 if it is RAM, 0 if not.
> + * If not found, or region is not fully contained, return -1
> + *
> + * Used by the ioremap functions to insure user not remapping RAM and is as
> + * vast speed up over walking through the resource table page by page.
> + */
> +int __weak region_is_ram(resource_size_t start, unsigned long size)
> +{
> +	struct resource *p;
> +	resource_size_t end = start + size - 1;
> +	int flags = IORESOURCE_MEM | IORESOURCE_BUSY;
> +	const char *name = "System RAM";
> +	int ret = -1;
> +
> +	read_lock(&resource_lock);
> +	for (p = iomem_resource.child; p ; p = p->sibling) {
> +		if (end < p->start)
> +			continue;
> +
> +		if (p->start <= start && end <= p->end) {
> +			/* resource fully contains region */
> +			if ((p->flags != flags) || strcmp(p->name, name))
> +				ret = 0;
> +			else
> +				ret = 1;
> +			break;
> +		}
> +		if (p->end < start)
> +			break;	/* not found */
> +	}
> +	read_unlock(&resource_lock);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(region_is_ram);

Exporting a __weak symbol is strange.  I guess it works, but neither
the __weak nor the export are actually needed?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/2] x86: Optimize resource lookups for ioremap

[Andrew Morton]

On Wed, 27 Aug 2014 17:59:28 -0500 Mike Travis <travis@sgi.com> wrote:

> Since the ioremap operation is verifying that the specified address range
> is NOT RAM, it will search the entire ioresource list if the condition
> is true.  To make matters worse, it does this one 4k page at a time.
> For a 128M BAR region this is 32 passes to determine the entire region
> does not contain any RAM addresses.
> 
> This patch provides another resource lookup function, region_is_ram,
> that searches for the entire region specified, verifying that it is
> completely contained within the resource region.  If it is found, then
> it is checked to be RAM or not, within a single pass.
> 
> The return result reflects if it was found or not (-1), and whether it is
> RAM (1) or not (0).  This allows the caller to fallback to the previous
> page by page search if it was not found.
> 
> ...
>
> --- linux.orig/kernel/resource.c
> +++ linux/kernel/resource.c
> @@ -494,6 +494,43 @@ int __weak page_is_ram(unsigned long pfn
>  }
>  EXPORT_SYMBOL_GPL(page_is_ram);
>  
> +/*
> + * Search for a resouce entry that fully contains the specified region.
> + * If found, return 1 if it is RAM, 0 if not.
> + * If not found, or region is not fully contained, return -1
> + *
> + * Used by the ioremap functions to insure user not remapping RAM and is as
> + * vast speed up over walking through the resource table page by page.
> + */
> +int __weak region_is_ram(resource_size_t start, unsigned long size)
> +{
> +	struct resource *p;
> +	resource_size_t end = start + size - 1;
> +	int flags = IORESOURCE_MEM | IORESOURCE_BUSY;
> +	const char *name = "System RAM";
> +	int ret = -1;
> +
> +	read_lock(&resource_lock);
> +	for (p = iomem_resource.child; p ; p = p->sibling) {
> +		if (end < p->start)
> +			continue;
> +
> +		if (p->start <= start && end <= p->end) {
> +			/* resource fully contains region */
> +			if ((p->flags != flags) || strcmp(p->name, name))
> +				ret = 0;
> +			else
> +				ret = 1;
> +			break;
> +		}
> +		if (p->end < start)
> +			break;	/* not found */
> +	}
> +	read_unlock(&resource_lock);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(region_is_ram);

Exporting a __weak symbol is strange.  I guess it works, but neither
the __weak nor the export are actually needed?

Re: [PATCH 1/2] x86: Optimize resource lookups for ioremap

[Mike Travis]

On 8/27/2014 4:05 PM, Andrew Morton wrote:
> On Wed, 27 Aug 2014 17:59:28 -0500 Mike Travis <travis@sgi.com> wrote:
> 
>> Since the ioremap operation is verifying that the specified address range
>> is NOT RAM, it will search the entire ioresource list if the condition
>> is true.  To make matters worse, it does this one 4k page at a time.
>> For a 128M BAR region this is 32 passes to determine the entire region
>> does not contain any RAM addresses.
>>
>> This patch provides another resource lookup function, region_is_ram,
>> that searches for the entire region specified, verifying that it is
>> completely contained within the resource region.  If it is found, then
>> it is checked to be RAM or not, within a single pass.
>>
>> The return result reflects if it was found or not (-1), and whether it is
>> RAM (1) or not (0).  This allows the caller to fallback to the previous
>> page by page search if it was not found.
>>
>> ...
>>
>> --- linux.orig/kernel/resource.c
>> +++ linux/kernel/resource.c
>> @@ -494,6 +494,43 @@ int __weak page_is_ram(unsigned long pfn
>>  }
>>  EXPORT_SYMBOL_GPL(page_is_ram);
>>  
>> +/*
>> + * Search for a resouce entry that fully contains the specified region.
>> + * If found, return 1 if it is RAM, 0 if not.
>> + * If not found, or region is not fully contained, return -1
>> + *
>> + * Used by the ioremap functions to insure user not remapping RAM and is as
>> + * vast speed up over walking through the resource table page by page.
>> + */
>> +int __weak region_is_ram(resource_size_t start, unsigned long size)
>> +{
>> +	struct resource *p;
>> +	resource_size_t end = start + size - 1;
>> +	int flags = IORESOURCE_MEM | IORESOURCE_BUSY;
>> +	const char *name = "System RAM";
>> +	int ret = -1;
>> +
>> +	read_lock(&resource_lock);
>> +	for (p = iomem_resource.child; p ; p = p->sibling) {
>> +		if (end < p->start)
>> +			continue;
>> +
>> +		if (p->start <= start && end <= p->end) {
>> +			/* resource fully contains region */
>> +			if ((p->flags != flags) || strcmp(p->name, name))
>> +				ret = 0;
>> +			else
>> +				ret = 1;
>> +			break;
>> +		}
>> +		if (p->end < start)
>> +			break;	/* not found */
>> +	}
>> +	read_unlock(&resource_lock);
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(region_is_ram);
> 
> Exporting a __weak symbol is strange.  I guess it works, but neither
> the __weak nor the export are actually needed?
> 

I mainly used 'weak' and export because that was what the page_is_ram
function was using.  Most likely this won't be used anywhere else but
I wasn't sure.  I can certainly remove the weak and export, at least
until it's actually needed?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/2] x86: Optimize resource lookups for ioremap

[Mike Travis]

On 8/27/2014 4:05 PM, Andrew Morton wrote:
> On Wed, 27 Aug 2014 17:59:28 -0500 Mike Travis <travis@sgi.com> wrote:
> 
>> Since the ioremap operation is verifying that the specified address range
>> is NOT RAM, it will search the entire ioresource list if the condition
>> is true.  To make matters worse, it does this one 4k page at a time.
>> For a 128M BAR region this is 32 passes to determine the entire region
>> does not contain any RAM addresses.
>>
>> This patch provides another resource lookup function, region_is_ram,
>> that searches for the entire region specified, verifying that it is
>> completely contained within the resource region.  If it is found, then
>> it is checked to be RAM or not, within a single pass.
>>
>> The return result reflects if it was found or not (-1), and whether it is
>> RAM (1) or not (0).  This allows the caller to fallback to the previous
>> page by page search if it was not found.
>>
>> ...
>>
>> --- linux.orig/kernel/resource.c
>> +++ linux/kernel/resource.c
>> @@ -494,6 +494,43 @@ int __weak page_is_ram(unsigned long pfn
>>  }
>>  EXPORT_SYMBOL_GPL(page_is_ram);
>>  
>> +/*
>> + * Search for a resouce entry that fully contains the specified region.
>> + * If found, return 1 if it is RAM, 0 if not.
>> + * If not found, or region is not fully contained, return -1
>> + *
>> + * Used by the ioremap functions to insure user not remapping RAM and is as
>> + * vast speed up over walking through the resource table page by page.
>> + */
>> +int __weak region_is_ram(resource_size_t start, unsigned long size)
>> +{
>> +	struct resource *p;
>> +	resource_size_t end = start + size - 1;
>> +	int flags = IORESOURCE_MEM | IORESOURCE_BUSY;
>> +	const char *name = "System RAM";
>> +	int ret = -1;
>> +
>> +	read_lock(&resource_lock);
>> +	for (p = iomem_resource.child; p ; p = p->sibling) {
>> +		if (end < p->start)
>> +			continue;
>> +
>> +		if (p->start <= start && end <= p->end) {
>> +			/* resource fully contains region */
>> +			if ((p->flags != flags) || strcmp(p->name, name))
>> +				ret = 0;
>> +			else
>> +				ret = 1;
>> +			break;
>> +		}
>> +		if (p->end < start)
>> +			break;	/* not found */
>> +	}
>> +	read_unlock(&resource_lock);
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(region_is_ram);
> 
> Exporting a __weak symbol is strange.  I guess it works, but neither
> the __weak nor the export are actually needed?
> 

I mainly used 'weak' and export because that was what the page_is_ram
function was using.  Most likely this won't be used anywhere else but
I wasn't sure.  I can certainly remove the weak and export, at least
until it's actually needed?

Re: [PATCH 1/2] x86: Optimize resource lookups for ioremap

[Andrew Morton]

On Wed, 27 Aug 2014 16:09:09 -0700 Mike Travis <travis@sgi.com> wrote:

> 
> >>
> >> ...
> >>
> >> --- linux.orig/kernel/resource.c
> >> +++ linux/kernel/resource.c
> >> @@ -494,6 +494,43 @@ int __weak page_is_ram(unsigned long pfn
> >>  }
> >>  EXPORT_SYMBOL_GPL(page_is_ram);
> >>  
> >> +/*
> >> + * Search for a resouce entry that fully contains the specified region.
> >> + * If found, return 1 if it is RAM, 0 if not.
> >> + * If not found, or region is not fully contained, return -1
> >> + *
> >> + * Used by the ioremap functions to insure user not remapping RAM and is as
> >> + * vast speed up over walking through the resource table page by page.
> >> + */
> >> +int __weak region_is_ram(resource_size_t start, unsigned long size)
> >> +{
> >> +	struct resource *p;
> >> +	resource_size_t end = start + size - 1;
> >> +	int flags = IORESOURCE_MEM | IORESOURCE_BUSY;
> >> +	const char *name = "System RAM";
> >> +	int ret = -1;
> >> +
> >> +	read_lock(&resource_lock);
> >> +	for (p = iomem_resource.child; p ; p = p->sibling) {
> >> +		if (end < p->start)
> >> +			continue;
> >> +
> >> +		if (p->start <= start && end <= p->end) {
> >> +			/* resource fully contains region */
> >> +			if ((p->flags != flags) || strcmp(p->name, name))
> >> +				ret = 0;
> >> +			else
> >> +				ret = 1;
> >> +			break;
> >> +		}
> >> +		if (p->end < start)
> >> +			break;	/* not found */
> >> +	}
> >> +	read_unlock(&resource_lock);
> >> +	return ret;
> >> +}
> >> +EXPORT_SYMBOL_GPL(region_is_ram);
> > 
> > Exporting a __weak symbol is strange.  I guess it works, but neither
> > the __weak nor the export are actually needed?
> > 
> 
> I mainly used 'weak' and export because that was what the page_is_ram
> function was using.  Most likely this won't be used anywhere else but
> I wasn't sure.  I can certainly remove the weak and export, at least
> until it's actually needed?

Several architectures implement custom page_is_ram(), so they need the
__weak.  region_is_ram() needs neither so yes, they should be removed.

<looks at the code>

Doing strcmp("System RAM") is rather a hack.  Is there nothing in
resource.flags which can be used?  Or added otherwise?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/2] x86: Optimize resource lookups for ioremap

[Andrew Morton]

On Wed, 27 Aug 2014 16:09:09 -0700 Mike Travis <travis@sgi.com> wrote:

> 
> >>
> >> ...
> >>
> >> --- linux.orig/kernel/resource.c
> >> +++ linux/kernel/resource.c
> >> @@ -494,6 +494,43 @@ int __weak page_is_ram(unsigned long pfn
> >>  }
> >>  EXPORT_SYMBOL_GPL(page_is_ram);
> >>  
> >> +/*
> >> + * Search for a resouce entry that fully contains the specified region.
> >> + * If found, return 1 if it is RAM, 0 if not.
> >> + * If not found, or region is not fully contained, return -1
> >> + *
> >> + * Used by the ioremap functions to insure user not remapping RAM and is as
> >> + * vast speed up over walking through the resource table page by page.
> >> + */
> >> +int __weak region_is_ram(resource_size_t start, unsigned long size)
> >> +{
> >> +	struct resource *p;
> >> +	resource_size_t end = start + size - 1;
> >> +	int flags = IORESOURCE_MEM | IORESOURCE_BUSY;
> >> +	const char *name = "System RAM";
> >> +	int ret = -1;
> >> +
> >> +	read_lock(&resource_lock);
> >> +	for (p = iomem_resource.child; p ; p = p->sibling) {
> >> +		if (end < p->start)
> >> +			continue;
> >> +
> >> +		if (p->start <= start && end <= p->end) {
> >> +			/* resource fully contains region */
> >> +			if ((p->flags != flags) || strcmp(p->name, name))
> >> +				ret = 0;
> >> +			else
> >> +				ret = 1;
> >> +			break;
> >> +		}
> >> +		if (p->end < start)
> >> +			break;	/* not found */
> >> +	}
> >> +	read_unlock(&resource_lock);
> >> +	return ret;
> >> +}
> >> +EXPORT_SYMBOL_GPL(region_is_ram);
> > 
> > Exporting a __weak symbol is strange.  I guess it works, but neither
> > the __weak nor the export are actually needed?
> > 
> 
> I mainly used 'weak' and export because that was what the page_is_ram
> function was using.  Most likely this won't be used anywhere else but
> I wasn't sure.  I can certainly remove the weak and export, at least
> until it's actually needed?

Several architectures implement custom page_is_ram(), so they need the
__weak.  region_is_ram() needs neither so yes, they should be removed.

<looks at the code>

Doing strcmp("System RAM") is rather a hack.  Is there nothing in
resource.flags which can be used?  Or added otherwise?

Re: [PATCH 1/2] x86: Optimize resource lookups for ioremap

[Mike Travis]

On 8/27/2014 4:18 PM, Andrew Morton wrote:
> On Wed, 27 Aug 2014 16:09:09 -0700 Mike Travis <travis@sgi.com> wrote:
> 
>>
>>>>
>>>> ...
>>>>
>>>> --- linux.orig/kernel/resource.c
>>>> +++ linux/kernel/resource.c
>>>> @@ -494,6 +494,43 @@ int __weak page_is_ram(unsigned long pfn
>>>>  }
>>>>  EXPORT_SYMBOL_GPL(page_is_ram);
>>>>  
>>>> +/*
>>>> + * Search for a resouce entry that fully contains the specified region.
>>>> + * If found, return 1 if it is RAM, 0 if not.
>>>> + * If not found, or region is not fully contained, return -1
>>>> + *
>>>> + * Used by the ioremap functions to insure user not remapping RAM and is as
>>>> + * vast speed up over walking through the resource table page by page.
>>>> + */
>>>> +int __weak region_is_ram(resource_size_t start, unsigned long size)
>>>> +{
>>>> +	struct resource *p;
>>>> +	resource_size_t end = start + size - 1;
>>>> +	int flags = IORESOURCE_MEM | IORESOURCE_BUSY;
>>>> +	const char *name = "System RAM";
>>>> +	int ret = -1;
>>>> +
>>>> +	read_lock(&resource_lock);
>>>> +	for (p = iomem_resource.child; p ; p = p->sibling) {
>>>> +		if (end < p->start)
>>>> +			continue;
>>>> +
>>>> +		if (p->start <= start && end <= p->end) {
>>>> +			/* resource fully contains region */
>>>> +			if ((p->flags != flags) || strcmp(p->name, name))
>>>> +				ret = 0;
>>>> +			else
>>>> +				ret = 1;
>>>> +			break;
>>>> +		}
>>>> +		if (p->end < start)
>>>> +			break;	/* not found */
>>>> +	}
>>>> +	read_unlock(&resource_lock);
>>>> +	return ret;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(region_is_ram);
>>>
>>> Exporting a __weak symbol is strange.  I guess it works, but neither
>>> the __weak nor the export are actually needed?
>>>
>>
>> I mainly used 'weak' and export because that was what the page_is_ram
>> function was using.  Most likely this won't be used anywhere else but
>> I wasn't sure.  I can certainly remove the weak and export, at least
>> until it's actually needed?
> 
> Several architectures implement custom page_is_ram(), so they need the
> __weak.  region_is_ram() needs neither so yes, they should be removed.

Okay.
> 
> <looks at the code>
> 
> Doing strcmp("System RAM") is rather a hack.  Is there nothing in
> resource.flags which can be used?  Or added otherwise?

I agree except this mimics the page_is_ram function:

        while ((res.start < res.end) &&
                (find_next_iomem_res(&res, "System RAM", true) >= 0)) {

So it passes the same literal string which then find_next does the
same strcmp on it:

                if (p->flags != res->flags)
                        continue;
                if (name && strcmp(p->name, name))
                        continue;

I should add back in the check to insure name is not NULL.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/2] x86: Optimize resource lookups for ioremap

[Mike Travis]

On 8/27/2014 4:18 PM, Andrew Morton wrote:
> On Wed, 27 Aug 2014 16:09:09 -0700 Mike Travis <travis@sgi.com> wrote:
> 
>>
>>>>
>>>> ...
>>>>
>>>> --- linux.orig/kernel/resource.c
>>>> +++ linux/kernel/resource.c
>>>> @@ -494,6 +494,43 @@ int __weak page_is_ram(unsigned long pfn
>>>>  }
>>>>  EXPORT_SYMBOL_GPL(page_is_ram);
>>>>  
>>>> +/*
>>>> + * Search for a resouce entry that fully contains the specified region.
>>>> + * If found, return 1 if it is RAM, 0 if not.
>>>> + * If not found, or region is not fully contained, return -1
>>>> + *
>>>> + * Used by the ioremap functions to insure user not remapping RAM and is as
>>>> + * vast speed up over walking through the resource table page by page.
>>>> + */
>>>> +int __weak region_is_ram(resource_size_t start, unsigned long size)
>>>> +{
>>>> +	struct resource *p;
>>>> +	resource_size_t end = start + size - 1;
>>>> +	int flags = IORESOURCE_MEM | IORESOURCE_BUSY;
>>>> +	const char *name = "System RAM";
>>>> +	int ret = -1;
>>>> +
>>>> +	read_lock(&resource_lock);
>>>> +	for (p = iomem_resource.child; p ; p = p->sibling) {
>>>> +		if (end < p->start)
>>>> +			continue;
>>>> +
>>>> +		if (p->start <= start && end <= p->end) {
>>>> +			/* resource fully contains region */
>>>> +			if ((p->flags != flags) || strcmp(p->name, name))
>>>> +				ret = 0;
>>>> +			else
>>>> +				ret = 1;
>>>> +			break;
>>>> +		}
>>>> +		if (p->end < start)
>>>> +			break;	/* not found */
>>>> +	}
>>>> +	read_unlock(&resource_lock);
>>>> +	return ret;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(region_is_ram);
>>>
>>> Exporting a __weak symbol is strange.  I guess it works, but neither
>>> the __weak nor the export are actually needed?
>>>
>>
>> I mainly used 'weak' and export because that was what the page_is_ram
>> function was using.  Most likely this won't be used anywhere else but
>> I wasn't sure.  I can certainly remove the weak and export, at least
>> until it's actually needed?
> 
> Several architectures implement custom page_is_ram(), so they need the
> __weak.  region_is_ram() needs neither so yes, they should be removed.

Okay.
> 
> <looks at the code>
> 
> Doing strcmp("System RAM") is rather a hack.  Is there nothing in
> resource.flags which can be used?  Or added otherwise?

I agree except this mimics the page_is_ram function:

        while ((res.start < res.end) &&
                (find_next_iomem_res(&res, "System RAM", true) >= 0)) {

So it passes the same literal string which then find_next does the
same strcmp on it:

                if (p->flags != res->flags)
                        continue;
                if (name && strcmp(p->name, name))
                        continue;

I should add back in the check to insure name is not NULL.

Re: [PATCH 1/2] x86: Optimize resource lookups for ioremap

[Andrew Morton]

On Wed, 27 Aug 2014 16:25:24 -0700 Mike Travis <travis@sgi.com> wrote:

> > 
> > <looks at the code>
> > 
> > Doing strcmp("System RAM") is rather a hack.  Is there nothing in
> > resource.flags which can be used?  Or added otherwise?
> 
> I agree except this mimics the page_is_ram function:
> 
>         while ((res.start < res.end) &&
>                 (find_next_iomem_res(&res, "System RAM", true) >= 0)) {

Yeah.  Sigh.

> So it passes the same literal string which then find_next does the
> same strcmp on it:
> 
>                 if (p->flags != res->flags)
>                         continue;
>                 if (name && strcmp(p->name, name))
>                         continue;
> 
> I should add back in the check to insure name is not NULL.

If we're still at 1+ hours then little bodges like this are nowhere
near sufficient and sterner stuff will be needed.

Do we actually need the test?  My googling turns up zero instances of
anyone reporting the "ioremap on RAM pfn" warning.

Where's the rest of the time being spent?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/2] x86: Optimize resource lookups for ioremap

[Andrew Morton]

On Wed, 27 Aug 2014 16:25:24 -0700 Mike Travis <travis@sgi.com> wrote:

> > 
> > <looks at the code>
> > 
> > Doing strcmp("System RAM") is rather a hack.  Is there nothing in
> > resource.flags which can be used?  Or added otherwise?
> 
> I agree except this mimics the page_is_ram function:
> 
>         while ((res.start < res.end) &&
>                 (find_next_iomem_res(&res, "System RAM", true) >= 0)) {

Yeah.  Sigh.

> So it passes the same literal string which then find_next does the
> same strcmp on it:
> 
>                 if (p->flags != res->flags)
>                         continue;
>                 if (name && strcmp(p->name, name))
>                         continue;
> 
> I should add back in the check to insure name is not NULL.

If we're still at 1+ hours then little bodges like this are nowhere
near sufficient and sterner stuff will be needed.

Do we actually need the test?  My googling turns up zero instances of
anyone reporting the "ioremap on RAM pfn" warning.

Where's the rest of the time being spent?

Re: [PATCH 1/2] x86: Optimize resource lookups for ioremap

[Mike Travis]

On 8/27/2014 4:37 PM, Andrew Morton wrote:
> On Wed, 27 Aug 2014 16:25:24 -0700 Mike Travis <travis@sgi.com> wrote:
> 
>>>
>>> <looks at the code>
>>>
>>> Doing strcmp("System RAM") is rather a hack.  Is there nothing in
>>> resource.flags which can be used?  Or added otherwise?
>>
>> I agree except this mimics the page_is_ram function:
>>
>>         while ((res.start < res.end) &&
>>                 (find_next_iomem_res(&res, "System RAM", true) >= 0)) {
> 
> Yeah.  Sigh.
> 
>> So it passes the same literal string which then find_next does the
>> same strcmp on it:
>>
>>                 if (p->flags != res->flags)
>>                         continue;
>>                 if (name && strcmp(p->name, name))
>>                         continue;
>>
>> I should add back in the check to insure name is not NULL.
> 
> If we're still at 1+ hours then little bodges like this are nowhere
> near sufficient and sterner stuff will be needed.
> 
> Do we actually need the test?  My googling turns up zero instances of
> anyone reporting the "ioremap on RAM pfn" warning.

We get them more than we like, mostly from 3rd party vendors, and
esp. those that merely port their windows drivers to linux.
> 
> Where's the rest of the time being spent?

This device has a huge internal memory and  many processing devices.
So it loads up an operating system and starts a bunch of pseudo network
connections through the PCI-e/driver interface.  It was hard to
determine what percentage the ioremap played in the overall starting
time (based on what info we were able to collect).  But the ioremap
was definitely the largest part of the 'modprobe' operation.  I think
realistically that's all we have control over.

(But as I mentioned, we are encouraging the vendor to look into starting
the devices in parallel.  The overlap will cut down the overall time by
quite a bit, being there are 31 devices.)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/2] x86: Optimize resource lookups for ioremap

[Mike Travis]

On 8/27/2014 4:37 PM, Andrew Morton wrote:
> On Wed, 27 Aug 2014 16:25:24 -0700 Mike Travis <travis@sgi.com> wrote:
> 
>>>
>>> <looks at the code>
>>>
>>> Doing strcmp("System RAM") is rather a hack.  Is there nothing in
>>> resource.flags which can be used?  Or added otherwise?
>>
>> I agree except this mimics the page_is_ram function:
>>
>>         while ((res.start < res.end) &&
>>                 (find_next_iomem_res(&res, "System RAM", true) >= 0)) {
> 
> Yeah.  Sigh.
> 
>> So it passes the same literal string which then find_next does the
>> same strcmp on it:
>>
>>                 if (p->flags != res->flags)
>>                         continue;
>>                 if (name && strcmp(p->name, name))
>>                         continue;
>>
>> I should add back in the check to insure name is not NULL.
> 
> If we're still at 1+ hours then little bodges like this are nowhere
> near sufficient and sterner stuff will be needed.
> 
> Do we actually need the test?  My googling turns up zero instances of
> anyone reporting the "ioremap on RAM pfn" warning.

We get them more than we like, mostly from 3rd party vendors, and
esp. those that merely port their windows drivers to linux.
> 
> Where's the rest of the time being spent?

This device has a huge internal memory and  many processing devices.
So it loads up an operating system and starts a bunch of pseudo network
connections through the PCI-e/driver interface.  It was hard to
determine what percentage the ioremap played in the overall starting
time (based on what info we were able to collect).  But the ioremap
was definitely the largest part of the 'modprobe' operation.  I think
realistically that's all we have control over.

(But as I mentioned, we are encouraging the vendor to look into starting
the devices in parallel.  The overlap will cut down the overall time by
quite a bit, being there are 31 devices.)

Re: [PATCH 1/2] x86: Optimize resource lookups for ioremap

[Andrew Morton]

On Wed, 27 Aug 2014 16:54:18 -0700 Mike Travis <travis@sgi.com> wrote:

> > If we're still at 1+ hours then little bodges like this are nowhere
> > near sufficient and sterner stuff will be needed.
> > 
> > Do we actually need the test?  My googling turns up zero instances of
> > anyone reporting the "ioremap on RAM pfn" warning.
> 
> We get them more than we like, mostly from 3rd party vendors, and
> esp. those that merely port their windows drivers to linux.

Dang.  So wrapping the check in CONFIG_DEBUG_VM would be problematic?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/2] x86: Optimize resource lookups for ioremap

[Andrew Morton]

On Wed, 27 Aug 2014 16:54:18 -0700 Mike Travis <travis@sgi.com> wrote:

> > If we're still at 1+ hours then little bodges like this are nowhere
> > near sufficient and sterner stuff will be needed.
> > 
> > Do we actually need the test?  My googling turns up zero instances of
> > anyone reporting the "ioremap on RAM pfn" warning.
> 
> We get them more than we like, mostly from 3rd party vendors, and
> esp. those that merely port their windows drivers to linux.

Dang.  So wrapping the check in CONFIG_DEBUG_VM would be problematic?

[PATCH 2/2] x86: Use optimized ioresource lookup in ioremap function

[Mike Travis]

[-- Attachment #1: use-get-resource-type --]
[-- Type: text/plain, Size: 2024 bytes --]

This patch uses the optimized ioresource lookup, "region_is_ram", for
the ioremap function.  If the region is not found, it falls back to the
"page_is_ram" function.  If it is found and it is RAM, then the usual
warning message is issued, and the ioremap operation is aborted.
Otherwise, the ioremap operation continues.

Signed-off-by: Mike Travis <travis@sgi.com>
Acked-by: Alex Thorlton <athorlton@sgi.com>
Reviewed-by: Cliff Wickman <cpw@sgi.com>
---
 arch/x86/mm/ioremap.c |   22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

--- linux.orig/arch/x86/mm/ioremap.c
+++ linux/arch/x86/mm/ioremap.c
@@ -86,6 +86,7 @@ static void __iomem *__ioremap_caller(re
 	pgprot_t prot;
 	int retval;
 	void __iomem *ret_addr;
+	int ram_region;
 
 	/* Don't allow wraparound or zero size */
 	last_addr = phys_addr + size - 1;
@@ -108,12 +109,23 @@ static void __iomem *__ioremap_caller(re
 	/*
 	 * Don't allow anybody to remap normal RAM that we're using..
 	 */
-	pfn      = phys_addr >> PAGE_SHIFT;
-	last_pfn = last_addr >> PAGE_SHIFT;
-	if (walk_system_ram_range(pfn, last_pfn - pfn + 1, NULL,
-				  __ioremap_check_ram) == 1)
-		return NULL;
+	/* First check if whole region can be identified as RAM or not */
+	ram_region = region_is_ram(phys_addr, size);
 
+	/* If is RAM(1) or could not be identified(-1), check page by page */
+	if (ram_region) {
+		pfn      = phys_addr >> PAGE_SHIFT;
+		last_pfn = last_addr >> PAGE_SHIFT;
+		if (ram_region > 0) {
+			WARN_ONCE(1, "ioremap on RAM at 0x%lx - 0x%lx\n",
+					(long unsigned int)phys_addr,
+					(long unsigned int)last_addr);
+			return NULL;
+		}
+		if (walk_system_ram_range(pfn, last_pfn - pfn + 1, NULL,
+					  __ioremap_check_ram) == 1)
+			return NULL;
+	}
 	/*
 	 * Mappings have to be page-aligned
 	 */

-- 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 2/2] x86: Use optimized ioresource lookup in ioremap function

[Mike Travis]

[-- Attachment #1: use-get-resource-type --]
[-- Type: text/plain, Size: 1798 bytes --]

This patch uses the optimized ioresource lookup, "region_is_ram", for
the ioremap function.  If the region is not found, it falls back to the
"page_is_ram" function.  If it is found and it is RAM, then the usual
warning message is issued, and the ioremap operation is aborted.
Otherwise, the ioremap operation continues.

Signed-off-by: Mike Travis <travis@sgi.com>
Acked-by: Alex Thorlton <athorlton@sgi.com>
Reviewed-by: Cliff Wickman <cpw@sgi.com>
---
 arch/x86/mm/ioremap.c |   22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

--- linux.orig/arch/x86/mm/ioremap.c
+++ linux/arch/x86/mm/ioremap.c
@@ -86,6 +86,7 @@ static void __iomem *__ioremap_caller(re
 	pgprot_t prot;
 	int retval;
 	void __iomem *ret_addr;
+	int ram_region;
 
 	/* Don't allow wraparound or zero size */
 	last_addr = phys_addr + size - 1;
@@ -108,12 +109,23 @@ static void __iomem *__ioremap_caller(re
 	/*
 	 * Don't allow anybody to remap normal RAM that we're using..
 	 */
-	pfn      = phys_addr >> PAGE_SHIFT;
-	last_pfn = last_addr >> PAGE_SHIFT;
-	if (walk_system_ram_range(pfn, last_pfn - pfn + 1, NULL,
-				  __ioremap_check_ram) == 1)
-		return NULL;
+	/* First check if whole region can be identified as RAM or not */
+	ram_region = region_is_ram(phys_addr, size);
 
+	/* If is RAM(1) or could not be identified(-1), check page by page */
+	if (ram_region) {
+		pfn      = phys_addr >> PAGE_SHIFT;
+		last_pfn = last_addr >> PAGE_SHIFT;
+		if (ram_region > 0) {
+			WARN_ONCE(1, "ioremap on RAM at 0x%lx - 0x%lx\n",
+					(long unsigned int)phys_addr,
+					(long unsigned int)last_addr);
+			return NULL;
+		}
+		if (walk_system_ram_range(pfn, last_pfn - pfn + 1, NULL,
+					  __ioremap_check_ram) == 1)
+			return NULL;
+	}
 	/*
 	 * Mappings have to be page-aligned
 	 */

-- 

Re: [PATCH 0/2] x86: Speed up ioremap operations

[Andrew Morton]

On Wed, 27 Aug 2014 17:59:27 -0500 Mike Travis <travis@sgi.com> wrote:

> 
> We have a large university system in the UK that is experiencing
> very long delays modprobing the driver for a specific I/O device.
> The delay is from 8-10 minutes per device and there are 31 devices
> in the system.  This 4 to 5 hour delay in starting up those I/O
> devices is very much a burden on the customer.

That's nuts.

> There are two causes for requiring a restart/reload of the drivers.
> First is periodic preventive maintenance (PM) and the second is if
> any of the devices experience a fatal error.  Both of these trigger
> this excessively long delay in bringing the system back up to full
> capability.
> 
> The problem was tracked down to a very slow IOREMAP operation and
> the excessively long ioresource lookup to insure that the user is
> not attempting to ioremap RAM.  These patches provide a speed up
> to that function.

With what result?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 0/2] x86: Speed up ioremap operations

[Andrew Morton]

On Wed, 27 Aug 2014 17:59:27 -0500 Mike Travis <travis@sgi.com> wrote:

> 
> We have a large university system in the UK that is experiencing
> very long delays modprobing the driver for a specific I/O device.
> The delay is from 8-10 minutes per device and there are 31 devices
> in the system.  This 4 to 5 hour delay in starting up those I/O
> devices is very much a burden on the customer.

That's nuts.

> There are two causes for requiring a restart/reload of the drivers.
> First is periodic preventive maintenance (PM) and the second is if
> any of the devices experience a fatal error.  Both of these trigger
> this excessively long delay in bringing the system back up to full
> capability.
> 
> The problem was tracked down to a very slow IOREMAP operation and
> the excessively long ioresource lookup to insure that the user is
> not attempting to ioremap RAM.  These patches provide a speed up
> to that function.

With what result?

Re: [PATCH 0/2] x86: Speed up ioremap operations

[Mike Travis]

On 8/27/2014 4:06 PM, Andrew Morton wrote:
> On Wed, 27 Aug 2014 17:59:27 -0500 Mike Travis <travis@sgi.com> wrote:
> 
>>
>> We have a large university system in the UK that is experiencing
>> very long delays modprobing the driver for a specific I/O device.
>> The delay is from 8-10 minutes per device and there are 31 devices
>> in the system.  This 4 to 5 hour delay in starting up those I/O
>> devices is very much a burden on the customer.
> 
> That's nuts.

Exactly!  The customer was (as expected) not terribly pleased... :)
> 
>> There are two causes for requiring a restart/reload of the drivers.
>> First is periodic preventive maintenance (PM) and the second is if
>> any of the devices experience a fatal error.  Both of these trigger
>> this excessively long delay in bringing the system back up to full
>> capability.
>>
>> The problem was tracked down to a very slow IOREMAP operation and
>> the excessively long ioresource lookup to insure that the user is
>> not attempting to ioremap RAM.  These patches provide a speed up
>> to that function.
> 
> With what result?
> 

Early measurements on our in house lab system (with far fewer cpus
and memory) shows about a 60-75% increase.  They have a 31 devices,
3000+ cpus, 10+Tb of memory.  We have 20 devices, 480 cpus, ~2Tb of
memory.  I expect their ioresource list to be about 5-10 times longer.
[But their system is in production so we have to wait for the next
scheduled PM interval before a live test can be done.]

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 0/2] x86: Speed up ioremap operations

[Mike Travis]

On 8/27/2014 4:06 PM, Andrew Morton wrote:
> On Wed, 27 Aug 2014 17:59:27 -0500 Mike Travis <travis@sgi.com> wrote:
> 
>>
>> We have a large university system in the UK that is experiencing
>> very long delays modprobing the driver for a specific I/O device.
>> The delay is from 8-10 minutes per device and there are 31 devices
>> in the system.  This 4 to 5 hour delay in starting up those I/O
>> devices is very much a burden on the customer.
> 
> That's nuts.

Exactly!  The customer was (as expected) not terribly pleased... :)
> 
>> There are two causes for requiring a restart/reload of the drivers.
>> First is periodic preventive maintenance (PM) and the second is if
>> any of the devices experience a fatal error.  Both of these trigger
>> this excessively long delay in bringing the system back up to full
>> capability.
>>
>> The problem was tracked down to a very slow IOREMAP operation and
>> the excessively long ioresource lookup to insure that the user is
>> not attempting to ioremap RAM.  These patches provide a speed up
>> to that function.
> 
> With what result?
> 

Early measurements on our in house lab system (with far fewer cpus
and memory) shows about a 60-75% increase.  They have a 31 devices,
3000+ cpus, 10+Tb of memory.  We have 20 devices, 480 cpus, ~2Tb of
memory.  I expect their ioresource list to be about 5-10 times longer.
[But their system is in production so we have to wait for the next
scheduled PM interval before a live test can be done.]

Re: [PATCH 0/2] x86: Speed up ioremap operations

[Andrew Morton]

On Wed, 27 Aug 2014 16:15:28 -0700 Mike Travis <travis@sgi.com> wrote:

> 
> > 
> >> There are two causes for requiring a restart/reload of the drivers.
> >> First is periodic preventive maintenance (PM) and the second is if
> >> any of the devices experience a fatal error.  Both of these trigger
> >> this excessively long delay in bringing the system back up to full
> >> capability.
> >>
> >> The problem was tracked down to a very slow IOREMAP operation and
> >> the excessively long ioresource lookup to insure that the user is
> >> not attempting to ioremap RAM.  These patches provide a speed up
> >> to that function.
> > 
> > With what result?
> > 
> 
> Early measurements on our in house lab system (with far fewer cpus
> and memory) shows about a 60-75% increase.  They have a 31 devices,
> 3000+ cpus, 10+Tb of memory.  We have 20 devices, 480 cpus, ~2Tb of
> memory.  I expect their ioresource list to be about 5-10 times longer.
> [But their system is in production so we have to wait for the next
> scheduled PM interval before a live test can be done.]

So you expect 1+ hours?  That's still nuts.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 0/2] x86: Speed up ioremap operations

[Andrew Morton]

On Wed, 27 Aug 2014 16:15:28 -0700 Mike Travis <travis@sgi.com> wrote:

> 
> > 
> >> There are two causes for requiring a restart/reload of the drivers.
> >> First is periodic preventive maintenance (PM) and the second is if
> >> any of the devices experience a fatal error.  Both of these trigger
> >> this excessively long delay in bringing the system back up to full
> >> capability.
> >>
> >> The problem was tracked down to a very slow IOREMAP operation and
> >> the excessively long ioresource lookup to insure that the user is
> >> not attempting to ioremap RAM.  These patches provide a speed up
> >> to that function.
> > 
> > With what result?
> > 
> 
> Early measurements on our in house lab system (with far fewer cpus
> and memory) shows about a 60-75% increase.  They have a 31 devices,
> 3000+ cpus, 10+Tb of memory.  We have 20 devices, 480 cpus, ~2Tb of
> memory.  I expect their ioresource list to be about 5-10 times longer.
> [But their system is in production so we have to wait for the next
> scheduled PM interval before a live test can be done.]

So you expect 1+ hours?  That's still nuts.

Re: [PATCH 0/2] x86: Speed up ioremap operations

[Mike Travis]

On 8/27/2014 4:20 PM, Andrew Morton wrote:
> On Wed, 27 Aug 2014 16:15:28 -0700 Mike Travis <travis@sgi.com> wrote:
> 
>>
>>>
>>>> There are two causes for requiring a restart/reload of the drivers.
>>>> First is periodic preventive maintenance (PM) and the second is if
>>>> any of the devices experience a fatal error.  Both of these trigger
>>>> this excessively long delay in bringing the system back up to full
>>>> capability.
>>>>
>>>> The problem was tracked down to a very slow IOREMAP operation and
>>>> the excessively long ioresource lookup to insure that the user is
>>>> not attempting to ioremap RAM.  These patches provide a speed up
>>>> to that function.
>>>
>>> With what result?
>>>
>>
>> Early measurements on our in house lab system (with far fewer cpus
>> and memory) shows about a 60-75% increase.  They have a 31 devices,
>> 3000+ cpus, 10+Tb of memory.  We have 20 devices, 480 cpus, ~2Tb of
>> memory.  I expect their ioresource list to be about 5-10 times longer.
>> [But their system is in production so we have to wait for the next
>> scheduled PM interval before a live test can be done.]
> 
> So you expect 1+ hours?  That's still nuts.
> 

Actually I expect a lot better improvement.  We are removing cycles
through the I/O resource list and the longer the list, the longer
it takes to pass completely through it.  As mentioned for a 128M
I/O BAR region, that is 32 passes, so we are removing 31 of them.
31 times a list 5-10 times longer should be a much better overall
improvement in the ioremap time.  The startup time of the device
will still be there, though we are encouraging the vendor to look
at starting them up in parallel.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 0/2] x86: Speed up ioremap operations

[Mike Travis]

On 8/27/2014 4:20 PM, Andrew Morton wrote:
> On Wed, 27 Aug 2014 16:15:28 -0700 Mike Travis <travis@sgi.com> wrote:
> 
>>
>>>
>>>> There are two causes for requiring a restart/reload of the drivers.
>>>> First is periodic preventive maintenance (PM) and the second is if
>>>> any of the devices experience a fatal error.  Both of these trigger
>>>> this excessively long delay in bringing the system back up to full
>>>> capability.
>>>>
>>>> The problem was tracked down to a very slow IOREMAP operation and
>>>> the excessively long ioresource lookup to insure that the user is
>>>> not attempting to ioremap RAM.  These patches provide a speed up
>>>> to that function.
>>>
>>> With what result?
>>>
>>
>> Early measurements on our in house lab system (with far fewer cpus
>> and memory) shows about a 60-75% increase.  They have a 31 devices,
>> 3000+ cpus, 10+Tb of memory.  We have 20 devices, 480 cpus, ~2Tb of
>> memory.  I expect their ioresource list to be about 5-10 times longer.
>> [But their system is in production so we have to wait for the next
>> scheduled PM interval before a live test can be done.]
> 
> So you expect 1+ hours?  That's still nuts.
> 

Actually I expect a lot better improvement.  We are removing cycles
through the I/O resource list and the longer the list, the longer
it takes to pass completely through it.  As mentioned for a 128M
I/O BAR region, that is 32 passes, so we are removing 31 of them.
31 times a list 5-10 times longer should be a much better overall
improvement in the ioremap time.  The startup time of the device
will still be there, though we are encouraging the vendor to look
at starting them up in parallel.

Re: [PATCH 0/2] x86: Speed up ioremap operations

[Ingo Molnar]

* Andrew Morton <akpm@linux-foundation.org> wrote:

> On Wed, 27 Aug 2014 17:59:27 -0500 Mike Travis <travis@sgi.com> wrote:
> 
> > 
> > We have a large university system in the UK that is experiencing
> > very long delays modprobing the driver for a specific I/O device.
> > The delay is from 8-10 minutes per device and there are 31 devices
> > in the system.  This 4 to 5 hour delay in starting up those I/O
> > devices is very much a burden on the customer.
> 
> That's nuts.

Agreed, and I'd suggest marking this for -stable, once it's all 
settled and tested.

Thanks,

	Ingo

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 0/2] x86: Speed up ioremap operations

[Ingo Molnar]

* Andrew Morton <akpm@linux-foundation.org> wrote:

> On Wed, 27 Aug 2014 17:59:27 -0500 Mike Travis <travis@sgi.com> wrote:
> 
> > 
> > We have a large university system in the UK that is experiencing
> > very long delays modprobing the driver for a specific I/O device.
> > The delay is from 8-10 minutes per device and there are 31 devices
> > in the system.  This 4 to 5 hour delay in starting up those I/O
> > devices is very much a burden on the customer.
> 
> That's nuts.

Agreed, and I'd suggest marking this for -stable, once it's all 
settled and tested.

Thanks,

	Ingo

[PATCH 0/2] x86: Speed up ioremap operations

[Mike Travis]

We have a large university system in the UK that is experiencing
very long delays modprobing the driver for a specific I/O device.
The delay is from 8-10 minutes per device and there are 31 devices
in the system.  This 4 to 5 hour delay in starting up those I/O
devices is very much a burden on the customer.

There are two causes for requiring a restart/reload of the drivers.
First is periodic preventive maintenance (PM) and the second is if
any of the devices experience a fatal error.  Both of these trigger
this excessively long delay in bringing the system back up to full
capability.

The problem was tracked down to a very slow IOREMAP operation and
the excessively long ioresource lookup to insure that the user is
not attempting to ioremap RAM.  These patches provide a speed up
to that function.

-- 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 1/2] x86: Optimize resource lookups for ioremap

[Mike Travis]

[-- Attachment #1: add-get-resource-type --]
[-- Type: text/plain, Size: 2894 bytes --]

Since the ioremap operation is verifying that the specified address range
is NOT RAM, it will search the entire ioresource list if the condition
is true.  To make matters worse, it does this one 4k page at a time.
For a 128M BAR region this is 32 passes to determine the entire region
does not contain any RAM addresses.

This patch provides another resource lookup function, region_is_ram,
that searches for the entire region specified, verifying that it is
completely contained within the resource region.  If it is found, then
it is checked to be RAM or not, within a single pass.

The return result reflects if it was found or not (-1), and whether it is
RAM (1) or not (0).  This allows the caller to fallback to the previous
page by page search if it was not found.

Signed-off-by: Mike Travis <travis@sgi.com>
Acked-by: Alex Thorlton <athorlton@sgi.com>
Reviewed-by: Cliff Wickman <cpw@sgi.com>
---
v2: remove 'weak' and EXPORT_SYMBOL_GPL from region_is_ram()
---
 include/linux/mm.h |    1 +
 kernel/resource.c  |   36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+)

--- linux.orig/include/linux/mm.h
+++ linux/include/linux/mm.h
@@ -346,6 +346,7 @@ static inline int put_page_unless_one(st
 }
 
 extern int page_is_ram(unsigned long pfn);
+extern int region_is_ram(resource_size_t phys_addr, unsigned long size);
 
 /* Support for virtually mapped pages */
 struct page *vmalloc_to_page(const void *addr);
--- linux.orig/kernel/resource.c
+++ linux/kernel/resource.c
@@ -494,6 +494,42 @@ int __weak page_is_ram(unsigned long pfn
 }
 EXPORT_SYMBOL_GPL(page_is_ram);
 
+/*
+ * Search for a resouce entry that fully contains the specified region.
+ * If found, return 1 if it is RAM, 0 if not.
+ * If not found, or region is not fully contained, return -1
+ *
+ * Used by the ioremap functions to insure user not remapping RAM and is as
+ * vast speed up over walking through the resource table page by page.
+ */
+int region_is_ram(resource_size_t start, unsigned long size)
+{
+	struct resource *p;
+	resource_size_t end = start + size - 1;
+	int flags = IORESOURCE_MEM | IORESOURCE_BUSY;
+	const char *name = "System RAM";
+	int ret = -1;
+
+	read_lock(&resource_lock);
+	for (p = iomem_resource.child; p ; p = p->sibling) {
+		if (end < p->start)
+			continue;
+
+		if (p->start <= start && end <= p->end) {
+			/* resource fully contains region */
+			if ((p->flags != flags) || strcmp(p->name, name))
+				ret = 0;
+			else
+				ret = 1;
+			break;
+		}
+		if (p->end < start)
+			break;	/* not found */
+	}
+	read_unlock(&resource_lock);
+	return ret;
+}
+
 void __weak arch_remove_reservations(struct resource *avail)
 {
 }

-- 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 1/2] x86: Optimize resource lookups for ioremap

[Mike Travis]

[-- Attachment #1: add-get-resource-type --]
[-- Type: text/plain, Size: 2668 bytes --]

Since the ioremap operation is verifying that the specified address range
is NOT RAM, it will search the entire ioresource list if the condition
is true.  To make matters worse, it does this one 4k page at a time.
For a 128M BAR region this is 32 passes to determine the entire region
does not contain any RAM addresses.

This patch provides another resource lookup function, region_is_ram,
that searches for the entire region specified, verifying that it is
completely contained within the resource region.  If it is found, then
it is checked to be RAM or not, within a single pass.

The return result reflects if it was found or not (-1), and whether it is
RAM (1) or not (0).  This allows the caller to fallback to the previous
page by page search if it was not found.

Signed-off-by: Mike Travis <travis@sgi.com>
Acked-by: Alex Thorlton <athorlton@sgi.com>
Reviewed-by: Cliff Wickman <cpw@sgi.com>
---
v2: remove 'weak' and EXPORT_SYMBOL_GPL from region_is_ram()
---
 include/linux/mm.h |    1 +
 kernel/resource.c  |   36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+)

--- linux.orig/include/linux/mm.h
+++ linux/include/linux/mm.h
@@ -346,6 +346,7 @@ static inline int put_page_unless_one(st
 }
 
 extern int page_is_ram(unsigned long pfn);
+extern int region_is_ram(resource_size_t phys_addr, unsigned long size);
 
 /* Support for virtually mapped pages */
 struct page *vmalloc_to_page(const void *addr);
--- linux.orig/kernel/resource.c
+++ linux/kernel/resource.c
@@ -494,6 +494,42 @@ int __weak page_is_ram(unsigned long pfn
 }
 EXPORT_SYMBOL_GPL(page_is_ram);
 
+/*
+ * Search for a resouce entry that fully contains the specified region.
+ * If found, return 1 if it is RAM, 0 if not.
+ * If not found, or region is not fully contained, return -1
+ *
+ * Used by the ioremap functions to insure user not remapping RAM and is as
+ * vast speed up over walking through the resource table page by page.
+ */
+int region_is_ram(resource_size_t start, unsigned long size)
+{
+	struct resource *p;
+	resource_size_t end = start + size - 1;
+	int flags = IORESOURCE_MEM | IORESOURCE_BUSY;
+	const char *name = "System RAM";
+	int ret = -1;
+
+	read_lock(&resource_lock);
+	for (p = iomem_resource.child; p ; p = p->sibling) {
+		if (end < p->start)
+			continue;
+
+		if (p->start <= start && end <= p->end) {
+			/* resource fully contains region */
+			if ((p->flags != flags) || strcmp(p->name, name))
+				ret = 0;
+			else
+				ret = 1;
+			break;
+		}
+		if (p->end < start)
+			break;	/* not found */
+	}
+	read_unlock(&resource_lock);
+	return ret;
+}
+
 void __weak arch_remove_reservations(struct resource *avail)
 {
 }

-- 

[PATCH 0/2] x86: Speed up ioremap operations

[Mike Travis]

We have a large university system in the UK that is experiencing
very long delays modprobing the driver for a specific I/O device.
The delay is from 8-10 minutes per device and there are 31 devices
in the system.  This 4 to 5 hour delay in starting up those I/O
devices is very much a burden on the customer.

There are two causes for requiring a restart/reload of the drivers.
First is periodic preventive maintenance (PM) and the second is if
any of the devices experience a fatal error.  Both of these trigger
this excessively long delay in bringing the system back up to full
capability.

The problem was tracked down to a very slow IOREMAP operation and
the excessively long ioresource lookup to insure that the user is
not attempting to ioremap RAM.  These patches provide a speed up
to that function.

-- 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 1/2] x86: Optimize resource lookups for ioremap

[Mike Travis]

[-- Attachment #1: add-get-resource-type --]
[-- Type: text/plain, Size: 2944 bytes --]

Since the ioremap operation is verifying that the specified address range
is NOT RAM, it will search the entire ioresource list if the condition
is true.  To make matters worse, it does this one 4k page at a time.
For a 128M BAR region this is 32 passes to determine the entire region
does not contain any RAM addresses.

This patch provides another resource lookup function, region_is_ram,
that searches for the entire region specified, verifying that it is
completely contained within the resource region.  If it is found, then
it is checked to be RAM or not, within a single pass.

The return result reflects if it was found or not (-1), and whether it is
RAM (1) or not (0).  This allows the caller to fallback to the previous
page by page search if it was not found.

Signed-off-by: Mike Travis <travis@sgi.com>
Acked-by: Alex Thorlton <athorlton@sgi.com>
Reviewed-by: Cliff Wickman <cpw@sgi.com>
Cc: <stable@vger.kernel.org>
---
v2: remove 'weak' and EXPORT_SYMBOL_GPL from region_is_ram()
v3: added Cc: stable
---
 include/linux/mm.h |    1 +
 kernel/resource.c  |   36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+)

--- linux.orig/include/linux/mm.h
+++ linux/include/linux/mm.h
@@ -346,6 +346,7 @@ static inline int put_page_unless_one(st
 }
 
 extern int page_is_ram(unsigned long pfn);
+extern int region_is_ram(resource_size_t phys_addr, unsigned long size);
 
 /* Support for virtually mapped pages */
 struct page *vmalloc_to_page(const void *addr);
--- linux.orig/kernel/resource.c
+++ linux/kernel/resource.c
@@ -494,6 +494,42 @@ int __weak page_is_ram(unsigned long pfn
 }
 EXPORT_SYMBOL_GPL(page_is_ram);
 
+/*
+ * Search for a resouce entry that fully contains the specified region.
+ * If found, return 1 if it is RAM, 0 if not.
+ * If not found, or region is not fully contained, return -1
+ *
+ * Used by the ioremap functions to insure user not remapping RAM and is as
+ * vast speed up over walking through the resource table page by page.
+ */
+int region_is_ram(resource_size_t start, unsigned long size)
+{
+	struct resource *p;
+	resource_size_t end = start + size - 1;
+	int flags = IORESOURCE_MEM | IORESOURCE_BUSY;
+	const char *name = "System RAM";
+	int ret = -1;
+
+	read_lock(&resource_lock);
+	for (p = iomem_resource.child; p ; p = p->sibling) {
+		if (end < p->start)
+			continue;
+
+		if (p->start <= start && end <= p->end) {
+			/* resource fully contains region */
+			if ((p->flags != flags) || strcmp(p->name, name))
+				ret = 0;
+			else
+				ret = 1;
+			break;
+		}
+		if (p->end < start)
+			break;	/* not found */
+	}
+	read_unlock(&resource_lock);
+	return ret;
+}
+
 void __weak arch_remove_reservations(struct resource *avail)
 {
 }

-- 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 1/2] x86: Optimize resource lookups for ioremap

[Mike Travis]

[-- Attachment #1: add-get-resource-type --]
[-- Type: text/plain, Size: 2718 bytes --]

Since the ioremap operation is verifying that the specified address range
is NOT RAM, it will search the entire ioresource list if the condition
is true.  To make matters worse, it does this one 4k page at a time.
For a 128M BAR region this is 32 passes to determine the entire region
does not contain any RAM addresses.

This patch provides another resource lookup function, region_is_ram,
that searches for the entire region specified, verifying that it is
completely contained within the resource region.  If it is found, then
it is checked to be RAM or not, within a single pass.

The return result reflects if it was found or not (-1), and whether it is
RAM (1) or not (0).  This allows the caller to fallback to the previous
page by page search if it was not found.

Signed-off-by: Mike Travis <travis@sgi.com>
Acked-by: Alex Thorlton <athorlton@sgi.com>
Reviewed-by: Cliff Wickman <cpw@sgi.com>
Cc: <stable@vger.kernel.org>
---
v2: remove 'weak' and EXPORT_SYMBOL_GPL from region_is_ram()
v3: added Cc: stable
---
 include/linux/mm.h |    1 +
 kernel/resource.c  |   36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+)

--- linux.orig/include/linux/mm.h
+++ linux/include/linux/mm.h
@@ -346,6 +346,7 @@ static inline int put_page_unless_one(st
 }
 
 extern int page_is_ram(unsigned long pfn);
+extern int region_is_ram(resource_size_t phys_addr, unsigned long size);
 
 /* Support for virtually mapped pages */
 struct page *vmalloc_to_page(const void *addr);
--- linux.orig/kernel/resource.c
+++ linux/kernel/resource.c
@@ -494,6 +494,42 @@ int __weak page_is_ram(unsigned long pfn
 }
 EXPORT_SYMBOL_GPL(page_is_ram);
 
+/*
+ * Search for a resouce entry that fully contains the specified region.
+ * If found, return 1 if it is RAM, 0 if not.
+ * If not found, or region is not fully contained, return -1
+ *
+ * Used by the ioremap functions to insure user not remapping RAM and is as
+ * vast speed up over walking through the resource table page by page.
+ */
+int region_is_ram(resource_size_t start, unsigned long size)
+{
+	struct resource *p;
+	resource_size_t end = start + size - 1;
+	int flags = IORESOURCE_MEM | IORESOURCE_BUSY;
+	const char *name = "System RAM";
+	int ret = -1;
+
+	read_lock(&resource_lock);
+	for (p = iomem_resource.child; p ; p = p->sibling) {
+		if (end < p->start)
+			continue;
+
+		if (p->start <= start && end <= p->end) {
+			/* resource fully contains region */
+			if ((p->flags != flags) || strcmp(p->name, name))
+				ret = 0;
+			else
+				ret = 1;
+			break;
+		}
+		if (p->end < start)
+			break;	/* not found */
+	}
+	read_unlock(&resource_lock);
+	return ret;
+}
+
 void __weak arch_remove_reservations(struct resource *avail)
 {
 }

--