* [PATCH] bcache: fix panic due to cache_set is null @ 2020-11-30 11:21 Yi Li 2020-12-01 4:35 ` Yi Li 2020-12-01 16:38 ` [PATCH] " Coly Li 0 siblings, 2 replies; 11+ messages in thread From: Yi Li @ 2020-11-30 11:21 UTC (permalink / raw) To: colyli Cc: yilikernel, kent.overstreet, linux-bcache, linux-kernel, Yi Li, Guo Chao bcache_device_detach will release the cache_set after hotunplug cache disk. update_writeback_rate should check validate of cache_set. IP: [<ffffffffa03730c9>] update_writeback_rate+0x59/0x3a0 [bcache] PGD 879620067 PUD 8755d3067 PMD 0 Oops: 0000 [#1] SMP CPU: 8 PID: 1005702 Comm: kworker/8:0 Tainted: G 4.4.0+10 #1 Hardware name: Intel BIOS SE5C610.86B.01.01.0021.032120170601 03/21/2017 Workqueue: events update_writeback_rate [bcache] task: ffff8808786f3800 ti: ffff88077082c000 task.ti: ffff88077082c000 RIP: e030:[<ffffffffa03730c9>] update_writeback_rate+0x59/0x3a0 [bcache] RSP: e02b:ffff88077082fde0 EFLAGS: 00010202 RAX: 0000000000000018 RBX: ffff8808047f0b08 RCX: 0000000000000000 RDX: 0000000000000001 RSI: ffff88088170dab8 RDI: ffff88088170dab8 RBP: ffff88077082fe18 R08: 000000000000000a R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000017bc8 R12: 0000000000000000 R13: ffff8808047f0000 R14: 0000000000000200 R15: ffff8808047f0b08 FS: 00007f157b6d6700(0000) GS:ffff880881700000(0000) knlGS:0000000000000000 CS: e033 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000368 CR3: 0000000875c05000 CR4: 0000000000040660 Stack: 0000000000000001 0000000000007ff0 ffff88085ff600c0 ffff880881714e80 ffff880881719500 0000000000000200 ffff8808047f0b08 ffff88077082fe60 ffffffff81088c0c 0000000081714e80 0000000000000000 ffff880881714e80 Call Trace: [<ffffffff81088c0c>] process_one_work+0x1fc/0x3b0 [<ffffffff81089575>] worker_thread+0x2a5/0x470 [<ffffffff815a2f58>] ? __schedule+0x648/0x870 [<ffffffff810892d0>] ? rescuer_thread+0x300/0x300 [<ffffffff8108e3d5>] kthread+0xd5/0xe0 [<ffffffff8108e300>] ? kthread_stop+0x110/0x110 [<ffffffff815a704f>] ret_from_fork+0x3f/0x70 [<ffffffff8108e300>] ? kthread_stop+0x110/0x110 Reported-by: Guo Chao <guochao@winhong.com> Signed-off-by: Guo Chao <guochao@winhong.com> Signed-off-by: Yi Li <yili@winhong.com> --- drivers/md/bcache/writeback.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/drivers/md/bcache/writeback.c b/drivers/md/bcache/writeback.c index 3c74996978da..186c4c6e1607 100644 --- a/drivers/md/bcache/writeback.c +++ b/drivers/md/bcache/writeback.c @@ -175,7 +175,15 @@ static void update_writeback_rate(struct work_struct *work) struct cached_dev *dc = container_of(to_delayed_work(work), struct cached_dev, writeback_rate_update); - struct cache_set *c = dc->disk.c; + struct cache_set *c = NULL; + + mutex_lock(&bch_register_lock); + c = dc->disk.c; + + if (c == NULL) { + mutex_unlock(&bch_register_lock); + return; + } /* * should check BCACHE_DEV_RATE_DW_RUNNING before calling @@ -194,6 +202,7 @@ static void update_writeback_rate(struct work_struct *work) clear_bit(BCACHE_DEV_RATE_DW_RUNNING, &dc->disk.flags); /* paired with where BCACHE_DEV_RATE_DW_RUNNING is tested */ smp_mb__after_atomic(); + mutex_unlock(&bch_register_lock); return; } @@ -230,6 +239,7 @@ static void update_writeback_rate(struct work_struct *work) clear_bit(BCACHE_DEV_RATE_DW_RUNNING, &dc->disk.flags); /* paired with where BCACHE_DEV_RATE_DW_RUNNING is tested */ smp_mb__after_atomic(); + mutex_unlock(&bch_register_lock); } static unsigned int writeback_delay(struct cached_dev *dc, -- 2.25.3 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] bcache: fix panic due to cache_set is null 2020-11-30 11:21 [PATCH] bcache: fix panic due to cache_set is null Yi Li @ 2020-12-01 4:35 ` Yi Li 2020-12-01 16:42 ` Coly Li 2020-12-01 16:38 ` [PATCH] " Coly Li 1 sibling, 1 reply; 11+ messages in thread From: Yi Li @ 2020-12-01 4:35 UTC (permalink / raw) To: Yi Li; +Cc: colyli, kent.overstreet, linux-bcache, linux-kernel, Guo Chao sorry, This patch will cause deadlock, i will check and redo it. On 11/30/20, Yi Li <yili@winhong.com> wrote: > bcache_device_detach will release the cache_set after hotunplug cache > disk. update_writeback_rate should check validate of cache_set. > > IP: [<ffffffffa03730c9>] update_writeback_rate+0x59/0x3a0 [bcache] > PGD 879620067 PUD 8755d3067 PMD 0 > Oops: 0000 [#1] SMP > CPU: 8 PID: 1005702 Comm: kworker/8:0 Tainted: G 4.4.0+10 #1 > Hardware name: Intel BIOS SE5C610.86B.01.01.0021.032120170601 03/21/2017 > Workqueue: events update_writeback_rate [bcache] > task: ffff8808786f3800 ti: ffff88077082c000 task.ti: ffff88077082c000 > RIP: e030:[<ffffffffa03730c9>] update_writeback_rate+0x59/0x3a0 [bcache] > RSP: e02b:ffff88077082fde0 EFLAGS: 00010202 > RAX: 0000000000000018 RBX: ffff8808047f0b08 RCX: 0000000000000000 > RDX: 0000000000000001 RSI: ffff88088170dab8 RDI: ffff88088170dab8 > RBP: ffff88077082fe18 R08: 000000000000000a R09: 0000000000000000 > R10: 0000000000000000 R11: 0000000000017bc8 R12: 0000000000000000 > R13: ffff8808047f0000 R14: 0000000000000200 R15: ffff8808047f0b08 > FS: 00007f157b6d6700(0000) GS:ffff880881700000(0000) > knlGS:0000000000000000 > CS: e033 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 0000000000000368 CR3: 0000000875c05000 CR4: 0000000000040660 > Stack: > 0000000000000001 0000000000007ff0 ffff88085ff600c0 ffff880881714e80 > ffff880881719500 0000000000000200 ffff8808047f0b08 ffff88077082fe60 > ffffffff81088c0c 0000000081714e80 0000000000000000 ffff880881714e80 > Call Trace: > [<ffffffff81088c0c>] process_one_work+0x1fc/0x3b0 > [<ffffffff81089575>] worker_thread+0x2a5/0x470 > [<ffffffff815a2f58>] ? __schedule+0x648/0x870 > [<ffffffff810892d0>] ? rescuer_thread+0x300/0x300 > [<ffffffff8108e3d5>] kthread+0xd5/0xe0 > [<ffffffff8108e300>] ? kthread_stop+0x110/0x110 > [<ffffffff815a704f>] ret_from_fork+0x3f/0x70 > [<ffffffff8108e300>] ? kthread_stop+0x110/0x110 > > Reported-by: Guo Chao <guochao@winhong.com> > Signed-off-by: Guo Chao <guochao@winhong.com> > Signed-off-by: Yi Li <yili@winhong.com> > --- > drivers/md/bcache/writeback.c | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/drivers/md/bcache/writeback.c b/drivers/md/bcache/writeback.c > index 3c74996978da..186c4c6e1607 100644 > --- a/drivers/md/bcache/writeback.c > +++ b/drivers/md/bcache/writeback.c > @@ -175,7 +175,15 @@ static void update_writeback_rate(struct work_struct > *work) > struct cached_dev *dc = container_of(to_delayed_work(work), > struct cached_dev, > writeback_rate_update); > - struct cache_set *c = dc->disk.c; > + struct cache_set *c = NULL; > + > + mutex_lock(&bch_register_lock); > + c = dc->disk.c; > + > + if (c == NULL) { > + mutex_unlock(&bch_register_lock); > + return; > + } > > /* > * should check BCACHE_DEV_RATE_DW_RUNNING before calling > @@ -194,6 +202,7 @@ static void update_writeback_rate(struct work_struct > *work) > clear_bit(BCACHE_DEV_RATE_DW_RUNNING, &dc->disk.flags); > /* paired with where BCACHE_DEV_RATE_DW_RUNNING is tested */ > smp_mb__after_atomic(); > + mutex_unlock(&bch_register_lock); > return; > } > > @@ -230,6 +239,7 @@ static void update_writeback_rate(struct work_struct > *work) > clear_bit(BCACHE_DEV_RATE_DW_RUNNING, &dc->disk.flags); > /* paired with where BCACHE_DEV_RATE_DW_RUNNING is tested */ > smp_mb__after_atomic(); > + mutex_unlock(&bch_register_lock); > } > > static unsigned int writeback_delay(struct cached_dev *dc, > -- > 2.25.3 > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] bcache: fix panic due to cache_set is null 2020-12-01 4:35 ` Yi Li @ 2020-12-01 16:42 ` Coly Li 2020-12-03 6:25 ` Yi Li 0 siblings, 1 reply; 11+ messages in thread From: Coly Li @ 2020-12-01 16:42 UTC (permalink / raw) To: Yi Li, Yi Li; +Cc: kent.overstreet, linux-bcache, linux-kernel, Guo Chao On 12/1/20 12:35 PM, Yi Li wrote: > sorry, This patch will cause deadlock, i will check and redo it. Can you try latest upstream kernel firstly ? Before spending more time on the fix. If I remember correctly, when cancel_writeback_rate_update_dwork() is not timed out, the cache set memory won't be freed before the writeback_rate_update worker terminates. It is possible that I miss something in the code, but I suggest to test with a kernel after v5.3, and better a v5.8+ kernel. Coly Li > > On 11/30/20, Yi Li <yili@winhong.com> wrote: >> bcache_device_detach will release the cache_set after hotunplug cache >> disk. update_writeback_rate should check validate of cache_set. >> >> IP: [<ffffffffa03730c9>] update_writeback_rate+0x59/0x3a0 [bcache] >> PGD 879620067 PUD 8755d3067 PMD 0 >> Oops: 0000 [#1] SMP >> CPU: 8 PID: 1005702 Comm: kworker/8:0 Tainted: G 4.4.0+10 #1 >> Hardware name: Intel BIOS SE5C610.86B.01.01.0021.032120170601 03/21/2017 >> Workqueue: events update_writeback_rate [bcache] >> task: ffff8808786f3800 ti: ffff88077082c000 task.ti: ffff88077082c000 >> RIP: e030:[<ffffffffa03730c9>] update_writeback_rate+0x59/0x3a0 [bcache] >> RSP: e02b:ffff88077082fde0 EFLAGS: 00010202 >> RAX: 0000000000000018 RBX: ffff8808047f0b08 RCX: 0000000000000000 >> RDX: 0000000000000001 RSI: ffff88088170dab8 RDI: ffff88088170dab8 >> RBP: ffff88077082fe18 R08: 000000000000000a R09: 0000000000000000 >> R10: 0000000000000000 R11: 0000000000017bc8 R12: 0000000000000000 >> R13: ffff8808047f0000 R14: 0000000000000200 R15: ffff8808047f0b08 >> FS: 00007f157b6d6700(0000) GS:ffff880881700000(0000) >> knlGS:0000000000000000 >> CS: e033 DS: 0000 ES: 0000 CR0: 0000000080050033 >> CR2: 0000000000000368 CR3: 0000000875c05000 CR4: 0000000000040660 >> Stack: >> 0000000000000001 0000000000007ff0 ffff88085ff600c0 ffff880881714e80 >> ffff880881719500 0000000000000200 ffff8808047f0b08 ffff88077082fe60 >> ffffffff81088c0c 0000000081714e80 0000000000000000 ffff880881714e80 >> Call Trace: >> [<ffffffff81088c0c>] process_one_work+0x1fc/0x3b0 >> [<ffffffff81089575>] worker_thread+0x2a5/0x470 >> [<ffffffff815a2f58>] ? __schedule+0x648/0x870 >> [<ffffffff810892d0>] ? rescuer_thread+0x300/0x300 >> [<ffffffff8108e3d5>] kthread+0xd5/0xe0 >> [<ffffffff8108e300>] ? kthread_stop+0x110/0x110 >> [<ffffffff815a704f>] ret_from_fork+0x3f/0x70 >> [<ffffffff8108e300>] ? kthread_stop+0x110/0x110 >> >> Reported-by: Guo Chao <guochao@winhong.com> >> Signed-off-by: Guo Chao <guochao@winhong.com> >> Signed-off-by: Yi Li <yili@winhong.com> >> --- >> drivers/md/bcache/writeback.c | 12 +++++++++++- >> 1 file changed, 11 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/md/bcache/writeback.c b/drivers/md/bcache/writeback.c >> index 3c74996978da..186c4c6e1607 100644 >> --- a/drivers/md/bcache/writeback.c >> +++ b/drivers/md/bcache/writeback.c >> @@ -175,7 +175,15 @@ static void update_writeback_rate(struct work_struct >> *work) >> struct cached_dev *dc = container_of(to_delayed_work(work), >> struct cached_dev, >> writeback_rate_update); >> - struct cache_set *c = dc->disk.c; >> + struct cache_set *c = NULL; >> + >> + mutex_lock(&bch_register_lock); >> + c = dc->disk.c; >> + >> + if (c == NULL) { >> + mutex_unlock(&bch_register_lock); >> + return; >> + } >> >> /* >> * should check BCACHE_DEV_RATE_DW_RUNNING before calling >> @@ -194,6 +202,7 @@ static void update_writeback_rate(struct work_struct >> *work) >> clear_bit(BCACHE_DEV_RATE_DW_RUNNING, &dc->disk.flags); >> /* paired with where BCACHE_DEV_RATE_DW_RUNNING is tested */ >> smp_mb__after_atomic(); >> + mutex_unlock(&bch_register_lock); >> return; >> } >> >> @@ -230,6 +239,7 @@ static void update_writeback_rate(struct work_struct >> *work) >> clear_bit(BCACHE_DEV_RATE_DW_RUNNING, &dc->disk.flags); >> /* paired with where BCACHE_DEV_RATE_DW_RUNNING is tested */ >> smp_mb__after_atomic(); >> + mutex_unlock(&bch_register_lock); >> } >> >> static unsigned int writeback_delay(struct cached_dev *dc, >> -- >> 2.25.3 >> >> >> >> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] bcache: fix panic due to cache_set is null 2020-12-01 16:42 ` Coly Li @ 2020-12-03 6:25 ` Yi Li 2020-12-03 6:56 ` Coly Li 0 siblings, 1 reply; 11+ messages in thread From: Yi Li @ 2020-12-03 6:25 UTC (permalink / raw) To: Coly Li; +Cc: Yi Li, kent.overstreet, linux-bcache, linux-kernel, Guo Chao > On 12/1/20 12:35 PM, Yi Li wrote: >> sorry, This patch will cause deadlock, i will check and redo it. > > Can you try latest upstream kernel firstly ? Before spending more time > on the fix. > This issue just happened three times （xenserver7.5 dom0 kernel） on the same machine and cannot reproduce it now. and have not reproduce it using the lastest uptream kernel. > If I remember correctly, when cancel_writeback_rate_update_dwork() is > not timed out, the cache set memory won't be freed before the > writeback_rate_update worker terminates. It is possible that I miss > something in the code, but I suggest to test with a kernel after v5.3, > and better a v5.8+ kernel. > > Coly Li > Thanks. it is confused that why writeback_rate_update worker run again after cancel_delayed_work_sync( kernel log telled). Maybe i should dig more. Attach some debug info as below. Nov 30 18:00:44 host-126 kernel: [ 1697.906137] bcache: update_writeback_rate() dc = ffff880818580000 Nov 30 18:00:44 host-126 kernel: [ 1697.906145] bcache: update_writeback_rate() c = ffff8808182c0000 Nov 30 18:00:44 host-126 kernel: [ 1697.906148] bcache: update_writeback_rate() dc->disk.flags = ffff8808185800a0 Nov 30 18:00:44 host-126 kernel: [ 1697.906150] bcache: update_writeback_rate() c->flags = ffff8808182c0368 Nov 30 18:00:45 host-126 kernel: [ 1698.578104] bcache: update_writeback_rate() dc = ffff880839bd0000 Nov 30 18:00:45 host-126 kernel: [ 1698.578108] bcache: update_writeback_rate() c = ffff8808182c0000 Nov 30 18:00:45 host-126 kernel: [ 1698.578109] bcache: update_writeback_rate() dc->disk.flags = ffff880839bd00a0 Nov 30 18:00:45 host-126 kernel: [ 1698.578111] bcache: update_writeback_rate() c->flags = ffff8808182c0368 Nov 30 18:00:49 host-126 kernel: [ 1702.914126] bcache: update_writeback_rate() dc = ffff880818580000 Nov 30 18:00:49 host-126 kernel: [ 1702.914130] bcache: update_writeback_rate() c = ffff8808182c0000 Nov 30 18:00:49 host-126 kernel: [ 1702.914132] bcache: update_writeback_rate() dc->disk.flags = ffff8808185800a0 Nov 30 18:00:49 host-126 kernel: [ 1702.914133] bcache: update_writeback_rate() c->flags = ffff8808182c0368 Nov 30 18:00:50 host-126 kernel: [ 1703.586182] bcache: update_writeback_rate() dc = ffff880839bd0000 Nov 30 18:00:50 host-126 kernel: [ 1703.586188] bcache: update_writeback_rate() c = ffff8808182c0000 Nov 30 18:00:50 host-126 kernel: [ 1703.586191] bcache: update_writeback_rate() dc->disk.flags = ffff880839bd00a0 Nov 30 18:00:50 host-126 kernel: [ 1703.586193] bcache: update_writeback_rate() c->flags = ffff8808182c0368 Nov 30 18:00:54 host-126 kernel: [ 1707.922215] bcache: update_writeback_rate() dc = ffff880818580000 Nov 30 18:00:54 host-126 kernel: [ 1707.922222] bcache: update_writeback_rate() c = ffff8808182c0000 Nov 30 18:00:54 host-126 kernel: [ 1707.922225] bcache: update_writeback_rate() dc->disk.flags = ffff8808185800a0 Nov 30 18:00:54 host-126 kernel: [ 1707.922227] bcache: update_writeback_rate() c->flags = ffff8808182c0368 Nov 30 18:00:55 host-126 kernel: [ 1708.594202] bcache: update_writeback_rate() dc = ffff880839bd0000 Nov 30 18:00:55 host-126 kernel: [ 1708.594206] bcache: update_writeback_rate() c = ffff8808182c0000 Nov 30 18:00:55 host-126 kernel: [ 1708.594208] bcache: update_writeback_rate() dc->disk.flags = ffff880839bd00a0 Nov 30 18:00:55 host-126 kernel: [ 1708.594210] bcache: update_writeback_rate() c->flags = ffff8808182c0368 Nov 30 18:00:55 host-126 kernel: [ 1709.118221] sd 0:0:1:0: device_block, handle(0x0009) Nov 30 18:00:58 host-126 kernel: [ 1711.368197] sd 0:0:1:0: device_unblock and setting to running, handle(0x0009) Nov 30 18:00:58 host-126 kernel: [ 1711.368263] sd 0:0:1:0: [sdb] tag#0 FAILED Result: hostbyte=DID_NO_CONNECT driverbyte=DRIVER_OK Nov 30 18:00:58 host-126 kernel: [ 1711.368277] sd 0:0:1:0: [sdb] tag#0 CDB: Synchronize Cache(10) 35 00 00 00 00 00 00 00 00 00 Nov 30 18:00:58 host-126 kernel: [ 1711.368289] blk_update_request: I/O error, dev sdb, sector 36160 Nov 30 18:00:58 host-126 kernel: [ 1711.368326] bcache: error on 96083de4-6b3e-4ede-81d1-44edc1a93729: journal io error, disabling caching Nov 30 18:00:58 host-126 kernel: [ 1711.368372] sd 0:0:1:0: [sdb] tag#1 FAILED Result: hostbyte=DID_NO_CONNECT driverbyte=DRIVER_OK Nov 30 18:00:58 host-126 kernel: [ 1711.368387] sd 0:0:1:0: [sdb] tag#1 CDB: Write(10) 2a 00 00 81 b0 40 00 00 08 00 Nov 30 18:00:58 host-126 kernel: [ 1711.368392] blk_update_request: I/O error, dev sdb, sector 8499264 Nov 30 18:00:58 host-126 kernel: [ 1711.368407] bcache: bch_count_io_errors() sdb2: IO error on writing data to cache. Nov 30 18:00:58 host-126 kernel: [ 1711.368447] bcache: bch_cached_dev_detach() bch_cached_dev_detach start Nov 30 18:00:58 host-126 kernel: [ 1711.368458] bcache: bch_cached_dev_detach() bch_cached_dev_detach end Nov 30 18:00:58 host-126 kernel: [ 1711.368463] bcache: conditional_stop_bcache_device() stop_when_cache_set_failed of bcache1 is "auto" and cache is dirty, stop it to avoid potential data corruption. Nov 30 18:00:58 host-126 kernel: [ 1711.368466] bcache: bch_cached_dev_detach() bch_cached_dev_detach start Nov 30 18:00:58 host-126 kernel: [ 1711.368474] bcache: bch_cached_dev_detach() bch_cached_dev_detach end Nov 30 18:00:58 host-126 kernel: [ 1711.368479] bcache: conditional_stop_bcache_device() stop_when_cache_set_failed of bcache0 is "auto" and cache is clean, keep it alive. Nov 30 18:00:58 host-126 kernel: [ 1711.368494] Buffer I/O error on dev bcache1, logical block 210979209, lost async page write Nov 30 18:00:58 host-126 kernel: [ 1711.368585] bcache: cached_dev_detach_finish() cached_dev_detach_finish start Nov 30 18:00:58 host-126 kernel: [ 1711.368589] bcache: cached_dev_detach_finish() dc->disk.flags = f Nov 30 18:00:58 host-126 kernel: [ 1711.368591] bcache: cancel_writeback_rate_update_dwork() cancel_writeback_rate_update_dwork start Nov 30 18:00:58 host-126 kernel: [ 1711.368594] bcache: cancel_writeback_rate_update_dwork() cancel_writeback_rate_update_dwork end Nov 30 18:00:58 host-126 kernel: [ 1711.368617] bcache: cached_dev_detach_finish() cached_dev_detach_finish start Nov 30 18:00:58 host-126 kernel: [ 1711.369022] bcache: cached_dev_detach_finish() Caching disabled for sda2 Nov 30 18:00:58 host-126 kernel: [ 1711.369025] bcache: cached_dev_detach_finish() cached_dev_detach_finish end Nov 30 18:00:58 host-126 kernel: [ 1711.369035] bcache: cached_dev_detach_finish() dc->disk.flags = a Nov 30 18:00:58 host-126 kernel: [ 1711.369039] bcache: cancel_writeback_rate_update_dwork() cancel_writeback_rate_update_dwork start Nov 30 18:00:58 host-126 kernel: [ 1711.369042] bcache: cancel_writeback_rate_update_dwork() cancel_writeback_rate_update_dwork end Nov 30 18:00:58 host-126 kernel: [ 1711.369460] bcache: cached_dev_detach_finish() Caching disabled for sdf2 Nov 30 18:00:58 host-126 kernel: [ 1711.369464] bcache: cached_dev_detach_finish() cached_dev_detach_finish end Nov 30 18:00:58 host-126 kernel: [ 1711.374327] sd 0:0:1:0: [sdb] Synchronizing SCSI cache Nov 30 18:00:58 host-126 kernel: [ 1711.374516] sd 0:0:1:0: [sdb] Synchronize Cache(10) failed: Result: hostbyte=DID_NO_CONNECT driverbyte=DRIVER_OK Nov 30 18:00:58 host-126 kernel: [ 1711.376907] bcache: cache_set_free() Cache set 96083de4-6b3e-4ede-81d1-44edc1a93729 unregistered Nov 30 18:00:58 host-126 kernel: [ 1711.418979] mpt3sas_cm0: removing handle(0x0009), sas_addr(0x4433221102000000) Nov 30 18:00:58 host-126 kernel: [ 1711.418984] mpt3sas_cm0: removing :enclosure logical id(0x5a4bf013514f4000), slot(4) Nov 30 18:00:58 host-126 kernel: [ 1711.418988] mpt3sas_cm0: removing :enclosure level(0x0000), connector name( ) Nov 30 18:01:05 host-126 kernel: [ 1718.873725] bcache: cached_dev_free() cached_dev_free start Nov 30 18:01:05 host-126 kernel: [ 1718.873776] bcache: bcache_device_free() bcache1 stopped Nov 30 18:01:05 host-126 kernel: [ 1718.926598] bcache: cached_dev_free() cached_dev_free end Nov 30 18:01:06 host-126 kernel: [ 1719.668540] sda: sda1 sda2 Nov 30 18:01:11 host-126 kernel: [ 1725.010305] bcache: update_writeback_rate() dc = ffff880818580000 Nov 30 18:01:11 host-126 kernel: [ 1725.010309] bcache: update_writeback_rate() c = (null) ------------the kernel log show that cache_set is null. Nov 30 18:01:11 host-126 kernel: [ 1725.010311] bcache: update_writeback_rate() dc->disk.flags = ffff8808185800a0 Nov 30 18:01:12 host-126 kernel: [ 1725.369791] mpt3sas_cm0: detecting: handle(0x0009), sas_address(0x4433221102000000), phy(2) Nov 30 18:01:12 host-126 kernel: [ 1725.369802] mpt3sas_cm0: REPORT_LUNS: handle(0x0009), retries(0) Nov 30 18:01:12 host-126 kernel: [ 1725.618138] mpt3sas_cm0: TEST_UNIT_READY: handle(0x0009), lun(0) Nov 30 18:01:13 host-126 kernel: [ 1726.619148] mpt3sas_cm0: detecting: handle(0x0009), sas_address(0x4433221102000000), phy(2) Nov 30 18:01:13 host-126 kernel: [ 1726.619156] mpt3sas_cm0: REPORT_LUNS: handle(0x0009), retries(0) Nov 30 18:01:13 host-126 kernel: [ 1726.619300] mpt3sas_cm0: TEST_UNIT_READY: handle(0x0009), lun(0) Nov 30 18:01:13 host-126 kernel: [ 1726.622323] scsi 0:0:6:0: Direct-Access ATA INTEL SSDSC2BB48 0112 PQ: 0 ANSI: 6 Nov 30 18:01:13 host-126 kernel: [ 1726.622839] scsi 0:0:6:0: SATA: handle(0x0009), sas_addr(0x4433221102000000), phy(2), device_name(0x55cd2e414d6dcd4b) Nov 30 18:01:13 host-126 kernel: [ 1726.622845] scsi 0:0:6:0: SATA: enclosure logical id(0x5a4bf013514f4000), slot(4) Nov 30 18:01:13 host-126 kernel: [ 1726.622847] scsi 0:0:6:0: SATA: enclosure level(0x0000), connector name( ) Nov 30 18:01:13 host-126 kernel: [ 1726.622990] scsi 0:0:6:0: atapi(n), ncq(y), asyn_notify(n), smart(y), fua(y), sw_preserve(y) Nov 30 18:01:13 host-126 kernel: [ 1726.623642] scsi 0:0:6:0: serial_number(PHDV7102003Q480BGN ) Nov 30 18:01:13 host-126 kernel: [ 1726.626116] sd 0:0:6:0: Attached scsi generic sg1 type 0 Nov 30 18:01:13 host-126 kernel: [ 1726.626784] sd 0:0:6:0: [sdb] 937703088 512-byte logical blocks: (480 GB/447 GiB) Nov 30 18:01:13 host-126 kernel: [ 1726.626789] sd 0:0:6:0: [sdb] 4096-byte physical blocks Nov 30 18:01:13 host-126 kernel: [ 1726.631161] sd 0:0:6:0: [sdb] Write Protect is off Nov 30 18:01:13 host-126 kernel: [ 1726.631165] sd 0:0:6:0: [sdb] Mode Sense: 9b 00 10 08 Nov 30 18:01:13 host-126 kernel: [ 1726.632513] sd 0:0:6:0: [sdb] Write cache: enabled, read cache: enabled, supports DPO and FUA Nov 30 18:01:13 host-126 kernel: [ 1726.648068] sdb: sdb1 sdb2 Nov 30 18:01:13 host-126 kernel: [ 1726.656841] sd 0:0:6:0: [sdb] Attached SCSI disk >> >> On 11/30/20, Yi Li <yili@winhong.com> wrote: >>> bcache_device_detach will release the cache_set after hotunplug cache >>> disk. update_writeback_rate should check validate of cache_set. >>> >>> IP: [<ffffffffa03730c9>] update_writeback_rate+0x59/0x3a0 [bcache] >>> PGD 879620067 PUD 8755d3067 PMD 0 >>> Oops: 0000 [#1] SMP >>> CPU: 8 PID: 1005702 Comm: kworker/8:0 Tainted: G 4.4.0+10 #1 >>> Hardware name: Intel BIOS SE5C610.86B.01.01.0021.032120170601 >>> 03/21/2017 >>> Workqueue: events update_writeback_rate [bcache] >>> task: ffff8808786f3800 ti: ffff88077082c000 task.ti: ffff88077082c000 >>> RIP: e030:[<ffffffffa03730c9>] update_writeback_rate+0x59/0x3a0 >>> [bcache] >>> RSP: e02b:ffff88077082fde0 EFLAGS: 00010202 >>> RAX: 0000000000000018 RBX: ffff8808047f0b08 RCX: 0000000000000000 >>> RDX: 0000000000000001 RSI: ffff88088170dab8 RDI: ffff88088170dab8 >>> RBP: ffff88077082fe18 R08: 000000000000000a R09: 0000000000000000 >>> R10: 0000000000000000 R11: 0000000000017bc8 R12: 0000000000000000 >>> R13: ffff8808047f0000 R14: 0000000000000200 R15: ffff8808047f0b08 >>> FS: 00007f157b6d6700(0000) GS:ffff880881700000(0000) >>> knlGS:0000000000000000 >>> CS: e033 DS: 0000 ES: 0000 CR0: 0000000080050033 >>> CR2: 0000000000000368 CR3: 0000000875c05000 CR4: 0000000000040660 >>> Stack: >>> 0000000000000001 0000000000007ff0 ffff88085ff600c0 ffff880881714e80 >>> ffff880881719500 0000000000000200 ffff8808047f0b08 ffff88077082fe60 >>> ffffffff81088c0c 0000000081714e80 0000000000000000 ffff880881714e80 >>> Call Trace: >>> [<ffffffff81088c0c>] process_one_work+0x1fc/0x3b0 >>> [<ffffffff81089575>] worker_thread+0x2a5/0x470 >>> [<ffffffff815a2f58>] ? __schedule+0x648/0x870 >>> [<ffffffff810892d0>] ? rescuer_thread+0x300/0x300 >>> [<ffffffff8108e3d5>] kthread+0xd5/0xe0 >>> [<ffffffff8108e300>] ? kthread_stop+0x110/0x110 >>> [<ffffffff815a704f>] ret_from_fork+0x3f/0x70 >>> [<ffffffff8108e300>] ? kthread_stop+0x110/0x110 >>> >>> Reported-by: Guo Chao <guochao@winhong.com> >>> Signed-off-by: Guo Chao <guochao@winhong.com> >>> Signed-off-by: Yi Li <yili@winhong.com> >>> --- >>> drivers/md/bcache/writeback.c | 12 +++++++++++- >>> 1 file changed, 11 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/md/bcache/writeback.c >>> b/drivers/md/bcache/writeback.c >>> index 3c74996978da..186c4c6e1607 100644 >>> --- a/drivers/md/bcache/writeback.c >>> +++ b/drivers/md/bcache/writeback.c >>> @@ -175,7 +175,15 @@ static void update_writeback_rate(struct >>> work_struct >>> *work) >>> struct cached_dev *dc = container_of(to_delayed_work(work), >>> struct cached_dev, >>> writeback_rate_update); >>> - struct cache_set *c = dc->disk.c; >>> + struct cache_set *c = NULL; >>> + >>> + mutex_lock(&bch_register_lock); >>> + c = dc->disk.c; >>> + >>> + if (c == NULL) { >>> + mutex_unlock(&bch_register_lock); >>> + return; >>> + } >>> >>> /* >>> * should check BCACHE_DEV_RATE_DW_RUNNING before calling >>> @@ -194,6 +202,7 @@ static void update_writeback_rate(struct work_struct >>> *work) >>> clear_bit(BCACHE_DEV_RATE_DW_RUNNING, &dc->disk.flags); >>> /* paired with where BCACHE_DEV_RATE_DW_RUNNING is tested */ >>> smp_mb__after_atomic(); >>> + mutex_unlock(&bch_register_lock); >>> return; >>> } >>> >>> @@ -230,6 +239,7 @@ static void update_writeback_rate(struct work_struct >>> *work) >>> clear_bit(BCACHE_DEV_RATE_DW_RUNNING, &dc->disk.flags); >>> /* paired with where BCACHE_DEV_RATE_DW_RUNNING is tested */ >>> smp_mb__after_atomic(); >>> + mutex_unlock(&bch_register_lock); >>> } >>> >>> static unsigned int writeback_delay(struct cached_dev *dc, >>> -- >>> 2.25.3 >>> >>> >>> >>> > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] bcache: fix panic due to cache_set is null 2020-12-03 6:25 ` Yi Li @ 2020-12-03 6:56 ` Coly Li 2020-12-03 9:06 ` Yi Li 0 siblings, 1 reply; 11+ messages in thread From: Coly Li @ 2020-12-03 6:56 UTC (permalink / raw) To: Yi Li; +Cc: Yi Li, kent.overstreet, linux-bcache, linux-kernel, Guo Chao On 12/3/20 2:25 PM, Yi Li wrote: >> On 12/1/20 12:35 PM, Yi Li wrote: >>> sorry, This patch will cause deadlock, i will check and redo it. >> >> Can you try latest upstream kernel firstly ? Before spending more time >> on the fix. >> > > This issue just happened three times （xenserver7.5 dom0 kernel） on the > same machine and cannot reproduce it now. and have not reproduce it > using the lastest uptream kernel. > Hmm, this is something very probably that I am not able to help. It seems the kernel is a third-part maintained Linux v4.4 based kernel + bcache backport, which is out of my view. If similar problem happens on latest upstream kernel, or at least v5.8+ kernel, I can help to take a look. >> If I remember correctly, when cancel_writeback_rate_update_dwork() is >> not timed out, the cache set memory won't be freed before the >> writeback_rate_update worker terminates. It is possible that I miss >> something in the code, but I suggest to test with a kernel after v5.3, >> and better a v5.8+ kernel. >> >> Coly Li >> > Thanks. > > it is confused that why writeback_rate_update worker run again after > cancel_delayed_work_sync( kernel log telled). > [snipped] Coly Li ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] bcache: fix panic due to cache_set is null 2020-12-03 6:56 ` Coly Li @ 2020-12-03 9:06 ` Yi Li 2020-12-03 9:47 ` [PATCH v2] " Yi Li 0 siblings, 1 reply; 11+ messages in thread From: Yi Li @ 2020-12-03 9:06 UTC (permalink / raw) To: Coly Li; +Cc: Yi Li, kent.overstreet, linux-bcache, linux-kernel, Guo Chao The root cause: After just cached_dev_free do cancel_writeback_rate_update_dwork without bch_register_lock . at the same time. Wirting the writeback_percent by sysfs witch bch_register_lock will insert a writeback_rate_update work. cached_dev_free with bch_register_lock to do bcache_device_free. (it is introduce by patch 80265d8dfd77792e133793cef44a21323aac2908) pls： 1： run the shell script #!/bin/bash while [ true ] do echo 0 > /sys/block/bcache0/bcache/writeback_percent done 2: hotplug the cache disk On 12/3/20, Coly Li <colyli@suse.de> wrote: > On 12/3/20 2:25 PM, Yi Li wrote: >>> On 12/1/20 12:35 PM, Yi Li wrote: >>>> sorry, This patch will cause deadlock, i will check and redo it. >>> >>> Can you try latest upstream kernel firstly ? Before spending more time >>> on the fix. >>> >> >> This issue just happened three times （xenserver7.5 dom0 kernel） on the >> same machine and cannot reproduce it now. and have not reproduce it >> using the lastest uptream kernel. >> > > Hmm, this is something very probably that I am not able to help. It > seems the kernel is a third-part maintained Linux v4.4 based kernel + > bcache backport, which is out of my view. > > If similar problem happens on latest upstream kernel, or at least v5.8+ > kernel, I can help to take a look. > > >>> If I remember correctly, when cancel_writeback_rate_update_dwork() is >>> not timed out, the cache set memory won't be freed before the >>> writeback_rate_update worker terminates. It is possible that I miss >>> something in the code, but I suggest to test with a kernel after v5.3, >>> and better a v5.8+ kernel. >>> >>> Coly Li >>> >> Thanks. >> >> it is confused that why writeback_rate_update worker run again after >> cancel_delayed_work_sync( kernel log telled). >> > > [snipped] > > Coly Li > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2] bcache: fix panic due to cache_set is null 2020-12-03 9:06 ` Yi Li @ 2020-12-03 9:47 ` Yi Li 2020-12-03 11:27 ` Coly Li 0 siblings, 1 reply; 11+ messages in thread From: Yi Li @ 2020-12-03 9:47 UTC (permalink / raw) To: colyli Cc: yilikernel, kent.overstreet, linux-bcache, linux-kernel, Yi Li, Guo Chao bcache_device_detach will release the cache_set after hotunplug cache disk. Here is how the issue happens. 1) cached_dev_free do cancel_writeback_rate_update_dwork without bch_register_lock. 2) Wirting the writeback_percent by sysfs with bch_register_lock will insert a writeback_rate_update work. 3) cached_dev_free with bch_register_lock to do bcache_device_free. dc->disk.cl will be set NULL 4) update_writeback_rate will crash when access dc->disk.cl Fixes: 80265d8dfd77 ("bcache: acquire bch_register_lock later in cached_dev_free()") IP: [<ffffffffa03730c9>] update_writeback_rate+0x59/0x3a0 [bcache] PGD 879620067 PUD 8755d3067 PMD 0 Oops: 0000 [#1] SMP CPU: 8 PID: 1005702 Comm: kworker/8:0 Tainted: G 4.4.0+10 #1 Hardware name: Intel BIOS SE5C610.86B.01.01.0021.032120170601 03/21/2017 Workqueue: events update_writeback_rate [bcache] task: ffff8808786f3800 ti: ffff88077082c000 task.ti: ffff88077082c000 RIP: e030:[<ffffffffa03730c9>] update_writeback_rate+0x59/0x3a0 [bcache] RSP: e02b:ffff88077082fde0 EFLAGS: 00010202 RAX: 0000000000000018 RBX: ffff8808047f0b08 RCX: 0000000000000000 RDX: 0000000000000001 RSI: ffff88088170dab8 RDI: ffff88088170dab8 RBP: ffff88077082fe18 R08: 000000000000000a R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000017bc8 R12: 0000000000000000 R13: ffff8808047f0000 R14: 0000000000000200 R15: ffff8808047f0b08 FS: 00007f157b6d6700(0000) GS:ffff880881700000(0000) knlGS:0000000000000000 CS: e033 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000368 CR3: 0000000875c05000 CR4: 0000000000040660 Stack: 0000000000000001 0000000000007ff0 ffff88085ff600c0 ffff880881714e80 ffff880881719500 0000000000000200 ffff8808047f0b08 ffff88077082fe60 ffffffff81088c0c 0000000081714e80 0000000000000000 ffff880881714e80 Call Trace: [<ffffffff81088c0c>] process_one_work+0x1fc/0x3b0 [<ffffffff81089575>] worker_thread+0x2a5/0x470 [<ffffffff815a2f58>] ? __schedule+0x648/0x870 [<ffffffff810892d0>] ? rescuer_thread+0x300/0x300 [<ffffffff8108e3d5>] kthread+0xd5/0xe0 [<ffffffff8108e300>] ? kthread_stop+0x110/0x110 [<ffffffff815a704f>] ret_from_fork+0x3f/0x70 [<ffffffff8108e300>] ? kthread_stop+0x110/0x110 Reported-by: Guo Chao <guochao@winhong.com> Signed-off-by: Yi Li <yili@winhong.com> --- drivers/md/bcache/super.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c index 46a00134a36a..8b341f756ac0 100644 --- a/drivers/md/bcache/super.c +++ b/drivers/md/bcache/super.c @@ -1334,9 +1334,6 @@ static void cached_dev_free(struct closure *cl) { struct cached_dev *dc = container_of(cl, struct cached_dev, disk.cl); - if (test_and_clear_bit(BCACHE_DEV_WB_RUNNING, &dc->disk.flags)) - cancel_writeback_rate_update_dwork(dc); - if (!IS_ERR_OR_NULL(dc->writeback_thread)) kthread_stop(dc->writeback_thread); if (!IS_ERR_OR_NULL(dc->status_update_thread)) @@ -1344,6 +1341,9 @@ static void cached_dev_free(struct closure *cl) mutex_lock(&bch_register_lock); + if (test_and_clear_bit(BCACHE_DEV_WB_RUNNING, &dc->disk.flags)) + cancel_writeback_rate_update_dwork(dc); + if (atomic_read(&dc->running)) bd_unlink_disk_holder(dc->bdev, dc->disk.disk); bcache_device_free(&dc->disk); -- 2.25.3 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2] bcache: fix panic due to cache_set is null 2020-12-03 9:47 ` [PATCH v2] " Yi Li @ 2020-12-03 11:27 ` Coly Li 2020-12-04 1:33 ` Yi Li 0 siblings, 1 reply; 11+ messages in thread From: Coly Li @ 2020-12-03 11:27 UTC (permalink / raw) To: Yi Li; +Cc: yilikernel, kent.overstreet, linux-bcache, linux-kernel, Guo Chao On 12/3/20 5:47 PM, Yi Li wrote: > bcache_device_detach will release the cache_set after hotunplug cache > disk. > > Here is how the issue happens. > 1) cached_dev_free do cancel_writeback_rate_update_dwork > without bch_register_lock. > 2) Wirting the writeback_percent by sysfs with > bch_register_lock will insert a writeback_rate_update work. > 3) cached_dev_free with bch_register_lock to do bcache_device_free. > dc->disk.cl will be set NULL > 4) update_writeback_rate will crash when access dc->disk.cl The analysis makes sense, good catch! Thank you for make me understand the problem. > > Fixes: 80265d8dfd77 ("bcache: acquire bch_register_lock later in cached_dev_free()") > > IP: [<ffffffffa03730c9>] update_writeback_rate+0x59/0x3a0 [bcache] > PGD 879620067 PUD 8755d3067 PMD 0 > Oops: 0000 [#1] SMP > CPU: 8 PID: 1005702 Comm: kworker/8:0 Tainted: G 4.4.0+10 #1 > Hardware name: Intel BIOS SE5C610.86B.01.01.0021.032120170601 03/21/2017 > Workqueue: events update_writeback_rate [bcache] > task: ffff8808786f3800 ti: ffff88077082c000 task.ti: ffff88077082c000 > RIP: e030:[<ffffffffa03730c9>] update_writeback_rate+0x59/0x3a0 [bcache] > RSP: e02b:ffff88077082fde0 EFLAGS: 00010202 > RAX: 0000000000000018 RBX: ffff8808047f0b08 RCX: 0000000000000000 > RDX: 0000000000000001 RSI: ffff88088170dab8 RDI: ffff88088170dab8 > RBP: ffff88077082fe18 R08: 000000000000000a R09: 0000000000000000 > R10: 0000000000000000 R11: 0000000000017bc8 R12: 0000000000000000 > R13: ffff8808047f0000 R14: 0000000000000200 R15: ffff8808047f0b08 > FS: 00007f157b6d6700(0000) GS:ffff880881700000(0000) knlGS:0000000000000000 > CS: e033 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 0000000000000368 CR3: 0000000875c05000 CR4: 0000000000040660 > Stack: > 0000000000000001 0000000000007ff0 ffff88085ff600c0 ffff880881714e80 > ffff880881719500 0000000000000200 ffff8808047f0b08 ffff88077082fe60 > ffffffff81088c0c 0000000081714e80 0000000000000000 ffff880881714e80 > Call Trace: > [<ffffffff81088c0c>] process_one_work+0x1fc/0x3b0 > [<ffffffff81089575>] worker_thread+0x2a5/0x470 > [<ffffffff815a2f58>] ? __schedule+0x648/0x870 > [<ffffffff810892d0>] ? rescuer_thread+0x300/0x300 > [<ffffffff8108e3d5>] kthread+0xd5/0xe0 > [<ffffffff8108e300>] ? kthread_stop+0x110/0x110 > [<ffffffff815a704f>] ret_from_fork+0x3f/0x70 > [<ffffffff8108e300>] ? kthread_stop+0x110/0x110 > > Reported-by: Guo Chao <guochao@winhong.com> > Signed-off-by: Yi Li <yili@winhong.com> > --- > drivers/md/bcache/super.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c > index 46a00134a36a..8b341f756ac0 100644 > --- a/drivers/md/bcache/super.c > +++ b/drivers/md/bcache/super.c > @@ -1334,9 +1334,6 @@ static void cached_dev_free(struct closure *cl) > { > struct cached_dev *dc = container_of(cl, struct cached_dev, disk.cl); > > - if (test_and_clear_bit(BCACHE_DEV_WB_RUNNING, &dc->disk.flags)) > - cancel_writeback_rate_update_dwork(dc); > - > if (!IS_ERR_OR_NULL(dc->writeback_thread)) > kthread_stop(dc->writeback_thread); > if (!IS_ERR_OR_NULL(dc->status_update_thread)) > @@ -1344,6 +1341,9 @@ static void cached_dev_free(struct closure *cl) > > mutex_lock(&bch_register_lock); > > + if (test_and_clear_bit(BCACHE_DEV_WB_RUNNING, &dc->disk.flags)) > + cancel_writeback_rate_update_dwork(dc); > + > if (atomic_read(&dc->running)) > bd_unlink_disk_holder(dc->bdev, dc->disk.disk); > bcache_device_free(&dc->disk); > Such change is problematic, the writeback rate kworker mush stopped before writeback and status_update thread, otherwise you may encounter other problem. And when I review your patch I find another similar potential problem. This is tricky, let me think how to fix it .... Thank you again, for catch such issue. Coly Li ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] bcache: fix panic due to cache_set is null 2020-12-03 11:27 ` Coly Li @ 2020-12-04 1:33 ` Yi Li 2020-12-04 5:54 ` [PATCH v3] " Yi Li 0 siblings, 1 reply; 11+ messages in thread From: Yi Li @ 2020-12-04 1:33 UTC (permalink / raw) To: Coly Li; +Cc: Yi Li, kent.overstreet, linux-bcache, linux-kernel, Guo Chao On 12/3/20, Coly Li <colyli@suse.de> wrote: > On 12/3/20 5:47 PM, Yi Li wrote: >> bcache_device_detach will release the cache_set after hotunplug cache >> disk. >> >> Here is how the issue happens. >> 1) cached_dev_free do cancel_writeback_rate_update_dwork >> without bch_register_lock. >> 2) Wirting the writeback_percent by sysfs with >> bch_register_lock will insert a writeback_rate_update work. >> 3) cached_dev_free with bch_register_lock to do bcache_device_free. >> dc->disk.cl will be set NULL >> 4) update_writeback_rate will crash when access dc->disk.cl > > The analysis makes sense, good catch! Thank you for make me understand > the problem. > > >> >> Fixes: 80265d8dfd77 ("bcache: acquire bch_register_lock later in >> cached_dev_free()") >> >> IP: [<ffffffffa03730c9>] update_writeback_rate+0x59/0x3a0 [bcache] >> PGD 879620067 PUD 8755d3067 PMD 0 >> Oops: 0000 [#1] SMP >> CPU: 8 PID: 1005702 Comm: kworker/8:0 Tainted: G 4.4.0+10 #1 >> Hardware name: Intel BIOS SE5C610.86B.01.01.0021.032120170601 >> 03/21/2017 >> Workqueue: events update_writeback_rate [bcache] >> task: ffff8808786f3800 ti: ffff88077082c000 task.ti: ffff88077082c000 >> RIP: e030:[<ffffffffa03730c9>] update_writeback_rate+0x59/0x3a0 >> [bcache] >> RSP: e02b:ffff88077082fde0 EFLAGS: 00010202 >> RAX: 0000000000000018 RBX: ffff8808047f0b08 RCX: 0000000000000000 >> RDX: 0000000000000001 RSI: ffff88088170dab8 RDI: ffff88088170dab8 >> RBP: ffff88077082fe18 R08: 000000000000000a R09: 0000000000000000 >> R10: 0000000000000000 R11: 0000000000017bc8 R12: 0000000000000000 >> R13: ffff8808047f0000 R14: 0000000000000200 R15: ffff8808047f0b08 >> FS: 00007f157b6d6700(0000) GS:ffff880881700000(0000) >> knlGS:0000000000000000 >> CS: e033 DS: 0000 ES: 0000 CR0: 0000000080050033 >> CR2: 0000000000000368 CR3: 0000000875c05000 CR4: 0000000000040660 >> Stack: >> 0000000000000001 0000000000007ff0 ffff88085ff600c0 ffff880881714e80 >> ffff880881719500 0000000000000200 ffff8808047f0b08 ffff88077082fe60 >> ffffffff81088c0c 0000000081714e80 0000000000000000 ffff880881714e80 >> Call Trace: >> [<ffffffff81088c0c>] process_one_work+0x1fc/0x3b0 >> [<ffffffff81089575>] worker_thread+0x2a5/0x470 >> [<ffffffff815a2f58>] ? __schedule+0x648/0x870 >> [<ffffffff810892d0>] ? rescuer_thread+0x300/0x300 >> [<ffffffff8108e3d5>] kthread+0xd5/0xe0 >> [<ffffffff8108e300>] ? kthread_stop+0x110/0x110 >> [<ffffffff815a704f>] ret_from_fork+0x3f/0x70 >> [<ffffffff8108e300>] ? kthread_stop+0x110/0x110 >> >> Reported-by: Guo Chao <guochao@winhong.com> >> Signed-off-by: Yi Li <yili@winhong.com> >> --- >> drivers/md/bcache/super.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c >> index 46a00134a36a..8b341f756ac0 100644 >> --- a/drivers/md/bcache/super.c >> +++ b/drivers/md/bcache/super.c >> @@ -1334,9 +1334,6 @@ static void cached_dev_free(struct closure *cl) >> { >> struct cached_dev *dc = container_of(cl, struct cached_dev, disk.cl); >> >> - if (test_and_clear_bit(BCACHE_DEV_WB_RUNNING, &dc->disk.flags)) >> - cancel_writeback_rate_update_dwork(dc); >> - >> if (!IS_ERR_OR_NULL(dc->writeback_thread)) >> kthread_stop(dc->writeback_thread); >> if (!IS_ERR_OR_NULL(dc->status_update_thread)) >> @@ -1344,6 +1341,9 @@ static void cached_dev_free(struct closure *cl) >> >> mutex_lock(&bch_register_lock); >> >> + if (test_and_clear_bit(BCACHE_DEV_WB_RUNNING, &dc->disk.flags)) >> + cancel_writeback_rate_update_dwork(dc); >> + >> if (atomic_read(&dc->running)) >> bd_unlink_disk_holder(dc->bdev, dc->disk.disk); >> bcache_device_free(&dc->disk); >> > > Such change is problematic, the writeback rate kworker mush stopped > before writeback and status_update thread, otherwise you may encounter > other problem. > enn, It is possible that I miss something. 1: writeback_rate_update work will add to the system_wq by schedule_delayed_work. 2: The issue 80265d8dfd77 --" After moving mutex_lock(&bch_register_lock) to a later location where before atomic_read(&dc->running) in cached_dev_free()" . > And when I review your patch I find another similar potential problem. > > This is tricky, let me think how to fix it .... > > Thank you again, for catch such issue. > > Coly Li > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3] bcache: fix panic due to cache_set is null 2020-12-04 1:33 ` Yi Li @ 2020-12-04 5:54 ` Yi Li 0 siblings, 0 replies; 11+ messages in thread From: Yi Li @ 2020-12-04 5:54 UTC (permalink / raw) To: colyli Cc: yilikernel, kent.overstreet, linux-bcache, linux-kernel, Yi Li, Guo Chao bcache_device_detach will release the cache_set after hotunplug cache disk. Here is how the issue happens. 1) cached_dev_free do cancel_writeback_rate_update_dwork without bch_register_lock. 2) Wirting the writeback_percent by sysfs with bch_register_lock will insert a writeback_rate_update work. 3) cached_dev_free with bch_register_lock to do bcache_device_free. dc->disk.c will be set NULL. 4) update_writeback_rate will crash when access dc->disk.c. After Patch: 1) cached_dev_free do cancel_writeback_rate_update_dwork and set dc->disk.c to NULL with bch_register_lock. 2) dc->disk.c = NULL will avoid that Wirting the writeback_percent by sysfs insert a writeback_rate_update work. Fixes: 80265d8dfd77 ("bcache: acquire bch_register_lock later in cached_dev_free()") IP: [<ffffffffa03730c9>] update_writeback_rate+0x59/0x3a0 [bcache] PGD 879620067 PUD 8755d3067 PMD 0 Oops: 0000 [#1] SMP CPU: 8 PID: 1005702 Comm: kworker/8:0 Tainted: G 4.4.0+10 #1 Hardware name: Intel BIOS SE5C610.86B.01.01.0021.032120170601 03/21/2017 Workqueue: events update_writeback_rate [bcache] task: ffff8808786f3800 ti: ffff88077082c000 task.ti: ffff88077082c000 RIP: e030:[<ffffffffa03730c9>] update_writeback_rate+0x59/0x3a0 [bcache] RSP: e02b:ffff88077082fde0 EFLAGS: 00010202 RAX: 0000000000000018 RBX: ffff8808047f0b08 RCX: 0000000000000000 RDX: 0000000000000001 RSI: ffff88088170dab8 RDI: ffff88088170dab8 RBP: ffff88077082fe18 R08: 000000000000000a R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000017bc8 R12: 0000000000000000 R13: ffff8808047f0000 R14: 0000000000000200 R15: ffff8808047f0b08 FS: 00007f157b6d6700(0000) GS:ffff880881700000(0000) knlGS:0000000000000000 CS: e033 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000368 CR3: 0000000875c05000 CR4: 0000000000040660 Stack: 0000000000000001 0000000000007ff0 ffff88085ff600c0 ffff880881714e80 ffff880881719500 0000000000000200 ffff8808047f0b08 ffff88077082fe60 ffffffff81088c0c 0000000081714e80 0000000000000000 ffff880881714e80 Call Trace: [<ffffffff81088c0c>] process_one_work+0x1fc/0x3b0 [<ffffffff81089575>] worker_thread+0x2a5/0x470 [<ffffffff815a2f58>] ? __schedule+0x648/0x870 [<ffffffff810892d0>] ? rescuer_thread+0x300/0x300 [<ffffffff8108e3d5>] kthread+0xd5/0xe0 [<ffffffff8108e300>] ? kthread_stop+0x110/0x110 [<ffffffff815a704f>] ret_from_fork+0x3f/0x70 [<ffffffff8108e300>] ? kthread_stop+0x110/0x110 Reported-by: Guo Chao <guochao@winhong.com> Signed-off-by: Yi Li <yili@winhong.com> --- drivers/md/bcache/super.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c index 46a00134a36a..381f9fbcd765 100644 --- a/drivers/md/bcache/super.c +++ b/drivers/md/bcache/super.c @@ -1122,9 +1122,6 @@ static void cached_dev_detach_finish(struct work_struct *w) BUG_ON(refcount_read(&dc->count)); - if (test_and_clear_bit(BCACHE_DEV_WB_RUNNING, &dc->disk.flags)) - cancel_writeback_rate_update_dwork(dc); - if (!IS_ERR_OR_NULL(dc->writeback_thread)) { kthread_stop(dc->writeback_thread); dc->writeback_thread = NULL; @@ -1138,6 +1135,9 @@ static void cached_dev_detach_finish(struct work_struct *w) mutex_lock(&bch_register_lock); + if (test_and_clear_bit(BCACHE_DEV_WB_RUNNING, &dc->disk.flags)) + cancel_writeback_rate_update_dwork(dc); + calc_cached_dev_sectors(dc->disk.c); bcache_device_detach(&dc->disk); list_move(&dc->list, &uncached_devices); @@ -1334,9 +1334,6 @@ static void cached_dev_free(struct closure *cl) { struct cached_dev *dc = container_of(cl, struct cached_dev, disk.cl); - if (test_and_clear_bit(BCACHE_DEV_WB_RUNNING, &dc->disk.flags)) - cancel_writeback_rate_update_dwork(dc); - if (!IS_ERR_OR_NULL(dc->writeback_thread)) kthread_stop(dc->writeback_thread); if (!IS_ERR_OR_NULL(dc->status_update_thread)) @@ -1344,6 +1341,9 @@ static void cached_dev_free(struct closure *cl) mutex_lock(&bch_register_lock); + if (test_and_clear_bit(BCACHE_DEV_WB_RUNNING, &dc->disk.flags)) + cancel_writeback_rate_update_dwork(dc); + if (atomic_read(&dc->running)) bd_unlink_disk_holder(dc->bdev, dc->disk.disk); bcache_device_free(&dc->disk); -- 2.25.3 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] bcache: fix panic due to cache_set is null 2020-11-30 11:21 [PATCH] bcache: fix panic due to cache_set is null Yi Li 2020-12-01 4:35 ` Yi Li @ 2020-12-01 16:38 ` Coly Li 1 sibling, 0 replies; 11+ messages in thread From: Coly Li @ 2020-12-01 16:38 UTC (permalink / raw) To: Yi Li; +Cc: yilikernel, kent.overstreet, linux-bcache, linux-kernel, Guo Chao On 11/30/20 7:21 PM, Yi Li wrote: > bcache_device_detach will release the cache_set after hotunplug cache > disk. update_writeback_rate should check validate of cache_set. > I see the kernel is 4.4.0+10, do you try the the v5.9 kernel ? I don't see your kernel code, it is not easy to response. Thanks. Coly LI > IP: [<ffffffffa03730c9>] update_writeback_rate+0x59/0x3a0 [bcache] > PGD 879620067 PUD 8755d3067 PMD 0 > Oops: 0000 [#1] SMP > CPU: 8 PID: 1005702 Comm: kworker/8:0 Tainted: G 4.4.0+10 #1 > Hardware name: Intel BIOS SE5C610.86B.01.01.0021.032120170601 03/21/2017 > Workqueue: events update_writeback_rate [bcache] > task: ffff8808786f3800 ti: ffff88077082c000 task.ti: ffff88077082c000 > RIP: e030:[<ffffffffa03730c9>] update_writeback_rate+0x59/0x3a0 [bcache] > RSP: e02b:ffff88077082fde0 EFLAGS: 00010202 > RAX: 0000000000000018 RBX: ffff8808047f0b08 RCX: 0000000000000000 > RDX: 0000000000000001 RSI: ffff88088170dab8 RDI: ffff88088170dab8 > RBP: ffff88077082fe18 R08: 000000000000000a R09: 0000000000000000 > R10: 0000000000000000 R11: 0000000000017bc8 R12: 0000000000000000 > R13: ffff8808047f0000 R14: 0000000000000200 R15: ffff8808047f0b08 > FS: 00007f157b6d6700(0000) GS:ffff880881700000(0000) knlGS:0000000000000000 > CS: e033 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 0000000000000368 CR3: 0000000875c05000 CR4: 0000000000040660 > Stack: > 0000000000000001 0000000000007ff0 ffff88085ff600c0 ffff880881714e80 > ffff880881719500 0000000000000200 ffff8808047f0b08 ffff88077082fe60 > ffffffff81088c0c 0000000081714e80 0000000000000000 ffff880881714e80 > Call Trace: > [<ffffffff81088c0c>] process_one_work+0x1fc/0x3b0 > [<ffffffff81089575>] worker_thread+0x2a5/0x470 > [<ffffffff815a2f58>] ? __schedule+0x648/0x870 > [<ffffffff810892d0>] ? rescuer_thread+0x300/0x300 > [<ffffffff8108e3d5>] kthread+0xd5/0xe0 > [<ffffffff8108e300>] ? kthread_stop+0x110/0x110 > [<ffffffff815a704f>] ret_from_fork+0x3f/0x70 > [<ffffffff8108e300>] ? kthread_stop+0x110/0x110 > > Reported-by: Guo Chao <guochao@winhong.com> > Signed-off-by: Guo Chao <guochao@winhong.com> > Signed-off-by: Yi Li <yili@winhong.com> > --- > drivers/md/bcache/writeback.c | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/drivers/md/bcache/writeback.c b/drivers/md/bcache/writeback.c > index 3c74996978da..186c4c6e1607 100644 > --- a/drivers/md/bcache/writeback.c > +++ b/drivers/md/bcache/writeback.c > @@ -175,7 +175,15 @@ static void update_writeback_rate(struct work_struct *work) > struct cached_dev *dc = container_of(to_delayed_work(work), > struct cached_dev, > writeback_rate_update); > - struct cache_set *c = dc->disk.c; > + struct cache_set *c = NULL; > + > + mutex_lock(&bch_register_lock); > + c = dc->disk.c; > + > + if (c == NULL) { > + mutex_unlock(&bch_register_lock); > + return; > + } > > /* > * should check BCACHE_DEV_RATE_DW_RUNNING before calling > @@ -194,6 +202,7 @@ static void update_writeback_rate(struct work_struct *work) > clear_bit(BCACHE_DEV_RATE_DW_RUNNING, &dc->disk.flags); > /* paired with where BCACHE_DEV_RATE_DW_RUNNING is tested */ > smp_mb__after_atomic(); > + mutex_unlock(&bch_register_lock); > return; > } > > @@ -230,6 +239,7 @@ static void update_writeback_rate(struct work_struct *work) > clear_bit(BCACHE_DEV_RATE_DW_RUNNING, &dc->disk.flags); > /* paired with where BCACHE_DEV_RATE_DW_RUNNING is tested */ > smp_mb__after_atomic(); > + mutex_unlock(&bch_register_lock); > } > > static unsigned int writeback_delay(struct cached_dev *dc, > ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-12-04 6:01 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-30 11:21 [PATCH] bcache: fix panic due to cache_set is null Yi Li 2020-12-01 4:35 ` Yi Li 2020-12-01 16:42 ` Coly Li 2020-12-03 6:25 ` Yi Li 2020-12-03 6:56 ` Coly Li 2020-12-03 9:06 ` Yi Li 2020-12-03 9:47 ` [PATCH v2] " Yi Li 2020-12-03 11:27 ` Coly Li 2020-12-04 1:33 ` Yi Li 2020-12-04 5:54 ` [PATCH v3] " Yi Li 2020-12-01 16:38 ` [PATCH] " Coly Li
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).