LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* 2.6.24-rc5 sysfs pci bridge duplicate symlink
@ 2007-12-28 13:11 Bahadir Balban
  2007-12-28 23:03 ` Andrew Morton
  0 siblings, 1 reply; 5+ messages in thread
From: Bahadir Balban @ 2007-12-28 13:11 UTC (permalink / raw)
  To: linux-kernel

Hi,

On ARM with PCI, I get this error since -rc2 (didn't try rc1):

sysfs: duplicate filename 'bridge' can not be created
WARNING: at fs/sysfs/dir.c:424 sysfs_add_one()
[<c00287fc>] (dump_stack+0x0/0x14) from [<c00cbc64>] (sysfs_add_one+0x50/0xf4)
[<c00cbc14>] (sysfs_add_one+0x0/0xf4) from [<c00ccd30>]
(sysfs_create_link+0xec/0x184)
 r6:cfca8b48 r5:cfca96c8 r4:cfc2fec0
[<c00ccc44>] (sysfs_create_link+0x0/0x184) from [<c0160b44>]
(pci_bus_add_devices+0xf0/0x144)
 r7:cfc5a2d4 r6:cfc5a2d4 r5:cfc5a2c0 r4:cfcad800
[<c0160a54>] (pci_bus_add_devices+0x0/0x144) from [<c0160b30>]
(pci_bus_add_devices+0xdc/0x144)
 r7:cfc5a3d4 r6:cfc5a3d4 r5:cfc5a3c0 r4:cfcadc00
[<c0160a54>] (pci_bus_add_devices+0x0/0x144) from [<c0160b30>]
(pci_bus_add_devices+0xdc/0x144)
 r7:cfc5a4d4 r6:cfc5a4d4 r5:cfc5a4c0 r4:cfc5d400
[<c0160a54>] (pci_bus_add_devices+0x0/0x144) from [<c000a934>]
(pci_common_init+0x148/0x184)
 r7:c035fd58 r6:cfc3e760 r5:c001e030 r4:cfc5a4c0


Any idea why?

Thanks,
Bahadir

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

* Re: 2.6.24-rc5 sysfs pci bridge duplicate symlink
  2007-12-28 13:11 2.6.24-rc5 sysfs pci bridge duplicate symlink Bahadir Balban
@ 2007-12-28 23:03 ` Andrew Morton
  2007-12-28 23:50   ` Russell King - ARM Linux
  0 siblings, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2007-12-28 23:03 UTC (permalink / raw)
  To: Bahadir Balban; +Cc: linux-kernel, linux-arm-kernel

On Fri, 28 Dec 2007 13:11:37 +0000 "Bahadir Balban" <bahadir.balban@gmail.com> wrote:

> Hi,
> 
> On ARM with PCI, I get this error since -rc2 (didn't try rc1):
> 
> sysfs: duplicate filename 'bridge' can not be created
> WARNING: at fs/sysfs/dir.c:424 sysfs_add_one()
> [<c00287fc>] (dump_stack+0x0/0x14) from [<c00cbc64>] (sysfs_add_one+0x50/0xf4)
> [<c00cbc14>] (sysfs_add_one+0x0/0xf4) from [<c00ccd30>]
> (sysfs_create_link+0xec/0x184)
>  r6:cfca8b48 r5:cfca96c8 r4:cfc2fec0
> [<c00ccc44>] (sysfs_create_link+0x0/0x184) from [<c0160b44>]
> (pci_bus_add_devices+0xf0/0x144)
>  r7:cfc5a2d4 r6:cfc5a2d4 r5:cfc5a2c0 r4:cfcad800
> [<c0160a54>] (pci_bus_add_devices+0x0/0x144) from [<c0160b30>]
> (pci_bus_add_devices+0xdc/0x144)
>  r7:cfc5a3d4 r6:cfc5a3d4 r5:cfc5a3c0 r4:cfcadc00
> [<c0160a54>] (pci_bus_add_devices+0x0/0x144) from [<c0160b30>]
> (pci_bus_add_devices+0xdc/0x144)
>  r7:cfc5a4d4 r6:cfc5a4d4 r5:cfc5a4c0 r4:cfc5d400
> [<c0160a54>] (pci_bus_add_devices+0x0/0x144) from [<c000a934>]
> (pci_common_init+0x148/0x184)
>  r7:c035fd58 r6:cfc3e760 r5:c001e030 r4:cfc5a4c0
> 
> 
> Any idea why?
> 

(suitable cc added)

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

* Re: 2.6.24-rc5 sysfs pci bridge duplicate symlink
  2007-12-28 23:03 ` Andrew Morton
@ 2007-12-28 23:50   ` Russell King - ARM Linux
  2008-01-10  3:55     ` Greg KH
  0 siblings, 1 reply; 5+ messages in thread
From: Russell King - ARM Linux @ 2007-12-28 23:50 UTC (permalink / raw)
  To: Andrew Morton, gregkh; +Cc: Bahadir Balban, linux-kernel, linux-arm-kernel

On Fri, Dec 28, 2007 at 03:03:22PM -0800, Andrew Morton wrote:
> On Fri, 28 Dec 2007 13:11:37 +0000 "Bahadir Balban" <bahadir.balban@gmail.com> wrote:
> 
> > Hi,
> > 
> > On ARM with PCI, I get this error since -rc2 (didn't try rc1):
> > 
> > sysfs: duplicate filename 'bridge' can not be created
> > WARNING: at fs/sysfs/dir.c:424 sysfs_add_one()
> > [<c00287fc>] (dump_stack+0x0/0x14) from [<c00cbc64>] (sysfs_add_one+0x50/0xf4)
> > [<c00cbc14>] (sysfs_add_one+0x0/0xf4) from [<c00ccd30>]
> > (sysfs_create_link+0xec/0x184)
> >  r6:cfca8b48 r5:cfca96c8 r4:cfc2fec0
> > [<c00ccc44>] (sysfs_create_link+0x0/0x184) from [<c0160b44>]
> > (pci_bus_add_devices+0xf0/0x144)
> >  r7:cfc5a2d4 r6:cfc5a2d4 r5:cfc5a2c0 r4:cfcad800
> > [<c0160a54>] (pci_bus_add_devices+0x0/0x144) from [<c0160b30>]
> > (pci_bus_add_devices+0xdc/0x144)
> >  r7:cfc5a3d4 r6:cfc5a3d4 r5:cfc5a3c0 r4:cfcadc00
> > [<c0160a54>] (pci_bus_add_devices+0x0/0x144) from [<c0160b30>]
> > (pci_bus_add_devices+0xdc/0x144)
> >  r7:cfc5a4d4 r6:cfc5a4d4 r5:cfc5a4c0 r4:cfc5d400
> > [<c0160a54>] (pci_bus_add_devices+0x0/0x144) from [<c000a934>]
> > (pci_common_init+0x148/0x184)
> >  r7:c035fd58 r6:cfc3e760 r5:c001e030 r4:cfc5a4c0
> > 
> > 
> > Any idea why?
> > 
> 
> (suitable cc added)

Looks to me as if the code added to pci_bus_add_devices() is off it's
rocker.

In brief, pci_bus_add_devices() is supposed to be callable given *any*
state of the bus, and it will add any new devices found in the bus tree
from the bus that it's called for downwards.  (That's how I designed
that bit of PCI code - so it can cope with hotpluggable PCI bridges.)

It still does this.  However, some bright spark decided to add sysfs
links to it - and break it.  Now, whenever we descend into a subordinate
bus (and return to a higher level) we will try to create a sysfs symlink
even if the bus is one that we already registered.

So... that sysfs_create_link() call in there should not be unconditional.
It should _only_ ever do that call if list_empty(&dev->subordinate->node)
was true.  IOW, we only create symlinks for subordinate bus that we don't
already know about.

It looks like it was introduced by Greg's "PCI: fix __must_check warnings"
but why a patch advertised _solely_ as a fix is adding additional sysfs
attributes I've no idea.

Something like the following should fix it.  Bahadir - please test.

diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
index 9e5ea07..132a91f 100644
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -131,18 +131,21 @@ void pci_bus_add_devices(struct pci_bus *bus)
 		 * it and then scan for unattached PCI devices.
 		 */
 		if (dev->subordinate) {
-		       if (list_empty(&dev->subordinate->node)) {
+			int new_bus = list_empty(&dev->subordinate->node);
+			if (new_bus) {
 			       down_write(&pci_bus_sem);
 			       list_add_tail(&dev->subordinate->node,
 					       &dev->bus->children);
 			       up_write(&pci_bus_sem);
 			}
 			pci_bus_add_devices(dev->subordinate);
-			retval = sysfs_create_link(&dev->subordinate->class_dev.kobj,
-						   &dev->dev.kobj, "bridge");
-			if (retval)
-				dev_err(&dev->dev, "Error creating sysfs "
-					"bridge symlink, continuing...\n");
+			if (new_bus) {
+				retval = sysfs_create_link(&dev->subordinate->class_dev.kobj,
+							   &dev->dev.kobj, "bridge");
+				if (retval)
+					dev_err(&dev->dev, "Error creating sysfs "
+						"bridge symlink, continuing...\n");
+			}
 		}
 	}
 }


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

* Re: 2.6.24-rc5 sysfs pci bridge duplicate symlink
  2007-12-28 23:50   ` Russell King - ARM Linux
@ 2008-01-10  3:55     ` Greg KH
  2008-01-16 19:02       ` Bahadir Balban
  0 siblings, 1 reply; 5+ messages in thread
From: Greg KH @ 2008-01-10  3:55 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Andrew Morton, Bahadir Balban, linux-kernel, linux-arm-kernel

On Fri, Dec 28, 2007 at 11:50:49PM +0000, Russell King - ARM Linux wrote:
> On Fri, Dec 28, 2007 at 03:03:22PM -0800, Andrew Morton wrote:
> > On Fri, 28 Dec 2007 13:11:37 +0000 "Bahadir Balban" <bahadir.balban@gmail.com> wrote:
> > 
> > > Hi,
> > > 
> > > On ARM with PCI, I get this error since -rc2 (didn't try rc1):
> > > 
> > > sysfs: duplicate filename 'bridge' can not be created
> > > WARNING: at fs/sysfs/dir.c:424 sysfs_add_one()
> > > [<c00287fc>] (dump_stack+0x0/0x14) from [<c00cbc64>] (sysfs_add_one+0x50/0xf4)
> > > [<c00cbc14>] (sysfs_add_one+0x0/0xf4) from [<c00ccd30>]
> > > (sysfs_create_link+0xec/0x184)
> > >  r6:cfca8b48 r5:cfca96c8 r4:cfc2fec0
> > > [<c00ccc44>] (sysfs_create_link+0x0/0x184) from [<c0160b44>]
> > > (pci_bus_add_devices+0xf0/0x144)
> > >  r7:cfc5a2d4 r6:cfc5a2d4 r5:cfc5a2c0 r4:cfcad800
> > > [<c0160a54>] (pci_bus_add_devices+0x0/0x144) from [<c0160b30>]
> > > (pci_bus_add_devices+0xdc/0x144)
> > >  r7:cfc5a3d4 r6:cfc5a3d4 r5:cfc5a3c0 r4:cfcadc00
> > > [<c0160a54>] (pci_bus_add_devices+0x0/0x144) from [<c0160b30>]
> > > (pci_bus_add_devices+0xdc/0x144)
> > >  r7:cfc5a4d4 r6:cfc5a4d4 r5:cfc5a4c0 r4:cfc5d400
> > > [<c0160a54>] (pci_bus_add_devices+0x0/0x144) from [<c000a934>]
> > > (pci_common_init+0x148/0x184)
> > >  r7:c035fd58 r6:cfc3e760 r5:c001e030 r4:cfc5a4c0
> > > 
> > > 
> > > Any idea why?
> > > 
> > 
> > (suitable cc added)
> 
> Looks to me as if the code added to pci_bus_add_devices() is off it's
> rocker.
> 
> In brief, pci_bus_add_devices() is supposed to be callable given *any*
> state of the bus, and it will add any new devices found in the bus tree
> from the bus that it's called for downwards.  (That's how I designed
> that bit of PCI code - so it can cope with hotpluggable PCI bridges.)
> 
> It still does this.  However, some bright spark decided to add sysfs
> links to it - and break it.  Now, whenever we descend into a subordinate
> bus (and return to a higher level) we will try to create a sysfs symlink
> even if the bus is one that we already registered.
> 
> So... that sysfs_create_link() call in there should not be unconditional.
> It should _only_ ever do that call if list_empty(&dev->subordinate->node)
> was true.  IOW, we only create symlinks for subordinate bus that we don't
> already know about.
> 
> It looks like it was introduced by Greg's "PCI: fix __must_check warnings"
> but why a patch advertised _solely_ as a fix is adding additional sysfs
> attributes I've no idea.

I'm sorry, but I don't see where that patch added new sysfs attributes
to anything.  It just reports any errors that might have occured and
hopefully properly handle them.  Did I mess up somewhere with this?

> Something like the following should fix it.  Bahadir - please test.

Any test results?

thanks,

greg k-h

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

* Re: 2.6.24-rc5 sysfs pci bridge duplicate symlink
  2008-01-10  3:55     ` Greg KH
@ 2008-01-16 19:02       ` Bahadir Balban
  0 siblings, 0 replies; 5+ messages in thread
From: Bahadir Balban @ 2008-01-16 19:02 UTC (permalink / raw)
  To: Greg KH
  Cc: Russell King - ARM Linux, Andrew Morton, linux-kernel, linux-arm-kernel

On 1/10/08, Greg KH <gregkh@suse.de> wrote:
> On Fri, Dec 28, 2007 at 11:50:49PM +0000, Russell King - ARM Linux wrote:
> > On Fri, Dec 28, 2007 at 03:03:22PM -0800, Andrew Morton wrote:
> > > On Fri, 28 Dec 2007 13:11:37 +0000 "Bahadir Balban" <bahadir.balban@gmail.com> wrote:
> > >
> > > > Hi,
> > > >
> > > > On ARM with PCI, I get this error since -rc2 (didn't try rc1):
> > > >
> > > > sysfs: duplicate filename 'bridge' can not be created
> > > > WARNING: at fs/sysfs/dir.c:424 sysfs_add_one()
> > > > [<c00287fc>] (dump_stack+0x0/0x14) from [<c00cbc64>] (sysfs_add_one+0x50/0xf4)
> > > > [<c00cbc14>] (sysfs_add_one+0x0/0xf4) from [<c00ccd30>]
> > > > (sysfs_create_link+0xec/0x184)
> > > >  r6:cfca8b48 r5:cfca96c8 r4:cfc2fec0
> > > > [<c00ccc44>] (sysfs_create_link+0x0/0x184) from [<c0160b44>]
> > > > (pci_bus_add_devices+0xf0/0x144)
> > > >  r7:cfc5a2d4 r6:cfc5a2d4 r5:cfc5a2c0 r4:cfcad800
> > > > [<c0160a54>] (pci_bus_add_devices+0x0/0x144) from [<c0160b30>]
> > > > (pci_bus_add_devices+0xdc/0x144)
> > > >  r7:cfc5a3d4 r6:cfc5a3d4 r5:cfc5a3c0 r4:cfcadc00
> > > > [<c0160a54>] (pci_bus_add_devices+0x0/0x144) from [<c0160b30>]
> > > > (pci_bus_add_devices+0xdc/0x144)
> > > >  r7:cfc5a4d4 r6:cfc5a4d4 r5:cfc5a4c0 r4:cfc5d400
> > > > [<c0160a54>] (pci_bus_add_devices+0x0/0x144) from [<c000a934>]
> > > > (pci_common_init+0x148/0x184)
> > > >  r7:c035fd58 r6:cfc3e760 r5:c001e030 r4:cfc5a4c0
> > > >
> > > >
> > > > Any idea why?
> > > >
> > >
> > > (suitable cc added)
> >
> > Looks to me as if the code added to pci_bus_add_devices() is off it's
> > rocker.
> >
> > In brief, pci_bus_add_devices() is supposed to be callable given *any*
> > state of the bus, and it will add any new devices found in the bus tree
> > from the bus that it's called for downwards.  (That's how I designed
> > that bit of PCI code - so it can cope with hotpluggable PCI bridges.)
> >
> > It still does this.  However, some bright spark decided to add sysfs
> > links to it - and break it.  Now, whenever we descend into a subordinate
> > bus (and return to a higher level) we will try to create a sysfs symlink
> > even if the bus is one that we already registered.
> >
> > So... that sysfs_create_link() call in there should not be unconditional.
> > It should _only_ ever do that call if list_empty(&dev->subordinate->node)
> > was true.  IOW, we only create symlinks for subordinate bus that we don't
> > already know about.
> >
> > It looks like it was introduced by Greg's "PCI: fix __must_check warnings"
> > but why a patch advertised _solely_ as a fix is adding additional sysfs
> > attributes I've no idea.
>
> I'm sorry, but I don't see where that patch added new sysfs attributes
> to anything.  It just reports any errors that might have occured and
> hopefully properly handle them.  Did I mess up somewhere with this?
>
> > Something like the following should fix it.  Bahadir - please test.
>
> Any test results?
>
> thanks,
>
> greg k-h
>

Hi,

I have tested and it works OK with this patch on -rc8. Without the
patch it breaks as in my original email.

While at it, could you please apply the below patch as well? It
enables PCI transactions through bridges on ARM.

Thanks,
Bahadir

From: Bahadir Balban <bahadir.balban@arm.com>


---

 arch/arm/kernel/bios32.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
index e574754..7d3ebd4 100644
--- a/arch/arm/kernel/bios32.c
+++ b/arch/arm/kernel/bios32.c
@@ -607,6 +607,11 @@ void __init pci_common_init(struct hw_pci *hw)
 			 * Assign resources.
 			 */
 			pci_bus_assign_resources(bus);
+
+			/*
+			 * Enable bridges
+			 */
+			pci_enable_bridges(bus);
 		}

 		/*

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

end of thread, other threads:[~2008-01-16 19:02 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-12-28 13:11 2.6.24-rc5 sysfs pci bridge duplicate symlink Bahadir Balban
2007-12-28 23:03 ` Andrew Morton
2007-12-28 23:50   ` Russell King - ARM Linux
2008-01-10  3:55     ` Greg KH
2008-01-16 19:02       ` Bahadir Balban

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).