linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] resources: fix call to alignf() in allocate_resource()
@ 2009-12-20  9:50 Dominik Brodowski
  2009-12-20 18:33 ` Linus Torvalds
  0 siblings, 1 reply; 12+ messages in thread
From: Dominik Brodowski @ 2009-12-20  9:50 UTC (permalink / raw)
  To: Linus Torvalds, Bjorn Helgaas; +Cc: Yinghai Lu, Jesse Barnes, linux-kernel

The second parameter to alignf() in allocate_resource() must
reflect what new resource is attempted to be allocated, else
functions like pcibios_align_resource() (at least on x86) or
pcmcia_align() can't work correctly.

Commit 1e5ad9679016275d422e36b12a98b0927d76f556 broke this by
setting the "new" resource until we're about to return success.
To keep the resource untouched when allocate_resource() fails,
a "tmp" resource is introduced.

CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Yinghai Lu <yhlu.kernel@gmail.com>
CC: Bjorn Helgaas <bjorn.helgaas@hp.com>
CC: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Dominik Brodowski <linux@dominikbrodowski.net>

diff --git a/kernel/resource.c b/kernel/resource.c
index dc15686..af96c1e 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -308,37 +308,37 @@ static int find_resource(struct resource *root, struct resource *new,
 			 void *alignf_data)
 {
 	struct resource *this = root->child;
-	resource_size_t start, end;
+	struct resource tmp = *new;
 
-	start = root->start;
+	tmp.start = root->start;
 	/*
 	 * Skip past an allocated resource that starts at 0, since the assignment
-	 * of this->start - 1 to new->end below would cause an underflow.
+	 * of this->start - 1 to tmp->end below would cause an underflow.
 	 */
 	if (this && this->start == 0) {
-		start = this->end + 1;
+		tmp.start = this->end + 1;
 		this = this->sibling;
 	}
 	for(;;) {
 		if (this)
-			end = this->start - 1;
+			tmp.end = this->start - 1;
 		else
-			end = root->end;
-		if (start < min)
-			start = min;
-		if (end > max)
-			end = max;
-		start = ALIGN(start, align);
+			tmp.end = root->end;
+		if (tmp.start < min)
+			tmp.start = min;
+		if (tmp.end > max)
+			tmp.end = max;
+		tmp.start = ALIGN(tmp.start, align);
 		if (alignf)
-			alignf(alignf_data, new, size, align);
-		if (start < end && end - start >= size - 1) {
-			new->start = start;
-			new->end = start + size - 1;
+			alignf(alignf_data, &tmp, size, align);
+		if (tmp.start < tmp.end && tmp.end - tmp.start >= size - 1) {
+			new->start = tmp.start;
+			new->end = tmp.start + size - 1;
 			return 0;
 		}
 		if (!this)
 			break;
-		start = this->end + 1;
+		tmp.start = this->end + 1;
 		this = this->sibling;
 	}
 	return -EBUSY;

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] resources: fix call to alignf() in allocate_resource()
  2009-12-20  9:50 [PATCH] resources: fix call to alignf() in allocate_resource() Dominik Brodowski
@ 2009-12-20 18:33 ` Linus Torvalds
  2009-12-20 22:12   ` Bjorn Helgaas
  2009-12-21  8:40   ` Dominik Brodowski
  0 siblings, 2 replies; 12+ messages in thread
From: Linus Torvalds @ 2009-12-20 18:33 UTC (permalink / raw)
  To: Dominik Brodowski; +Cc: Bjorn Helgaas, Yinghai Lu, Jesse Barnes, linux-kernel



On Sun, 20 Dec 2009, Dominik Brodowski wrote:
>
> The second parameter to alignf() in allocate_resource() must
> reflect what new resource is attempted to be allocated, else
> functions like pcibios_align_resource() (at least on x86) or
> pcmcia_align() can't work correctly.
> 
> Commit 1e5ad9679016275d422e36b12a98b0927d76f556 broke this by
> setting the "new" resource until we're about to return success.
> To keep the resource untouched when allocate_resource() fails,
> a "tmp" resource is introduced.

Ack. That was subtle. 

That said, maybe a nicer fix to this would be to actually return 'start' 
from the 'alignf' macro. That "modify the resource inside the alignment 
function" thing was always pretty ugly.

And then we'd pass in 'start' instead of 'size' (I have _no_ idea why we 
pass in 'size' to the alignment function, but whatever).

We'd still need to pass in the 'struct resource', but that would be so 
that it can figure out 'flags' (and 'size' if it really needs it) from it, 
but now it would be for reading only. So we could mark it 'const'.

Comments?

But Dominik's patch is ok too - the problem is not his patch, it's our 
longstanding horrible sh*t-for-brains calling convention (for which you 
can probably blame me - mea culpa).

		Linus

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] resources: fix call to alignf() in allocate_resource()
  2009-12-20 18:33 ` Linus Torvalds
@ 2009-12-20 22:12   ` Bjorn Helgaas
  2009-12-21  8:40   ` Dominik Brodowski
  1 sibling, 0 replies; 12+ messages in thread
From: Bjorn Helgaas @ 2009-12-20 22:12 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Dominik Brodowski, Yinghai Lu, Jesse Barnes, linux-kernel

On Sun, 2009-12-20 at 10:33 -0800, Linus Torvalds wrote:
> 
> On Sun, 20 Dec 2009, Dominik Brodowski wrote:
> >
> > The second parameter to alignf() in allocate_resource() must
> > reflect what new resource is attempted to be allocated, else
> > functions like pcibios_align_resource() (at least on x86) or
> > pcmcia_align() can't work correctly.
> > 
> > Commit 1e5ad9679016275d422e36b12a98b0927d76f556 broke this by
> > setting the "new" resource until we're about to return success.
> > To keep the resource untouched when allocate_resource() fails,
> > a "tmp" resource is introduced.
> 
> Ack. That was subtle. 
> 
> That said, maybe a nicer fix to this would be to actually return 'start' 
> from the 'alignf' macro. That "modify the resource inside the alignment 
> function" thing was always pretty ugly.
> 
> And then we'd pass in 'start' instead of 'size' (I have _no_ idea why we 
> pass in 'size' to the alignment function, but whatever).
> 
> We'd still need to pass in the 'struct resource', but that would be so 
> that it can figure out 'flags' (and 'size' if it really needs it) from it, 
> but now it would be for reading only. So we could mark it 'const'.

Ouch, sorry about that, I should have noticed that alignf() can modify
'new' before we know whether we're going to succeed.

Linus' proposal requires more code change, but has the advantage that
future similar mistakes would be less likely.

Bjorn



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] resources: fix call to alignf() in allocate_resource()
  2009-12-20 18:33 ` Linus Torvalds
  2009-12-20 22:12   ` Bjorn Helgaas
@ 2009-12-21  8:40   ` Dominik Brodowski
  2009-12-21 16:03     ` Linus Torvalds
  1 sibling, 1 reply; 12+ messages in thread
From: Dominik Brodowski @ 2009-12-21  8:40 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Bjorn Helgaas, Yinghai Lu, Jesse Barnes, linux-kernel

On Sun, Dec 20, 2009 at 10:33:05AM -0800, Linus Torvalds wrote:
> 
> 
> On Sun, 20 Dec 2009, Dominik Brodowski wrote:
> >
> > The second parameter to alignf() in allocate_resource() must
> > reflect what new resource is attempted to be allocated, else
> > functions like pcibios_align_resource() (at least on x86) or
> > pcmcia_align() can't work correctly.
> > 
> > Commit 1e5ad9679016275d422e36b12a98b0927d76f556 broke this by
> > setting the "new" resource until we're about to return success.
> > To keep the resource untouched when allocate_resource() fails,
> > a "tmp" resource is introduced.
> 
> Ack. That was subtle. 
> 
> That said, maybe a nicer fix to this would be to actually return 'start' 
> from the 'alignf' macro. That "modify the resource inside the alignment 
> function" thing was always pretty ugly.
> 
> And then we'd pass in 'start' instead of 'size' (I have _no_ idea why we 
> pass in 'size' to the alignment function, but whatever).

At least the PCMCIA "align" function makes excessive use of the "size"
parameter, so we'd still need this.

> We'd still need to pass in the 'struct resource', but that would be so 
> that it can figure out 'flags' (and 'size' if it really needs it) from it, 
> but now it would be for reading only. So we could mark it 'const'.

AFAICS, you can't determine the size out of "struct resource" as "start +
size" may be less than "end" (else we couldn't align anything, couldn't we?).

> But Dominik's patch is ok too - the problem is not his patch, it's our 
> longstanding horrible sh*t-for-brains calling convention (for which you 
> can probably blame me - mea culpa).

What about taking my patch for 2.6.33, and deferring the change to the
calling convention to the 2.6.34 merge window? (I'll try to cook something
up and get it into linux-next during the next few weeks).

Best,
	Dominik

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] resources: fix call to alignf() in allocate_resource()
  2009-12-21  8:40   ` Dominik Brodowski
@ 2009-12-21 16:03     ` Linus Torvalds
  2010-01-01 16:38       ` [RFC] improve alignf() calling convention " Dominik Brodowski
  2010-01-14  7:28       ` why can not receive kernel mail anymore? goldenwang1979
  0 siblings, 2 replies; 12+ messages in thread
From: Linus Torvalds @ 2009-12-21 16:03 UTC (permalink / raw)
  To: Dominik Brodowski; +Cc: Bjorn Helgaas, Yinghai Lu, Jesse Barnes, linux-kernel



On Mon, 21 Dec 2009, Dominik Brodowski wrote:
> 
> At least the PCMCIA "align" function makes excessive use of the "size"
> parameter, so we'd still need this.

I noticed that it also uses 'res->end' (in the meaning "skip this one as 
unacceptable").

> What about taking my patch for 2.6.33, and deferring the change to the
> calling convention to the 2.6.34 merge window? (I'll try to cook something
> up and get it into linux-next during the next few weeks).

Sounds like a plan,

		Linus

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC] improve alignf() calling convention in allocate_resource()
  2009-12-21 16:03     ` Linus Torvalds
@ 2010-01-01 16:38       ` Dominik Brodowski
  2010-01-01 16:40         ` [PATCH 1/2] resource: align functions now return start of resource Dominik Brodowski
  2010-01-01 16:40         ` [PATCH 2/2] resource: mark struct resource as const Dominik Brodowski
  2010-01-14  7:28       ` why can not receive kernel mail anymore? goldenwang1979
  1 sibling, 2 replies; 12+ messages in thread
From: Dominik Brodowski @ 2010-01-01 16:38 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Bjorn Helgaas, Yinghai Lu, Jesse Barnes, linux-kernel, linux-pci,
	linux-arch

On Mon, Dec 21, 2009 at 08:03:02AM -0800, Linus Torvalds wrote:
> > What about taking my patch for 2.6.33, and deferring the change to the
> > calling convention to the 2.6.34 merge window? (I'll try to cook something
> > up and get it into linux-next during the next few weeks).

Based on the previous discussion, I've prepared two patches which change the
calling convention of alignf() to

resource_size_t (*alignf)(void *data,
			  const struct resource *new,
			  resource_size_t size,
			  resource_size_t align)

. Any feedback is welcome.

 arch/alpha/kernel/pci.c                 |    6 +++---
 arch/arm/kernel/bios32.c                |    8 +++++---
 arch/cris/arch-v32/drivers/pci/bios.c   |   16 +++++++---------
 arch/frv/mb93090-mb00/pci-frv.c         |   16 +++++++---------
 arch/ia64/pci/pci.c                     |    5 +++--
 arch/mips/pci/pci.c                     |    6 +++---
 arch/mips/pmc-sierra/yosemite/ht.c      |   10 +++++-----
 arch/mn10300/unit-asb2305/pci-asb2305.c |   16 +++++++---------
 arch/parisc/kernel/pci.c                |   10 +++++-----
 arch/powerpc/kernel/pci-common.c        |   13 ++++++-------
 arch/sh/drivers/pci/pci.c               |    6 +++---
 arch/sparc/kernel/pci.c                 |    5 +++--
 arch/sparc/kernel/pcic.c                |    5 +++--
 arch/x86/pci/i386.c                     |   14 ++++++--------
 arch/xtensa/kernel/pci.c                |   15 +++++++--------
 drivers/pci/bus.c                       |    6 ++++--
 drivers/pcmcia/rsrc_mgr.c               |   13 +++++++------
 drivers/pcmcia/rsrc_nonstatic.c         |   22 ++++++++++++----------
 include/linux/ioport.h                  |    6 ++++--
 include/linux/pci.h                     |    9 ++++++---
 kernel/resource.c                       |   14 +++++++++-----
 21 files changed, 115 insertions(+), 106 deletions(-)


Best,
	Dominik

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] resource: align functions now return start of resource
  2010-01-01 16:38       ` [RFC] improve alignf() calling convention " Dominik Brodowski
@ 2010-01-01 16:40         ` Dominik Brodowski
  2010-01-01 22:12           ` Yinghai Lu
  2010-01-15 18:23           ` Jesse Barnes
  2010-01-01 16:40         ` [PATCH 2/2] resource: mark struct resource as const Dominik Brodowski
  1 sibling, 2 replies; 12+ messages in thread
From: Dominik Brodowski @ 2010-01-01 16:40 UTC (permalink / raw)
  To: linux-kernel, linux-pci, linux-arch, torvalds
  Cc: Dominik Brodowski, Bjorn Helgaas, Yinghai Lu, Jesse Barnes

As suggested by Linus, align functions should return the start
of a resource, not void. An update of "res->start" is no longer
necessary.

Cc: Bjorn Helgaas <bjorn.helgaas@hp.com>
Cc: Yinghai Lu <yhlu.kernel@gmail.com>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Dominik Brodowski <linux@dominikbrodowski.net>
---
 arch/alpha/kernel/pci.c                 |    4 ++--
 arch/arm/kernel/bios32.c                |    8 +++++---
 arch/cris/arch-v32/drivers/pci/bios.c   |   14 ++++++--------
 arch/frv/mb93090-mb00/pci-frv.c         |   14 ++++++--------
 arch/ia64/pci/pci.c                     |    3 ++-
 arch/mips/pci/pci.c                     |    4 ++--
 arch/mips/pmc-sierra/yosemite/ht.c      |   10 +++++-----
 arch/mn10300/unit-asb2305/pci-asb2305.c |   16 +++++++---------
 arch/parisc/kernel/pci.c                |   10 +++++-----
 arch/powerpc/kernel/pci-common.c        |   13 ++++++-------
 arch/sh/drivers/pci/pci.c               |    6 +++---
 arch/sparc/kernel/pci.c                 |    5 +++--
 arch/sparc/kernel/pcic.c                |    5 +++--
 arch/x86/pci/i386.c                     |   12 +++++-------
 arch/xtensa/kernel/pci.c                |   11 +++++------
 drivers/pci/bus.c                       |    6 ++++--
 drivers/pcmcia/rsrc_mgr.c               |   12 ++++++------
 drivers/pcmcia/rsrc_nonstatic.c         |   16 +++++++++-------
 include/linux/ioport.h                  |    6 ++++--
 include/linux/pci.h                     |    8 +++++---
 kernel/resource.c                       |   14 +++++++++-----
 21 files changed, 102 insertions(+), 95 deletions(-)

diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
index a91ba28..5cf111e 100644
--- a/arch/alpha/kernel/pci.c
+++ b/arch/alpha/kernel/pci.c
@@ -126,7 +126,7 @@ DECLARE_PCI_FIXUP_FINAL(PCI_ANY_ID, PCI_ANY_ID, pcibios_fixup_final);
 #define MB			(1024*KB)
 #define GB			(1024*MB)
 
-void
+resource_size_t
 pcibios_align_resource(void *data, struct resource *res,
 		       resource_size_t size, resource_size_t align)
 {
@@ -184,7 +184,7 @@ pcibios_align_resource(void *data, struct resource *res,
 		}
 	}
 
-	res->start = start;
+	return start;
 }
 #undef KB
 #undef MB
diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
index 8096819..a7c85f8 100644
--- a/arch/arm/kernel/bios32.c
+++ b/arch/arm/kernel/bios32.c
@@ -616,15 +616,17 @@ char * __init pcibios_setup(char *str)
  * but we want to try to avoid allocating at 0x2900-0x2bff
  * which might be mirrored at 0x0100-0x03ff..
  */
-void pcibios_align_resource(void *data, struct resource *res,
-			    resource_size_t size, resource_size_t align)
+resource_size_t pcibios_align_resource(void *data, struct resource *res,
+				resource_size_t size, resource_size_t align)
 {
 	resource_size_t start = res->start;
 
 	if (res->flags & IORESOURCE_IO && start & 0x300)
 		start = (start + 0x3ff) & ~0x3ff;
 
-	res->start = (start + align - 1) & ~(align - 1);
+	start = (start + align - 1) & ~(align - 1);
+
+	return start;
 }
 
 /**
diff --git a/arch/cris/arch-v32/drivers/pci/bios.c b/arch/cris/arch-v32/drivers/pci/bios.c
index 77ee319..5811e2f 100644
--- a/arch/cris/arch-v32/drivers/pci/bios.c
+++ b/arch/cris/arch-v32/drivers/pci/bios.c
@@ -41,18 +41,16 @@ int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
 	return 0;
 }
 
-void
+resource_size_t
 pcibios_align_resource(void *data, struct resource *res,
 		       resource_size_t size, resource_size_t align)
 {
-	if (res->flags & IORESOURCE_IO) {
-		resource_size_t start = res->start;
+	resource_size_t start = res->start;
 
-		if (start & 0x300) {
-			start = (start + 0x3ff) & ~0x3ff;
-			res->start = start;
-		}
-	}
+	if ((res->flags & IORESOURCE_IO) && (start & 0x300))
+		start = (start + 0x3ff) & ~0x3ff;
+
+	return start
 }
 
 int pcibios_enable_resources(struct pci_dev *dev, int mask)
diff --git a/arch/frv/mb93090-mb00/pci-frv.c b/arch/frv/mb93090-mb00/pci-frv.c
index 566bdeb..c947aa4 100644
--- a/arch/frv/mb93090-mb00/pci-frv.c
+++ b/arch/frv/mb93090-mb00/pci-frv.c
@@ -32,18 +32,16 @@
  * but we want to try to avoid allocating at 0x2900-0x2bff
  * which might have be mirrored at 0x0100-0x03ff..
  */
-void
+resource_size_t
 pcibios_align_resource(void *data, struct resource *res,
 		       resource_size_t size, resource_size_t align)
 {
-	if (res->flags & IORESOURCE_IO) {
-		resource_size_t start = res->start;
+	resource_size_t start = res->start;
 
-		if (start & 0x300) {
-			start = (start + 0x3ff) & ~0x3ff;
-			res->start = start;
-		}
-	}
+	if ((res->flags & IORESOURCE_IO) && (start & 0x300))
+		start = (start + 0x3ff) & ~0x3ff;
+
+	return start
 }
 
 
diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
index df639db..ef574cd 100644
--- a/arch/ia64/pci/pci.c
+++ b/arch/ia64/pci/pci.c
@@ -547,10 +547,11 @@ pcibios_disable_device (struct pci_dev *dev)
 		acpi_pci_irq_disable(dev);
 }
 
-void
+resource_size_t
 pcibios_align_resource (void *data, struct resource *res,
 		        resource_size_t size, resource_size_t align)
 {
+	return res->start;
 }
 
 /*
diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
index 9a11c22..9085988 100644
--- a/arch/mips/pci/pci.c
+++ b/arch/mips/pci/pci.c
@@ -49,7 +49,7 @@ static int pci_initialized;
  * but we want to try to avoid allocating at 0x2900-0x2bff
  * which might have be mirrored at 0x0100-0x03ff..
  */
-void
+resource_size_t
 pcibios_align_resource(void *data, struct resource *res,
 		       resource_size_t size, resource_size_t align)
 {
@@ -73,7 +73,7 @@ pcibios_align_resource(void *data, struct resource *res,
 			start = PCIBIOS_MIN_MEM + hose->mem_resource->start;
 	}
 
-	res->start = start;
+	return start;
 }
 
 static void __devinit pcibios_scanbus(struct pci_controller *hose)
diff --git a/arch/mips/pmc-sierra/yosemite/ht.c b/arch/mips/pmc-sierra/yosemite/ht.c
index 678388f..5e41008 100644
--- a/arch/mips/pmc-sierra/yosemite/ht.c
+++ b/arch/mips/pmc-sierra/yosemite/ht.c
@@ -345,14 +345,13 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
         return pcibios_enable_resources(dev);
 }
 
-void pcibios_align_resource(void *data, struct resource *res,
-                            resource_size_t size, resource_size_t align)
+resource_size_t pcibios_align_resource(void *data, struct resource *res,
+				resource_size_t size, resource_size_t align)
 {
         struct pci_dev *dev = data;
+	resource_size_t start = res->start;
 
         if (res->flags & IORESOURCE_IO) {
-                resource_size_t start = res->start;
-
                 /* We need to avoid collisions with `mirrored' VGA ports
                    and other strange ISA hardware, so we always want the
                    addresses kilobyte aligned.  */
@@ -363,8 +362,9 @@ void pcibios_align_resource(void *data, struct resource *res,
                 }
 
                 start = (start + 1024 - 1) & ~(1024 - 1);
-                res->start = start;
         }
+
+	return start;
 }
 
 struct pci_ops titan_pci_ops = {
diff --git a/arch/mn10300/unit-asb2305/pci-asb2305.c b/arch/mn10300/unit-asb2305/pci-asb2305.c
index d100ca7..60f24a7 100644
--- a/arch/mn10300/unit-asb2305/pci-asb2305.c
+++ b/arch/mn10300/unit-asb2305/pci-asb2305.c
@@ -31,9 +31,11 @@
  * but we want to try to avoid allocating at 0x2900-0x2bff
  * which might have be mirrored at 0x0100-0x03ff..
  */
-void pcibios_align_resource(void *data, struct resource *res,
-			    resource_size_t size, resource_size_t align)
+resource_size_t pcibios_align_resource(void *data, struct resource *res,
+				resource_size_t size, resource_size_t align)
 {
+	resource_size_t start = res->start;
+
 #if 0
 	struct pci_dev *dev = data;
 
@@ -47,14 +49,10 @@ void pcibios_align_resource(void *data, struct resource *res,
 	       );
 #endif
 
-	if (res->flags & IORESOURCE_IO) {
-		unsigned long start = res->start;
+	if ((res->flags & IORESOURCE_IO) && (start & 0x300))
+		start = (start + 0x3ff) & ~0x3ff;
 
-		if (start & 0x300) {
-			start = (start + 0x3ff) & ~0x3ff;
-			res->start = start;
-		}
-	}
+	return start;
 }
 
 
diff --git a/arch/parisc/kernel/pci.c b/arch/parisc/kernel/pci.c
index f7064ab..4463a31 100644
--- a/arch/parisc/kernel/pci.c
+++ b/arch/parisc/kernel/pci.c
@@ -254,10 +254,10 @@ EXPORT_SYMBOL(pcibios_bus_to_resource);
  * Since we are just checking candidates, don't use any fields other
  * than res->start.
  */
-void pcibios_align_resource(void *data, struct resource *res,
+resource_size_t pcibios_align_resource(void *data, struct resource *res,
 				resource_size_t size, resource_size_t alignment)
 {
-	resource_size_t mask, align;
+	resource_size_t mask, align, start = res->start;
 
 	DBG_RES("pcibios_align_resource(%s, (%p) [%lx,%lx]/%x, 0x%lx, 0x%lx)\n",
 		pci_name(((struct pci_dev *) data)),
@@ -269,10 +269,10 @@ void pcibios_align_resource(void *data, struct resource *res,
 
 	/* Align to largest of MIN or input size */
 	mask = max(alignment, align) - 1;
-	res->start += mask;
-	res->start &= ~mask;
+	start += mask;
+	start &= ~mask;
 
-	/* The caller updates the end field, we don't.  */
+	return start;
 }
 
 
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index e8dfdbd..3e343b8 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1168,21 +1168,20 @@ static int skip_isa_ioresource_align(struct pci_dev *dev)
  * but we want to try to avoid allocating at 0x2900-0x2bff
  * which might have be mirrored at 0x0100-0x03ff..
  */
-void pcibios_align_resource(void *data, struct resource *res,
+resource_size_t pcibios_align_resource(void *data, struct resource *res,
 				resource_size_t size, resource_size_t align)
 {
 	struct pci_dev *dev = data;
+	resource_size_t start = res->start;
 
 	if (res->flags & IORESOURCE_IO) {
-		resource_size_t start = res->start;
-
 		if (skip_isa_ioresource_align(dev))
-			return;
-		if (start & 0x300) {
+			return start;
+		if (start & 0x300)
 			start = (start + 0x3ff) & ~0x3ff;
-			res->start = start;
-		}
 	}
+
+	return start;
 }
 EXPORT_SYMBOL(pcibios_align_resource);
 
diff --git a/arch/sh/drivers/pci/pci.c b/arch/sh/drivers/pci/pci.c
index c481df6..b36ca82 100644
--- a/arch/sh/drivers/pci/pci.c
+++ b/arch/sh/drivers/pci/pci.c
@@ -148,8 +148,8 @@ void __devinit pcibios_fixup_bus(struct pci_bus *bus)
  * addresses to be allocated in the 0x000-0x0ff region
  * modulo 0x400.
  */
-void pcibios_align_resource(void *data, struct resource *res,
-			    resource_size_t size, resource_size_t align)
+resource_size_t pcibios_align_resource(void *data, struct resource *res,
+				resource_size_t size, resource_size_t align)
 {
 	struct pci_dev *dev = data;
 	struct pci_channel *chan = dev->sysdata;
@@ -171,7 +171,7 @@ void pcibios_align_resource(void *data, struct resource *res,
 			start = PCIBIOS_MIN_MEM + chan->mem_resource->start;
 	}
 
-	res->start = start;
+	return start;
 }
 
 void pcibios_resource_to_bus(struct pci_dev *dev, struct pci_bus_region *region,
diff --git a/arch/sparc/kernel/pci.c b/arch/sparc/kernel/pci.c
index 539e83f..6381ceb 100644
--- a/arch/sparc/kernel/pci.c
+++ b/arch/sparc/kernel/pci.c
@@ -715,9 +715,10 @@ void pcibios_update_irq(struct pci_dev *pdev, int irq)
 {
 }
 
-void pcibios_align_resource(void *data, struct resource *res,
-			    resource_size_t size, resource_size_t align)
+resource_size_t pcibios_align_resource(void *data, struct resource *res,
+				resource_size_t size, resource_size_t align)
 {
+	return res->start;
 }
 
 int pcibios_enable_device(struct pci_dev *dev, int mask)
diff --git a/arch/sparc/kernel/pcic.c b/arch/sparc/kernel/pcic.c
index 85e7037..96fe494 100644
--- a/arch/sparc/kernel/pcic.c
+++ b/arch/sparc/kernel/pcic.c
@@ -839,9 +839,10 @@ char * __devinit pcibios_setup(char *str)
 	return str;
 }
 
-void pcibios_align_resource(void *data, struct resource *res,
-			    resource_size_t size, resource_size_t align)
+resource_size_t pcibios_align_resource(void *data, struct resource *res,
+				resource_size_t size, resource_size_t align)
 {
+	return res->start;
 }
 
 int pcibios_enable_device(struct pci_dev *pdev, int mask)
diff --git a/arch/x86/pci/i386.c b/arch/x86/pci/i386.c
index 5dc9e8c..924e40c 100644
--- a/arch/x86/pci/i386.c
+++ b/arch/x86/pci/i386.c
@@ -60,22 +60,20 @@ skip_isa_ioresource_align(struct pci_dev *dev) {
  * but we want to try to avoid allocating at 0x2900-0x2bff
  * which might have be mirrored at 0x0100-0x03ff..
  */
-void
+resource_size_t
 pcibios_align_resource(void *data, struct resource *res,
 			resource_size_t size, resource_size_t align)
 {
 	struct pci_dev *dev = data;
+	resource_size_t start = res->start;
 
 	if (res->flags & IORESOURCE_IO) {
-		resource_size_t start = res->start;
-
 		if (skip_isa_ioresource_align(dev))
-			return;
-		if (start & 0x300) {
+			return start;
+		if (start & 0x300)
 			start = (start + 0x3ff) & ~0x3ff;
-			res->start = start;
-		}
 	}
+	return start;
 }
 EXPORT_SYMBOL(pcibios_align_resource);
 
diff --git a/arch/xtensa/kernel/pci.c b/arch/xtensa/kernel/pci.c
index b7c0734..d7efab0 100644
--- a/arch/xtensa/kernel/pci.c
+++ b/arch/xtensa/kernel/pci.c
@@ -69,26 +69,25 @@ static int pci_bus_count;
  * but we want to try to avoid allocating at 0x2900-0x2bff
  * which might have be mirrored at 0x0100-0x03ff..
  */
-void
+resource_size_t
 pcibios_align_resource(void *data, struct resource *res, resource_size_t size,
     		       resource_size_t align)
 {
 	struct pci_dev *dev = data;
+	resource_size_t start = res->start;
 
 	if (res->flags & IORESOURCE_IO) {
-		resource_size_t start = res->start;
-
 		if (size > 0x100) {
 			printk(KERN_ERR "PCI: I/O Region %s/%d too large"
 			       " (%ld bytes)\n", pci_name(dev),
 			       dev->resource - res, size);
 		}
 
-		if (start & 0x300) {
+		if (start & 0x300)
 			start = (start + 0x3ff) & ~0x3ff;
-			res->start = start;
-		}
 	}
+
+	return start;
 }
 
 int
diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
index cef28a7..d29d69a 100644
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -36,8 +36,10 @@ int
 pci_bus_alloc_resource(struct pci_bus *bus, struct resource *res,
 		resource_size_t size, resource_size_t align,
 		resource_size_t min, unsigned int type_mask,
-		void (*alignf)(void *, struct resource *, resource_size_t,
-				resource_size_t),
+		resource_size_t (*alignf)(void *,
+					  struct resource *,
+					  resource_size_t,
+					  resource_size_t),
 		void *alignf_data)
 {
 	int i, ret = -ENOMEM;
diff --git a/drivers/pcmcia/rsrc_mgr.c b/drivers/pcmcia/rsrc_mgr.c
index 52db172..f92a2da 100644
--- a/drivers/pcmcia/rsrc_mgr.c
+++ b/drivers/pcmcia/rsrc_mgr.c
@@ -114,22 +114,20 @@ struct pcmcia_align_data {
 	unsigned long	offset;
 };
 
-static void pcmcia_align(void *align_data, struct resource *res,
-			unsigned long size, unsigned long align)
+static resource_size_t pcmcia_align(void *align_data, struct resource *res,
+				resource_size_t size, resource_size_t align)
 {
 	struct pcmcia_align_data *data = align_data;
-	unsigned long start;
+	resource_size_t start;
 
 	start = (res->start & ~data->mask) + data->offset;
 	if (start < res->start)
 		start += data->mask + 1;
-	res->start = start;
 
 #ifdef CONFIG_X86
 	if (res->flags & IORESOURCE_IO) {
 		if (start & 0x300) {
 			start = (start + 0x3ff) & ~0x3ff;
-			res->start = start;
 		}
 	}
 #endif
@@ -137,9 +135,11 @@ static void pcmcia_align(void *align_data, struct resource *res,
 #ifdef CONFIG_M68K
 	if (res->flags & IORESOURCE_IO) {
 		if ((res->start + size - 1) >= 1024)
-			res->start = res->end;
+			start = res->end;
 	}
 #endif
+
+	return start;
 }
 
 
diff --git a/drivers/pcmcia/rsrc_nonstatic.c b/drivers/pcmcia/rsrc_nonstatic.c
index 9b0dc43..b659028 100644
--- a/drivers/pcmcia/rsrc_nonstatic.c
+++ b/drivers/pcmcia/rsrc_nonstatic.c
@@ -533,7 +533,7 @@ struct pcmcia_align_data {
 	struct resource_map	*map;
 };
 
-static void
+static resource_size_t
 pcmcia_common_align(void *align_data, struct resource *res,
 			resource_size_t size, resource_size_t align)
 {
@@ -545,17 +545,18 @@ pcmcia_common_align(void *align_data, struct resource *res,
 	start = (res->start & ~data->mask) + data->offset;
 	if (start < res->start)
 		start += data->mask + 1;
-	res->start = start;
+	return start;
 }
 
-static void
+static resource_size_t
 pcmcia_align(void *align_data, struct resource *res, resource_size_t size,
 		resource_size_t align)
 {
 	struct pcmcia_align_data *data = align_data;
 	struct resource_map *m;
+	resource_size_t start;
 
-	pcmcia_common_align(data, res, size, align);
+	start = pcmcia_common_align(data, res, size, align);
 
 	for (m = data->map->next; m != data->map; m = m->next) {
 		unsigned long start = m->base;
@@ -567,8 +568,7 @@ pcmcia_align(void *align_data, struct resource *res, resource_size_t size,
 		 * fit here.
 		 */
 		if (res->start < start) {
-			res->start = start;
-			pcmcia_common_align(data, res, size, align);
+			start = pcmcia_common_align(data, res, size, align);
 		}
 
 		/*
@@ -586,7 +586,9 @@ pcmcia_align(void *align_data, struct resource *res, resource_size_t size,
 	 * If we failed to find something suitable, ensure we fail.
 	 */
 	if (m == data->map)
-		res->start = res->end;
+		start = res->end;
+
+	return start;
 }
 
 /*
diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index 7129504..f4195de 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -120,8 +120,10 @@ extern void insert_resource_expand_to_fit(struct resource *root, struct resource
 extern int allocate_resource(struct resource *root, struct resource *new,
 			     resource_size_t size, resource_size_t min,
 			     resource_size_t max, resource_size_t align,
-			     void (*alignf)(void *, struct resource *,
-					    resource_size_t, resource_size_t),
+			     resource_size_t (*alignf)(void *,
+						       struct resource *,
+						       resource_size_t,
+						       resource_size_t),
 			     void *alignf_data);
 int adjust_resource(struct resource *res, resource_size_t start,
 		    resource_size_t size);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index bf1e670..30a6e9b 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -562,7 +562,8 @@ int __must_check pcibios_enable_device(struct pci_dev *, int mask);
 char *pcibios_setup(char *str);
 
 /* Used only when drivers/pci/setup.c is used */
-void pcibios_align_resource(void *, struct resource *, resource_size_t,
+resource_size_t pcibios_align_resource(void *, struct resource *,
+				resource_size_t,
 				resource_size_t);
 void pcibios_update_irq(struct pci_dev *, int irq);
 
@@ -789,8 +790,9 @@ int __must_check pci_bus_alloc_resource(struct pci_bus *bus,
 			struct resource *res, resource_size_t size,
 			resource_size_t align, resource_size_t min,
 			unsigned int type_mask,
-			void (*alignf)(void *, struct resource *,
-				resource_size_t, resource_size_t),
+			resource_size_t (*alignf)(void *, struct resource *,
+						  resource_size_t,
+						  resource_size_t),
 			void *alignf_data);
 void pci_enable_bridges(struct pci_bus *bus);
 
diff --git a/kernel/resource.c b/kernel/resource.c
index af96c1e..e697f20 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -303,8 +303,10 @@ int walk_system_ram_range(unsigned long start_pfn, unsigned long nr_pages,
 static int find_resource(struct resource *root, struct resource *new,
 			 resource_size_t size, resource_size_t min,
 			 resource_size_t max, resource_size_t align,
-			 void (*alignf)(void *, struct resource *,
-					resource_size_t, resource_size_t),
+			 resource_size_t (*alignf)(void *,
+						   struct resource *,
+						   resource_size_t,
+						   resource_size_t),
 			 void *alignf_data)
 {
 	struct resource *this = root->child;
@@ -330,7 +332,7 @@ static int find_resource(struct resource *root, struct resource *new,
 			tmp.end = max;
 		tmp.start = ALIGN(tmp.start, align);
 		if (alignf)
-			alignf(alignf_data, &tmp, size, align);
+			tmp.start = alignf(alignf_data, &tmp, size, align);
 		if (tmp.start < tmp.end && tmp.end - tmp.start >= size - 1) {
 			new->start = tmp.start;
 			new->end = tmp.start + size - 1;
@@ -358,8 +360,10 @@ static int find_resource(struct resource *root, struct resource *new,
 int allocate_resource(struct resource *root, struct resource *new,
 		      resource_size_t size, resource_size_t min,
 		      resource_size_t max, resource_size_t align,
-		      void (*alignf)(void *, struct resource *,
-				     resource_size_t, resource_size_t),
+		      resource_size_t (*alignf)(void *,
+						struct resource *,
+						resource_size_t,
+						resource_size_t),
 		      void *alignf_data)
 {
 	int err;
-- 
1.6.3.3


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] resource: mark struct resource as const
  2010-01-01 16:38       ` [RFC] improve alignf() calling convention " Dominik Brodowski
  2010-01-01 16:40         ` [PATCH 1/2] resource: align functions now return start of resource Dominik Brodowski
@ 2010-01-01 16:40         ` Dominik Brodowski
  1 sibling, 0 replies; 12+ messages in thread
From: Dominik Brodowski @ 2010-01-01 16:40 UTC (permalink / raw)
  To: linux-kernel, linux-pci, linux-arch, torvalds
  Cc: Dominik Brodowski, Bjorn Helgaas, Yinghai Lu, Jesse Barnes

Now that we return the new resource start position, there is no
need to update "struct resource" inside the align function.
Therefore, mark the struct resource as const.

Cc: Bjorn Helgaas <bjorn.helgaas@hp.com>
Cc: Yinghai Lu <yhlu.kernel@gmail.com>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Dominik Brodowski <linux@dominikbrodowski.net>
---
 arch/alpha/kernel/pci.c                 |    2 +-
 arch/arm/kernel/bios32.c                |    2 +-
 arch/cris/arch-v32/drivers/pci/bios.c   |    2 +-
 arch/frv/mb93090-mb00/pci-frv.c         |    2 +-
 arch/ia64/pci/pci.c                     |    2 +-
 arch/mips/pci/pci.c                     |    2 +-
 arch/mips/pmc-sierra/yosemite/ht.c      |    2 +-
 arch/mn10300/unit-asb2305/pci-asb2305.c |    2 +-
 arch/parisc/kernel/pci.c                |    2 +-
 arch/powerpc/kernel/pci-common.c        |    2 +-
 arch/sh/drivers/pci/pci.c               |    2 +-
 arch/sparc/kernel/pci.c                 |    2 +-
 arch/sparc/kernel/pcic.c                |    2 +-
 arch/x86/pci/i386.c                     |    2 +-
 arch/xtensa/kernel/pci.c                |    4 ++--
 drivers/pci/bus.c                       |    2 +-
 drivers/pcmcia/rsrc_mgr.c               |    3 ++-
 drivers/pcmcia/rsrc_nonstatic.c         |    6 +++---
 include/linux/ioport.h                  |    2 +-
 include/linux/pci.h                     |    5 +++--
 kernel/resource.c                       |    4 ++--
 21 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
index 5cf111e..c9ab94e 100644
--- a/arch/alpha/kernel/pci.c
+++ b/arch/alpha/kernel/pci.c
@@ -127,7 +127,7 @@ DECLARE_PCI_FIXUP_FINAL(PCI_ANY_ID, PCI_ANY_ID, pcibios_fixup_final);
 #define GB			(1024*MB)
 
 resource_size_t
-pcibios_align_resource(void *data, struct resource *res,
+pcibios_align_resource(void *data, const struct resource *res,
 		       resource_size_t size, resource_size_t align)
 {
 	struct pci_dev *dev = data;
diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
index a7c85f8..bd397e0 100644
--- a/arch/arm/kernel/bios32.c
+++ b/arch/arm/kernel/bios32.c
@@ -616,7 +616,7 @@ char * __init pcibios_setup(char *str)
  * but we want to try to avoid allocating at 0x2900-0x2bff
  * which might be mirrored at 0x0100-0x03ff..
  */
-resource_size_t pcibios_align_resource(void *data, struct resource *res,
+resource_size_t pcibios_align_resource(void *data, const struct resource *res,
 				resource_size_t size, resource_size_t align)
 {
 	resource_size_t start = res->start;
diff --git a/arch/cris/arch-v32/drivers/pci/bios.c b/arch/cris/arch-v32/drivers/pci/bios.c
index 5811e2f..d4b9c36 100644
--- a/arch/cris/arch-v32/drivers/pci/bios.c
+++ b/arch/cris/arch-v32/drivers/pci/bios.c
@@ -42,7 +42,7 @@ int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
 }
 
 resource_size_t
-pcibios_align_resource(void *data, struct resource *res,
+pcibios_align_resource(void *data, const struct resource *res,
 		       resource_size_t size, resource_size_t align)
 {
 	resource_size_t start = res->start;
diff --git a/arch/frv/mb93090-mb00/pci-frv.c b/arch/frv/mb93090-mb00/pci-frv.c
index c947aa4..1ed15d7 100644
--- a/arch/frv/mb93090-mb00/pci-frv.c
+++ b/arch/frv/mb93090-mb00/pci-frv.c
@@ -33,7 +33,7 @@
  * which might have be mirrored at 0x0100-0x03ff..
  */
 resource_size_t
-pcibios_align_resource(void *data, struct resource *res,
+pcibios_align_resource(void *data, const struct resource *res,
 		       resource_size_t size, resource_size_t align)
 {
 	resource_size_t start = res->start;
diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
index ef574cd..783c83b 100644
--- a/arch/ia64/pci/pci.c
+++ b/arch/ia64/pci/pci.c
@@ -548,7 +548,7 @@ pcibios_disable_device (struct pci_dev *dev)
 }
 
 resource_size_t
-pcibios_align_resource (void *data, struct resource *res,
+pcibios_align_resource (void *data, const struct resource *res,
 		        resource_size_t size, resource_size_t align)
 {
 	return res->start;
diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
index 9085988..f87f5e1 100644
--- a/arch/mips/pci/pci.c
+++ b/arch/mips/pci/pci.c
@@ -50,7 +50,7 @@ static int pci_initialized;
  * which might have be mirrored at 0x0100-0x03ff..
  */
 resource_size_t
-pcibios_align_resource(void *data, struct resource *res,
+pcibios_align_resource(void *data, const struct resource *res,
 		       resource_size_t size, resource_size_t align)
 {
 	struct pci_dev *dev = data;
diff --git a/arch/mips/pmc-sierra/yosemite/ht.c b/arch/mips/pmc-sierra/yosemite/ht.c
index 5e41008..fd22597 100644
--- a/arch/mips/pmc-sierra/yosemite/ht.c
+++ b/arch/mips/pmc-sierra/yosemite/ht.c
@@ -345,7 +345,7 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
         return pcibios_enable_resources(dev);
 }
 
-resource_size_t pcibios_align_resource(void *data, struct resource *res,
+resource_size_t pcibios_align_resource(void *data, const struct resource *res,
 				resource_size_t size, resource_size_t align)
 {
         struct pci_dev *dev = data;
diff --git a/arch/mn10300/unit-asb2305/pci-asb2305.c b/arch/mn10300/unit-asb2305/pci-asb2305.c
index 60f24a7..8182fff 100644
--- a/arch/mn10300/unit-asb2305/pci-asb2305.c
+++ b/arch/mn10300/unit-asb2305/pci-asb2305.c
@@ -31,7 +31,7 @@
  * but we want to try to avoid allocating at 0x2900-0x2bff
  * which might have be mirrored at 0x0100-0x03ff..
  */
-resource_size_t pcibios_align_resource(void *data, struct resource *res,
+resource_size_t pcibios_align_resource(void *data, const struct resource *res,
 				resource_size_t size, resource_size_t align)
 {
 	resource_size_t start = res->start;
diff --git a/arch/parisc/kernel/pci.c b/arch/parisc/kernel/pci.c
index 4463a31..5179e5e 100644
--- a/arch/parisc/kernel/pci.c
+++ b/arch/parisc/kernel/pci.c
@@ -254,7 +254,7 @@ EXPORT_SYMBOL(pcibios_bus_to_resource);
  * Since we are just checking candidates, don't use any fields other
  * than res->start.
  */
-resource_size_t pcibios_align_resource(void *data, struct resource *res,
+resource_size_t pcibios_align_resource(void *data, const struct resource *res,
 				resource_size_t size, resource_size_t alignment)
 {
 	resource_size_t mask, align, start = res->start;
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 3e343b8..55045e7 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1168,7 +1168,7 @@ static int skip_isa_ioresource_align(struct pci_dev *dev)
  * but we want to try to avoid allocating at 0x2900-0x2bff
  * which might have be mirrored at 0x0100-0x03ff..
  */
-resource_size_t pcibios_align_resource(void *data, struct resource *res,
+resource_size_t pcibios_align_resource(void *data, const struct resource *res,
 				resource_size_t size, resource_size_t align)
 {
 	struct pci_dev *dev = data;
diff --git a/arch/sh/drivers/pci/pci.c b/arch/sh/drivers/pci/pci.c
index b36ca82..96213fd 100644
--- a/arch/sh/drivers/pci/pci.c
+++ b/arch/sh/drivers/pci/pci.c
@@ -148,7 +148,7 @@ void __devinit pcibios_fixup_bus(struct pci_bus *bus)
  * addresses to be allocated in the 0x000-0x0ff region
  * modulo 0x400.
  */
-resource_size_t pcibios_align_resource(void *data, struct resource *res,
+resource_size_t pcibios_align_resource(void *data, const struct resource *res,
 				resource_size_t size, resource_size_t align)
 {
 	struct pci_dev *dev = data;
diff --git a/arch/sparc/kernel/pci.c b/arch/sparc/kernel/pci.c
index 6381ceb..e71e9ce 100644
--- a/arch/sparc/kernel/pci.c
+++ b/arch/sparc/kernel/pci.c
@@ -715,7 +715,7 @@ void pcibios_update_irq(struct pci_dev *pdev, int irq)
 {
 }
 
-resource_size_t pcibios_align_resource(void *data, struct resource *res,
+resource_size_t pcibios_align_resource(void *data, const struct resource *res,
 				resource_size_t size, resource_size_t align)
 {
 	return res->start;
diff --git a/arch/sparc/kernel/pcic.c b/arch/sparc/kernel/pcic.c
index 96fe494..49fcf5f 100644
--- a/arch/sparc/kernel/pcic.c
+++ b/arch/sparc/kernel/pcic.c
@@ -839,7 +839,7 @@ char * __devinit pcibios_setup(char *str)
 	return str;
 }
 
-resource_size_t pcibios_align_resource(void *data, struct resource *res,
+resource_size_t pcibios_align_resource(void *data, const struct resource *res,
 				resource_size_t size, resource_size_t align)
 {
 	return res->start;
diff --git a/arch/x86/pci/i386.c b/arch/x86/pci/i386.c
index 924e40c..5a8fbf8 100644
--- a/arch/x86/pci/i386.c
+++ b/arch/x86/pci/i386.c
@@ -61,7 +61,7 @@ skip_isa_ioresource_align(struct pci_dev *dev) {
  * which might have be mirrored at 0x0100-0x03ff..
  */
 resource_size_t
-pcibios_align_resource(void *data, struct resource *res,
+pcibios_align_resource(void *data, const struct resource *res,
 			resource_size_t size, resource_size_t align)
 {
 	struct pci_dev *dev = data;
diff --git a/arch/xtensa/kernel/pci.c b/arch/xtensa/kernel/pci.c
index d7efab0..cd10269 100644
--- a/arch/xtensa/kernel/pci.c
+++ b/arch/xtensa/kernel/pci.c
@@ -70,8 +70,8 @@ static int pci_bus_count;
  * which might have be mirrored at 0x0100-0x03ff..
  */
 resource_size_t
-pcibios_align_resource(void *data, struct resource *res, resource_size_t size,
-    		       resource_size_t align)
+pcibios_align_resource(void *data, const struct resource *res,
+		       resource_size_t size, resource_size_t align)
 {
 	struct pci_dev *dev = data;
 	resource_size_t start = res->start;
diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
index d29d69a..a26135b 100644
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -37,7 +37,7 @@ pci_bus_alloc_resource(struct pci_bus *bus, struct resource *res,
 		resource_size_t size, resource_size_t align,
 		resource_size_t min, unsigned int type_mask,
 		resource_size_t (*alignf)(void *,
-					  struct resource *,
+					  const struct resource *,
 					  resource_size_t,
 					  resource_size_t),
 		void *alignf_data)
diff --git a/drivers/pcmcia/rsrc_mgr.c b/drivers/pcmcia/rsrc_mgr.c
index f92a2da..f8401a0 100644
--- a/drivers/pcmcia/rsrc_mgr.c
+++ b/drivers/pcmcia/rsrc_mgr.c
@@ -114,7 +114,8 @@ struct pcmcia_align_data {
 	unsigned long	offset;
 };
 
-static resource_size_t pcmcia_align(void *align_data, struct resource *res,
+static resource_size_t pcmcia_align(void *align_data,
+				const struct resource *res,
 				resource_size_t size, resource_size_t align)
 {
 	struct pcmcia_align_data *data = align_data;
diff --git a/drivers/pcmcia/rsrc_nonstatic.c b/drivers/pcmcia/rsrc_nonstatic.c
index b659028..45d75dc 100644
--- a/drivers/pcmcia/rsrc_nonstatic.c
+++ b/drivers/pcmcia/rsrc_nonstatic.c
@@ -534,7 +534,7 @@ struct pcmcia_align_data {
 };
 
 static resource_size_t
-pcmcia_common_align(void *align_data, struct resource *res,
+pcmcia_common_align(void *align_data, const struct resource *res,
 			resource_size_t size, resource_size_t align)
 {
 	struct pcmcia_align_data *data = align_data;
@@ -549,8 +549,8 @@ pcmcia_common_align(void *align_data, struct resource *res,
 }
 
 static resource_size_t
-pcmcia_align(void *align_data, struct resource *res, resource_size_t size,
-		resource_size_t align)
+pcmcia_align(void *align_data, const struct resource *res,
+	resource_size_t size, resource_size_t align)
 {
 	struct pcmcia_align_data *data = align_data;
 	struct resource_map *m;
diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index f4195de..4a81189 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -121,7 +121,7 @@ extern int allocate_resource(struct resource *root, struct resource *new,
 			     resource_size_t size, resource_size_t min,
 			     resource_size_t max, resource_size_t align,
 			     resource_size_t (*alignf)(void *,
-						       struct resource *,
+						       const struct resource *,
 						       resource_size_t,
 						       resource_size_t),
 			     void *alignf_data);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 30a6e9b..f5cd77e 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -562,7 +562,7 @@ int __must_check pcibios_enable_device(struct pci_dev *, int mask);
 char *pcibios_setup(char *str);
 
 /* Used only when drivers/pci/setup.c is used */
-resource_size_t pcibios_align_resource(void *, struct resource *,
+resource_size_t pcibios_align_resource(void *, const struct resource *,
 				resource_size_t,
 				resource_size_t);
 void pcibios_update_irq(struct pci_dev *, int irq);
@@ -790,7 +790,8 @@ int __must_check pci_bus_alloc_resource(struct pci_bus *bus,
 			struct resource *res, resource_size_t size,
 			resource_size_t align, resource_size_t min,
 			unsigned int type_mask,
-			resource_size_t (*alignf)(void *, struct resource *,
+			resource_size_t (*alignf)(void *,
+						  const struct resource *,
 						  resource_size_t,
 						  resource_size_t),
 			void *alignf_data);
diff --git a/kernel/resource.c b/kernel/resource.c
index e697f20..7fd123a 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -304,7 +304,7 @@ static int find_resource(struct resource *root, struct resource *new,
 			 resource_size_t size, resource_size_t min,
 			 resource_size_t max, resource_size_t align,
 			 resource_size_t (*alignf)(void *,
-						   struct resource *,
+						   const struct resource *,
 						   resource_size_t,
 						   resource_size_t),
 			 void *alignf_data)
@@ -361,7 +361,7 @@ int allocate_resource(struct resource *root, struct resource *new,
 		      resource_size_t size, resource_size_t min,
 		      resource_size_t max, resource_size_t align,
 		      resource_size_t (*alignf)(void *,
-						struct resource *,
+						const struct resource *,
 						resource_size_t,
 						resource_size_t),
 		      void *alignf_data)
-- 
1.6.3.3


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] resource: align functions now return start of  resource
  2010-01-01 16:40         ` [PATCH 1/2] resource: align functions now return start of resource Dominik Brodowski
@ 2010-01-01 22:12           ` Yinghai Lu
  2010-01-03 16:30             ` Dominik Brodowski
  2010-01-15 18:23           ` Jesse Barnes
  1 sibling, 1 reply; 12+ messages in thread
From: Yinghai Lu @ 2010-01-01 22:12 UTC (permalink / raw)
  To: Dominik Brodowski
  Cc: linux-kernel, linux-pci, linux-arch, torvalds, Bjorn Helgaas,
	Jesse Barnes

On Fri, Jan 1, 2010 at 8:40 AM, Dominik Brodowski
<linux@dominikbrodowski.net> wrote:
> As suggested by Linus, align functions should return the start
> of a resource, not void. An update of "res->start" is no longer
> necessary.
>
> Cc: Bjorn Helgaas <bjorn.helgaas@hp.com>
> Cc: Yinghai Lu <yhlu.kernel@gmail.com>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Signed-off-by: Dominik Brodowski <linux@dominikbrodowski.net>
> ---
>  arch/alpha/kernel/pci.c                 |    4 ++--
>  arch/arm/kernel/bios32.c                |    8 +++++---
>  arch/cris/arch-v32/drivers/pci/bios.c   |   14 ++++++--------
>  arch/frv/mb93090-mb00/pci-frv.c         |   14 ++++++--------
>  arch/ia64/pci/pci.c                     |    3 ++-
>  arch/mips/pci/pci.c                     |    4 ++--
>  arch/mips/pmc-sierra/yosemite/ht.c      |   10 +++++-----
>  arch/mn10300/unit-asb2305/pci-asb2305.c |   16 +++++++---------
>  arch/parisc/kernel/pci.c                |   10 +++++-----
>  arch/powerpc/kernel/pci-common.c        |   13 ++++++-------
>  arch/sh/drivers/pci/pci.c               |    6 +++---
>  arch/sparc/kernel/pci.c                 |    5 +++--
>  arch/sparc/kernel/pcic.c                |    5 +++--
>  arch/x86/pci/i386.c                     |   12 +++++-------
>  arch/xtensa/kernel/pci.c                |   11 +++++------
>  drivers/pci/bus.c                       |    6 ++++--
>  drivers/pcmcia/rsrc_mgr.c               |   12 ++++++------
>  drivers/pcmcia/rsrc_nonstatic.c         |   16 +++++++++-------
>  include/linux/ioport.h                  |    6 ++++--
>  include/linux/pci.h                     |    8 +++++---
>  kernel/resource.c                       |   14 +++++++++-----
>  21 files changed, 102 insertions(+), 95 deletions(-)
>
> diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
> index a91ba28..5cf111e 100644
> --- a/arch/alpha/kernel/pci.c
> +++ b/arch/alpha/kernel/pci.c
> @@ -126,7 +126,7 @@ DECLARE_PCI_FIXUP_FINAL(PCI_ANY_ID, PCI_ANY_ID, pcibios_fixup_final);
>  #define MB                     (1024*KB)
>  #define GB                     (1024*MB)
>
> -void
> +resource_size_t
>  pcibios_align_resource(void *data, struct resource *res,
>                       resource_size_t size, resource_size_t align)
>  {
> @@ -184,7 +184,7 @@ pcibios_align_resource(void *data, struct resource *res,
>                }
>        }
>
> -       res->start = start;
> +       return start;
>  }
>  #undef KB
>  #undef MB
> diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
> index 8096819..a7c85f8 100644
> --- a/arch/arm/kernel/bios32.c
> +++ b/arch/arm/kernel/bios32.c
> @@ -616,15 +616,17 @@ char * __init pcibios_setup(char *str)
>  * but we want to try to avoid allocating at 0x2900-0x2bff
>  * which might be mirrored at 0x0100-0x03ff..
>  */
> -void pcibios_align_resource(void *data, struct resource *res,
> -                           resource_size_t size, resource_size_t align)
> +resource_size_t pcibios_align_resource(void *data, struct resource *res,
> +                               resource_size_t size, resource_size_t align)
>  {
>        resource_size_t start = res->start;
>
>        if (res->flags & IORESOURCE_IO && start & 0x300)
>                start = (start + 0x3ff) & ~0x3ff;
>
> -       res->start = (start + align - 1) & ~(align - 1);
> +       start = (start + align - 1) & ~(align - 1);
> +
> +       return start;
>  }
>
>  /**
> diff --git a/arch/cris/arch-v32/drivers/pci/bios.c b/arch/cris/arch-v32/drivers/pci/bios.c
> index 77ee319..5811e2f 100644
> --- a/arch/cris/arch-v32/drivers/pci/bios.c
> +++ b/arch/cris/arch-v32/drivers/pci/bios.c
> @@ -41,18 +41,16 @@ int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
>        return 0;
>  }
>
> -void
> +resource_size_t
>  pcibios_align_resource(void *data, struct resource *res,
>                       resource_size_t size, resource_size_t align)
>  {
> -       if (res->flags & IORESOURCE_IO) {
> -               resource_size_t start = res->start;
> +       resource_size_t start = res->start;
>
> -               if (start & 0x300) {
> -                       start = (start + 0x3ff) & ~0x3ff;
> -                       res->start = start;
> -               }
> -       }
> +       if ((res->flags & IORESOURCE_IO) && (start & 0x300))
> +               start = (start + 0x3ff) & ~0x3ff;
> +
> +       return start

missed ; here?

YH

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] resource: align functions now return start of resource
  2010-01-01 22:12           ` Yinghai Lu
@ 2010-01-03 16:30             ` Dominik Brodowski
  0 siblings, 0 replies; 12+ messages in thread
From: Dominik Brodowski @ 2010-01-03 16:30 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: linux-kernel, linux-pci, linux-arch, torvalds, Bjorn Helgaas,
	Jesse Barnes

Hey,

On Fri, Jan 01, 2010 at 02:12:09PM -0800, Yinghai Lu wrote:
> > -                       res->start = start;
> > -               }
> > -       }
> > +       if ((res->flags & IORESOURCE_IO) && (start & 0x300))
> > +               start = (start + 0x3ff) & ~0x3ff;
> > +
> > +       return start
> 
> missed ; here?

Indeed -- thanks for the hint.

Best,
	Dominik

^ permalink raw reply	[flat|nested] 12+ messages in thread

* why can not receive kernel mail anymore?
  2009-12-21 16:03     ` Linus Torvalds
  2010-01-01 16:38       ` [RFC] improve alignf() calling convention " Dominik Brodowski
@ 2010-01-14  7:28       ` goldenwang1979
  1 sibling, 0 replies; 12+ messages in thread
From: goldenwang1979 @ 2010-01-14  7:28 UTC (permalink / raw)
  To: linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="gb2312", Size: 578 bytes --]

hi,all£»
	
	addr:	linux-kernel@vger.kernel.org



Best regards. 		
goldenwang1979
2010-01-14

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

ÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿá¶Úÿÿ÷'þf¢—üš†Šÿr‰ÿý:.žË›±Êâmçë¢kaŠÉb²ßìzwm…ébïîžË›±Êâmébžìÿ‘êçz_âžØ^n‡r¡ö¦zË\x1aëh™¨è­Ú&£ûàz¿äz¹Þ—ú+€Ê+zf£¢·hšˆ§~†­†Ûiÿÿïêÿ‘êçz_è®\x0fæj:+v‰¨þ)ߣømšSåy«\x1e­æ¶\x17…\x01\x06­†ÛiÿÿðÃ\x0fí»\x1fè®\x0få’i\x7f

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] resource: align functions now return start of resource
  2010-01-01 16:40         ` [PATCH 1/2] resource: align functions now return start of resource Dominik Brodowski
  2010-01-01 22:12           ` Yinghai Lu
@ 2010-01-15 18:23           ` Jesse Barnes
  1 sibling, 0 replies; 12+ messages in thread
From: Jesse Barnes @ 2010-01-15 18:23 UTC (permalink / raw)
  To: Dominik Brodowski
  Cc: linux-kernel, linux-pci, linux-arch, torvalds, Dominik Brodowski,
	Bjorn Helgaas, Yinghai Lu

On Fri,  1 Jan 2010 17:40:49 +0100
Dominik Brodowski <linux@dominikbrodowski.net> wrote:

> As suggested by Linus, align functions should return the start
> of a resource, not void. An update of "res->start" is no longer
> necessary.

Applied these two to my linux-next branch, thanks Dominik.

-- 
Jesse Barnes, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2010-01-15 18:23 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-12-20  9:50 [PATCH] resources: fix call to alignf() in allocate_resource() Dominik Brodowski
2009-12-20 18:33 ` Linus Torvalds
2009-12-20 22:12   ` Bjorn Helgaas
2009-12-21  8:40   ` Dominik Brodowski
2009-12-21 16:03     ` Linus Torvalds
2010-01-01 16:38       ` [RFC] improve alignf() calling convention " Dominik Brodowski
2010-01-01 16:40         ` [PATCH 1/2] resource: align functions now return start of resource Dominik Brodowski
2010-01-01 22:12           ` Yinghai Lu
2010-01-03 16:30             ` Dominik Brodowski
2010-01-15 18:23           ` Jesse Barnes
2010-01-01 16:40         ` [PATCH 2/2] resource: mark struct resource as const Dominik Brodowski
2010-01-14  7:28       ` why can not receive kernel mail anymore? goldenwang1979

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).