| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 23 Apr 2009 08:37:13 +0200 From: Jens Axboe <jens.axboe@...cle.com> To: Sage Weil <sage@...dream.net> Cc: linux-kernel@...r.kernel.org, jeff@...zik.org, neilb@...e.de, yehuda@...dream.net Subject: Re: [PATCH] umem: fix request_queue lock warning On Wed, Apr 22 2009, Sage Weil wrote: > Hi, > > The umem driver issues two warnings on boot, due to blk_plug_device() and > blk_remove_plug() being called without q->queue_lock held. Starting with > e48ec690 (block: extend queue_flag bitops), the queue_flag_* functions > warn if q->queue_lock doesn't appear to be locked. In fact, q->queue_lock > is NULL (though that apparently isn't otherwise a problem as the driver is > using card->lock for everything). > > Although blk_init_queue() with take a request_fn_proc and spinlock_t*, > there isn't a corresponding init helper that takes a make_request_fn. > Setting queue_lock to &card->lock explicitly seems to work fine for me. > The warning goes away and the device appears to behave. > > If there is a better solution, I'd be happy to test it out. It looks like > this problem showed up in 2.6.27. There clearly aren't too many people > using these NVRAM cards, but it'd be nice to see this fixed. > > Thanks- > sage > > > [ 1.531881] v2.3 : Micro Memory(tm) PCI memory board block driver > [ 1.538136] umem 0000:02:01.0: PCI INT A -> GSI 20 (level, low) -> IRQ 20 > [ 1.545018] umem 0000:02:01.0: Micro Memory(tm) controller found (PCI Mem Module (Battery Backup)) > [ 1.554176] umem 0000:02:01.0: CSR 0xfc9ffc00 -> 0xffffc200013d0c00 (0x100) > [ 1.561279] umem 0000:02:01.0: Size 1048576 KB, Battery 1 Disabled (FAILURE), Battery 2 Disabled (FAILURE) > [ 1.571114] umem 0000:02:01.0: Window size 16777216 bytes, IRQ 20 > [ 1.577304] umem 0000:02:01.0: memory NOT initialized. Consider over-writing whole device. > [ 1.585989] umema:<4>------------[ cut here ]------------ > [ 1.591775] WARNING: at include/linux/blkdev.h:492 blk_plug_device+0x6d/0x106() > [ 1.592025] Hardware name: H8SSL > [ 1.592025] Modules linked in: > [ 1.592025] Pid: 1, comm: swapper Not tainted 2.6.29 #8 > [ 1.592025] Call Trace: > [ 1.592025] [<ffffffff8023c994>] warn_slowpath+0xd3/0xf2 > [ 1.592025] [<ffffffff8025a5b5>] ? save_trace+0x3f/0x9b > [ 1.592025] [<ffffffff8025a68b>] ? add_lock_to_list+0x7a/0xba > [ 1.592025] [<ffffffff8025e609>] ? validate_chain+0xb3b/0xce8 > [ 1.592025] [<ffffffff80441556>] ? mm_make_request+0x27/0x59 > [ 1.592025] [<ffffffff80441556>] ? mm_make_request+0x27/0x59 > [ 1.592025] [<ffffffff8025ef04>] ? __lock_acquire+0x74e/0x7b9 > [ 1.592025] [<ffffffff8025a70e>] ? get_lock_stats+0x34/0x5e > [ 1.592025] [<ffffffff8025a746>] ? put_lock_stats+0xe/0x27 > [ 1.592025] [<ffffffff80441556>] ? mm_make_request+0x27/0x59 > [ 1.592025] [<ffffffff803ad165>] blk_plug_device+0x6d/0x106 > [ 1.592025] [<ffffffff80441575>] mm_make_request+0x46/0x59 > [ 1.592025] [<ffffffff803ac2d9>] generic_make_request+0x335/0x3cf > [ 1.592025] [<ffffffff8027fcc7>] ? mempool_alloc_slab+0x11/0x13 > [ 1.592025] [<ffffffff8027fdce>] ? mempool_alloc+0x45/0x101 > [ 1.592025] [<ffffffff8025a746>] ? put_lock_stats+0xe/0x27 > [ 1.592025] [<ffffffff803adda5>] submit_bio+0x10a/0x119 > [ 1.592025] [<ffffffff802c8d00>] submit_bh+0xe5/0x109 > [ 1.592025] [<ffffffff802cbf43>] block_read_full_page+0x2aa/0x2cb > [ 1.592025] [<ffffffff802cf4c4>] ? blkdev_get_block+0x0/0x4c > [ 1.592025] [<ffffffff805c90a8>] ? _spin_unlock_irq+0x36/0x51 > [ 1.592025] [<ffffffff80286836>] ? __lru_cache_add+0x92/0xb2 > [ 1.592025] [<ffffffff802cf008>] blkdev_readpage+0x13/0x15 > [ 1.592025] [<ffffffff8027de06>] read_cache_page_async+0x90/0x134 > [ 1.592025] [<ffffffff802ceff5>] ? blkdev_readpage+0x0/0x15 > [ 1.592025] [<ffffffff802f5f1c>] ? adfspart_check_ICS+0x0/0x16c > [ 1.592025] [<ffffffff8027deb8>] read_cache_page+0xe/0x45 > [ 1.592025] [<ffffffff802f5170>] read_dev_sector+0x2e/0x93 > [ 1.592025] [<ffffffff802f5f44>] adfspart_check_ICS+0x28/0x16c > [ 1.592025] [<ffffffff8025d427>] ? trace_hardirqs_on+0xd/0xf > [ 1.592025] [<ffffffff802f5f1c>] ? adfspart_check_ICS+0x0/0x16c > [ 1.592025] [<ffffffff802f59c5>] rescan_partitions+0x168/0x2fb > [ 1.592025] [<ffffffff802ceae9>] __blkdev_get+0x259/0x336 > [ 1.592025] [<ffffffff803ca1e2>] ? kobject_put+0x47/0x4b > [ 1.592025] [<ffffffff802cebd1>] blkdev_get+0xb/0xd > [ 1.592025] [<ffffffff802f5773>] register_disk+0xc4/0x12b > [ 1.592025] [<ffffffff803b2a7b>] add_disk+0xc3/0x12d > [ 1.592025] [<ffffffff808a1d4a>] ? mm_init+0x0/0x1a5 > [ 1.592025] [<ffffffff808a1e73>] mm_init+0x129/0x1a5 > [ 1.592025] [<ffffffff808a1d4a>] ? mm_init+0x0/0x1a5 > [ 1.592025] [<ffffffff80209056>] _stext+0x56/0x130 > [ 1.592025] [<ffffffff80274932>] ? register_irq_proc+0xae/0xca > [ 1.592025] [<ffffffff802f0000>] ? proc_pid_lookup+0xb4/0x18b > [ 1.592025] [<ffffffff8087f975>] kernel_init+0x132/0x18b > [ 1.592025] [<ffffffff8020d17a>] child_rip+0xa/0x20 > [ 1.592025] [<ffffffff8020cb40>] ? restore_args+0x0/0x30 > [ 1.592025] [<ffffffff8087f843>] ? kernel_init+0x0/0x18b > [ 1.592025] [<ffffffff8020d170>] ? child_rip+0x0/0x20 > [ 1.592025] ---[ end trace 7150b3b86da74e1e ]--- > [ 1.889858] ------------[ cut here ]------------[ve_plug+0x5f/0x91() > [ 1.893848] Hardware name: H8SSL > [ 1.893848] Modules linked in: > [ 1.893848] Pid: 1, comm: swapper Tainted: G W 2.6.29 #8 > [ 1.893848] Call Trace: > [ 1.893848] [<ffffffff8023c994>] warn_slowpath+0xd3/0xf2 > [ 1.893848] [<ffffffff805c8411>] ? trace_hardirqs_on_thunk+0x3a/0x3f > [ 1.893848] [<ffffffff8020cb40>] ? restore_args+0x0/0x30 > [ 1.893848] [<ffffffff80254245>] ? __atomic_notifier_call_chain+0x0/0xb2 > [ 1.893848] [<ffffffff805c90a3>] ? _spin_unlock_irq+0x31/0x51 > [ 1.893848] [<ffffffff805c90bf>] ? _spin_unlock_irq+0x4d/0x51 > [ 1.893848] [<ffffffff8044157d>] ? mm_make_request+0x4e/0x59 > [ 1.893848] [<ffffffff8025a70e>] ? get_lock_stats+0x34/0x5e > [ 1.893848] [<ffffffff8025a75d>] ? put_lock_stats+0x25/0x27 > [ 1.893848] [<ffffffff80441504>] ? mm_unplug_device+0x25/0x50 > [ 1.893848] [<ffffffff803acf23>] blk_remove_plug+0x5f/0x91 > [ 1.893848] [<ffffffff8044150f>] mm_unplug_device+0x30/0x50 > [ 1.893848] [<ffffffff803ab74a>] blk_unplug+0x78/0x7d > [ 1.893848] [<ffffffff803ab75c>] blk_backing_dev_unplug+0xd/0xf > [ 1.893848] [<ffffffff802c853c>] block_sync_page+0x4a/0x4c > [ 1.893848] [<ffffffff8027da1c>] sync_page+0x44/0x4d > [ 1.893848] [<ffffffff805c66fd>] __wait_on_bit_lock+0x42/0x8a > [ 1.893848] [<ffffffff8027d9d8>] ? sync_page+0x0/0x4d > [ 1.893848] [<ffffffff8027d9c4>] __lock_page+0x64/0x6b > [ 1.893848] [<ffffffff802508db>] ? wake_bit_function+0x0/0x2a > [ 1.893848] [<ffffffff8027de4a>] read_cache_page_async+0xd4/0x134 > [ 1.893848] [<ffffffff802ceff5>] ? blkdev_readpage+0x0/0x15 > [ 1.893848] [<ffffffff802f5f1c>] ? adfspart_check_ICS+0x0/0x16c > [ 1.893848] [<ffffffff8027deb8>] read_cache_page+0xe/0x45 > [ 1.893848] [<ffffffff802f5170>] read_dev_sector+0x2e/0x93 > [ 1.893848] [<ffffffff802f5f44>] adfspart_check_ICS+0x28/0x16c > [ 1.893848] [<ffffffff8025d427>] ? trace_hardirqs_on+0xd/0xf > [ 1.893848] [<ffffffff802f5f1c>] ? adfspart_check_ICS+0x0/0x16c > [ 1.893848] [<ffffffff802f59c5>] rescan_partitions+0x168/0x2fb > [ 1.893848] [<ffffffff802ceae9>] __blkdev_get+0x259/0x336 > [ 1.893848] [<ffffffff803ca1e2>] ? kobject_put+0x47/0x4b > [ 1.893848] [<ffffffff802cebd1>] blkdev_get+0xb/0xd > [ 1.893848] [<ffffffff802f5773>] register_disk+0xc4/0x12b > [ 1.893848] [<ffffffff803b2a7b>] add_disk+0xc3/0x12d > [ 1.893848] [<ffffffff808a1d4a>] ? mm_init+0x0/0x1a5 > [ 1.893848] [<ffffffff808a1e73>] mm_init+0x129/0x1a5 > [ 1.893848] [<ffffffff808a1d4a>] ? mm_init+0x0/0x1a5 > [ 1.893848] [<ffffffff80209056>] _stext+0x56/0x130 > [ 1.893848] [<ffffffff80274932>] ? register_irq_proc+0xae/0xca > [ 1.893848] [<ffffffff802f0000>] ? proc_pid_lookup+0xb4/0x18b > [ 1.893848] [<ffffffff8087f975>] kernel_init+0x132/0x18b > [ 1.893848] [<ffffffff8020d17a>] child_rip+0xa/0x20 > [ 1.893848] [<ffffffff8020cb40>] ? restore_args+0x0/0x30 > [ 1.893848] [<ffffffff8087f843>] ? kernel_init+0x0/0x18b > [ 1.893848] [<ffffffff8020d170>] ? child_rip+0x0/0x20 > [ 1.893848] ---[ end trace 7150b3b86da74e1f ]--- > > > Signed-off-by: Sage Weil <sage@...dream.net> > --- > drivers/block/umem.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/drivers/block/umem.c b/drivers/block/umem.c > index c24e1bd..de669fb 100644 > --- a/drivers/block/umem.c > +++ b/drivers/block/umem.c > @@ -906,6 +906,7 @@ static int __devinit mm_pci_probe(struct pci_dev *dev, > goto failed_alloc; > > blk_queue_make_request(card->queue, mm_make_request); > + card->queue->queue_lock = &card->lock; > card->queue->queuedata = card; > card->queue->unplug_fn = mm_unplug_device; That looks like the correct fix, we are already using that very lock to protect the plugging. I guess the original commit just forgot to inform the block layer that this is the queue lock. I'll queue up your fix. -- Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists