[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20211130124131.6pgu7enjgk6y536m@skn-laptop>
Date: Tue, 30 Nov 2021 13:41:31 +0100
From: Sean Nyekjaer <sean@...nix.com>
To: Marek Szyprowski <m.szyprowski@...sung.com>,
Miquel Raynal <miquel.raynal@...tlin.com>,
Boris Brezillon <boris.brezillon@...labora.com>
Cc: Richard Weinberger <richard@....at>,
Vignesh Raghavendra <vigneshr@...com>,
Boris Brezillon <bbrezillon@...nel.org>,
linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 3/4] mtd: core: protect access to MTD devices while in
suspend
> On 29.11.2021 10:41, Sean Nyekjaer wrote:
> > On Mon, Nov 29, 2021 at 10:19:08AM +0100, Miquel Raynal wrote:
> >> boris.brezillon@...labora.com wrote on Tue, 23 Nov 2021 14:07:15 +0100:
> >>> On Tue, 23 Nov 2021 13:50:12 +0100
> >>> Sean Nyekjaer <sean@...nix.com> wrote:
> >>>> @Boris do we need to do something similar here to what we did with the
> >>>> mtdconcat stuff?
> >>> Absolutely, physmasp subdevices are never initialized/registered, so
> >>> you can't call any of the mtd helpers taking the suspend lock on those.
> >>> I guess it'd be better to call mtd_suspend/resume() on the concat device
> >>> in though:
> >> Any chance that you will come up with a fix or v6 of the series anytime
> >> soon?
> >>
> >>> static void physmap_flash_shutdown(struct platform_device *dev)
> >>> {
> >>> struct physmap_flash_info *info = platform_get_drvdata(dev);
> >>>
> >>> mtd_suspend(info->cmtd);
There is one more issue when using concat during boot, I think we should
start here :)
It uses uninitialized rwsm.
[ 0.000000] Booting Linux on physical CPU 0x0000000000 [0x411fd070]
[ 0.000000] Linux version 5.16.0-rc1-00013-g67bcbe202b48 (sean@zen) (aarch64-linux-gnu-gcc (GCC) 11.2.0, GNU ld (GNU Binutils) 2.37) #207 SMP PREEMPT Tue Nov 30 13:22:48 CET 2021
[ 0.000000] Machine model: linux,dummy-virt
[ 0.000000] efi: UEFI not found.
[ 0.000000] NUMA: No NUMA configuration found
[ ... ]
[ 1.848327] 0.flash: Found 2 x16 devices at 0x0 in 32-bit bank. Manufacturer ID 0x000000 Chip ID 0x000000
[ 1.848567] Intel/Sharp Extended Query Table at 0x0031
[ 1.848999] Using buffer write method
[ 1.849237] Concatenating MTD devices:
[ 1.849347] (0): "0.flash"
[ 1.849637] (1): "0.flash"
[ 1.849726] into device "0.flash"
[ 1.904985] libphy: Fixed MDIO Bus: probed
[ 1.915812] tun: Universal TUN/TAP device driver, 1.6
[ ... ]
[ 6.064628] The code is fine but needs lockdep annotation, or maybe
[ 6.064756] you didn't initialize this object before use?
[ 6.064857] turning off the locking correctness validator.
[ 6.065111] CPU: 0 PID: 49 Comm: kworker/0:1H Not tainted 5.16.0-rc1-00013-g67bcbe202b48 #207
[ 6.065273] Hardware name: linux,dummy-virt (DT)
[ 6.065572] Workqueue: kblockd blk_mq_run_work_fn
[ 6.065879] Call trace:
[ 6.065943] dump_backtrace+0x0/0x1a0
[ 6.066057] show_stack+0x1c/0x30
[ 6.066130] dump_stack_lvl+0x8c/0xb8
[ 6.066206] dump_stack+0x1c/0x38
[ 6.066270] register_lock_class+0x49c/0x4c0
[ 6.066352] __lock_acquire+0x78/0x20cc
[ 6.066422] lock_acquire.part.0+0xe0/0x230
[ 6.066497] lock_acquire+0x6c/0x90
[ 6.066562] down_read+0x58/0x7c
[ 6.066626] mtd_read_oob_std+0x98/0x170
[ 6.066703] mtd_read_oob+0x80/0x13c
[ 6.066770] mtd_read+0x64/0xa0
[ 6.066834] concat_read+0xd4/0x1b0
[ 6.066900] mtd_read_oob_std+0x158/0x170
[ 6.066972] mtd_read_oob+0x80/0x13c
[ 6.067038] mtd_read+0x64/0xa0
[ 6.067099] mtdblock_readsect+0x6c/0x160
[ 6.067169] mtd_queue_rq+0x240/0x460
[ 6.067234] blk_mq_dispatch_rq_list+0x19c/0x830
[ 6.067313] __blk_mq_do_dispatch_sched+0x248/0x2d4
[ 6.067397] __blk_mq_sched_dispatch_requests+0x110/0x170
[ 6.067487] blk_mq_sched_dispatch_requests+0x3c/0x80
[ 6.067573] __blk_mq_run_hw_queue+0x60/0xa0
[ 6.067647] blk_mq_run_work_fn+0x24/0x30
[ 6.067718] process_one_work+0x28c/0x6e4
[ 6.067790] worker_thread+0x78/0x464
[ 6.067856] kthread+0x180/0x190
[ 6.067919] ret_from_fork+0x10/0x20
[ 6.068868] ------------[ cut here ]------------
[ 6.068993] DEBUG_RWSEMS_WARN_ON(sem->magic != sem): count = 0x100, magic = 0x0, owner = 0xffff528f84c617c1, curr 0xffff528f84c617c0, list not empty
[ 6.069592] WARNING: CPU: 0 PID: 49 at kernel/locking/rwsem.c:1302 __up_read+0x1e8/0x270
[ 6.069790] Modules linked in:
[ 6.069963] CPU: 0 PID: 49 Comm: kworker/0:1H Not tainted 5.16.0-rc1-00013-g67bcbe202b48 #207
[ 6.070099] Hardware name: linux,dummy-virt (DT)
[ 6.070179] Workqueue: kblockd blk_mq_run_work_fn
[ 6.070347] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 6.070487] pc : __up_read+0x1e8/0x270
[ 6.070577] lr : __up_read+0x1e8/0x270
[ 6.070660] sp : ffff80001026b740
[ 6.070728] x29: ffff80001026b740 x28: 000000000003ff88 x27: 0000000000040000
[ 6.070912] x26: ffff80001026ba28 x25: ffff528f84e92000 x24: ffff528f8576d000
[ 6.071040] x23: 0000000000000000 x22: 0000000003ff0000 x21: ffff80001026b878
[ 6.071165] x20: ffffadee32213000 x19: ffff528f84e918b0 x18: ffffffffffffffff
[ 6.071292] x17: 672d33313030302d x16: 3163722d302e3631 x15: ffff80009026b457
[ 6.071418] x14: 0000000000000007 x13: ffffadee32236658 x12: 0000000000000384
[ 6.071543] x11: 000000000000012c x10: ffffadee32236658 x9 : ffffadee32236658
[ 6.071692] x8 : 00000000ffffefff x7 : ffffadee3228e658 x6 : ffffadee3228e658
[ 6.071817] x5 : 000000000000bff4 x4 : 0000000000000000 x3 : 0000000000000000
[ 6.071942] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff528f84c617c0
[ 6.072094] Call trace:
[ 6.072163] __up_read+0x1e8/0x270
[ 6.072238] up_read+0x44/0x300
[ 6.072301] mtd_read_oob_std+0xd0/0x170
[ 6.072374] mtd_read_oob+0x80/0x13c
[ 6.072441] mtd_read+0x64/0xa0
[ 6.072502] concat_read+0xd4/0x1b0
[ 6.072567] mtd_read_oob_std+0x158/0x170
[ 6.072640] mtd_read_oob+0x80/0x13c
[ 6.072706] mtd_read+0x64/0xa0
[ 6.072766] mtdblock_readsect+0x6c/0x160
[ 6.072843] mtd_queue_rq+0x240/0x460
[ 6.072909] blk_mq_dispatch_rq_list+0x19c/0x830
[ 6.072990] __blk_mq_do_dispatch_sched+0x248/0x2d4
[ 6.073073] __blk_mq_sched_dispatch_requests+0x110/0x170
[ 6.073163] blk_mq_sched_dispatch_requests+0x3c/0x80
[ 6.073249] __blk_mq_run_hw_queue+0x60/0xa0
[ 6.073325] blk_mq_run_work_fn+0x24/0x30
[ 6.073421] process_one_work+0x28c/0x6e4
[ 6.073568] worker_thread+0x78/0x464
[ 6.073643] kthread+0x180/0x190
[ 6.073710] ret_from_fork+0x10/0x20
[ 6.073820] irq event stamp: 1529
[ 6.073891] hardirqs last enabled at (1529): [<ffffadee3114576c>] _raw_spin_unlock_irq+0x48/0x9c
[ 6.074067] hardirqs last disabled at (1528): [<ffffadee31145b70>] _raw_spin_lock_irq+0xac/0xb0
[ 6.074211] softirqs last enabled at (1444): [<ffffadee30010528>] __do_softirq+0x478/0x5f0
[ 6.074350] softirqs last disabled at (1429): [<ffffadee3009f064>] __irq_exit_rcu+0x184/0x1b0
[ 6.074506] ---[ end trace 738f7ffe764b66d5 ]---
I can solve this with this hack from mtd_set_dev_defaults():
diff --git a/drivers/mtd/mtdconcat.c b/drivers/mtd/mtdconcat.c
index f4a4274489b4..a4b69b9558c9 100644
--- a/drivers/mtd/mtdconcat.c
+++ b/drivers/mtd/mtdconcat.c
@@ -693,6 +693,8 @@ struct mtd_info *mtd_concat_create(struct mtd_info *subdev[], /* subdevices to c
concat->mtd.oobavail = subdev[0]->oobavail;
subdev_master = mtd_get_master(subdev[0]);
+ init_waitqueue_head(&subdev_master->master.resume_wq);
+ init_rwsem(&subdev_master->master.suspend_lock);
if (subdev_master->_writev)
concat->mtd._writev = concat_writev;
if (subdev_master->_read_oob)
@@ -740,6 +742,8 @@ struct mtd_info *mtd_concat_create(struct mtd_info *subdev[], /* subdevices to c
}
subdev_master = mtd_get_master(subdev[i]);
+ init_waitqueue_head(&subdev_master->master.resume_wq);
+ init_rwsem(&subdev_master->master.suspend_lock);
concat->mtd.size += subdev[i]->size;
concat->mtd.ecc_stats.badblocks +=
subdev[i]->ecc_stats.badblocks;
I do not have a great overview of what is happing here with all these
master devices :/
Any ideas Boris/Miquel?
/Sean
Powered by blists - more mailing lists