• Zhao Lei's avatar
    btrfs: Fix lockdep warning of wr_ctx->wr_lock in scrub_free_wr_ctx() · 20b2e302
    Zhao Lei authored
    lockdep report following warning in test:
     [25176.843958] =================================
     [25176.844519] [ INFO: inconsistent lock state ]
     [25176.845047] 4.1.0-rc3 #22 Tainted: G        W
     [25176.845591] ---------------------------------
     [25176.846153] inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
     [25176.846713] fsstress/26661 [HC0[0]:SC1[1]:HE1:SE0] takes:
     [25176.847246]  (&wr_ctx->wr_lock){+.?...}, at: [<ffffffffa04cdc6d>] scrub_free_ctx+0x2d/0xf0 [btrfs]
     [25176.847838] {SOFTIRQ-ON-W} state was registered at:
     [25176.848396]   [<ffffffff810bf460>] __lock_acquire+0x6a0/0xe10
     [25176.848955]   [<ffffffff810bfd1e>] lock_acquire+0xce/0x2c0
     [25176.849491]   [<ffffffff816489af>] mutex_lock_nested+0x7f/0x410
     [25176.850029]   [<ffffffffa04d04ff>] scrub_stripe+0x4df/0x1080 [btrfs]
     [25176.850575]   [<ffffffffa04d11b1>] scrub_chunk.isra.19+0x111/0x130 [btrfs]
     [25176.851110]   [<ffffffffa04d144c>] scrub_enumerate_chunks+0x27c/0x510 [btrfs]
     [25176.851660]   [<ffffffffa04d3b87>] btrfs_scrub_dev+0x1c7/0x6c0 [btrfs]
     [25176.852189]   [<ffffffffa04e918e>] btrfs_dev_replace_start+0x36e/0x450 [btrfs]
     [25176.852771]   [<ffffffffa04a98e0>] btrfs_ioctl+0x1e10/0x2d20 [btrfs]
     [25176.853315]   [<ffffffff8121c5b8>] do_vfs_ioctl+0x318/0x570
     [25176.853868]   [<ffffffff8121c851>] SyS_ioctl+0x41/0x80
     [25176.854406]   [<ffffffff8164da17>] system_call_fastpath+0x12/0x6f
     [25176.854935] irq event stamp: 51506
     [25176.855511] hardirqs last  enabled at (51506): [<ffffffff810d4ce5>] vprintk_emit+0x225/0x5e0
     [25176.856059] hardirqs last disabled at (51505): [<ffffffff810d4b77>] vprintk_emit+0xb7/0x5e0
     [25176.856642] softirqs last  enabled at (50886): [<ffffffff81067a23>] __do_softirq+0x363/0x640
     [25176.857184] softirqs last disabled at (50949): [<ffffffff8106804d>] irq_exit+0x10d/0x120
     [25176.857746]
     other info that might help us debug this:
     [25176.858845]  Possible unsafe locking scenario:
     [25176.859981]        CPU0
     [25176.860537]        ----
     [25176.861059]   lock(&wr_ctx->wr_lock);
     [25176.861705]   <Interrupt>
     [25176.862272]     lock(&wr_ctx->wr_lock);
     [25176.862881]
      *** DEADLOCK ***
    
    Reason:
     Above warning is caused by:
     Interrupt
     -> bio_endio()
     -> ...
     -> scrub_put_ctx()
     -> scrub_free_ctx() *1
     -> ...
     -> mutex_lock(&wr_ctx->wr_lock);
    
     scrub_put_ctx() is allowed to be called in end_bio interrupt, but
     in code design, it will never call scrub_free_ctx(sctx) in interrupe
     context(above *1), because btrfs_scrub_dev() get one additional
     reference of sctx->refs, which makes scrub_free_ctx() only called
     withine btrfs_scrub_dev().
    
     Now the code runs out of our wish, because free sequence in
     scrub_pending_bio_dec() have a gap.
    
     Current code:
     -----------------------------------+-----------------------------------
     scrub_pending_bio_dec()            |  btrfs_scrub_dev
     -----------------------------------+-----------------------------------
     atomic_dec(&sctx->bios_in_flight); |
     wake_up(&sctx->list_wait);         |
                                        | scrub_put_ctx()
                                        | -> atomic_dec_and_test(&sctx->refs)
     scrub_put_ctx(sctx);               |
     -> atomic_dec_and_test(&sctx->refs)|
     -> scrub_free_ctx()                |
     -----------------------------------+-----------------------------------
    
     We expected:
     -----------------------------------+-----------------------------------
     scrub_pending_bio_dec()            |  btrfs_scrub_dev
     -----------------------------------+-----------------------------------
     atomic_dec(&sctx->bios_in_flight); |
     wake_up(&sctx->list_wait);         |
     scrub_put_ctx(sctx);               |
     -> atomic_dec_and_test(&sctx->refs)|
                                        | scrub_put_ctx()
                                        | -> atomic_dec_and_test(&sctx->refs)
                                        | -> scrub_free_ctx()
     -----------------------------------+-----------------------------------
    
    Fix:
     Move scrub_pending_bio_dec() to a workqueue, to avoid this function run
     in interrupt context.
     Tested by check tracelog in debug.
    
    Changelog v1->v2:
     Use workqueue instead of adjust function call sequence in v1,
     because v1 will introduce a bug pointed out by:
     Filipe David Manana <fdmanana@gmail.com>
    Reported-by: default avatarQu Wenruo <quwenruo@cn.fujitsu.com>
    Signed-off-by: default avatarZhao Lei <zhaolei@cn.fujitsu.com>
    Reviewed-by: default avatarFilipe Manana <fdmanana@suse.com>
    Signed-off-by: default avatarChris Mason <clm@fb.com>
    20b2e302
async-thread.h 2.87 KB