Re: [PATCH] block/genhd: use atomic_t for disk_event->block

From: Hannes Reinecke <hare@suse.de>
To: Ming Lei <ming.lei@redhat.com>
Cc: Jens Axboe <axboe@kernel.dk>, Christoph Hellwig <hch@lst.de>,
	linux-block@vger.kernel.org,
	Linux Kernel Mailinglist <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] block/genhd: use atomic_t for disk_event->block
Date: Tue, 1 Jun 2021 15:41:35 +0200	[thread overview]
Message-ID: <73783f6f-a3ec-907f-ea19-966e9d1457dc@suse.de> (raw)
In-Reply-To: <YLY1V/dTMeo3RGZd@T590>

On 6/1/21 3:25 PM, Ming Lei wrote:
> On Tue, Jun 01, 2021 at 01:01:45PM +0200, Hannes Reinecke wrote:
>> __disk_unblock_events() will call queue_delayed_work() with a '0' argument
>> under a spin lock. This might cause the queue_work item to be executed
>> immediately, and run into a deadlock in disk_check_events() waiting for
>> the lock to be released.
> 
> Do you have lockdep warning on this 'deadlock'?
> 
> Executed immediately doesn't mean the work fn is run in the current
> task context, and it is actually run in one wq worker(task) context, so
> __queue_work() usually wakes up one wq worker for running the work fn,
> then there shouldn't be the 'deadlock' you mentioned.
> 

That's what I thought, too, but then we have a customer report
complaining about a stuck installation, and this kernel message:

> [  990.305908] INFO: task init:1 blocked for more than 491 seconds.
> [  990.311904]       Not tainted 5.3.18-22-default #1
> [  990.316682] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs"
disables this message.
> [  990.324483] init            D    0     1      0 0x00000000
> [  990.329950] Call Trace:
> [  990.332396]  __schedule+0x28a/0x6d0
> [  990.335876]  ? work_busy+0x80/0x80
> [  990.339267]  schedule+0x2f/0xa0
> [  990.342399]  schedule_timeout+0x1dd/0x300
> [  990.346399]  ? check_preempt_curr+0x29/0x80
> [  990.350569]  ? ttwu_do_wakeup+0x19/0x150
> [  990.354480]  ? work_busy+0x80/0x80
> [  990.357869]  wait_for_completion+0xba/0x140
> [  990.362040]  ? wake_up_q+0xa0/0xa0
> [  990.365430]  __flush_work+0x177/0x1d0
> [  990.369080]  ? worker_detach_from_pool+0xa0/0xa0
> [  990.373682]  __cancel_work_timer+0x12b/0x1b0
> [  990.377940]  ? exact_lock+0xd/0x20
> [  990.381329]  ? kobj_lookup+0x113/0x160
> [  990.385067]  disk_block_events+0x78/0x90
> [  990.388979]  __blkdev_get+0x6d/0x7e0
> [  990.392542]  ? blkdev_get_by_dev+0x40/0x40
> [  990.396627]  do_dentry_open+0x1ea/0x380
> [  990.400450]  path_openat+0x2fc/0x1520
> [  990.404103]  do_filp_open+0x9b/0x110
> [  990.407667]  ? kmem_cache_alloc+0x3d/0x250
> [  990.411749]  ? do_sys_open+0x1bd/0x250
> [  990.415486]  do_sys_open+0x1bd/0x250
> [  990.419052]  do_syscall_64+0x5b/0x1e0
> [  990.422701]  entry_SYSCALL_64_after_hwframe+0x44/0xa9

Which does vanish with this patch.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		        Kernel Storage Architect
hare@suse.de			               +49 911 74053 688
SUSE Software Solutions Germany GmbH, 90409 Nürnberg
GF: F. Imendörffer, HRB 36809 (AG Nürnberg)