| Message ID | 20120416092610.GA28908@richard |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
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 -- 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
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
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 */
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(-)