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? Thanks, Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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@valinux.co.jp> 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@lists.xensource.com http://lists.xensource.com/xen-devel
Yes, thanks, this way it at least makes sense. You know whether it does what it''s supposed to do. Jan>>> Isaku Yamahata <yamahata@valinux.co.jp> 16.04.09 04:13 >>>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@valinux.co.jp> 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@lists.xensource.com http://lists.xensource.com/xen-devel