diff mbox

PCI: export MSI mode using attributes, not kobjects

Message ID CAE9FiQUeyxN5OAaXqfhdQkzTwjV83BN-L3r6kH0sO0t1b1-63Q@mail.gmail.com
State Accepted
Headers show

Commit Message

Yinghai Lu Dec. 12, 2013, 11:17 p.m. UTC
On Sat, Dec 7, 2013 at 12:41 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Wed, Nov 27, 2013 at 11:46 AM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
>> From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>
>> The PCI MSI sysfs code is a mess with kobjects for things that don't
>> really need to be kobjects.  This patch creates attributes dynamically
>> for the MSI interrupts instead of using kobjects.
>>
>> Note, this removes a directory from the current MSI interrupt sysfs
>> code:
>>
>> old MSI kobjects:
>> pci_device
>>    └── msi_irqs
>>        └── 40
>>            └── mode
>>
>> new MSI attributes:
>> pci_device
>>    └── msi_irqs
>>        └── 40
>>
>> As there was only one file "mode" with the kobject model, the interrupt
>> number is now a file that returns the "mode" of the interrupt (msi vs.
>> msix).
>>
>> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>
> I added the acks from Neil and Veaceslav, folded in the
> Documentation/ABI update, and applied the whole thing to my pci/misc
> branch for v3.14.  Thanks!

got:
[   71.429735] BUG: key ffff887fcf082a58 not in .data!
[   71.429737] ------------[ cut here ]------------
[   71.429742] WARNING: CPU: 0 PID: 4 at kernel/locking/lockdep.c:2987 lockdep_i
nit_map+0x127/0x5b0()
[   71.429743] DEBUG_LOCKS_WARN_ON(1)
[   71.429744] Modules linked in:
[   71.429747] CPU: 0 PID: 4 Comm: kworker/0:0 Tainted: G          I  3.13.0-rc3
-yh-01187-ge0e4e4e-dirty #45
[   71.429757] Workqueue: events work_for_cpu_fn
[   71.429792]  0000000000000009 ffff881fcf95da58 ffffffff82020475 ffff881fcf95d
aa0
[   71.429800]  ffff881fcf95da90 ffffffff81097c3d ffff887fcf082a58 ffff88dfcec1a
d28
[   71.429809]  0000000000000000 0000000000000000 ffff889fcf2eecd0 ffff881fcf95d
af0
[   71.429809] Call Trace:
[   71.429814]  [<ffffffff82020475>] dump_stack+0x45/0x56
[   71.429818]  [<ffffffff81097c3d>] warn_slowpath_common+0x7d/0xa0
[   71.429822]  [<ffffffff81097cac>] warn_slowpath_fmt+0x4c/0x50
[   71.429826]  [<ffffffff810e3d97>] lockdep_init_map+0x127/0x5b0
[   71.429835]  [<ffffffff81242fe8>] ? sysfs_new_dirent+0x98/0x140
[   71.429839]  [<ffffffff81242033>] sysfs_add_file_mode_ns+0x63/0xc0
[   71.429843]  [<ffffffff8124491d>] internal_create_group+0x18d/0x260
[   71.429853]  [<ffffffff81556345>] ? populate_msi_sysfs+0x185/0x1d0
[   71.429857]  [<ffffffff81244a52>] sysfs_create_groups+0x42/0xa0
[   71.429861]  [<ffffffff81556367>] populate_msi_sysfs+0x1a7/0x1d0
[   71.429865]  [<ffffffff81556b67>] pci_enable_msi_block+0x1f7/0x2a0
[   71.429870]  [<ffffffff81549795>] pcie_port_device_register+0x335/0x520
[   71.429874]  [<ffffffff81549f08>] pcie_portdrv_probe+0x68/0xa0
[   71.429883]  [<ffffffff8153cf75>] local_pci_probe+0x45/0xa0
[   71.429887]  [<ffffffff810b0af4>] work_for_cpu_fn+0x14/0x20
[   71.429891]  [<ffffffff810b431b>] process_one_work+0x28b/0x4a0
[   71.429895]  [<ffffffff810b4292>] ? process_one_work+0x202/0x4a0
[   71.429899]  [<ffffffff810b55db>] worker_thread+0x26b/0x3a0
[   71.429903]  [<ffffffff810e262d>] ? trace_hardirqs_on+0xd/0x10
[   71.429906]  [<ffffffff810b5370>] ? manage_workers.isra.17+0x340/0x340
[   71.429912]  [<ffffffff810bc031>] kthread+0x111/0x120
[   71.429919]  [<ffffffff810cddab>] ? local_clock+0x2b/0x40
[   71.429923]  [<ffffffff810bbf20>] ? kthread_stop+0xf0/0xf0
[   71.429927]  [<ffffffff8203842c>] ret_from_fork+0x7c/0xb0
[   71.429931]  [<ffffffff810bbf20>] ? kthread_stop+0xf0/0xf0
[   71.429933] ---[ end trace c511e3d74efea94e ]---

looks like Greg forgot adding attr init.

Can you fold attached patch into Greg's patch ?

Comments

Greg KH Dec. 12, 2013, 11:22 p.m. UTC | #1
On Thu, Dec 12, 2013 at 03:17:50PM -0800, Yinghai Lu wrote:
> On Sat, Dec 7, 2013 at 12:41 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> > On Wed, Nov 27, 2013 at 11:46 AM, Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> >> From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >>
> >> The PCI MSI sysfs code is a mess with kobjects for things that don't
> >> really need to be kobjects.  This patch creates attributes dynamically
> >> for the MSI interrupts instead of using kobjects.
> >>
> >> Note, this removes a directory from the current MSI interrupt sysfs
> >> code:
> >>
> >> old MSI kobjects:
> >> pci_device
> >>    └── msi_irqs
> >>        └── 40
> >>            └── mode
> >>
> >> new MSI attributes:
> >> pci_device
> >>    └── msi_irqs
> >>        └── 40
> >>
> >> As there was only one file "mode" with the kobject model, the interrupt
> >> number is now a file that returns the "mode" of the interrupt (msi vs.
> >> msix).
> >>
> >> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >
> > I added the acks from Neil and Veaceslav, folded in the
> > Documentation/ABI update, and applied the whole thing to my pci/misc
> > branch for v3.14.  Thanks!
> 
> got:
> [   71.429735] BUG: key ffff887fcf082a58 not in .data!
> [   71.429737] ------------[ cut here ]------------
> [   71.429742] WARNING: CPU: 0 PID: 4 at kernel/locking/lockdep.c:2987 lockdep_i
> nit_map+0x127/0x5b0()
> [   71.429743] DEBUG_LOCKS_WARN_ON(1)
> [   71.429744] Modules linked in:
> [   71.429747] CPU: 0 PID: 4 Comm: kworker/0:0 Tainted: G          I  3.13.0-rc3
> -yh-01187-ge0e4e4e-dirty #45
> [   71.429757] Workqueue: events work_for_cpu_fn
> [   71.429792]  0000000000000009 ffff881fcf95da58 ffffffff82020475 ffff881fcf95d
> aa0
> [   71.429800]  ffff881fcf95da90 ffffffff81097c3d ffff887fcf082a58 ffff88dfcec1a
> d28
> [   71.429809]  0000000000000000 0000000000000000 ffff889fcf2eecd0 ffff881fcf95d
> af0
> [   71.429809] Call Trace:
> [   71.429814]  [<ffffffff82020475>] dump_stack+0x45/0x56
> [   71.429818]  [<ffffffff81097c3d>] warn_slowpath_common+0x7d/0xa0
> [   71.429822]  [<ffffffff81097cac>] warn_slowpath_fmt+0x4c/0x50
> [   71.429826]  [<ffffffff810e3d97>] lockdep_init_map+0x127/0x5b0
> [   71.429835]  [<ffffffff81242fe8>] ? sysfs_new_dirent+0x98/0x140
> [   71.429839]  [<ffffffff81242033>] sysfs_add_file_mode_ns+0x63/0xc0
> [   71.429843]  [<ffffffff8124491d>] internal_create_group+0x18d/0x260
> [   71.429853]  [<ffffffff81556345>] ? populate_msi_sysfs+0x185/0x1d0
> [   71.429857]  [<ffffffff81244a52>] sysfs_create_groups+0x42/0xa0
> [   71.429861]  [<ffffffff81556367>] populate_msi_sysfs+0x1a7/0x1d0
> [   71.429865]  [<ffffffff81556b67>] pci_enable_msi_block+0x1f7/0x2a0
> [   71.429870]  [<ffffffff81549795>] pcie_port_device_register+0x335/0x520
> [   71.429874]  [<ffffffff81549f08>] pcie_portdrv_probe+0x68/0xa0
> [   71.429883]  [<ffffffff8153cf75>] local_pci_probe+0x45/0xa0
> [   71.429887]  [<ffffffff810b0af4>] work_for_cpu_fn+0x14/0x20
> [   71.429891]  [<ffffffff810b431b>] process_one_work+0x28b/0x4a0
> [   71.429895]  [<ffffffff810b4292>] ? process_one_work+0x202/0x4a0
> [   71.429899]  [<ffffffff810b55db>] worker_thread+0x26b/0x3a0
> [   71.429903]  [<ffffffff810e262d>] ? trace_hardirqs_on+0xd/0x10
> [   71.429906]  [<ffffffff810b5370>] ? manage_workers.isra.17+0x340/0x340
> [   71.429912]  [<ffffffff810bc031>] kthread+0x111/0x120
> [   71.429919]  [<ffffffff810cddab>] ? local_clock+0x2b/0x40
> [   71.429923]  [<ffffffff810bbf20>] ? kthread_stop+0xf0/0xf0
> [   71.429927]  [<ffffffff8203842c>] ret_from_fork+0x7c/0xb0
> [   71.429931]  [<ffffffff810bbf20>] ? kthread_stop+0xf0/0xf0
> [   71.429933] ---[ end trace c511e3d74efea94e ]---
> 
> looks like Greg forgot adding attr init.
> 
> Can you fold attached patch into Greg's patch ?
> 
> Index: linux-2.6/drivers/pci/msi.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/msi.c
> +++ linux-2.6/drivers/pci/msi.c
> @@ -547,6 +547,7 @@ static int populate_msi_sysfs(struct pci
>                 msi_dev_attr = kzalloc(sizeof(*msi_dev_attr), GFP_KERNEL);
>                 if (!msi_dev_attr)
>                         return -ENOMEM;
> +               sysfs_attr_init(&msi_dev_attr->attr);
>                 sprintf(name, "%d", entry->irq);
>                 msi_dev_attr->attr.name = name;
>                 msi_dev_attr->attr.mode = S_IRUGO;

> ---
>  drivers/pci/msi.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> Index: linux-2.6/drivers/pci/msi.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/msi.c
> +++ linux-2.6/drivers/pci/msi.c
> @@ -547,6 +547,7 @@ static int populate_msi_sysfs(struct pci
>  		msi_dev_attr = kzalloc(sizeof(*msi_dev_attr), GFP_KERNEL);
>  		if (!msi_dev_attr)
>  			return -ENOMEM;
> +		sysfs_attr_init(&msi_dev_attr->attr);
>  		sprintf(name, "%d", entry->irq);
>  		msi_dev_attr->attr.name = name;
>  		msi_dev_attr->attr.mode = S_IRUGO;

Ah, crap, yes, you are right, my fault, very nice catch.

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bjorn Helgaas Dec. 12, 2013, 11:56 p.m. UTC | #2
On Thu, Dec 12, 2013 at 4:17 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Sat, Dec 7, 2013 at 12:41 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> On Wed, Nov 27, 2013 at 11:46 AM, Greg Kroah-Hartman
>> <gregkh@linuxfoundation.org> wrote:
>>> From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>>
>>> The PCI MSI sysfs code is a mess with kobjects for things that don't
>>> really need to be kobjects.  This patch creates attributes dynamically
>>> for the MSI interrupts instead of using kobjects.
>>>
>>> Note, this removes a directory from the current MSI interrupt sysfs
>>> code:
>>>
>>> old MSI kobjects:
>>> pci_device
>>>    └── msi_irqs
>>>        └── 40
>>>            └── mode
>>>
>>> new MSI attributes:
>>> pci_device
>>>    └── msi_irqs
>>>        └── 40
>>>
>>> As there was only one file "mode" with the kobject model, the interrupt
>>> number is now a file that returns the "mode" of the interrupt (msi vs.
>>> msix).
>>>
>>> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>
>> I added the acks from Neil and Veaceslav, folded in the
>> Documentation/ABI update, and applied the whole thing to my pci/misc
>> branch for v3.14.  Thanks!
>
> got:
> [   71.429735] BUG: key ffff887fcf082a58 not in .data!
> [   71.429737] ------------[ cut here ]------------
> [   71.429742] WARNING: CPU: 0 PID: 4 at kernel/locking/lockdep.c:2987 lockdep_i
> nit_map+0x127/0x5b0()
> [   71.429743] DEBUG_LOCKS_WARN_ON(1)
> [   71.429744] Modules linked in:
> [   71.429747] CPU: 0 PID: 4 Comm: kworker/0:0 Tainted: G          I  3.13.0-rc3
> -yh-01187-ge0e4e4e-dirty #45
> [   71.429757] Workqueue: events work_for_cpu_fn
> [   71.429792]  0000000000000009 ffff881fcf95da58 ffffffff82020475 ffff881fcf95d
> aa0
> [   71.429800]  ffff881fcf95da90 ffffffff81097c3d ffff887fcf082a58 ffff88dfcec1a
> d28
> [   71.429809]  0000000000000000 0000000000000000 ffff889fcf2eecd0 ffff881fcf95d
> af0
> [   71.429809] Call Trace:
> [   71.429814]  [<ffffffff82020475>] dump_stack+0x45/0x56
> [   71.429818]  [<ffffffff81097c3d>] warn_slowpath_common+0x7d/0xa0
> [   71.429822]  [<ffffffff81097cac>] warn_slowpath_fmt+0x4c/0x50
> [   71.429826]  [<ffffffff810e3d97>] lockdep_init_map+0x127/0x5b0
> [   71.429835]  [<ffffffff81242fe8>] ? sysfs_new_dirent+0x98/0x140
> [   71.429839]  [<ffffffff81242033>] sysfs_add_file_mode_ns+0x63/0xc0
> [   71.429843]  [<ffffffff8124491d>] internal_create_group+0x18d/0x260
> [   71.429853]  [<ffffffff81556345>] ? populate_msi_sysfs+0x185/0x1d0
> [   71.429857]  [<ffffffff81244a52>] sysfs_create_groups+0x42/0xa0
> [   71.429861]  [<ffffffff81556367>] populate_msi_sysfs+0x1a7/0x1d0
> [   71.429865]  [<ffffffff81556b67>] pci_enable_msi_block+0x1f7/0x2a0
> [   71.429870]  [<ffffffff81549795>] pcie_port_device_register+0x335/0x520
> [   71.429874]  [<ffffffff81549f08>] pcie_portdrv_probe+0x68/0xa0
> [   71.429883]  [<ffffffff8153cf75>] local_pci_probe+0x45/0xa0
> [   71.429887]  [<ffffffff810b0af4>] work_for_cpu_fn+0x14/0x20
> [   71.429891]  [<ffffffff810b431b>] process_one_work+0x28b/0x4a0
> [   71.429895]  [<ffffffff810b4292>] ? process_one_work+0x202/0x4a0
> [   71.429899]  [<ffffffff810b55db>] worker_thread+0x26b/0x3a0
> [   71.429903]  [<ffffffff810e262d>] ? trace_hardirqs_on+0xd/0x10
> [   71.429906]  [<ffffffff810b5370>] ? manage_workers.isra.17+0x340/0x340
> [   71.429912]  [<ffffffff810bc031>] kthread+0x111/0x120
> [   71.429919]  [<ffffffff810cddab>] ? local_clock+0x2b/0x40
> [   71.429923]  [<ffffffff810bbf20>] ? kthread_stop+0xf0/0xf0
> [   71.429927]  [<ffffffff8203842c>] ret_from_fork+0x7c/0xb0
> [   71.429931]  [<ffffffff810bbf20>] ? kthread_stop+0xf0/0xf0
> [   71.429933] ---[ end trace c511e3d74efea94e ]---
>
> looks like Greg forgot adding attr init.
>
> Can you fold attached patch into Greg's patch ?

Sure.  I don't know how to reproduce it, so I couldn't verify the fix,
but I added it to Greg's patch.  Thanks!

> Index: linux-2.6/drivers/pci/msi.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/msi.c
> +++ linux-2.6/drivers/pci/msi.c
> @@ -547,6 +547,7 @@ static int populate_msi_sysfs(struct pci
>                 msi_dev_attr = kzalloc(sizeof(*msi_dev_attr), GFP_KERNEL);
>                 if (!msi_dev_attr)
>                         return -ENOMEM;
> +               sysfs_attr_init(&msi_dev_attr->attr);
>                 sprintf(name, "%d", entry->irq);
>                 msi_dev_attr->attr.name = name;
>                 msi_dev_attr->attr.mode = S_IRUGO;
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Greg KH Dec. 13, 2013, 2:06 a.m. UTC | #3
On Thu, Dec 12, 2013 at 04:56:20PM -0700, Bjorn Helgaas wrote:
> On Thu, Dec 12, 2013 at 4:17 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> > On Sat, Dec 7, 2013 at 12:41 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> >> On Wed, Nov 27, 2013 at 11:46 AM, Greg Kroah-Hartman
> >> <gregkh@linuxfoundation.org> wrote:
> >>> From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >>>
> >>> The PCI MSI sysfs code is a mess with kobjects for things that don't
> >>> really need to be kobjects.  This patch creates attributes dynamically
> >>> for the MSI interrupts instead of using kobjects.
> >>>
> >>> Note, this removes a directory from the current MSI interrupt sysfs
> >>> code:
> >>>
> >>> old MSI kobjects:
> >>> pci_device
> >>>    └── msi_irqs
> >>>        └── 40
> >>>            └── mode
> >>>
> >>> new MSI attributes:
> >>> pci_device
> >>>    └── msi_irqs
> >>>        └── 40
> >>>
> >>> As there was only one file "mode" with the kobject model, the interrupt
> >>> number is now a file that returns the "mode" of the interrupt (msi vs.
> >>> msix).
> >>>
> >>> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >>
> >> I added the acks from Neil and Veaceslav, folded in the
> >> Documentation/ABI update, and applied the whole thing to my pci/misc
> >> branch for v3.14.  Thanks!
> >
> > got:
> > [   71.429735] BUG: key ffff887fcf082a58 not in .data!
> > [   71.429737] ------------[ cut here ]------------
> > [   71.429742] WARNING: CPU: 0 PID: 4 at kernel/locking/lockdep.c:2987 lockdep_i
> > nit_map+0x127/0x5b0()
> > [   71.429743] DEBUG_LOCKS_WARN_ON(1)
> > [   71.429744] Modules linked in:
> > [   71.429747] CPU: 0 PID: 4 Comm: kworker/0:0 Tainted: G          I  3.13.0-rc3
> > -yh-01187-ge0e4e4e-dirty #45
> > [   71.429757] Workqueue: events work_for_cpu_fn
> > [   71.429792]  0000000000000009 ffff881fcf95da58 ffffffff82020475 ffff881fcf95d
> > aa0
> > [   71.429800]  ffff881fcf95da90 ffffffff81097c3d ffff887fcf082a58 ffff88dfcec1a
> > d28
> > [   71.429809]  0000000000000000 0000000000000000 ffff889fcf2eecd0 ffff881fcf95d
> > af0
> > [   71.429809] Call Trace:
> > [   71.429814]  [<ffffffff82020475>] dump_stack+0x45/0x56
> > [   71.429818]  [<ffffffff81097c3d>] warn_slowpath_common+0x7d/0xa0
> > [   71.429822]  [<ffffffff81097cac>] warn_slowpath_fmt+0x4c/0x50
> > [   71.429826]  [<ffffffff810e3d97>] lockdep_init_map+0x127/0x5b0
> > [   71.429835]  [<ffffffff81242fe8>] ? sysfs_new_dirent+0x98/0x140
> > [   71.429839]  [<ffffffff81242033>] sysfs_add_file_mode_ns+0x63/0xc0
> > [   71.429843]  [<ffffffff8124491d>] internal_create_group+0x18d/0x260
> > [   71.429853]  [<ffffffff81556345>] ? populate_msi_sysfs+0x185/0x1d0
> > [   71.429857]  [<ffffffff81244a52>] sysfs_create_groups+0x42/0xa0
> > [   71.429861]  [<ffffffff81556367>] populate_msi_sysfs+0x1a7/0x1d0
> > [   71.429865]  [<ffffffff81556b67>] pci_enable_msi_block+0x1f7/0x2a0
> > [   71.429870]  [<ffffffff81549795>] pcie_port_device_register+0x335/0x520
> > [   71.429874]  [<ffffffff81549f08>] pcie_portdrv_probe+0x68/0xa0
> > [   71.429883]  [<ffffffff8153cf75>] local_pci_probe+0x45/0xa0
> > [   71.429887]  [<ffffffff810b0af4>] work_for_cpu_fn+0x14/0x20
> > [   71.429891]  [<ffffffff810b431b>] process_one_work+0x28b/0x4a0
> > [   71.429895]  [<ffffffff810b4292>] ? process_one_work+0x202/0x4a0
> > [   71.429899]  [<ffffffff810b55db>] worker_thread+0x26b/0x3a0
> > [   71.429903]  [<ffffffff810e262d>] ? trace_hardirqs_on+0xd/0x10
> > [   71.429906]  [<ffffffff810b5370>] ? manage_workers.isra.17+0x340/0x340
> > [   71.429912]  [<ffffffff810bc031>] kthread+0x111/0x120
> > [   71.429919]  [<ffffffff810cddab>] ? local_clock+0x2b/0x40
> > [   71.429923]  [<ffffffff810bbf20>] ? kthread_stop+0xf0/0xf0
> > [   71.429927]  [<ffffffff8203842c>] ret_from_fork+0x7c/0xb0
> > [   71.429931]  [<ffffffff810bbf20>] ? kthread_stop+0xf0/0xf0
> > [   71.429933] ---[ end trace c511e3d74efea94e ]---
> >
> > looks like Greg forgot adding attr init.
> >
> > Can you fold attached patch into Greg's patch ?
> 
> Sure.  I don't know how to reproduce it, so I couldn't verify the fix,
> but I added it to Greg's patch.  Thanks!

I think lockdep has to be enabled for this to show up, I wasn't running
with that enabled when I tested the code, my fault, sorry.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bjorn Helgaas Dec. 13, 2013, 4:32 a.m. UTC | #4
On Thu, Dec 12, 2013 at 7:06 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Thu, Dec 12, 2013 at 04:56:20PM -0700, Bjorn Helgaas wrote:
>> On Thu, Dec 12, 2013 at 4:17 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> > On Sat, Dec 7, 2013 at 12:41 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> >> On Wed, Nov 27, 2013 at 11:46 AM, Greg Kroah-Hartman
>> >> <gregkh@linuxfoundation.org> wrote:
>> >>> From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> >>>
>> >>> The PCI MSI sysfs code is a mess with kobjects for things that don't
>> >>> really need to be kobjects.  This patch creates attributes dynamically
>> >>> for the MSI interrupts instead of using kobjects.
>> >>>
>> >>> Note, this removes a directory from the current MSI interrupt sysfs
>> >>> code:
>> >>>
>> >>> old MSI kobjects:
>> >>> pci_device
>> >>>    └── msi_irqs
>> >>>        └── 40
>> >>>            └── mode
>> >>>
>> >>> new MSI attributes:
>> >>> pci_device
>> >>>    └── msi_irqs
>> >>>        └── 40
>> >>>
>> >>> As there was only one file "mode" with the kobject model, the interrupt
>> >>> number is now a file that returns the "mode" of the interrupt (msi vs.
>> >>> msix).
>> >>>
>> >>> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> >>
>> >> I added the acks from Neil and Veaceslav, folded in the
>> >> Documentation/ABI update, and applied the whole thing to my pci/misc
>> >> branch for v3.14.  Thanks!
>> >
>> > got:
>> > [   71.429735] BUG: key ffff887fcf082a58 not in .data!
>> > [   71.429737] ------------[ cut here ]------------
>> > [   71.429742] WARNING: CPU: 0 PID: 4 at kernel/locking/lockdep.c:2987 lockdep_i
>> > nit_map+0x127/0x5b0()
>> > [   71.429743] DEBUG_LOCKS_WARN_ON(1)
>> > [   71.429744] Modules linked in:
>> > [   71.429747] CPU: 0 PID: 4 Comm: kworker/0:0 Tainted: G          I  3.13.0-rc3
>> > -yh-01187-ge0e4e4e-dirty #45
>> > [   71.429757] Workqueue: events work_for_cpu_fn
>> > [   71.429792]  0000000000000009 ffff881fcf95da58 ffffffff82020475 ffff881fcf95d
>> > aa0
>> > [   71.429800]  ffff881fcf95da90 ffffffff81097c3d ffff887fcf082a58 ffff88dfcec1a
>> > d28
>> > [   71.429809]  0000000000000000 0000000000000000 ffff889fcf2eecd0 ffff881fcf95d
>> > af0
>> > [   71.429809] Call Trace:
>> > [   71.429814]  [<ffffffff82020475>] dump_stack+0x45/0x56
>> > [   71.429818]  [<ffffffff81097c3d>] warn_slowpath_common+0x7d/0xa0
>> > [   71.429822]  [<ffffffff81097cac>] warn_slowpath_fmt+0x4c/0x50
>> > [   71.429826]  [<ffffffff810e3d97>] lockdep_init_map+0x127/0x5b0
>> > [   71.429835]  [<ffffffff81242fe8>] ? sysfs_new_dirent+0x98/0x140
>> > [   71.429839]  [<ffffffff81242033>] sysfs_add_file_mode_ns+0x63/0xc0
>> > [   71.429843]  [<ffffffff8124491d>] internal_create_group+0x18d/0x260
>> > [   71.429853]  [<ffffffff81556345>] ? populate_msi_sysfs+0x185/0x1d0
>> > [   71.429857]  [<ffffffff81244a52>] sysfs_create_groups+0x42/0xa0
>> > [   71.429861]  [<ffffffff81556367>] populate_msi_sysfs+0x1a7/0x1d0
>> > [   71.429865]  [<ffffffff81556b67>] pci_enable_msi_block+0x1f7/0x2a0
>> > [   71.429870]  [<ffffffff81549795>] pcie_port_device_register+0x335/0x520
>> > [   71.429874]  [<ffffffff81549f08>] pcie_portdrv_probe+0x68/0xa0
>> > [   71.429883]  [<ffffffff8153cf75>] local_pci_probe+0x45/0xa0
>> > [   71.429887]  [<ffffffff810b0af4>] work_for_cpu_fn+0x14/0x20
>> > [   71.429891]  [<ffffffff810b431b>] process_one_work+0x28b/0x4a0
>> > [   71.429895]  [<ffffffff810b4292>] ? process_one_work+0x202/0x4a0
>> > [   71.429899]  [<ffffffff810b55db>] worker_thread+0x26b/0x3a0
>> > [   71.429903]  [<ffffffff810e262d>] ? trace_hardirqs_on+0xd/0x10
>> > [   71.429906]  [<ffffffff810b5370>] ? manage_workers.isra.17+0x340/0x340
>> > [   71.429912]  [<ffffffff810bc031>] kthread+0x111/0x120
>> > [   71.429919]  [<ffffffff810cddab>] ? local_clock+0x2b/0x40
>> > [   71.429923]  [<ffffffff810bbf20>] ? kthread_stop+0xf0/0xf0
>> > [   71.429927]  [<ffffffff8203842c>] ret_from_fork+0x7c/0xb0
>> > [   71.429931]  [<ffffffff810bbf20>] ? kthread_stop+0xf0/0xf0
>> > [   71.429933] ---[ end trace c511e3d74efea94e ]---
>> >
>> > looks like Greg forgot adding attr init.
>> >
>> > Can you fold attached patch into Greg's patch ?
>>
>> Sure.  I don't know how to reproduce it, so I couldn't verify the fix,
>> but I added it to Greg's patch.  Thanks!
>
> I think lockdep has to be enabled for this to show up, I wasn't running
> with that enabled when I tested the code, my fault, sorry.

No problem.  I have

CONFIG_LOCKDEP_SUPPORT=y
CONFIG_LOCKDEP=y
CONFIG_DEBUG_LOCKDEP=y

and I still don't see the problem booting the qemu q35 model.
Whatever, I folded it in anyway.

Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yinghai Lu Dec. 13, 2013, 5:59 a.m. UTC | #5
On Thu, Dec 12, 2013 at 8:32 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> No problem.  I have
>
> CONFIG_LOCKDEP_SUPPORT=y
> CONFIG_LOCKDEP=y
> CONFIG_DEBUG_LOCKDEP=y
>
> and I still don't see the problem booting the qemu q35 model.
> Whatever, I folded it in anyway.

Does qemu q35 use MSI?

Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bjorn Helgaas Dec. 13, 2013, 3:43 p.m. UTC | #6
On Thu, Dec 12, 2013 at 10:59 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Thu, Dec 12, 2013 at 8:32 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> No problem.  I have
>>
>> CONFIG_LOCKDEP_SUPPORT=y
>> CONFIG_LOCKDEP=y
>> CONFIG_DEBUG_LOCKDEP=y
>>
>> and I still don't see the problem booting the qemu q35 model.
>> Whatever, I folded it in anyway.
>
> Does qemu q35 use MSI?

Oops, sorry, I *do* see the problem on qemu q35.  I must have messed
something up when I tried it yesterday.  And I verified that your
change fixes it.  Sorry for the noise.

Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

---
 drivers/pci/msi.c |    1 +
 1 file changed, 1 insertion(+)

Index: linux-2.6/drivers/pci/msi.c
===================================================================
--- linux-2.6.orig/drivers/pci/msi.c
+++ linux-2.6/drivers/pci/msi.c
@@ -547,6 +547,7 @@  static int populate_msi_sysfs(struct pci
 		msi_dev_attr = kzalloc(sizeof(*msi_dev_attr), GFP_KERNEL);
 		if (!msi_dev_attr)
 			return -ENOMEM;
+		sysfs_attr_init(&msi_dev_attr->attr);
 		sprintf(name, "%d", entry->irq);
 		msi_dev_attr->attr.name = name;
 		msi_dev_attr->attr.mode = S_IRUGO;