Re: [PATCH] Optimize the resource overlap check

Re: [PATCH] Optimize the resource overlap check

[Ram Pai]

On Fri, Apr 13, 2012 at 10:28:48AM +0800, Wei Yang wrote:
> In coalesce_windows() it tries to check whether the res1 and res2 overlap.
> This function do four comparisons, which could be done with one
> comparisons.
> 
> Also make the resource_check_overlap() a common function for others.
> 
> Signed-Off-By: Wei Yang <weiyang@linux.vnet.ibm.com>

Signed-Off-By: Ram Pai <linuxram@us.ibm.com>

Re: [PATCH] Optimize the resource overlap check

[Richard Yang]

Bjorn

Would you mind take a look at this one?

On Fri, Apr 13, 2012 at 05:44:50PM +0800, Ram Pai wrote:
>On Fri, Apr 13, 2012 at 10:28:48AM +0800, Wei Yang wrote:
>> In coalesce_windows() it tries to check whether the res1 and res2 overlap.
>> This function do four comparisons, which could be done with one
>> comparisons.
>> 
>> Also make the resource_check_overlap() a common function for others.
>> 
>> Signed-Off-By: Wei Yang <weiyang@linux.vnet.ibm.com>
>
>Signed-Off-By: Ram Pai <linuxram@us.ibm.com>
>

-- 
Richard Yang
Help you, Help me

[PATCH] Optimize the resource overlap check

[Richard Yang]

In coalesce_windows() it tries to check whether the res1 and res2 overlap.
This function do four comparisons, which could be done with one
comparisons.

Also make the resource_check_overlap() a common function for others.

Signed-Off-By: Wei Yang <weiyang@linux.vnet.ibm.com>
---
 arch/x86/pci/acpi.c    |   12 +-----------
 include/linux/ioport.h |    7 +++++++
 2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
index 68c3c13..f2bb99e 100644
--- a/arch/x86/pci/acpi.c
+++ b/arch/x86/pci/acpi.c
@@ -181,13 +181,6 @@ setup_resource(struct acpi_resource *acpi_res, void *data)
 	return AE_OK;
 }

-static bool resource_contains(struct resource *res, resource_size_t point)
-{
-	if (res->start <= point && point <= res->end)
-		return true;
-	return false;
-}
-
 static void coalesce_windows(struct pci_root_info *info, unsigned long type)
 {
 	int i, j;
@@ -208,10 +201,7 @@ static void coalesce_windows(struct pci_root_info *info, unsigned long type)
 			 * our resources no longer match the ACPI _CRS, but
 			 * the kernel resource tree doesn't allow overlaps.
 			 */
-			if (resource_contains(res1, res2->start) ||
-			    resource_contains(res1, res2->end) ||
-			    resource_contains(res2, res1->start) ||
-			    resource_contains(res2, res1->end)) {
+			if (resource_check_overlap(res1, res2)) {
 				res1->start = min(res1->start, res2->start);
 				res1->end = max(res1->end, res2->end);
 				dev_info(&info->bridge->dev,
diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index e9bb22c..374259b 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -198,5 +198,12 @@ extern int
 walk_system_ram_range(unsigned long start_pfn, unsigned long nr_pages,
 		void *arg, int (*func)(unsigned long, unsigned long, void *));

+static inline int resource_check_overlap(struct resource *r1,
+                                         struct resource *r2)
+{
+       return (r1->start <= r2->end && r1->end >= r2->start);
+}
+
+
 #endif /* __ASSEMBLY__ */
 #endif	/* _LINUX_IOPORT_H */
-- 
1.7.4.1

-- 
Richard Yang
Help you, Help me

Re: [PATCH] Optimize the resource overlap check

[Bjorn Helgaas]

On Mon, Apr 16, 2012 at 3:26 AM, Richard Yang
<weiyang@linux.vnet.ibm.com> wrote:
>
> In coalesce_windows() it tries to check whether the res1 and res2 overlap.
> This function do four comparisons, which could be done with one
> comparisons.
>
> Also make the resource_check_overlap() a common function for others.
>
> Signed-Off-By: Wei Yang <weiyang@linux.vnet.ibm.com>
> ---
>  arch/x86/pci/acpi.c    |   12 +-----------
>  include/linux/ioport.h |    7 +++++++
>  2 files changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
> index 68c3c13..f2bb99e 100644
> --- a/arch/x86/pci/acpi.c
> +++ b/arch/x86/pci/acpi.c
> @@ -181,13 +181,6 @@ setup_resource(struct acpi_resource *acpi_res, void *data)
>        return AE_OK;
>  }
>
> -static bool resource_contains(struct resource *res, resource_size_t point)
> -{
> -       if (res->start <= point && point <= res->end)
> -               return true;
> -       return false;
> -}
> -
>  static void coalesce_windows(struct pci_root_info *info, unsigned long type)
>  {
>        int i, j;
> @@ -208,10 +201,7 @@ static void coalesce_windows(struct pci_root_info *info, unsigned long type)
>                         * our resources no longer match the ACPI _CRS, but
>                         * the kernel resource tree doesn't allow overlaps.
>                         */
> -                       if (resource_contains(res1, res2->start) ||
> -                           resource_contains(res1, res2->end) ||
> -                           resource_contains(res2, res1->start) ||
> -                           resource_contains(res2, res1->end)) {
> +                       if (resource_check_overlap(res1, res2)) {
>                                res1->start = min(res1->start, res2->start);
>                                res1->end = max(res1->end, res2->end);
>                                dev_info(&info->bridge->dev,
> diff --git a/include/linux/ioport.h b/include/linux/ioport.h
> index e9bb22c..374259b 100644
> --- a/include/linux/ioport.h
> +++ b/include/linux/ioport.h
> @@ -198,5 +198,12 @@ extern int
>  walk_system_ram_range(unsigned long start_pfn, unsigned long nr_pages,
>                void *arg, int (*func)(unsigned long, unsigned long, void *));
>
> +static inline int resource_check_overlap(struct resource *r1,
> +                                         struct resource *r2)
> +{
> +       return (r1->start <= r2->end && r1->end >= r2->start);
> +}

I like the concept, but resource_check_overlap() is not a very good
name.  It returns a boolean, but the function name doesn't give any
clue about the sense.  "resource_overlaps()" or something similar
would be better.

Bjorn

Re: [PATCH] Optimize the resource overlap check

[Richard Yang]

On Mon, Apr 23, 2012 at 11:29:38AM -0600, Bjorn Helgaas wrote:
>On Mon, Apr 16, 2012 at 3:26 AM, Richard Yang
><weiyang@linux.vnet.ibm.com> wrote:
>>
>> In coalesce_windows() it tries to check whether the res1 and res2 overlap.
>> This function do four comparisons, which could be done with one
>> comparisons.
>>
>> Also make the resource_check_overlap() a common function for others.
>>
>> Signed-Off-By: Wei Yang <weiyang@linux.vnet.ibm.com>
>> ---
>>  arch/x86/pci/acpi.c    |   12 +-----------
>>  include/linux/ioport.h |    7 +++++++
>>  2 files changed, 8 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
>> index 68c3c13..f2bb99e 100644
>> --- a/arch/x86/pci/acpi.c
>> +++ b/arch/x86/pci/acpi.c
>> @@ -181,13 +181,6 @@ setup_resource(struct acpi_resource *acpi_res, void *data)
>>        return AE_OK;
>>  }
>>
>> -static bool resource_contains(struct resource *res, resource_size_t point)
>> -{
>> -       if (res->start <= point && point <= res->end)
>> -               return true;
>> -       return false;
>> -}
>> -
>>  static void coalesce_windows(struct pci_root_info *info, unsigned long type)
>>  {
>>        int i, j;
>> @@ -208,10 +201,7 @@ static void coalesce_windows(struct pci_root_info *info, unsigned long type)
>>                         * our resources no longer match the ACPI _CRS, but
>>                         * the kernel resource tree doesn't allow overlaps.
>>                         */
>> -                       if (resource_contains(res1, res2->start) ||
>> -                           resource_contains(res1, res2->end) ||
>> -                           resource_contains(res2, res1->start) ||
>> -                           resource_contains(res2, res1->end)) {
>> +                       if (resource_check_overlap(res1, res2)) {
>>                                res1->start = min(res1->start, res2->start);
>>                                res1->end = max(res1->end, res2->end);
>>                                dev_info(&info->bridge->dev,
>> diff --git a/include/linux/ioport.h b/include/linux/ioport.h
>> index e9bb22c..374259b 100644
>> --- a/include/linux/ioport.h
>> +++ b/include/linux/ioport.h
>> @@ -198,5 +198,12 @@ extern int
>>  walk_system_ram_range(unsigned long start_pfn, unsigned long nr_pages,
>>                void *arg, int (*func)(unsigned long, unsigned long, void *));
>>
>> +static inline int resource_check_overlap(struct resource *r1,
>> +                                         struct resource *r2)
>> +{
>> +       return (r1->start <= r2->end && r1->end >= r2->start);
>> +}
>
>I like the concept, but resource_check_overlap() is not a very good
>name.  It returns a boolean, but the function name doesn't give any
>clue about the sense.  "resource_overlaps()" or something similar
>would be better.
change to this will be better?

static inline bool resource_overlaps(struct resource *r1,
                                         
>
>Bjorn

-- 
Richard Yang
Help you, Help me

Re: [PATCH] Optimize the resource overlap check

[Richard Yang]

Bjorn,

How do you think of my this version?

On Thu, Apr 26, 2012 at 03:32:55PM +0800, Wei Yang wrote:
>In coalesce_windows() it tries to check whether the res1 and res2 overlap.
>This function do four comparisons, which could be done with one
>comparisons.
>
>Also make the resource_overlaps() a common function for others.
>
>Signed-Off-By: Wei Yang <weiyang@linux.vnet.ibm.com>
>---
> arch/x86/pci/acpi.c    |   12 +-----------
> include/linux/ioport.h |    7 +++++++
> 2 files changed, 8 insertions(+), 11 deletions(-)
>
>diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
>index 68c3c13..fd3f541 100644
>--- a/arch/x86/pci/acpi.c
>+++ b/arch/x86/pci/acpi.c
>@@ -181,13 +181,6 @@ setup_resource(struct acpi_resource *acpi_res, void *data)
> 	return AE_OK;
> }
>
>-static bool resource_contains(struct resource *res, resource_size_t point)
>-{
>-	if (res->start <= point && point <= res->end)
>-		return true;
>-	return false;
>-}
>-
> static void coalesce_windows(struct pci_root_info *info, unsigned long type)
> {
> 	int i, j;
>@@ -208,10 +201,7 @@ static void coalesce_windows(struct pci_root_info *info, unsigned long type)
> 			 * our resources no longer match the ACPI _CRS, but
> 			 * the kernel resource tree doesn't allow overlaps.
> 			 */
>-			if (resource_contains(res1, res2->start) ||
>-			    resource_contains(res1, res2->end) ||
>-			    resource_contains(res2, res1->start) ||
>-			    resource_contains(res2, res1->end)) {
>+			if (resource_overlaps(res1, res2)) {
> 				res1->start = min(res1->start, res2->start);
> 				res1->end = max(res1->end, res2->end);
> 				dev_info(&info->bridge->dev,
>diff --git a/include/linux/ioport.h b/include/linux/ioport.h
>index e9bb22c..05d2c70 100644
>--- a/include/linux/ioport.h
>+++ b/include/linux/ioport.h
>@@ -198,5 +198,12 @@ extern int
> walk_system_ram_range(unsigned long start_pfn, unsigned long nr_pages,
> 		void *arg, int (*func)(unsigned long, unsigned long, void *));
>
>+static inline bool resource_overlaps(struct resource *r1,
>+                                         struct resource *r2)
>+{
>+       return (r1->start <= r2->end && r1->end >= r2->start);
>+}
>+
>+
> #endif /* __ASSEMBLY__ */
> #endif	/* _LINUX_IOPORT_H */
>-- 
>1.7.4.1

-- 
Richard Yang
Help you, Help me

Re: [PATCH] Optimize the resource overlap check

[Bjorn Helgaas]

On Fri, May 4, 2012 at 8:47 PM, Richard Yang <weiyang@linux.vnet.ibm.com> wrote:
> Bjorn,
>
> How do you think of my this version?

Applied to my 'next' branch, thanks.

> On Thu, Apr 26, 2012 at 03:32:55PM +0800, Wei Yang wrote:
>>In coalesce_windows() it tries to check whether the res1 and res2 overlap.
>>This function do four comparisons, which could be done with one
>>comparisons.
>>
>>Also make the resource_overlaps() a common function for others.
>>
>>Signed-Off-By: Wei Yang <weiyang@linux.vnet.ibm.com>
>>---
>> arch/x86/pci/acpi.c    |   12 +-----------
>> include/linux/ioport.h |    7 +++++++
>> 2 files changed, 8 insertions(+), 11 deletions(-)
>>
>>diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
>>index 68c3c13..fd3f541 100644
>>--- a/arch/x86/pci/acpi.c
>>+++ b/arch/x86/pci/acpi.c
>>@@ -181,13 +181,6 @@ setup_resource(struct acpi_resource *acpi_res, void *data)
>>       return AE_OK;
>> }
>>
>>-static bool resource_contains(struct resource *res, resource_size_t point)
>>-{
>>-      if (res->start <= point && point <= res->end)
>>-              return true;
>>-      return false;
>>-}
>>-
>> static void coalesce_windows(struct pci_root_info *info, unsigned long type)
>> {
>>       int i, j;
>>@@ -208,10 +201,7 @@ static void coalesce_windows(struct pci_root_info *info, unsigned long type)
>>                        * our resources no longer match the ACPI _CRS, but
>>                        * the kernel resource tree doesn't allow overlaps.
>>                        */
>>-                      if (resource_contains(res1, res2->start) ||
>>-                          resource_contains(res1, res2->end) ||
>>-                          resource_contains(res2, res1->start) ||
>>-                          resource_contains(res2, res1->end)) {
>>+                      if (resource_overlaps(res1, res2)) {
>>                               res1->start = min(res1->start, res2->start);
>>                               res1->end = max(res1->end, res2->end);
>>                               dev_info(&info->bridge->dev,
>>diff --git a/include/linux/ioport.h b/include/linux/ioport.h
>>index e9bb22c..05d2c70 100644
>>--- a/include/linux/ioport.h
>>+++ b/include/linux/ioport.h
>>@@ -198,5 +198,12 @@ extern int
>> walk_system_ram_range(unsigned long start_pfn, unsigned long nr_pages,
>>               void *arg, int (*func)(unsigned long, unsigned long, void *));
>>
>>+static inline bool resource_overlaps(struct resource *r1,
>>+                                         struct resource *r2)
>>+{
>>+       return (r1->start <= r2->end && r1->end >= r2->start);
>>+}
>>+
>>+
>> #endif /* __ASSEMBLY__ */
>> #endif        /* _LINUX_IOPORT_H */
>>--
>>1.7.4.1
>
> --
> Richard Yang
> Help you, Help me
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Optimize the resource overlap check

[Richard Yang]

On Mon, May 07, 2012 at 10:01:40AM -0700, Bjorn Helgaas wrote:
>On Fri, May 4, 2012 at 8:47 PM, Richard Yang <weiyang@linux.vnet.ibm.com> wrote:
>> Bjorn,
>>
>> How do you think of my this version?
>
>Applied to my 'next' branch, thanks.
Thanks :)
>
>> On Thu, Apr 26, 2012 at 03:32:55PM +0800, Wei Yang wrote:
>>>In coalesce_windows() it tries to check whether the res1 and res2 overlap.
>>>This function do four comparisons, which could be done with one
>>>comparisons.
>>>
>>>Also make the resource_overlaps() a common function for others.
>>>
>>>Signed-Off-By: Wei Yang <weiyang@linux.vnet.ibm.com>
>>>---
>>> arch/x86/pci/acpi.c    |   12 +-----------
>>> include/linux/ioport.h |    7 +++++++
>>> 2 files changed, 8 insertions(+), 11 deletions(-)
>>>
>>>diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
>>>index 68c3c13..fd3f541 100644
>>>--- a/arch/x86/pci/acpi.c
>>>+++ b/arch/x86/pci/acpi.c
>>>@@ -181,13 +181,6 @@ setup_resource(struct acpi_resource *acpi_res, void *data)
>>>       return AE_OK;
>>> }
>>>
>>>-static bool resource_contains(struct resource *res, resource_size_t point)
>>>-{
>>>-      if (res->start <= point && point <= res->end)
>>>-              return true;
>>>-      return false;
>>>-}
>>>-
>>> static void coalesce_windows(struct pci_root_info *info, unsigned long type)
>>> {
>>>       int i, j;
>>>@@ -208,10 +201,7 @@ static void coalesce_windows(struct pci_root_info *info, unsigned long type)
>>>                        * our resources no longer match the ACPI _CRS, but
>>>                        * the kernel resource tree doesn't allow overlaps.
>>>                        */
>>>-                      if (resource_contains(res1, res2->start) ||
>>>-                          resource_contains(res1, res2->end) ||
>>>-                          resource_contains(res2, res1->start) ||
>>>-                          resource_contains(res2, res1->end)) {
>>>+                      if (resource_overlaps(res1, res2)) {
>>>                               res1->start = min(res1->start, res2->start);
>>>                               res1->end = max(res1->end, res2->end);
>>>                               dev_info(&info->bridge->dev,
>>>diff --git a/include/linux/ioport.h b/include/linux/ioport.h
>>>index e9bb22c..05d2c70 100644
>>>--- a/include/linux/ioport.h
>>>+++ b/include/linux/ioport.h
>>>@@ -198,5 +198,12 @@ extern int
>>> walk_system_ram_range(unsigned long start_pfn, unsigned long nr_pages,
>>>               void *arg, int (*func)(unsigned long, unsigned long, void *));
>>>
>>>+static inline bool resource_overlaps(struct resource *r1,
>>>+                                         struct resource *r2)
>>>+{
>>>+       return (r1->start <= r2->end && r1->end >= r2->start);
>>>+}
>>>+
>>>+
>>> #endif /* __ASSEMBLY__ */
>>> #endif        /* _LINUX_IOPORT_H */
>>>--
>>>1.7.4.1
>>
>> --
>> Richard Yang
>> Help you, Help me
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Richard Yang
Help you, Help me