[Xen-devel] Re: linux c/s 860

[Isaku Yamahata <yamahata@xxxxxxxxxxxxx>]

On Wed, Apr 15, 2009 at 03:59:56PM +0100, Jan Beulich wrote:
> Other than its comment says, this c/s doesn't only subtract 1 from r_align, 
> but
> also extends the surrounding condition to include !r_align. If r_align is 
> zero,
> ALIGN(r_align, PAGE_SIZE) will be zero too, and hence in the end r_align
> will become ~0. What's the purpose of all that? Or am I missing something
> here? Or is the added condition perhaps just inverted?

Thank you for pointing out.
The alignment logic should be same as in th following list loop.
How about this patch?

linux: clean up of linux c/s 860

The fixing logic was somewhat confused and doesn't produce right result.
This patch cleans it up.

Signed-off-by: Isaku Yamahata <yamahata@xxxxxxxxxxxxx>

diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -245,11 +245,6 @@ pdev_sort_resources(struct pci_dev *dev,
                if (!(r->flags) || r->parent)
                        continue;
                
-               if (i < PCI_BRIDGE_RESOURCES && (r->flags & IORESOURCE_MEM) &&
-                   !r_align && reassigndev)
-                       /* -1 is compensation for +1 in the following calc. */
-                       r_align = ALIGN(r_align, PAGE_SIZE) - 1;
-
                if (!r_align) {
                        printk(KERN_WARNING "PCI: Ignore bogus resource %d "
                                "[%llx:%llx] of %s\n",
@@ -258,6 +253,11 @@ pdev_sort_resources(struct pci_dev *dev,
                        continue;
                }
                r_align = (i < PCI_BRIDGE_RESOURCES) ? r_align + 1 : r->start;
+
+               if (i < PCI_BRIDGE_RESOURCES && (r->flags & IORESOURCE_MEM) &&
+                   reassigndev)
+                       r_align = ALIGN(r_align, PAGE_SIZE);
+
                for (list = head; ; list = list->next) {
                        resource_size_t align = 0;
                        struct resource_list *ln = list->next;


-- 
yamahata

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel