* [PATCH] PCI: Don't use Printk in raw_spinlocks @ 2020-08-01 1:25 Xingxing Su 2020-09-10 2:07 ` Bjorn Helgaas 0 siblings, 1 reply; 10+ messages in thread From: Xingxing Su @ 2020-08-01 1:25 UTC (permalink / raw) To: Bjorn Helgaas; +Cc: linux-pci, linux-kernel Do not use printk in raw_spinlocks, it will cause BUG: Invalid wait context. The trace reported by lockdep follows. [ 2.986113] ============================= [ 2.986115] [ BUG: Invalid wait context ] [ 2.986116] 5.8.0-rc1+ #11 Not tainted [ 2.986118] ----------------------------- [ 2.986120] swapper/0/1 is trying to lock: [ 2.986122] ffffffff80f5ddd8 (console_owner){....}-{3:3}, at: console_unlock+0x284/0x820 [ 2.986130] other info that might help us debug this: [ 2.986132] context-{5:5} [ 2.986134] 3 locks held by swapper/0/1: [ 2.986135] #0: 98000007fa03c990 (&dev->mutex){....}-{0:0}, at: device_driver_attach+0x28/0x90 [ 2.986144] #1: ffffffff80fb83a8 (pci_lock){....}-{2:2}, at: pci_bus_write_config_word+0x60/0xb8 [ 2.986152] #2: ffffffff80f5ded0 (console_lock){+.+.}-{0:0}, at: vprintk_emit+0x1b0/0x3b8 [ 2.986161] stack backtrace: [ 2.986163] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.8.0-rc1+ #11 [ 2.986164] Stack : 0000000000001d67 98000000030be9b0 0000000000000001 7b2aba74f6c4785b [ 2.986172] 7b2aba74f6c4785b 0000000000000000 98000007f89cb438 ffffffff80e7dc80 [ 2.986181] 0000000000000001 000000000000000a 0000000000000001 0000000000000001 [ 2.986189] ffffffff80f4e156 fffffffffffffffd ffffffff80cc2d98 fffffffff8000000 [ 2.986197] 0000000024000000 ffffffff80f40000 0000000000000000 0000000000000000 [ 2.986205] ffffffff9500cce0 0000000000000000 ffffffff80f50000 ffffffff81546318 [ 2.986213] ffffffff81c4c3c0 0000000000000018 ffffffffbc000000 0000000000000000 [ 2.986221] ffffffff81340000 98000007f89c8000 98000007f89cb430 98000007f8a00000 [ 2.986229] ffffffff806be568 0000000000000000 0000000000000000 0000000000000000 [ 2.986237] 0000000000000000 0000000000000000 ffffffff80211c1c 7b2aba74f6c4785b [ 2.986245] ... [ 2.986250] Call Trace: [ 2.986251] [<ffffffff80211c1c>] show_stack+0x9c/0x130 [ 2.986253] [<ffffffff806be568>] dump_stack+0xe8/0x150 [ 2.986255] [<ffffffff802ad408>] __lock_acquire+0x570/0x3250 [ 2.986257] [<ffffffff802abed0>] lock_acquire+0x118/0x558 [ 2.986259] [<ffffffff802be764>] console_unlock+0x2e4/0x820 [ 2.986261] [<ffffffff802c0a68>] vprintk_emit+0x1c0/0x3b8 [ 2.986263] [<ffffffff807f45a8>] dev_vprintk_emit+0x1c8/0x210 [ 2.986265] [<ffffffff807f462c>] dev_printk_emit+0x3c/0x60 [ 2.986267] [<ffffffff807f499c>] _dev_warn+0x5c/0x80 [ 2.986269] [<ffffffff806eea9c>] pci_generic_config_write32+0x154/0x160 [ 2.986271] [<ffffffff806edca4>] pci_bus_write_config_word+0x84/0xb8 [ 2.986273] [<ffffffff806f1664>] pci_setup_device+0x22c/0x768 [ 2.986275] [<ffffffff806f26a0>] pci_scan_single_device+0xc8/0x100 [ 2.986277] [<ffffffff806f2788>] pci_scan_slot+0xb0/0x178 [ 2.986279] [<ffffffff806f3ae4>] pci_scan_child_bus_extend+0x5c/0x370 [ 2.986281] [<ffffffff806f407c>] pci_scan_root_bus_bridge+0x6c/0xf0 [ 2.986283] [<ffffffff806f411c>] pci_host_probe+0x1c/0xd8 [ 2.986285] [<ffffffff807fa03c>] platform_drv_probe+0x54/0xb8 [ 2.986287] [<ffffffff807f71f8>] really_probe+0x130/0x388 [ 2.986289] [<ffffffff807f7594>] driver_probe_device+0x64/0xd8 [ 2.986291] [<ffffffff807f7844>] device_driver_attach+0x84/0x90 [ 2.986293] [<ffffffff807f7918>] __driver_attach+0xc8/0x128 [ 2.986295] [<ffffffff807f4cac>] bus_for_each_dev+0x74/0xd8 [ 2.986297] [<ffffffff807f6408>] bus_add_driver+0x170/0x250 [ 2.986299] [<ffffffff807f899c>] driver_register+0x84/0x150 [ 2.986301] [<ffffffff80200b08>] do_one_initcall+0x98/0x458 [ 2.986303] [<ffffffff810212dc>] kernel_init_freeable+0x2c0/0x36c [ 2.986305] [<ffffffff80be7540>] kernel_init+0x10/0x128 [ 2.986307] [<ffffffff80209d44>] ret_from_kernel_thread+0x14/0x1c Signed-off-by: Xingxing Su <suxingxing@loongson.cn> --- drivers/pci/access.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/pci/access.c b/drivers/pci/access.c index 79c4a2e..b3fc164 100644 --- a/drivers/pci/access.c +++ b/drivers/pci/access.c @@ -160,9 +160,6 @@ int pci_generic_config_write32(struct pci_bus *bus, unsigned int devfn, * write happen to have any RW1C (write-one-to-clear) bits set, we * just inadvertently cleared something we shouldn't have. */ - dev_warn_ratelimited(&bus->dev, "%d-byte config write to %04x:%02x:%02x.%d offset %#x may corrupt adjacent RW1C bits\n", - size, pci_domain_nr(bus), bus->number, - PCI_SLOT(devfn), PCI_FUNC(devfn), where); mask = ~(((1 << (size * 8)) - 1) << ((where & 0x3) * 8)); tmp = readl(addr) & mask; -- 2.1.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] PCI: Don't use Printk in raw_spinlocks 2020-08-01 1:25 [PATCH] PCI: Don't use Printk in raw_spinlocks Xingxing Su @ 2020-09-10 2:07 ` Bjorn Helgaas 2020-09-10 14:21 ` Rob Herring 0 siblings, 1 reply; 10+ messages in thread From: Bjorn Helgaas @ 2020-09-10 2:07 UTC (permalink / raw) To: Xingxing Su Cc: Bjorn Helgaas, linux-pci, linux-kernel, Mark Tomlinson, Florian Fainelli, Rob Herring, Scott Branden [+cc Mark, Florian, Rob, Scott] On Sat, Aug 01, 2020 at 09:25:49AM +0800, Xingxing Su wrote: > Do not use printk in raw_spinlocks, > it will cause BUG: Invalid wait context. > > The trace reported by lockdep follows. > > [ 2.986113] ============================= > [ 2.986115] [ BUG: Invalid wait context ] > [ 2.986116] 5.8.0-rc1+ #11 Not tainted > [ 2.986118] ----------------------------- > [ 2.986120] swapper/0/1 is trying to lock: > [ 2.986122] ffffffff80f5ddd8 (console_owner){....}-{3:3}, at: console_unlock+0x284/0x820 > [ 2.986130] other info that might help us debug this: > [ 2.986132] context-{5:5} > [ 2.986134] 3 locks held by swapper/0/1: > [ 2.986135] #0: 98000007fa03c990 (&dev->mutex){....}-{0:0}, at: device_driver_attach+0x28/0x90 > [ 2.986144] #1: ffffffff80fb83a8 (pci_lock){....}-{2:2}, at: pci_bus_write_config_word+0x60/0xb8 > [ 2.986152] #2: ffffffff80f5ded0 (console_lock){+.+.}-{0:0}, at: vprintk_emit+0x1b0/0x3b8 > [ 2.986161] stack backtrace: > [ 2.986163] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.8.0-rc1+ #11 > [ 2.986164] Stack : 0000000000001d67 98000000030be9b0 0000000000000001 7b2aba74f6c4785b > [ 2.986172] 7b2aba74f6c4785b 0000000000000000 98000007f89cb438 ffffffff80e7dc80 > [ 2.986181] 0000000000000001 000000000000000a 0000000000000001 0000000000000001 > [ 2.986189] ffffffff80f4e156 fffffffffffffffd ffffffff80cc2d98 fffffffff8000000 > [ 2.986197] 0000000024000000 ffffffff80f40000 0000000000000000 0000000000000000 > [ 2.986205] ffffffff9500cce0 0000000000000000 ffffffff80f50000 ffffffff81546318 > [ 2.986213] ffffffff81c4c3c0 0000000000000018 ffffffffbc000000 0000000000000000 > [ 2.986221] ffffffff81340000 98000007f89c8000 98000007f89cb430 98000007f8a00000 > [ 2.986229] ffffffff806be568 0000000000000000 0000000000000000 0000000000000000 > [ 2.986237] 0000000000000000 0000000000000000 ffffffff80211c1c 7b2aba74f6c4785b > [ 2.986245] ... > [ 2.986250] Call Trace: > [ 2.986251] [<ffffffff80211c1c>] show_stack+0x9c/0x130 > [ 2.986253] [<ffffffff806be568>] dump_stack+0xe8/0x150 > [ 2.986255] [<ffffffff802ad408>] __lock_acquire+0x570/0x3250 > [ 2.986257] [<ffffffff802abed0>] lock_acquire+0x118/0x558 > [ 2.986259] [<ffffffff802be764>] console_unlock+0x2e4/0x820 > [ 2.986261] [<ffffffff802c0a68>] vprintk_emit+0x1c0/0x3b8 > [ 2.986263] [<ffffffff807f45a8>] dev_vprintk_emit+0x1c8/0x210 > [ 2.986265] [<ffffffff807f462c>] dev_printk_emit+0x3c/0x60 > [ 2.986267] [<ffffffff807f499c>] _dev_warn+0x5c/0x80 > [ 2.986269] [<ffffffff806eea9c>] pci_generic_config_write32+0x154/0x160 > [ 2.986271] [<ffffffff806edca4>] pci_bus_write_config_word+0x84/0xb8 > [ 2.986273] [<ffffffff806f1664>] pci_setup_device+0x22c/0x768 > [ 2.986275] [<ffffffff806f26a0>] pci_scan_single_device+0xc8/0x100 > [ 2.986277] [<ffffffff806f2788>] pci_scan_slot+0xb0/0x178 > [ 2.986279] [<ffffffff806f3ae4>] pci_scan_child_bus_extend+0x5c/0x370 > [ 2.986281] [<ffffffff806f407c>] pci_scan_root_bus_bridge+0x6c/0xf0 > [ 2.986283] [<ffffffff806f411c>] pci_host_probe+0x1c/0xd8 > [ 2.986285] [<ffffffff807fa03c>] platform_drv_probe+0x54/0xb8 > [ 2.986287] [<ffffffff807f71f8>] really_probe+0x130/0x388 > [ 2.986289] [<ffffffff807f7594>] driver_probe_device+0x64/0xd8 > [ 2.986291] [<ffffffff807f7844>] device_driver_attach+0x84/0x90 > [ 2.986293] [<ffffffff807f7918>] __driver_attach+0xc8/0x128 > [ 2.986295] [<ffffffff807f4cac>] bus_for_each_dev+0x74/0xd8 > [ 2.986297] [<ffffffff807f6408>] bus_add_driver+0x170/0x250 > [ 2.986299] [<ffffffff807f899c>] driver_register+0x84/0x150 > [ 2.986301] [<ffffffff80200b08>] do_one_initcall+0x98/0x458 > [ 2.986303] [<ffffffff810212dc>] kernel_init_freeable+0x2c0/0x36c > [ 2.986305] [<ffffffff80be7540>] kernel_init+0x10/0x128 > [ 2.986307] [<ffffffff80209d44>] ret_from_kernel_thread+0x14/0x1c > > Signed-off-by: Xingxing Su <suxingxing@loongson.cn> > --- > drivers/pci/access.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/drivers/pci/access.c b/drivers/pci/access.c > index 79c4a2e..b3fc164 100644 > --- a/drivers/pci/access.c > +++ b/drivers/pci/access.c > @@ -160,9 +160,6 @@ int pci_generic_config_write32(struct pci_bus *bus, unsigned int devfn, > * write happen to have any RW1C (write-one-to-clear) bits set, we > * just inadvertently cleared something we shouldn't have. > */ > - dev_warn_ratelimited(&bus->dev, "%d-byte config write to %04x:%02x:%02x.%d offset %#x may corrupt adjacent RW1C bits\n", > - size, pci_domain_nr(bus), bus->number, > - PCI_SLOT(devfn), PCI_FUNC(devfn), where); We just changed this printk (see [1]), but I think we still have this lockdep problem even after Mark's change. So I guess we need another think about this. Maybe we can print something when registering pci_ops that use pci_generic_config_write32()? [1] https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/enumeration&id=92ff4b49c2a9 > mask = ~(((1 << (size * 8)) - 1) << ((where & 0x3) * 8)); > tmp = readl(addr) & mask; > -- > 2.1.0 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] PCI: Don't use Printk in raw_spinlocks 2020-09-10 2:07 ` Bjorn Helgaas @ 2020-09-10 14:21 ` Rob Herring 2020-09-10 16:07 ` Scott Branden 2020-09-10 18:46 ` Bjorn Helgaas 0 siblings, 2 replies; 10+ messages in thread From: Rob Herring @ 2020-09-10 14:21 UTC (permalink / raw) To: Bjorn Helgaas Cc: Xingxing Su, Bjorn Helgaas, PCI, linux-kernel, Mark Tomlinson, Florian Fainelli, Scott Branden On Wed, Sep 9, 2020 at 8:07 PM Bjorn Helgaas <helgaas@kernel.org> wrote: > > [+cc Mark, Florian, Rob, Scott] > > On Sat, Aug 01, 2020 at 09:25:49AM +0800, Xingxing Su wrote: > > Do not use printk in raw_spinlocks, > > it will cause BUG: Invalid wait context. > > > > The trace reported by lockdep follows. > > > > [ 2.986113] ============================= > > [ 2.986115] [ BUG: Invalid wait context ] > > [ 2.986116] 5.8.0-rc1+ #11 Not tainted > > [ 2.986118] ----------------------------- > > [ 2.986120] swapper/0/1 is trying to lock: > > [ 2.986122] ffffffff80f5ddd8 (console_owner){....}-{3:3}, at: console_unlock+0x284/0x820 > > [ 2.986130] other info that might help us debug this: > > [ 2.986132] context-{5:5} > > [ 2.986134] 3 locks held by swapper/0/1: > > [ 2.986135] #0: 98000007fa03c990 (&dev->mutex){....}-{0:0}, at: device_driver_attach+0x28/0x90 > > [ 2.986144] #1: ffffffff80fb83a8 (pci_lock){....}-{2:2}, at: pci_bus_write_config_word+0x60/0xb8 > > [ 2.986152] #2: ffffffff80f5ded0 (console_lock){+.+.}-{0:0}, at: vprintk_emit+0x1b0/0x3b8 > > [ 2.986161] stack backtrace: > > [ 2.986163] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.8.0-rc1+ #11 > > [ 2.986164] Stack : 0000000000001d67 98000000030be9b0 0000000000000001 7b2aba74f6c4785b > > [ 2.986172] 7b2aba74f6c4785b 0000000000000000 98000007f89cb438 ffffffff80e7dc80 > > [ 2.986181] 0000000000000001 000000000000000a 0000000000000001 0000000000000001 > > [ 2.986189] ffffffff80f4e156 fffffffffffffffd ffffffff80cc2d98 fffffffff8000000 > > [ 2.986197] 0000000024000000 ffffffff80f40000 0000000000000000 0000000000000000 > > [ 2.986205] ffffffff9500cce0 0000000000000000 ffffffff80f50000 ffffffff81546318 > > [ 2.986213] ffffffff81c4c3c0 0000000000000018 ffffffffbc000000 0000000000000000 > > [ 2.986221] ffffffff81340000 98000007f89c8000 98000007f89cb430 98000007f8a00000 > > [ 2.986229] ffffffff806be568 0000000000000000 0000000000000000 0000000000000000 > > [ 2.986237] 0000000000000000 0000000000000000 ffffffff80211c1c 7b2aba74f6c4785b > > [ 2.986245] ... > > [ 2.986250] Call Trace: > > [ 2.986251] [<ffffffff80211c1c>] show_stack+0x9c/0x130 > > [ 2.986253] [<ffffffff806be568>] dump_stack+0xe8/0x150 > > [ 2.986255] [<ffffffff802ad408>] __lock_acquire+0x570/0x3250 > > [ 2.986257] [<ffffffff802abed0>] lock_acquire+0x118/0x558 > > [ 2.986259] [<ffffffff802be764>] console_unlock+0x2e4/0x820 > > [ 2.986261] [<ffffffff802c0a68>] vprintk_emit+0x1c0/0x3b8 > > [ 2.986263] [<ffffffff807f45a8>] dev_vprintk_emit+0x1c8/0x210 > > [ 2.986265] [<ffffffff807f462c>] dev_printk_emit+0x3c/0x60 > > [ 2.986267] [<ffffffff807f499c>] _dev_warn+0x5c/0x80 > > [ 2.986269] [<ffffffff806eea9c>] pci_generic_config_write32+0x154/0x160 > > [ 2.986271] [<ffffffff806edca4>] pci_bus_write_config_word+0x84/0xb8 > > [ 2.986273] [<ffffffff806f1664>] pci_setup_device+0x22c/0x768 > > [ 2.986275] [<ffffffff806f26a0>] pci_scan_single_device+0xc8/0x100 > > [ 2.986277] [<ffffffff806f2788>] pci_scan_slot+0xb0/0x178 > > [ 2.986279] [<ffffffff806f3ae4>] pci_scan_child_bus_extend+0x5c/0x370 > > [ 2.986281] [<ffffffff806f407c>] pci_scan_root_bus_bridge+0x6c/0xf0 > > [ 2.986283] [<ffffffff806f411c>] pci_host_probe+0x1c/0xd8 > > [ 2.986285] [<ffffffff807fa03c>] platform_drv_probe+0x54/0xb8 > > [ 2.986287] [<ffffffff807f71f8>] really_probe+0x130/0x388 > > [ 2.986289] [<ffffffff807f7594>] driver_probe_device+0x64/0xd8 > > [ 2.986291] [<ffffffff807f7844>] device_driver_attach+0x84/0x90 > > [ 2.986293] [<ffffffff807f7918>] __driver_attach+0xc8/0x128 > > [ 2.986295] [<ffffffff807f4cac>] bus_for_each_dev+0x74/0xd8 > > [ 2.986297] [<ffffffff807f6408>] bus_add_driver+0x170/0x250 > > [ 2.986299] [<ffffffff807f899c>] driver_register+0x84/0x150 > > [ 2.986301] [<ffffffff80200b08>] do_one_initcall+0x98/0x458 > > [ 2.986303] [<ffffffff810212dc>] kernel_init_freeable+0x2c0/0x36c > > [ 2.986305] [<ffffffff80be7540>] kernel_init+0x10/0x128 > > [ 2.986307] [<ffffffff80209d44>] ret_from_kernel_thread+0x14/0x1c > > > > Signed-off-by: Xingxing Su <suxingxing@loongson.cn> > > --- > > drivers/pci/access.c | 3 --- > > 1 file changed, 3 deletions(-) > > > > diff --git a/drivers/pci/access.c b/drivers/pci/access.c > > index 79c4a2e..b3fc164 100644 > > --- a/drivers/pci/access.c > > +++ b/drivers/pci/access.c > > @@ -160,9 +160,6 @@ int pci_generic_config_write32(struct pci_bus *bus, unsigned int devfn, > > * write happen to have any RW1C (write-one-to-clear) bits set, we > > * just inadvertently cleared something we shouldn't have. > > */ > > - dev_warn_ratelimited(&bus->dev, "%d-byte config write to %04x:%02x:%02x.%d offset %#x may corrupt adjacent RW1C bits\n", > > - size, pci_domain_nr(bus), bus->number, > > - PCI_SLOT(devfn), PCI_FUNC(devfn), where); > > We just changed this printk (see [1]), but I think we still have this > lockdep problem even after Mark's change. So I guess we need another > think about this. > > Maybe we can print something when registering pci_ops that use > pci_generic_config_write32()? That was my suggestion, but as Mark pointed out that doesn't work if pci_generic_config_write32 is wrapped (which is 4 out of 8 cases). Also, 3 of the cases are only for the root bus (bridge). Are 32-bit writes to a bridge going to cause problems? For xgene, interestingly, with DT _write32 is needed, but for ACPI it is not (only _read32). I think xgene is practically dead though a few people still have systems, but likely xgene with DT is really dead. The ECAM case was for QCom server which is also pretty much dead. SA1100 nano-engine is really old and something only a few people have at most (Russell King). So ignoring all those, we're left with just loongson and iproc. Maybe just remove the warning? Rob ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] PCI: Don't use Printk in raw_spinlocks 2020-09-10 14:21 ` Rob Herring @ 2020-09-10 16:07 ` Scott Branden 2020-09-10 18:46 ` Bjorn Helgaas 1 sibling, 0 replies; 10+ messages in thread From: Scott Branden @ 2020-09-10 16:07 UTC (permalink / raw) To: Rob Herring, Bjorn Helgaas Cc: Xingxing Su, Bjorn Helgaas, PCI, linux-kernel, Mark Tomlinson, Florian Fainelli On 2020-09-10 7:21 a.m., Rob Herring wrote: > On Wed, Sep 9, 2020 at 8:07 PM Bjorn Helgaas <helgaas@kernel.org> wrote: >> [+cc Mark, Florian, Rob, Scott] >> >> On Sat, Aug 01, 2020 at 09:25:49AM +0800, Xingxing Su wrote: >>> Do not use printk in raw_spinlocks, >>> it will cause BUG: Invalid wait context. >>> >>> The trace reported by lockdep follows. >>> >>> [ 2.986113] ============================= >>> [ 2.986115] [ BUG: Invalid wait context ] >>> [ 2.986116] 5.8.0-rc1+ #11 Not tainted >>> [ 2.986118] ----------------------------- >>> [ 2.986120] swapper/0/1 is trying to lock: >>> [ 2.986122] ffffffff80f5ddd8 (console_owner){....}-{3:3}, at: console_unlock+0x284/0x820 >>> [ 2.986130] other info that might help us debug this: >>> [ 2.986132] context-{5:5} >>> [ 2.986134] 3 locks held by swapper/0/1: >>> [ 2.986135] #0: 98000007fa03c990 (&dev->mutex){....}-{0:0}, at: device_driver_attach+0x28/0x90 >>> [ 2.986144] #1: ffffffff80fb83a8 (pci_lock){....}-{2:2}, at: pci_bus_write_config_word+0x60/0xb8 >>> [ 2.986152] #2: ffffffff80f5ded0 (console_lock){+.+.}-{0:0}, at: vprintk_emit+0x1b0/0x3b8 >>> [ 2.986161] stack backtrace: >>> [ 2.986163] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.8.0-rc1+ #11 >>> [ 2.986164] Stack : 0000000000001d67 98000000030be9b0 0000000000000001 7b2aba74f6c4785b >>> [ 2.986172] 7b2aba74f6c4785b 0000000000000000 98000007f89cb438 ffffffff80e7dc80 >>> [ 2.986181] 0000000000000001 000000000000000a 0000000000000001 0000000000000001 >>> [ 2.986189] ffffffff80f4e156 fffffffffffffffd ffffffff80cc2d98 fffffffff8000000 >>> [ 2.986197] 0000000024000000 ffffffff80f40000 0000000000000000 0000000000000000 >>> [ 2.986205] ffffffff9500cce0 0000000000000000 ffffffff80f50000 ffffffff81546318 >>> [ 2.986213] ffffffff81c4c3c0 0000000000000018 ffffffffbc000000 0000000000000000 >>> [ 2.986221] ffffffff81340000 98000007f89c8000 98000007f89cb430 98000007f8a00000 >>> [ 2.986229] ffffffff806be568 0000000000000000 0000000000000000 0000000000000000 >>> [ 2.986237] 0000000000000000 0000000000000000 ffffffff80211c1c 7b2aba74f6c4785b >>> [ 2.986245] ... >>> [ 2.986250] Call Trace: >>> [ 2.986251] [<ffffffff80211c1c>] show_stack+0x9c/0x130 >>> [ 2.986253] [<ffffffff806be568>] dump_stack+0xe8/0x150 >>> [ 2.986255] [<ffffffff802ad408>] __lock_acquire+0x570/0x3250 >>> [ 2.986257] [<ffffffff802abed0>] lock_acquire+0x118/0x558 >>> [ 2.986259] [<ffffffff802be764>] console_unlock+0x2e4/0x820 >>> [ 2.986261] [<ffffffff802c0a68>] vprintk_emit+0x1c0/0x3b8 >>> [ 2.986263] [<ffffffff807f45a8>] dev_vprintk_emit+0x1c8/0x210 >>> [ 2.986265] [<ffffffff807f462c>] dev_printk_emit+0x3c/0x60 >>> [ 2.986267] [<ffffffff807f499c>] _dev_warn+0x5c/0x80 >>> [ 2.986269] [<ffffffff806eea9c>] pci_generic_config_write32+0x154/0x160 >>> [ 2.986271] [<ffffffff806edca4>] pci_bus_write_config_word+0x84/0xb8 >>> [ 2.986273] [<ffffffff806f1664>] pci_setup_device+0x22c/0x768 >>> [ 2.986275] [<ffffffff806f26a0>] pci_scan_single_device+0xc8/0x100 >>> [ 2.986277] [<ffffffff806f2788>] pci_scan_slot+0xb0/0x178 >>> [ 2.986279] [<ffffffff806f3ae4>] pci_scan_child_bus_extend+0x5c/0x370 >>> [ 2.986281] [<ffffffff806f407c>] pci_scan_root_bus_bridge+0x6c/0xf0 >>> [ 2.986283] [<ffffffff806f411c>] pci_host_probe+0x1c/0xd8 >>> [ 2.986285] [<ffffffff807fa03c>] platform_drv_probe+0x54/0xb8 >>> [ 2.986287] [<ffffffff807f71f8>] really_probe+0x130/0x388 >>> [ 2.986289] [<ffffffff807f7594>] driver_probe_device+0x64/0xd8 >>> [ 2.986291] [<ffffffff807f7844>] device_driver_attach+0x84/0x90 >>> [ 2.986293] [<ffffffff807f7918>] __driver_attach+0xc8/0x128 >>> [ 2.986295] [<ffffffff807f4cac>] bus_for_each_dev+0x74/0xd8 >>> [ 2.986297] [<ffffffff807f6408>] bus_add_driver+0x170/0x250 >>> [ 2.986299] [<ffffffff807f899c>] driver_register+0x84/0x150 >>> [ 2.986301] [<ffffffff80200b08>] do_one_initcall+0x98/0x458 >>> [ 2.986303] [<ffffffff810212dc>] kernel_init_freeable+0x2c0/0x36c >>> [ 2.986305] [<ffffffff80be7540>] kernel_init+0x10/0x128 >>> [ 2.986307] [<ffffffff80209d44>] ret_from_kernel_thread+0x14/0x1c >>> >>> Signed-off-by: Xingxing Su <suxingxing@loongson.cn> >>> --- >>> drivers/pci/access.c | 3 --- >>> 1 file changed, 3 deletions(-) >>> >>> diff --git a/drivers/pci/access.c b/drivers/pci/access.c >>> index 79c4a2e..b3fc164 100644 >>> --- a/drivers/pci/access.c >>> +++ b/drivers/pci/access.c >>> @@ -160,9 +160,6 @@ int pci_generic_config_write32(struct pci_bus *bus, unsigned int devfn, >>> * write happen to have any RW1C (write-one-to-clear) bits set, we >>> * just inadvertently cleared something we shouldn't have. >>> */ >>> - dev_warn_ratelimited(&bus->dev, "%d-byte config write to %04x:%02x:%02x.%d offset %#x may corrupt adjacent RW1C bits\n", >>> - size, pci_domain_nr(bus), bus->number, >>> - PCI_SLOT(devfn), PCI_FUNC(devfn), where); >> We just changed this printk (see [1]), but I think we still have this >> lockdep problem even after Mark's change. So I guess we need another >> think about this. >> >> Maybe we can print something when registering pci_ops that use >> pci_generic_config_write32()? > That was my suggestion, but as Mark pointed out that doesn't work if > pci_generic_config_write32 is wrapped (which is 4 out of 8 cases). > > Also, 3 of the cases are only for the root bus (bridge). Are 32-bit > writes to a bridge going to cause problems? For xgene, interestingly, > with DT _write32 is needed, but for ACPI it is not (only _read32). I > think xgene is practically dead though a few people still have > systems, but likely xgene with DT is really dead. The ECAM case was > for QCom server which is also pretty much dead. SA1100 nano-engine is > really old and something only a few people have at most (Russell > King). So ignoring all those, we're left with just loongson and iproc. > Maybe just remove the warning? Yes, removing the warning works fine. > > Rob ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] PCI: Don't use Printk in raw_spinlocks 2020-09-10 14:21 ` Rob Herring 2020-09-10 16:07 ` Scott Branden @ 2020-09-10 18:46 ` Bjorn Helgaas 2020-09-10 18:50 ` Florian Fainelli 2021-03-30 20:53 ` Tyler Hicks 1 sibling, 2 replies; 10+ messages in thread From: Bjorn Helgaas @ 2020-09-10 18:46 UTC (permalink / raw) To: Rob Herring Cc: Xingxing Su, Bjorn Helgaas, PCI, linux-kernel, Mark Tomlinson, Florian Fainelli, Scott Branden On Thu, Sep 10, 2020 at 08:21:06AM -0600, Rob Herring wrote: > On Wed, Sep 9, 2020 at 8:07 PM Bjorn Helgaas <helgaas@kernel.org> wrote: > > > > [+cc Mark, Florian, Rob, Scott] > > > > On Sat, Aug 01, 2020 at 09:25:49AM +0800, Xingxing Su wrote: > > > Do not use printk in raw_spinlocks, > > > it will cause BUG: Invalid wait context. > > > > > > The trace reported by lockdep follows. > > > > > > [ 2.986113] ============================= > > > [ 2.986115] [ BUG: Invalid wait context ] > > > [ 2.986116] 5.8.0-rc1+ #11 Not tainted > > > [ 2.986118] ----------------------------- > > > [ 2.986120] swapper/0/1 is trying to lock: > > > [ 2.986122] ffffffff80f5ddd8 (console_owner){....}-{3:3}, at: console_unlock+0x284/0x820 > > > [ 2.986130] other info that might help us debug this: > > > [ 2.986132] context-{5:5} > > > [ 2.986134] 3 locks held by swapper/0/1: > > > [ 2.986135] #0: 98000007fa03c990 (&dev->mutex){....}-{0:0}, at: device_driver_attach+0x28/0x90 > > > [ 2.986144] #1: ffffffff80fb83a8 (pci_lock){....}-{2:2}, at: pci_bus_write_config_word+0x60/0xb8 > > > [ 2.986152] #2: ffffffff80f5ded0 (console_lock){+.+.}-{0:0}, at: vprintk_emit+0x1b0/0x3b8 > > > [ 2.986161] stack backtrace: > > > [ 2.986163] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.8.0-rc1+ #11 > > > [ 2.986164] Stack : 0000000000001d67 98000000030be9b0 0000000000000001 7b2aba74f6c4785b > > > [ 2.986172] 7b2aba74f6c4785b 0000000000000000 98000007f89cb438 ffffffff80e7dc80 > > > [ 2.986181] 0000000000000001 000000000000000a 0000000000000001 0000000000000001 > > > [ 2.986189] ffffffff80f4e156 fffffffffffffffd ffffffff80cc2d98 fffffffff8000000 > > > [ 2.986197] 0000000024000000 ffffffff80f40000 0000000000000000 0000000000000000 > > > [ 2.986205] ffffffff9500cce0 0000000000000000 ffffffff80f50000 ffffffff81546318 > > > [ 2.986213] ffffffff81c4c3c0 0000000000000018 ffffffffbc000000 0000000000000000 > > > [ 2.986221] ffffffff81340000 98000007f89c8000 98000007f89cb430 98000007f8a00000 > > > [ 2.986229] ffffffff806be568 0000000000000000 0000000000000000 0000000000000000 > > > [ 2.986237] 0000000000000000 0000000000000000 ffffffff80211c1c 7b2aba74f6c4785b > > > [ 2.986245] ... > > > [ 2.986250] Call Trace: > > > [ 2.986251] [<ffffffff80211c1c>] show_stack+0x9c/0x130 > > > [ 2.986253] [<ffffffff806be568>] dump_stack+0xe8/0x150 > > > [ 2.986255] [<ffffffff802ad408>] __lock_acquire+0x570/0x3250 > > > [ 2.986257] [<ffffffff802abed0>] lock_acquire+0x118/0x558 > > > [ 2.986259] [<ffffffff802be764>] console_unlock+0x2e4/0x820 > > > [ 2.986261] [<ffffffff802c0a68>] vprintk_emit+0x1c0/0x3b8 > > > [ 2.986263] [<ffffffff807f45a8>] dev_vprintk_emit+0x1c8/0x210 > > > [ 2.986265] [<ffffffff807f462c>] dev_printk_emit+0x3c/0x60 > > > [ 2.986267] [<ffffffff807f499c>] _dev_warn+0x5c/0x80 > > > [ 2.986269] [<ffffffff806eea9c>] pci_generic_config_write32+0x154/0x160 > > > [ 2.986271] [<ffffffff806edca4>] pci_bus_write_config_word+0x84/0xb8 > > > [ 2.986273] [<ffffffff806f1664>] pci_setup_device+0x22c/0x768 > > > [ 2.986275] [<ffffffff806f26a0>] pci_scan_single_device+0xc8/0x100 > > > [ 2.986277] [<ffffffff806f2788>] pci_scan_slot+0xb0/0x178 > > > [ 2.986279] [<ffffffff806f3ae4>] pci_scan_child_bus_extend+0x5c/0x370 > > > [ 2.986281] [<ffffffff806f407c>] pci_scan_root_bus_bridge+0x6c/0xf0 > > > [ 2.986283] [<ffffffff806f411c>] pci_host_probe+0x1c/0xd8 > > > [ 2.986285] [<ffffffff807fa03c>] platform_drv_probe+0x54/0xb8 > > > [ 2.986287] [<ffffffff807f71f8>] really_probe+0x130/0x388 > > > [ 2.986289] [<ffffffff807f7594>] driver_probe_device+0x64/0xd8 > > > [ 2.986291] [<ffffffff807f7844>] device_driver_attach+0x84/0x90 > > > [ 2.986293] [<ffffffff807f7918>] __driver_attach+0xc8/0x128 > > > [ 2.986295] [<ffffffff807f4cac>] bus_for_each_dev+0x74/0xd8 > > > [ 2.986297] [<ffffffff807f6408>] bus_add_driver+0x170/0x250 > > > [ 2.986299] [<ffffffff807f899c>] driver_register+0x84/0x150 > > > [ 2.986301] [<ffffffff80200b08>] do_one_initcall+0x98/0x458 > > > [ 2.986303] [<ffffffff810212dc>] kernel_init_freeable+0x2c0/0x36c > > > [ 2.986305] [<ffffffff80be7540>] kernel_init+0x10/0x128 > > > [ 2.986307] [<ffffffff80209d44>] ret_from_kernel_thread+0x14/0x1c > > > > > > Signed-off-by: Xingxing Su <suxingxing@loongson.cn> > > > --- > > > drivers/pci/access.c | 3 --- > > > 1 file changed, 3 deletions(-) > > > > > > diff --git a/drivers/pci/access.c b/drivers/pci/access.c > > > index 79c4a2e..b3fc164 100644 > > > --- a/drivers/pci/access.c > > > +++ b/drivers/pci/access.c > > > @@ -160,9 +160,6 @@ int pci_generic_config_write32(struct pci_bus *bus, unsigned int devfn, > > > * write happen to have any RW1C (write-one-to-clear) bits set, we > > > * just inadvertently cleared something we shouldn't have. > > > */ > > > - dev_warn_ratelimited(&bus->dev, "%d-byte config write to %04x:%02x:%02x.%d offset %#x may corrupt adjacent RW1C bits\n", > > > - size, pci_domain_nr(bus), bus->number, > > > - PCI_SLOT(devfn), PCI_FUNC(devfn), where); > > > > We just changed this printk (see [1]), but I think we still have this > > lockdep problem even after Mark's change. So I guess we need another > > think about this. > > > > Maybe we can print something when registering pci_ops that use > > pci_generic_config_write32()? > > That was my suggestion, but as Mark pointed out that doesn't work if > pci_generic_config_write32 is wrapped (which is 4 out of 8 cases). > > Also, 3 of the cases are only for the root bus (bridge). Are 32-bit > writes to a bridge going to cause problems? For xgene, interestingly, > with DT _write32 is needed, but for ACPI it is not (only _read32). I > think xgene is practically dead though a few people still have > systems, but likely xgene with DT is really dead. The ECAM case was > for QCom server which is also pretty much dead. SA1100 nano-engine is > really old and something only a few people have at most (Russell > King). So ignoring all those, we're left with just loongson and iproc. > Maybe just remove the warning? Sigh, removing it sounds like the best option. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] PCI: Don't use Printk in raw_spinlocks 2020-09-10 18:46 ` Bjorn Helgaas @ 2020-09-10 18:50 ` Florian Fainelli 2020-09-10 19:20 ` Bjorn Helgaas 2021-03-30 20:53 ` Tyler Hicks 1 sibling, 1 reply; 10+ messages in thread From: Florian Fainelli @ 2020-09-10 18:50 UTC (permalink / raw) To: Bjorn Helgaas, Rob Herring Cc: Xingxing Su, Bjorn Helgaas, PCI, linux-kernel, Mark Tomlinson, Scott Branden On 9/10/2020 11:46 AM, Bjorn Helgaas wrote: > On Thu, Sep 10, 2020 at 08:21:06AM -0600, Rob Herring wrote: >> On Wed, Sep 9, 2020 at 8:07 PM Bjorn Helgaas <helgaas@kernel.org> wrote: >>> >>> [+cc Mark, Florian, Rob, Scott] >>> >>> On Sat, Aug 01, 2020 at 09:25:49AM +0800, Xingxing Su wrote: >>>> Do not use printk in raw_spinlocks, >>>> it will cause BUG: Invalid wait context. >>>> >>>> The trace reported by lockdep follows. >>>> >>>> [ 2.986113] ============================= >>>> [ 2.986115] [ BUG: Invalid wait context ] >>>> [ 2.986116] 5.8.0-rc1+ #11 Not tainted >>>> [ 2.986118] ----------------------------- >>>> [ 2.986120] swapper/0/1 is trying to lock: >>>> [ 2.986122] ffffffff80f5ddd8 (console_owner){....}-{3:3}, at: console_unlock+0x284/0x820 >>>> [ 2.986130] other info that might help us debug this: >>>> [ 2.986132] context-{5:5} >>>> [ 2.986134] 3 locks held by swapper/0/1: >>>> [ 2.986135] #0: 98000007fa03c990 (&dev->mutex){....}-{0:0}, at: device_driver_attach+0x28/0x90 >>>> [ 2.986144] #1: ffffffff80fb83a8 (pci_lock){....}-{2:2}, at: pci_bus_write_config_word+0x60/0xb8 >>>> [ 2.986152] #2: ffffffff80f5ded0 (console_lock){+.+.}-{0:0}, at: vprintk_emit+0x1b0/0x3b8 >>>> [ 2.986161] stack backtrace: >>>> [ 2.986163] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.8.0-rc1+ #11 >>>> [ 2.986164] Stack : 0000000000001d67 98000000030be9b0 0000000000000001 7b2aba74f6c4785b >>>> [ 2.986172] 7b2aba74f6c4785b 0000000000000000 98000007f89cb438 ffffffff80e7dc80 >>>> [ 2.986181] 0000000000000001 000000000000000a 0000000000000001 0000000000000001 >>>> [ 2.986189] ffffffff80f4e156 fffffffffffffffd ffffffff80cc2d98 fffffffff8000000 >>>> [ 2.986197] 0000000024000000 ffffffff80f40000 0000000000000000 0000000000000000 >>>> [ 2.986205] ffffffff9500cce0 0000000000000000 ffffffff80f50000 ffffffff81546318 >>>> [ 2.986213] ffffffff81c4c3c0 0000000000000018 ffffffffbc000000 0000000000000000 >>>> [ 2.986221] ffffffff81340000 98000007f89c8000 98000007f89cb430 98000007f8a00000 >>>> [ 2.986229] ffffffff806be568 0000000000000000 0000000000000000 0000000000000000 >>>> [ 2.986237] 0000000000000000 0000000000000000 ffffffff80211c1c 7b2aba74f6c4785b >>>> [ 2.986245] ... >>>> [ 2.986250] Call Trace: >>>> [ 2.986251] [<ffffffff80211c1c>] show_stack+0x9c/0x130 >>>> [ 2.986253] [<ffffffff806be568>] dump_stack+0xe8/0x150 >>>> [ 2.986255] [<ffffffff802ad408>] __lock_acquire+0x570/0x3250 >>>> [ 2.986257] [<ffffffff802abed0>] lock_acquire+0x118/0x558 >>>> [ 2.986259] [<ffffffff802be764>] console_unlock+0x2e4/0x820 >>>> [ 2.986261] [<ffffffff802c0a68>] vprintk_emit+0x1c0/0x3b8 >>>> [ 2.986263] [<ffffffff807f45a8>] dev_vprintk_emit+0x1c8/0x210 >>>> [ 2.986265] [<ffffffff807f462c>] dev_printk_emit+0x3c/0x60 >>>> [ 2.986267] [<ffffffff807f499c>] _dev_warn+0x5c/0x80 >>>> [ 2.986269] [<ffffffff806eea9c>] pci_generic_config_write32+0x154/0x160 >>>> [ 2.986271] [<ffffffff806edca4>] pci_bus_write_config_word+0x84/0xb8 >>>> [ 2.986273] [<ffffffff806f1664>] pci_setup_device+0x22c/0x768 >>>> [ 2.986275] [<ffffffff806f26a0>] pci_scan_single_device+0xc8/0x100 >>>> [ 2.986277] [<ffffffff806f2788>] pci_scan_slot+0xb0/0x178 >>>> [ 2.986279] [<ffffffff806f3ae4>] pci_scan_child_bus_extend+0x5c/0x370 >>>> [ 2.986281] [<ffffffff806f407c>] pci_scan_root_bus_bridge+0x6c/0xf0 >>>> [ 2.986283] [<ffffffff806f411c>] pci_host_probe+0x1c/0xd8 >>>> [ 2.986285] [<ffffffff807fa03c>] platform_drv_probe+0x54/0xb8 >>>> [ 2.986287] [<ffffffff807f71f8>] really_probe+0x130/0x388 >>>> [ 2.986289] [<ffffffff807f7594>] driver_probe_device+0x64/0xd8 >>>> [ 2.986291] [<ffffffff807f7844>] device_driver_attach+0x84/0x90 >>>> [ 2.986293] [<ffffffff807f7918>] __driver_attach+0xc8/0x128 >>>> [ 2.986295] [<ffffffff807f4cac>] bus_for_each_dev+0x74/0xd8 >>>> [ 2.986297] [<ffffffff807f6408>] bus_add_driver+0x170/0x250 >>>> [ 2.986299] [<ffffffff807f899c>] driver_register+0x84/0x150 >>>> [ 2.986301] [<ffffffff80200b08>] do_one_initcall+0x98/0x458 >>>> [ 2.986303] [<ffffffff810212dc>] kernel_init_freeable+0x2c0/0x36c >>>> [ 2.986305] [<ffffffff80be7540>] kernel_init+0x10/0x128 >>>> [ 2.986307] [<ffffffff80209d44>] ret_from_kernel_thread+0x14/0x1c >>>> >>>> Signed-off-by: Xingxing Su <suxingxing@loongson.cn> >>>> --- >>>> drivers/pci/access.c | 3 --- >>>> 1 file changed, 3 deletions(-) >>>> >>>> diff --git a/drivers/pci/access.c b/drivers/pci/access.c >>>> index 79c4a2e..b3fc164 100644 >>>> --- a/drivers/pci/access.c >>>> +++ b/drivers/pci/access.c >>>> @@ -160,9 +160,6 @@ int pci_generic_config_write32(struct pci_bus *bus, unsigned int devfn, >>>> * write happen to have any RW1C (write-one-to-clear) bits set, we >>>> * just inadvertently cleared something we shouldn't have. >>>> */ >>>> - dev_warn_ratelimited(&bus->dev, "%d-byte config write to %04x:%02x:%02x.%d offset %#x may corrupt adjacent RW1C bits\n", >>>> - size, pci_domain_nr(bus), bus->number, >>>> - PCI_SLOT(devfn), PCI_FUNC(devfn), where); >>> >>> We just changed this printk (see [1]), but I think we still have this >>> lockdep problem even after Mark's change. So I guess we need another >>> think about this. >>> >>> Maybe we can print something when registering pci_ops that use >>> pci_generic_config_write32()? >> >> That was my suggestion, but as Mark pointed out that doesn't work if >> pci_generic_config_write32 is wrapped (which is 4 out of 8 cases). >> >> Also, 3 of the cases are only for the root bus (bridge). Are 32-bit >> writes to a bridge going to cause problems? For xgene, interestingly, >> with DT _write32 is needed, but for ACPI it is not (only _read32). I >> think xgene is practically dead though a few people still have >> systems, but likely xgene with DT is really dead. The ECAM case was >> for QCom server which is also pretty much dead. SA1100 nano-engine is >> really old and something only a few people have at most (Russell >> King). So ignoring all those, we're left with just loongson and iproc. >> Maybe just remove the warning? > > Sigh, removing it sounds like the best option. Given that we have to go by a list of drivers, how about we add another pci_bus_flags_t flag to indicate 32-bit configuration space accessors are used and key off that bit to issue the warning? -- Florian ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] PCI: Don't use Printk in raw_spinlocks 2020-09-10 18:50 ` Florian Fainelli @ 2020-09-10 19:20 ` Bjorn Helgaas 2020-09-10 19:22 ` Florian Fainelli 0 siblings, 1 reply; 10+ messages in thread From: Bjorn Helgaas @ 2020-09-10 19:20 UTC (permalink / raw) To: Florian Fainelli Cc: Rob Herring, Xingxing Su, Bjorn Helgaas, PCI, linux-kernel, Mark Tomlinson, Scott Branden On Thu, Sep 10, 2020 at 11:50:07AM -0700, Florian Fainelli wrote: > On 9/10/2020 11:46 AM, Bjorn Helgaas wrote: > > On Thu, Sep 10, 2020 at 08:21:06AM -0600, Rob Herring wrote: > > > On Wed, Sep 9, 2020 at 8:07 PM Bjorn Helgaas <helgaas@kernel.org> wrote: > > > > > > > > [+cc Mark, Florian, Rob, Scott] > > > > > > > > On Sat, Aug 01, 2020 at 09:25:49AM +0800, Xingxing Su wrote: > > > > > Do not use printk in raw_spinlocks, > > > > > it will cause BUG: Invalid wait context. > > > > > > > > > > The trace reported by lockdep follows. > > > > > > > > > > [ 2.986113] ============================= > > > > > [ 2.986115] [ BUG: Invalid wait context ] > > > > > [ 2.986116] 5.8.0-rc1+ #11 Not tainted > > > > > [ 2.986118] ----------------------------- > > > > > [ 2.986120] swapper/0/1 is trying to lock: > > > > > [ 2.986122] ffffffff80f5ddd8 (console_owner){....}-{3:3}, at: console_unlock+0x284/0x820 > > > > > [ 2.986130] other info that might help us debug this: > > > > > [ 2.986132] context-{5:5} > > > > > [ 2.986134] 3 locks held by swapper/0/1: > > > > > [ 2.986135] #0: 98000007fa03c990 (&dev->mutex){....}-{0:0}, at: device_driver_attach+0x28/0x90 > > > > > [ 2.986144] #1: ffffffff80fb83a8 (pci_lock){....}-{2:2}, at: pci_bus_write_config_word+0x60/0xb8 > > > > > [ 2.986152] #2: ffffffff80f5ded0 (console_lock){+.+.}-{0:0}, at: vprintk_emit+0x1b0/0x3b8 > > > > > [ 2.986161] stack backtrace: > > > > > [ 2.986163] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.8.0-rc1+ #11 > > > > > [ 2.986164] Stack : 0000000000001d67 98000000030be9b0 0000000000000001 7b2aba74f6c4785b > > > > > [ 2.986172] 7b2aba74f6c4785b 0000000000000000 98000007f89cb438 ffffffff80e7dc80 > > > > > [ 2.986181] 0000000000000001 000000000000000a 0000000000000001 0000000000000001 > > > > > [ 2.986189] ffffffff80f4e156 fffffffffffffffd ffffffff80cc2d98 fffffffff8000000 > > > > > [ 2.986197] 0000000024000000 ffffffff80f40000 0000000000000000 0000000000000000 > > > > > [ 2.986205] ffffffff9500cce0 0000000000000000 ffffffff80f50000 ffffffff81546318 > > > > > [ 2.986213] ffffffff81c4c3c0 0000000000000018 ffffffffbc000000 0000000000000000 > > > > > [ 2.986221] ffffffff81340000 98000007f89c8000 98000007f89cb430 98000007f8a00000 > > > > > [ 2.986229] ffffffff806be568 0000000000000000 0000000000000000 0000000000000000 > > > > > [ 2.986237] 0000000000000000 0000000000000000 ffffffff80211c1c 7b2aba74f6c4785b > > > > > [ 2.986245] ... > > > > > [ 2.986250] Call Trace: > > > > > [ 2.986251] [<ffffffff80211c1c>] show_stack+0x9c/0x130 > > > > > [ 2.986253] [<ffffffff806be568>] dump_stack+0xe8/0x150 > > > > > [ 2.986255] [<ffffffff802ad408>] __lock_acquire+0x570/0x3250 > > > > > [ 2.986257] [<ffffffff802abed0>] lock_acquire+0x118/0x558 > > > > > [ 2.986259] [<ffffffff802be764>] console_unlock+0x2e4/0x820 > > > > > [ 2.986261] [<ffffffff802c0a68>] vprintk_emit+0x1c0/0x3b8 > > > > > [ 2.986263] [<ffffffff807f45a8>] dev_vprintk_emit+0x1c8/0x210 > > > > > [ 2.986265] [<ffffffff807f462c>] dev_printk_emit+0x3c/0x60 > > > > > [ 2.986267] [<ffffffff807f499c>] _dev_warn+0x5c/0x80 > > > > > [ 2.986269] [<ffffffff806eea9c>] pci_generic_config_write32+0x154/0x160 > > > > > [ 2.986271] [<ffffffff806edca4>] pci_bus_write_config_word+0x84/0xb8 > > > > > [ 2.986273] [<ffffffff806f1664>] pci_setup_device+0x22c/0x768 > > > > > [ 2.986275] [<ffffffff806f26a0>] pci_scan_single_device+0xc8/0x100 > > > > > [ 2.986277] [<ffffffff806f2788>] pci_scan_slot+0xb0/0x178 > > > > > [ 2.986279] [<ffffffff806f3ae4>] pci_scan_child_bus_extend+0x5c/0x370 > > > > > [ 2.986281] [<ffffffff806f407c>] pci_scan_root_bus_bridge+0x6c/0xf0 > > > > > [ 2.986283] [<ffffffff806f411c>] pci_host_probe+0x1c/0xd8 > > > > > [ 2.986285] [<ffffffff807fa03c>] platform_drv_probe+0x54/0xb8 > > > > > [ 2.986287] [<ffffffff807f71f8>] really_probe+0x130/0x388 > > > > > [ 2.986289] [<ffffffff807f7594>] driver_probe_device+0x64/0xd8 > > > > > [ 2.986291] [<ffffffff807f7844>] device_driver_attach+0x84/0x90 > > > > > [ 2.986293] [<ffffffff807f7918>] __driver_attach+0xc8/0x128 > > > > > [ 2.986295] [<ffffffff807f4cac>] bus_for_each_dev+0x74/0xd8 > > > > > [ 2.986297] [<ffffffff807f6408>] bus_add_driver+0x170/0x250 > > > > > [ 2.986299] [<ffffffff807f899c>] driver_register+0x84/0x150 > > > > > [ 2.986301] [<ffffffff80200b08>] do_one_initcall+0x98/0x458 > > > > > [ 2.986303] [<ffffffff810212dc>] kernel_init_freeable+0x2c0/0x36c > > > > > [ 2.986305] [<ffffffff80be7540>] kernel_init+0x10/0x128 > > > > > [ 2.986307] [<ffffffff80209d44>] ret_from_kernel_thread+0x14/0x1c > > > > > > > > > > Signed-off-by: Xingxing Su <suxingxing@loongson.cn> > > > > > --- > > > > > drivers/pci/access.c | 3 --- > > > > > 1 file changed, 3 deletions(-) > > > > > > > > > > diff --git a/drivers/pci/access.c b/drivers/pci/access.c > > > > > index 79c4a2e..b3fc164 100644 > > > > > --- a/drivers/pci/access.c > > > > > +++ b/drivers/pci/access.c > > > > > @@ -160,9 +160,6 @@ int pci_generic_config_write32(struct pci_bus *bus, unsigned int devfn, > > > > > * write happen to have any RW1C (write-one-to-clear) bits set, we > > > > > * just inadvertently cleared something we shouldn't have. > > > > > */ > > > > > - dev_warn_ratelimited(&bus->dev, "%d-byte config write to %04x:%02x:%02x.%d offset %#x may corrupt adjacent RW1C bits\n", > > > > > - size, pci_domain_nr(bus), bus->number, > > > > > - PCI_SLOT(devfn), PCI_FUNC(devfn), where); > > > > > > > > We just changed this printk (see [1]), but I think we still have this > > > > lockdep problem even after Mark's change. So I guess we need another > > > > think about this. > > > > > > > > Maybe we can print something when registering pci_ops that use > > > > pci_generic_config_write32()? > > > > > > That was my suggestion, but as Mark pointed out that doesn't work if > > > pci_generic_config_write32 is wrapped (which is 4 out of 8 cases). > > > > > > Also, 3 of the cases are only for the root bus (bridge). Are 32-bit > > > writes to a bridge going to cause problems? For xgene, interestingly, > > > with DT _write32 is needed, but for ACPI it is not (only _read32). I > > > think xgene is practically dead though a few people still have > > > systems, but likely xgene with DT is really dead. The ECAM case was > > > for QCom server which is also pretty much dead. SA1100 nano-engine is > > > really old and something only a few people have at most (Russell > > > King). So ignoring all those, we're left with just loongson and iproc. > > > Maybe just remove the warning? > > > > Sigh, removing it sounds like the best option. > > Given that we have to go by a list of drivers, how about we add another > pci_bus_flags_t flag to indicate 32-bit configuration space accessors are > used and key off that bit to issue the warning? I think that's basically what Mark's patch does: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/enumeration&id=92ff4b49c2a9 But of course we still have to deal with the lockdep problem. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] PCI: Don't use Printk in raw_spinlocks 2020-09-10 19:20 ` Bjorn Helgaas @ 2020-09-10 19:22 ` Florian Fainelli 0 siblings, 0 replies; 10+ messages in thread From: Florian Fainelli @ 2020-09-10 19:22 UTC (permalink / raw) To: Bjorn Helgaas Cc: Rob Herring, Xingxing Su, Bjorn Helgaas, PCI, linux-kernel, Mark Tomlinson, Scott Branden On 9/10/2020 12:20 PM, Bjorn Helgaas wrote: > On Thu, Sep 10, 2020 at 11:50:07AM -0700, Florian Fainelli wrote: >> On 9/10/2020 11:46 AM, Bjorn Helgaas wrote: >>> On Thu, Sep 10, 2020 at 08:21:06AM -0600, Rob Herring wrote: >>>> On Wed, Sep 9, 2020 at 8:07 PM Bjorn Helgaas <helgaas@kernel.org> wrote: >>>>> >>>>> [+cc Mark, Florian, Rob, Scott] >>>>> >>>>> On Sat, Aug 01, 2020 at 09:25:49AM +0800, Xingxing Su wrote: >>>>>> Do not use printk in raw_spinlocks, >>>>>> it will cause BUG: Invalid wait context. >>>>>> >>>>>> The trace reported by lockdep follows. >>>>>> >>>>>> [ 2.986113] ============================= >>>>>> [ 2.986115] [ BUG: Invalid wait context ] >>>>>> [ 2.986116] 5.8.0-rc1+ #11 Not tainted >>>>>> [ 2.986118] ----------------------------- >>>>>> [ 2.986120] swapper/0/1 is trying to lock: >>>>>> [ 2.986122] ffffffff80f5ddd8 (console_owner){....}-{3:3}, at: console_unlock+0x284/0x820 >>>>>> [ 2.986130] other info that might help us debug this: >>>>>> [ 2.986132] context-{5:5} >>>>>> [ 2.986134] 3 locks held by swapper/0/1: >>>>>> [ 2.986135] #0: 98000007fa03c990 (&dev->mutex){....}-{0:0}, at: device_driver_attach+0x28/0x90 >>>>>> [ 2.986144] #1: ffffffff80fb83a8 (pci_lock){....}-{2:2}, at: pci_bus_write_config_word+0x60/0xb8 >>>>>> [ 2.986152] #2: ffffffff80f5ded0 (console_lock){+.+.}-{0:0}, at: vprintk_emit+0x1b0/0x3b8 >>>>>> [ 2.986161] stack backtrace: >>>>>> [ 2.986163] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.8.0-rc1+ #11 >>>>>> [ 2.986164] Stack : 0000000000001d67 98000000030be9b0 0000000000000001 7b2aba74f6c4785b >>>>>> [ 2.986172] 7b2aba74f6c4785b 0000000000000000 98000007f89cb438 ffffffff80e7dc80 >>>>>> [ 2.986181] 0000000000000001 000000000000000a 0000000000000001 0000000000000001 >>>>>> [ 2.986189] ffffffff80f4e156 fffffffffffffffd ffffffff80cc2d98 fffffffff8000000 >>>>>> [ 2.986197] 0000000024000000 ffffffff80f40000 0000000000000000 0000000000000000 >>>>>> [ 2.986205] ffffffff9500cce0 0000000000000000 ffffffff80f50000 ffffffff81546318 >>>>>> [ 2.986213] ffffffff81c4c3c0 0000000000000018 ffffffffbc000000 0000000000000000 >>>>>> [ 2.986221] ffffffff81340000 98000007f89c8000 98000007f89cb430 98000007f8a00000 >>>>>> [ 2.986229] ffffffff806be568 0000000000000000 0000000000000000 0000000000000000 >>>>>> [ 2.986237] 0000000000000000 0000000000000000 ffffffff80211c1c 7b2aba74f6c4785b >>>>>> [ 2.986245] ... >>>>>> [ 2.986250] Call Trace: >>>>>> [ 2.986251] [<ffffffff80211c1c>] show_stack+0x9c/0x130 >>>>>> [ 2.986253] [<ffffffff806be568>] dump_stack+0xe8/0x150 >>>>>> [ 2.986255] [<ffffffff802ad408>] __lock_acquire+0x570/0x3250 >>>>>> [ 2.986257] [<ffffffff802abed0>] lock_acquire+0x118/0x558 >>>>>> [ 2.986259] [<ffffffff802be764>] console_unlock+0x2e4/0x820 >>>>>> [ 2.986261] [<ffffffff802c0a68>] vprintk_emit+0x1c0/0x3b8 >>>>>> [ 2.986263] [<ffffffff807f45a8>] dev_vprintk_emit+0x1c8/0x210 >>>>>> [ 2.986265] [<ffffffff807f462c>] dev_printk_emit+0x3c/0x60 >>>>>> [ 2.986267] [<ffffffff807f499c>] _dev_warn+0x5c/0x80 >>>>>> [ 2.986269] [<ffffffff806eea9c>] pci_generic_config_write32+0x154/0x160 >>>>>> [ 2.986271] [<ffffffff806edca4>] pci_bus_write_config_word+0x84/0xb8 >>>>>> [ 2.986273] [<ffffffff806f1664>] pci_setup_device+0x22c/0x768 >>>>>> [ 2.986275] [<ffffffff806f26a0>] pci_scan_single_device+0xc8/0x100 >>>>>> [ 2.986277] [<ffffffff806f2788>] pci_scan_slot+0xb0/0x178 >>>>>> [ 2.986279] [<ffffffff806f3ae4>] pci_scan_child_bus_extend+0x5c/0x370 >>>>>> [ 2.986281] [<ffffffff806f407c>] pci_scan_root_bus_bridge+0x6c/0xf0 >>>>>> [ 2.986283] [<ffffffff806f411c>] pci_host_probe+0x1c/0xd8 >>>>>> [ 2.986285] [<ffffffff807fa03c>] platform_drv_probe+0x54/0xb8 >>>>>> [ 2.986287] [<ffffffff807f71f8>] really_probe+0x130/0x388 >>>>>> [ 2.986289] [<ffffffff807f7594>] driver_probe_device+0x64/0xd8 >>>>>> [ 2.986291] [<ffffffff807f7844>] device_driver_attach+0x84/0x90 >>>>>> [ 2.986293] [<ffffffff807f7918>] __driver_attach+0xc8/0x128 >>>>>> [ 2.986295] [<ffffffff807f4cac>] bus_for_each_dev+0x74/0xd8 >>>>>> [ 2.986297] [<ffffffff807f6408>] bus_add_driver+0x170/0x250 >>>>>> [ 2.986299] [<ffffffff807f899c>] driver_register+0x84/0x150 >>>>>> [ 2.986301] [<ffffffff80200b08>] do_one_initcall+0x98/0x458 >>>>>> [ 2.986303] [<ffffffff810212dc>] kernel_init_freeable+0x2c0/0x36c >>>>>> [ 2.986305] [<ffffffff80be7540>] kernel_init+0x10/0x128 >>>>>> [ 2.986307] [<ffffffff80209d44>] ret_from_kernel_thread+0x14/0x1c >>>>>> >>>>>> Signed-off-by: Xingxing Su <suxingxing@loongson.cn> >>>>>> --- >>>>>> drivers/pci/access.c | 3 --- >>>>>> 1 file changed, 3 deletions(-) >>>>>> >>>>>> diff --git a/drivers/pci/access.c b/drivers/pci/access.c >>>>>> index 79c4a2e..b3fc164 100644 >>>>>> --- a/drivers/pci/access.c >>>>>> +++ b/drivers/pci/access.c >>>>>> @@ -160,9 +160,6 @@ int pci_generic_config_write32(struct pci_bus *bus, unsigned int devfn, >>>>>> * write happen to have any RW1C (write-one-to-clear) bits set, we >>>>>> * just inadvertently cleared something we shouldn't have. >>>>>> */ >>>>>> - dev_warn_ratelimited(&bus->dev, "%d-byte config write to %04x:%02x:%02x.%d offset %#x may corrupt adjacent RW1C bits\n", >>>>>> - size, pci_domain_nr(bus), bus->number, >>>>>> - PCI_SLOT(devfn), PCI_FUNC(devfn), where); >>>>> >>>>> We just changed this printk (see [1]), but I think we still have this >>>>> lockdep problem even after Mark's change. So I guess we need another >>>>> think about this. >>>>> >>>>> Maybe we can print something when registering pci_ops that use >>>>> pci_generic_config_write32()? >>>> >>>> That was my suggestion, but as Mark pointed out that doesn't work if >>>> pci_generic_config_write32 is wrapped (which is 4 out of 8 cases). >>>> >>>> Also, 3 of the cases are only for the root bus (bridge). Are 32-bit >>>> writes to a bridge going to cause problems? For xgene, interestingly, >>>> with DT _write32 is needed, but for ACPI it is not (only _read32). I >>>> think xgene is practically dead though a few people still have >>>> systems, but likely xgene with DT is really dead. The ECAM case was >>>> for QCom server which is also pretty much dead. SA1100 nano-engine is >>>> really old and something only a few people have at most (Russell >>>> King). So ignoring all those, we're left with just loongson and iproc. >>>> Maybe just remove the warning? >>> >>> Sigh, removing it sounds like the best option. >> >> Given that we have to go by a list of drivers, how about we add another >> pci_bus_flags_t flag to indicate 32-bit configuration space accessors are >> used and key off that bit to issue the warning? > > I think that's basically what Mark's patch does: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/enumeration&id=92ff4b49c2a9 Yes indeed, not enough coffee, clearly. > > But of course we still have to deal with the lockdep problem. > -- Florian ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] PCI: Don't use Printk in raw_spinlocks 2020-09-10 18:46 ` Bjorn Helgaas 2020-09-10 18:50 ` Florian Fainelli @ 2021-03-30 20:53 ` Tyler Hicks 2021-06-14 17:43 ` Tyler Hicks 1 sibling, 1 reply; 10+ messages in thread From: Tyler Hicks @ 2021-03-30 20:53 UTC (permalink / raw) To: Bjorn Helgaas Cc: Rob Herring, Xingxing Su, Bjorn Helgaas, PCI, linux-kernel, Mark Tomlinson, Florian Fainelli, Scott Branden On 2020-09-10 13:46:27, Bjorn Helgaas wrote: > On Thu, Sep 10, 2020 at 08:21:06AM -0600, Rob Herring wrote: > > On Wed, Sep 9, 2020 at 8:07 PM Bjorn Helgaas <helgaas@kernel.org> wrote: > > > > > > [+cc Mark, Florian, Rob, Scott] > > > > > > On Sat, Aug 01, 2020 at 09:25:49AM +0800, Xingxing Su wrote: > > > > Do not use printk in raw_spinlocks, > > > > it will cause BUG: Invalid wait context. > > > > > > > > The trace reported by lockdep follows. > > > > > > > > [ 2.986113] ============================= > > > > [ 2.986115] [ BUG: Invalid wait context ] > > > > [ 2.986116] 5.8.0-rc1+ #11 Not tainted > > > > [ 2.986118] ----------------------------- > > > > [ 2.986120] swapper/0/1 is trying to lock: > > > > [ 2.986122] ffffffff80f5ddd8 (console_owner){....}-{3:3}, at: console_unlock+0x284/0x820 > > > > [ 2.986130] other info that might help us debug this: > > > > [ 2.986132] context-{5:5} > > > > [ 2.986134] 3 locks held by swapper/0/1: > > > > [ 2.986135] #0: 98000007fa03c990 (&dev->mutex){....}-{0:0}, at: device_driver_attach+0x28/0x90 > > > > [ 2.986144] #1: ffffffff80fb83a8 (pci_lock){....}-{2:2}, at: pci_bus_write_config_word+0x60/0xb8 > > > > [ 2.986152] #2: ffffffff80f5ded0 (console_lock){+.+.}-{0:0}, at: vprintk_emit+0x1b0/0x3b8 > > > > [ 2.986161] stack backtrace: > > > > [ 2.986163] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.8.0-rc1+ #11 > > > > [ 2.986164] Stack : 0000000000001d67 98000000030be9b0 0000000000000001 7b2aba74f6c4785b > > > > [ 2.986172] 7b2aba74f6c4785b 0000000000000000 98000007f89cb438 ffffffff80e7dc80 > > > > [ 2.986181] 0000000000000001 000000000000000a 0000000000000001 0000000000000001 > > > > [ 2.986189] ffffffff80f4e156 fffffffffffffffd ffffffff80cc2d98 fffffffff8000000 > > > > [ 2.986197] 0000000024000000 ffffffff80f40000 0000000000000000 0000000000000000 > > > > [ 2.986205] ffffffff9500cce0 0000000000000000 ffffffff80f50000 ffffffff81546318 > > > > [ 2.986213] ffffffff81c4c3c0 0000000000000018 ffffffffbc000000 0000000000000000 > > > > [ 2.986221] ffffffff81340000 98000007f89c8000 98000007f89cb430 98000007f8a00000 > > > > [ 2.986229] ffffffff806be568 0000000000000000 0000000000000000 0000000000000000 > > > > [ 2.986237] 0000000000000000 0000000000000000 ffffffff80211c1c 7b2aba74f6c4785b > > > > [ 2.986245] ... > > > > [ 2.986250] Call Trace: > > > > [ 2.986251] [<ffffffff80211c1c>] show_stack+0x9c/0x130 > > > > [ 2.986253] [<ffffffff806be568>] dump_stack+0xe8/0x150 > > > > [ 2.986255] [<ffffffff802ad408>] __lock_acquire+0x570/0x3250 > > > > [ 2.986257] [<ffffffff802abed0>] lock_acquire+0x118/0x558 > > > > [ 2.986259] [<ffffffff802be764>] console_unlock+0x2e4/0x820 > > > > [ 2.986261] [<ffffffff802c0a68>] vprintk_emit+0x1c0/0x3b8 > > > > [ 2.986263] [<ffffffff807f45a8>] dev_vprintk_emit+0x1c8/0x210 > > > > [ 2.986265] [<ffffffff807f462c>] dev_printk_emit+0x3c/0x60 > > > > [ 2.986267] [<ffffffff807f499c>] _dev_warn+0x5c/0x80 > > > > [ 2.986269] [<ffffffff806eea9c>] pci_generic_config_write32+0x154/0x160 > > > > [ 2.986271] [<ffffffff806edca4>] pci_bus_write_config_word+0x84/0xb8 > > > > [ 2.986273] [<ffffffff806f1664>] pci_setup_device+0x22c/0x768 > > > > [ 2.986275] [<ffffffff806f26a0>] pci_scan_single_device+0xc8/0x100 > > > > [ 2.986277] [<ffffffff806f2788>] pci_scan_slot+0xb0/0x178 > > > > [ 2.986279] [<ffffffff806f3ae4>] pci_scan_child_bus_extend+0x5c/0x370 > > > > [ 2.986281] [<ffffffff806f407c>] pci_scan_root_bus_bridge+0x6c/0xf0 > > > > [ 2.986283] [<ffffffff806f411c>] pci_host_probe+0x1c/0xd8 > > > > [ 2.986285] [<ffffffff807fa03c>] platform_drv_probe+0x54/0xb8 > > > > [ 2.986287] [<ffffffff807f71f8>] really_probe+0x130/0x388 > > > > [ 2.986289] [<ffffffff807f7594>] driver_probe_device+0x64/0xd8 > > > > [ 2.986291] [<ffffffff807f7844>] device_driver_attach+0x84/0x90 > > > > [ 2.986293] [<ffffffff807f7918>] __driver_attach+0xc8/0x128 > > > > [ 2.986295] [<ffffffff807f4cac>] bus_for_each_dev+0x74/0xd8 > > > > [ 2.986297] [<ffffffff807f6408>] bus_add_driver+0x170/0x250 > > > > [ 2.986299] [<ffffffff807f899c>] driver_register+0x84/0x150 > > > > [ 2.986301] [<ffffffff80200b08>] do_one_initcall+0x98/0x458 > > > > [ 2.986303] [<ffffffff810212dc>] kernel_init_freeable+0x2c0/0x36c > > > > [ 2.986305] [<ffffffff80be7540>] kernel_init+0x10/0x128 > > > > [ 2.986307] [<ffffffff80209d44>] ret_from_kernel_thread+0x14/0x1c > > > > > > > > Signed-off-by: Xingxing Su <suxingxing@loongson.cn> > > > > --- > > > > drivers/pci/access.c | 3 --- > > > > 1 file changed, 3 deletions(-) > > > > > > > > diff --git a/drivers/pci/access.c b/drivers/pci/access.c > > > > index 79c4a2e..b3fc164 100644 > > > > --- a/drivers/pci/access.c > > > > +++ b/drivers/pci/access.c > > > > @@ -160,9 +160,6 @@ int pci_generic_config_write32(struct pci_bus *bus, unsigned int devfn, > > > > * write happen to have any RW1C (write-one-to-clear) bits set, we > > > > * just inadvertently cleared something we shouldn't have. > > > > */ > > > > - dev_warn_ratelimited(&bus->dev, "%d-byte config write to %04x:%02x:%02x.%d offset %#x may corrupt adjacent RW1C bits\n", > > > > - size, pci_domain_nr(bus), bus->number, > > > > - PCI_SLOT(devfn), PCI_FUNC(devfn), where); > > > > > > We just changed this printk (see [1]), but I think we still have this > > > lockdep problem even after Mark's change. So I guess we need another > > > think about this. > > > > > > Maybe we can print something when registering pci_ops that use > > > pci_generic_config_write32()? > > > > That was my suggestion, but as Mark pointed out that doesn't work if > > pci_generic_config_write32 is wrapped (which is 4 out of 8 cases). > > > > Also, 3 of the cases are only for the root bus (bridge). Are 32-bit > > writes to a bridge going to cause problems? For xgene, interestingly, > > with DT _write32 is needed, but for ACPI it is not (only _read32). I > > think xgene is practically dead though a few people still have > > systems, but likely xgene with DT is really dead. The ECAM case was > > for QCom server which is also pretty much dead. SA1100 nano-engine is > > really old and something only a few people have at most (Russell > > King). So ignoring all those, we're left with just loongson and iproc. > > Maybe just remove the warning? > > Sigh, removing it sounds like the best option. Hi Bjorn - Was this lockdep issue fixed in a different way than removing the use of printk? I'm mostly interested in reducing the RW1C warnings as Mark suggested here: https://lore.kernel.org/lkml/20200806041455.11070-1-mark.tomlinson@alliedtelesis.co.nz/ While thinking how best to rejuvenate that patch, I came across this thread in the archives and it seems like the plan shifted to completely removing this printk callsite to address the lockdep issue. However, neither patch ended up in Linus' tree nor in the current PCI tree (I only peeked in the next branch). What's everyone's preference at this point? Thanks! Tyler ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] PCI: Don't use Printk in raw_spinlocks 2021-03-30 20:53 ` Tyler Hicks @ 2021-06-14 17:43 ` Tyler Hicks 0 siblings, 0 replies; 10+ messages in thread From: Tyler Hicks @ 2021-06-14 17:43 UTC (permalink / raw) To: Bjorn Helgaas Cc: Rob Herring, Xingxing Su, Bjorn Helgaas, PCI, linux-kernel, Mark Tomlinson, Florian Fainelli, Scott Branden On 2021-03-30 15:53:35, Tyler Hicks wrote: > On 2020-09-10 13:46:27, Bjorn Helgaas wrote: > > On Thu, Sep 10, 2020 at 08:21:06AM -0600, Rob Herring wrote: > > > On Wed, Sep 9, 2020 at 8:07 PM Bjorn Helgaas <helgaas@kernel.org> wrote: > > > > > > > > [+cc Mark, Florian, Rob, Scott] > > > > > > > > On Sat, Aug 01, 2020 at 09:25:49AM +0800, Xingxing Su wrote: > > > > > Do not use printk in raw_spinlocks, > > > > > it will cause BUG: Invalid wait context. > > > > > > > > > > The trace reported by lockdep follows. > > > > > > > > > > [ 2.986113] ============================= > > > > > [ 2.986115] [ BUG: Invalid wait context ] > > > > > [ 2.986116] 5.8.0-rc1+ #11 Not tainted > > > > > [ 2.986118] ----------------------------- > > > > > [ 2.986120] swapper/0/1 is trying to lock: > > > > > [ 2.986122] ffffffff80f5ddd8 (console_owner){....}-{3:3}, at: console_unlock+0x284/0x820 > > > > > [ 2.986130] other info that might help us debug this: > > > > > [ 2.986132] context-{5:5} > > > > > [ 2.986134] 3 locks held by swapper/0/1: > > > > > [ 2.986135] #0: 98000007fa03c990 (&dev->mutex){....}-{0:0}, at: device_driver_attach+0x28/0x90 > > > > > [ 2.986144] #1: ffffffff80fb83a8 (pci_lock){....}-{2:2}, at: pci_bus_write_config_word+0x60/0xb8 > > > > > [ 2.986152] #2: ffffffff80f5ded0 (console_lock){+.+.}-{0:0}, at: vprintk_emit+0x1b0/0x3b8 > > > > > [ 2.986161] stack backtrace: > > > > > [ 2.986163] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.8.0-rc1+ #11 > > > > > [ 2.986164] Stack : 0000000000001d67 98000000030be9b0 0000000000000001 7b2aba74f6c4785b > > > > > [ 2.986172] 7b2aba74f6c4785b 0000000000000000 98000007f89cb438 ffffffff80e7dc80 > > > > > [ 2.986181] 0000000000000001 000000000000000a 0000000000000001 0000000000000001 > > > > > [ 2.986189] ffffffff80f4e156 fffffffffffffffd ffffffff80cc2d98 fffffffff8000000 > > > > > [ 2.986197] 0000000024000000 ffffffff80f40000 0000000000000000 0000000000000000 > > > > > [ 2.986205] ffffffff9500cce0 0000000000000000 ffffffff80f50000 ffffffff81546318 > > > > > [ 2.986213] ffffffff81c4c3c0 0000000000000018 ffffffffbc000000 0000000000000000 > > > > > [ 2.986221] ffffffff81340000 98000007f89c8000 98000007f89cb430 98000007f8a00000 > > > > > [ 2.986229] ffffffff806be568 0000000000000000 0000000000000000 0000000000000000 > > > > > [ 2.986237] 0000000000000000 0000000000000000 ffffffff80211c1c 7b2aba74f6c4785b > > > > > [ 2.986245] ... > > > > > [ 2.986250] Call Trace: > > > > > [ 2.986251] [<ffffffff80211c1c>] show_stack+0x9c/0x130 > > > > > [ 2.986253] [<ffffffff806be568>] dump_stack+0xe8/0x150 > > > > > [ 2.986255] [<ffffffff802ad408>] __lock_acquire+0x570/0x3250 > > > > > [ 2.986257] [<ffffffff802abed0>] lock_acquire+0x118/0x558 > > > > > [ 2.986259] [<ffffffff802be764>] console_unlock+0x2e4/0x820 > > > > > [ 2.986261] [<ffffffff802c0a68>] vprintk_emit+0x1c0/0x3b8 > > > > > [ 2.986263] [<ffffffff807f45a8>] dev_vprintk_emit+0x1c8/0x210 > > > > > [ 2.986265] [<ffffffff807f462c>] dev_printk_emit+0x3c/0x60 > > > > > [ 2.986267] [<ffffffff807f499c>] _dev_warn+0x5c/0x80 > > > > > [ 2.986269] [<ffffffff806eea9c>] pci_generic_config_write32+0x154/0x160 > > > > > [ 2.986271] [<ffffffff806edca4>] pci_bus_write_config_word+0x84/0xb8 > > > > > [ 2.986273] [<ffffffff806f1664>] pci_setup_device+0x22c/0x768 > > > > > [ 2.986275] [<ffffffff806f26a0>] pci_scan_single_device+0xc8/0x100 > > > > > [ 2.986277] [<ffffffff806f2788>] pci_scan_slot+0xb0/0x178 > > > > > [ 2.986279] [<ffffffff806f3ae4>] pci_scan_child_bus_extend+0x5c/0x370 > > > > > [ 2.986281] [<ffffffff806f407c>] pci_scan_root_bus_bridge+0x6c/0xf0 > > > > > [ 2.986283] [<ffffffff806f411c>] pci_host_probe+0x1c/0xd8 > > > > > [ 2.986285] [<ffffffff807fa03c>] platform_drv_probe+0x54/0xb8 > > > > > [ 2.986287] [<ffffffff807f71f8>] really_probe+0x130/0x388 > > > > > [ 2.986289] [<ffffffff807f7594>] driver_probe_device+0x64/0xd8 > > > > > [ 2.986291] [<ffffffff807f7844>] device_driver_attach+0x84/0x90 > > > > > [ 2.986293] [<ffffffff807f7918>] __driver_attach+0xc8/0x128 > > > > > [ 2.986295] [<ffffffff807f4cac>] bus_for_each_dev+0x74/0xd8 > > > > > [ 2.986297] [<ffffffff807f6408>] bus_add_driver+0x170/0x250 > > > > > [ 2.986299] [<ffffffff807f899c>] driver_register+0x84/0x150 > > > > > [ 2.986301] [<ffffffff80200b08>] do_one_initcall+0x98/0x458 > > > > > [ 2.986303] [<ffffffff810212dc>] kernel_init_freeable+0x2c0/0x36c > > > > > [ 2.986305] [<ffffffff80be7540>] kernel_init+0x10/0x128 > > > > > [ 2.986307] [<ffffffff80209d44>] ret_from_kernel_thread+0x14/0x1c > > > > > > > > > > Signed-off-by: Xingxing Su <suxingxing@loongson.cn> > > > > > --- > > > > > drivers/pci/access.c | 3 --- > > > > > 1 file changed, 3 deletions(-) > > > > > > > > > > diff --git a/drivers/pci/access.c b/drivers/pci/access.c > > > > > index 79c4a2e..b3fc164 100644 > > > > > --- a/drivers/pci/access.c > > > > > +++ b/drivers/pci/access.c > > > > > @@ -160,9 +160,6 @@ int pci_generic_config_write32(struct pci_bus *bus, unsigned int devfn, > > > > > * write happen to have any RW1C (write-one-to-clear) bits set, we > > > > > * just inadvertently cleared something we shouldn't have. > > > > > */ > > > > > - dev_warn_ratelimited(&bus->dev, "%d-byte config write to %04x:%02x:%02x.%d offset %#x may corrupt adjacent RW1C bits\n", > > > > > - size, pci_domain_nr(bus), bus->number, > > > > > - PCI_SLOT(devfn), PCI_FUNC(devfn), where); > > > > > > > > We just changed this printk (see [1]), but I think we still have this > > > > lockdep problem even after Mark's change. So I guess we need another > > > > think about this. > > > > > > > > Maybe we can print something when registering pci_ops that use > > > > pci_generic_config_write32()? > > > > > > That was my suggestion, but as Mark pointed out that doesn't work if > > > pci_generic_config_write32 is wrapped (which is 4 out of 8 cases). > > > > > > Also, 3 of the cases are only for the root bus (bridge). Are 32-bit > > > writes to a bridge going to cause problems? For xgene, interestingly, > > > with DT _write32 is needed, but for ACPI it is not (only _read32). I > > > think xgene is practically dead though a few people still have > > > systems, but likely xgene with DT is really dead. The ECAM case was > > > for QCom server which is also pretty much dead. SA1100 nano-engine is > > > really old and something only a few people have at most (Russell > > > King). So ignoring all those, we're left with just loongson and iproc. > > > Maybe just remove the warning? > > > > Sigh, removing it sounds like the best option. > > Hi Bjorn - Was this lockdep issue fixed in a different way than removing > the use of printk? I'm mostly interested in reducing the RW1C warnings > as Mark suggested here: > > https://lore.kernel.org/lkml/20200806041455.11070-1-mark.tomlinson@alliedtelesis.co.nz/ > > While thinking how best to rejuvenate that patch, I came across this > thread in the archives and it seems like the plan shifted to completely > removing this printk callsite to address the lockdep issue. However, > neither patch ended up in Linus' tree nor in the current PCI tree (I > only peeked in the next branch). > > What's everyone's preference at this point? Hi Bjorn - friendly ping on this topic. Thanks! Tyler > > Thanks! > > Tyler ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-06-14 17:43 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-08-01 1:25 [PATCH] PCI: Don't use Printk in raw_spinlocks Xingxing Su 2020-09-10 2:07 ` Bjorn Helgaas 2020-09-10 14:21 ` Rob Herring 2020-09-10 16:07 ` Scott Branden 2020-09-10 18:46 ` Bjorn Helgaas 2020-09-10 18:50 ` Florian Fainelli 2020-09-10 19:20 ` Bjorn Helgaas 2020-09-10 19:22 ` Florian Fainelli 2021-03-30 20:53 ` Tyler Hicks 2021-06-14 17:43 ` Tyler Hicks
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).