Re: Blockdev 6.13-rc lockdep splat regressions

[Ming Lei <ming.lei@redhat.com>]

On Sun, Jan 12, 2025 at 12:33:13PM +0100, Thomas Hellström wrote:
> On Sat, 2025-01-11 at 11:05 +0800, Ming Lei wrote:

...

> 
> Ah, You're right, it's a different warning this time. Posted the
> warning below. (Note: This is also with Christoph's series applied on
> top).
> 
> May I also humbly suggest the following lockdep priming to be able to
> catch the reclaim lockdep splats early without reclaim needing to
> happen. That will also pick up splat #2 below.
> 
> 8<-------------------------------------------------------------
> 
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 32fb28a6372c..2dd8dc9aed7f 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -458,6 +458,11 @@ struct request_queue *blk_alloc_queue(struct
> queue_limits *lim, int node_id)
>  
>         q->nr_requests = BLKDEV_DEFAULT_RQ;
>  
> +       fs_reclaim_acquire(GFP_KERNEL);
> +       rwsem_acquire_read(&q->io_lockdep_map, 0, 0, _RET_IP_);
> +       rwsem_release(&q->io_lockdep_map, _RET_IP_);
> +       fs_reclaim_release(GFP_KERNEL);
> +
>         return q;

Looks one nice idea for injecting fs_reclaim, maybe it can be
added to inject framework?

>  
>  fail_stats:
> 
> 8<-------------------------------------------------------------
> 
> #1:
>   106.921533] ======================================================
> [  106.921716] WARNING: possible circular locking dependency detected
> [  106.921725] 6.13.0-rc6+ #121 Tainted: G     U            
> [  106.921734] ------------------------------------------------------
> [  106.921743] kswapd0/117 is trying to acquire lock:
> [  106.921751] ffff8ff4e2da09f0 (&q->q_usage_counter(io)){++++}-{0:0},
> at: __submit_bio+0x80/0x220
> [  106.921769] 
>                but task is already holding lock:
> [  106.921778] ffffffff8e65e1c0 (fs_reclaim){+.+.}-{0:0}, at:
> balance_pgdat+0xe2/0xa10
> [  106.921791] 
>                which lock already depends on the new lock.
> 
> [  106.921803] 
>                the existing dependency chain (in reverse order) is:
> [  106.921814] 
>                -> #1 (fs_reclaim){+.+.}-{0:0}:
> [  106.921824]        fs_reclaim_acquire+0x9d/0xd0
> [  106.921833]        __kmalloc_cache_node_noprof+0x5d/0x3f0
> [  106.921842]        blk_mq_init_tags+0x3d/0xb0
> [  106.921851]        blk_mq_alloc_map_and_rqs+0x4e/0x3d0
> [  106.921860]        blk_mq_init_sched+0x100/0x260
> [  106.921868]        elevator_switch+0x8d/0x2e0
> [  106.921877]        elv_iosched_store+0x174/0x1e0
> [  106.921885]        queue_attr_store+0x142/0x180
> [  106.921893]        kernfs_fop_write_iter+0x168/0x240
> [  106.921902]        vfs_write+0x2b2/0x540
> [  106.921910]        ksys_write+0x72/0xf0
> [  106.921916]        do_syscall_64+0x95/0x180
> [  106.921925]        entry_SYSCALL_64_after_hwframe+0x76/0x7e

That is another regression from commit

	af2814149883 block: freeze the queue in queue_attr_store

and queue_wb_lat_store() has same risk too.

I will cook a patch to fix it.

Thanks,
Ming