Thread (2 messages) 2 messages, 2 authors, 2020-08-17

Re: [PATCH] block: don't read block device if it's invalid

From: Christoph Hellwig <hch@infradead.org>
Date: 2020-08-17 06:42:46
Also in: linux-nvme, lkml

On Tue, Aug 11, 2020 at 09:43:06AM -0400, Xianting Tian wrote:
We found several processes in 'D' state after nvme device hot-removed,
The call trace as below, we can see process 848 got lock 'bdev->bd_mutex'
in blkdev_reread_part(), but scheduled out due to wait for IO done. But
the IO won't be completed as the device is hot-removed. Then it caused
the lock 'bdev->bd_mutex' can't be unlocked. As a result, it caused
other processes, which need to get the same lock 'bdev->bd_mutex',
blocked on this lock.

When nvme device hot-removed, kernel will start a thread to handle the
task of nvme device removing, as the call trace of process 1111504 shows
below. I listed the call trace of nvme_kill_queues() in detail as below,
we can see 'NVME_NS_DEAD' is set, then when executing
nvme_revalidate_disk(), it found 'NVME_NS_DEAD' is set and
'set_capacity(disk, 0)' will be called to set disk capacity to 0.
    nvme_kill_queues()
        if (test_and_set_bit(NVME_NS_DEAD, &ns->flags)) return;
            revalidate_disk(disk)
                disk->fops->revalidate_disk(disk) <=for nvme device, revalidate_disk=nvme_revalidate_disk()
                     mutex_lock(&bdev->bd_mutex)

This patch is to reduce the probability of such problem. Before getting
the lock of 'bdev->bd_mutex' in blkdev_reread_part(), add the code to
check if the capacity of the disk is 0, just return. Then we can avoid
the happen of the issue:
nvme device is hot-removed, and its capacity is alreday set to 0; then
if there is process like 848 want to read the device, it will return
directly in blkdev_reread_part(), then it will not get the lock
"bdev->bd_mutex", which can't be unlocked by the process itself as IO
can't be completed.
We need to fix this for real, as you stated at best this reduces the
window that the race can happen.

I think our main problem is that due to bd_mutex we can't update the
block device size from arbitrary context.  If we instead add an irqsave
spinlock just for the size we'd get rid of the limitation and can stop
papering over the problem.  Give m a little time to try to do that.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help