New: BUG: using smp_processor_id() in preemptible [00000001] code: nfsd/2783
Subsystems (2)
It looks like qla2x00_queuecommand()'s use of smp_processor_id() is just a heuristic, in which case it should be sufficient to simply use _smp_processor_id() to quash the warning.
Begin forwarded message:
Date: Fri, 4 Feb 2005 13:37:14 -0800
From: [email protected]
To: [email protected]
Subject: [Bugme-new] [Bug 4166] New: BUG: using smp_processor_id() in preemptible [00000001] code: nfsd/2783
http://bugme.osdl.org/show_bug.cgi?id=4166
Summary: BUG: using smp_processor_id() in preemptible [00000001]
code: nfsd/2783
Kernel Version: 2.6.11-rc3
Status: NEW
Severity: normal
Owner: [email protected]
Submitter: [email protected]Distribution: debian sid
Hardware Environment: Fibre Channel: QLogic Corp. QLA2312 Fibre Channel Adapter Problem Description:
Kernel floods logs with
caller is qla2x00_queuecommand+0x20c/0x660 [qla2xxx]
[<c023f926>] smp_processor_id+0xa6/0xc0 [<f8bee89c>] qla2x00_queuecommand+0x20c/0x660 [qla2xxx] [<f8bee89c>] qla2x00_queuecommand+0x20c/0x660 [qla2xxx]
....
Steps to reproduce:
didn't try to reproduce but the bug came with upgrade from 2.6.8.1 to 2.6.11-rc3 on the system with QLA2312
The rest of the nodes in the cluster which do not have Qlogic run fine with the kernel
Here is full bt
BUG: using smp_processor_id() in preemptible [00000001] code: nfsd/2783 caller is qla2x00_queuecommand+0x20c/0x660 [qla2xxx] [<c023f926>] smp_processor_id+0xa6/0xc0 [<f8bee89c>] qla2x00_queuecommand+0x20c/0x660 [qla2xxx] [<f8bee89c>] qla2x00_queuecommand+0x20c/0x660 [qla2xxx] [<c0407766>] _spin_lock+0x16/0x90 [<c0407bbf>] _spin_unlock_irqrestore+0xf/0x30 [<c01231d8>] __mod_timer+0xe8/0x130 [<c02e20e5>] scsi_dispatch_cmd+0x135/0x1d0 [<c02e22e0>] scsi_done+0x0/0x30 [<c02e4c10>] scsi_times_out+0x0/0xb0 [<c02e7977>] scsi_request_fn+0x1c7/0x350 [<c02a2560>] elv_next_request+0x50/0x110 [<c02a498b>] __generic_unplug_device+0x3b/0x40 [<c02a49ae>] generic_unplug_device+0x1e/0x30 [<c02a49db>] blk_backing_dev_unplug+0x1b/0x20 [<c0158a4e>] sync_buffer+0x4e/0x50 [<c0407196>] __wait_on_bit+0x56/0x60 [<c0158a00>] sync_buffer+0x0/0x50 [<c0158a00>] sync_buffer+0x0/0x50 [<c0407231>] out_of_line_wait_on_bit+0x91/0xa0 [<c012f360>] wake_bit_function+0x0/0x60 [<c012f360>] wake_bit_function+0x0/0x60 [<c01b3b8f>] search_by_key+0xef/0x1180 [<c01b39fd>] is_tree_node+0x6d/0x70 [<c03a5d54>] kfree_skbmem+0x24/0x30 [<c03abdba>] net_tx_action+0x4a/0x120 [<c011f6e2>] __do_softirq+0x62/0xd0 [<c019fca1>] reiserfs_read_locked_inode+0x71/0x110 [<c019fd40>] reiserfs_find_actor+0x0/0x30 [<c019fe13>] reiserfs_iget+0xa3/0xb0 [<c019fd40>] reiserfs_find_actor+0x0/0x30 [<c019fc10>] reiserfs_init_locked_inode+0x0/0x20 [<c019abda>] reiserfs_lookup+0x16a/0x190 [<c0170692>] __d_lookup+0x92/0x140 [<c0407bbf>] _spin_unlock_irqrestore+0xf/0x30 [<c01700be>] d_alloc+0x13e/0x1a0 [<c0166c35>] __lookup_hash+0xa5/0xe0 [<c0166c8f>] lookup_hash+0x1f/0x30 [<c0166d01>] lookup_one_len+0x61/0x70 [<f919c2d2>] nfsd_lookup+0x112/0x4c0 [nfsd] [<c01290cd>] groups_alloc+0x3d/0xc0 [<f91a47a1>] nfsd3_proc_lookup+0xa1/0xe0 [nfsd] [<f91996a9>] nfsd_dispatch+0xd9/0x230 [nfsd] [<f8ea0a22>] svc_process+0x4a2/0x680 [sunrpc] [<c0116240>] default_wake_function+0x0/0x20 [<f919944a>] nfsd+0x1ba/0x340 [nfsd] [<f9199290>] nfsd+0x0/0x340 [nfsd] [<c01009d1>] kernel_thread_helper+0x5/0x14
printk: 460 messages suppressed.
repeats
more details about the system can be found from
http://www.onerussian.com/Linux/bugs/020405ra
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
Quoted 4 lines · Andrew Morton
It looks like qla2x00_queuecommand()'s use of
smp_processor_id()is just a heuristic, in which case it should be sufficient to simply use_smp_processor_id()to quash the warning.
Hmm, this was also reported by another customer on Friday -- hopefully the attached patch can make it in by 2.6.11...
Hopeully once the dust settles with the transport stuffs, a bulk of this code can be removed...
Thanks,
AV
===== drivers/scsi/qla2xxx/qla_os.c 1.53 vs edited =====
drivers/scsi/qla2xxx/qla_os.c +1 -1
@@ -859,7 +859,7 @@* In all other cases we'll let an irq pick up our IO and submit it * to the controller to improve affinity. */ -if (smp_processor_id () == ha->last_irq_cpu || was_empty) +if (_smp_processor_id () == ha->last_irq_cpu || was_empty)qla2x00_next (ha);spin_lock_irq (ha->host->host_lock);
drivers/scsi/qla2xxx/qla_os.c line 862
Quoted 3 lines · Andrew Vasquez
drivers/scsi/qla2xxx/qla_os.c +1
-1-if (smp_processor_id () == ha->last_irq_cpu || was_empty) +if (_smp_processor_id () == ha->last_irq_cpu || was_empty)
Am I correct in assuming that if we are preempted, and if _smp_processor_id() returns the wrong number, the driver will continue to function correctly? And that we simply missed a little optimisation opportunity?
Quoted 9 lines · Andrew Morton
drivers/scsi/qla2xxx/qla_os.c line 862
Quoted 3 lines · Andrew Vasquez
drivers/scsi/qla2xxx/qla_os.c +1
-1-if (smp_processor_id () == ha->last_irq_cpu || was_empty) +if (_smp_processor_id () == ha->last_irq_cpu || was_empty)Am I correct in assuming that if we are preempted, and if
_smp_processor_id()returns the wrong number, the driver will continue to function correctly? And that we simply missed a little optimisation opportunity?
Yes, the driver will continue to operate correctly. Another suggestion made was to explicitly disable/enable preemption around the 'if' statement. The s/smp_processor_id/_smp_processor_id/ seemed like the least intrusive option in the interim...
Signature · 1 line
Andrew Vasquez
Quoted 13 lines · Andrew Vasquez
Quoted 9 lines · Andrew Morton
drivers/scsi/qla2xxx/qla_os.c line 862
Quoted 3 lines · Andrew Vasquez
drivers/scsi/qla2xxx/qla_os.c +1
-1-if (smp_processor_id () == ha->last_irq_cpu || was_empty) +if (_smp_processor_id () == ha->last_irq_cpu || was_empty)Am I correct in assuming that if we are preempted, and if
_smp_processor_id()returns the wrong number, the driver will continue to function correctly? And that we simply missed a little optimisation opportunity?
Quoted 4 lines
Yes, the driver will continue to operate correctly. Another suggestion made was to explicitly disable/enable preemption around the 'if' statement. The s/smp_processor_id/_smp_processor_id/ seemed like the least intrusive option in the interim...
It makes perfect sense. James, I'll add this to this evening's patchbatch.