[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.

-- 

[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)
 {
 }

-- 

[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 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 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 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 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 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 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 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 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 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.)

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?

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

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

[Greg KH]

On Fri, Aug 29, 2014 at 01:52:00PM -0700, Andrew Morton wrote:
> On Fri, 29 Aug 2014 13:44:31 -0700 Mike Travis <travis@sgi.com> wrote:
> 
> > 
> > 
> > On 8/29/2014 1:16 PM, Andrew Morton wrote:
> > > On Fri, 29 Aug 2014 14:53:28 -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.
> > >>
> > >> 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.
> > >>
> > > 
> > > Really would prefer to have some quantitative testing results in here,
> > > as that is the entire point of the patchset.  And it leaves the reader
> > > wondering "how much of this severe problem remains?".
> > 
> > Okay, I have some results from testing.  The modprobe time appears to
> > be affected quite a bit by previous activity on the ioresource list,
> > which I suspect is due to cache preloading.  While the overall
> > improvement is impacted by other overhead of starting the devices,
> > this drastically improves the modprobe time.
> > 
> > Also our system is considerably smaller so the percentages gained
> > will not be the same.  Best case improvement with the modprobe
> > on our 20 device smallish system was from 'real    5m51.913s' to
> > 'real    0m18.275s'.
> 
> Thanks, I slurped that into the changelog.
> 
> > > Also, the -stable backport is a big ask, isn't it?  It's arguably
> > > notabug and the affected number of machines is small.
> > > 
> > 
> > Ingo had suggested this.  We are definitely pushing it to our distro
> > suppliers for our customers.  Whether it's a big deal for smaller
> > systems is up in the air.  Note that the customer system has 31 devices
> > on an SSI that includes a large number of other IB and SAS devices
> > as well as a number of nodes which all which have discontiguous memory
> > segments.  I'm envisioning an ioresource list that numbers at least
> > several hundred entries.  While that's somewhat indicative of typical
> > UV systems it is generally not that common otherwise.
> > 
> > So I guess the -stable is merely a suggestion, not a request.
> 
> Cc Greg for his thoughts!

Sounds like a good thing for stable.

thanks,

greg k-h

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

[Andrew Morton]

On Fri, 29 Aug 2014 13:44:31 -0700 Mike Travis <travis@sgi.com> wrote:

> 
> 
> On 8/29/2014 1:16 PM, Andrew Morton wrote:
> > On Fri, 29 Aug 2014 14:53:28 -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.
> >>
> >> 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.
> >>
> > 
> > Really would prefer to have some quantitative testing results in here,
> > as that is the entire point of the patchset.  And it leaves the reader
> > wondering "how much of this severe problem remains?".
> 
> Okay, I have some results from testing.  The modprobe time appears to
> be affected quite a bit by previous activity on the ioresource list,
> which I suspect is due to cache preloading.  While the overall
> improvement is impacted by other overhead of starting the devices,
> this drastically improves the modprobe time.
> 
> Also our system is considerably smaller so the percentages gained
> will not be the same.  Best case improvement with the modprobe
> on our 20 device smallish system was from 'real    5m51.913s' to
> 'real    0m18.275s'.

Thanks, I slurped that into the changelog.

> > Also, the -stable backport is a big ask, isn't it?  It's arguably
> > notabug and the affected number of machines is small.
> > 
> 
> Ingo had suggested this.  We are definitely pushing it to our distro
> suppliers for our customers.  Whether it's a big deal for smaller
> systems is up in the air.  Note that the customer system has 31 devices
> on an SSI that includes a large number of other IB and SAS devices
> as well as a number of nodes which all which have discontiguous memory
> segments.  I'm envisioning an ioresource list that numbers at least
> several hundred entries.  While that's somewhat indicative of typical
> UV systems it is generally not that common otherwise.
> 
> So I guess the -stable is merely a suggestion, not a request.

Cc Greg for his thoughts!

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

[Mike Travis]

On 8/29/2014 1:16 PM, Andrew Morton wrote:
> On Fri, 29 Aug 2014 14:53:28 -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.
>>
>> 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.
>>
> 
> Really would prefer to have some quantitative testing results in here,
> as that is the entire point of the patchset.  And it leaves the reader
> wondering "how much of this severe problem remains?".

Okay, I have some results from testing.  The modprobe time appears to
be affected quite a bit by previous activity on the ioresource list,
which I suspect is due to cache preloading.  While the overall
improvement is impacted by other overhead of starting the devices,
this drastically improves the modprobe time.

Also our system is considerably smaller so the percentages gained
will not be the same.  Best case improvement with the modprobe
on our 20 device smallish system was from 'real    5m51.913s' to
'real    0m18.275s'.

> Also, the -stable backport is a big ask, isn't it?  It's arguably
> notabug and the affected number of machines is small.
> 

Ingo had suggested this.  We are definitely pushing it to our distro
suppliers for our customers.  Whether it's a big deal for smaller
systems is up in the air.  Note that the customer system has 31 devices
on an SSI that includes a large number of other IB and SAS devices
as well as a number of nodes which all which have discontiguous memory
segments.  I'm envisioning an ioresource list that numbers at least
several hundred entries.  While that's somewhat indicative of typical
UV systems it is generally not that common otherwise.

So I guess the -stable is merely a suggestion, not a request.

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

[Andrew Morton]

On Fri, 29 Aug 2014 14:53:28 -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.
> 
> 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.
> 

Really would prefer to have some quantitative testing results in here,
as that is the entire point of the patchset.  And it leaves the reader
wondering "how much of this severe problem remains?".

Also, the -stable backport is a big ask, isn't it?  It's arguably
notabug and the affected number of machines is small.

[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 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.

--