linux-ext4 - Re: [ext4 io hang] buffered write io hang in balance_dirty

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 28 Apr 2023 01:47:22 -0400
From:   "Theodore Ts'o" <tytso@....edu>
To:     Baokun Li <libaokun1@...wei.com>
Cc:     Ming Lei <ming.lei@...hat.com>,
        Matthew Wilcox <willy@...radead.org>,
        linux-ext4@...r.kernel.org,
        Andreas Dilger <adilger.kernel@...ger.ca>,
        linux-block@...r.kernel.org,
        Andrew Morton <akpm@...ux-foundation.org>,
        linux-fsdevel@...r.kernel.org, linux-mm@...ck.org,
        Dave Chinner <dchinner@...hat.com>,
        Eric Sandeen <sandeen@...hat.com>,
        Christoph Hellwig <hch@....de>, Zhang Yi <yi.zhang@...hat.com>,
        yangerkun <yangerkun@...wei.com>
Subject: Re: [ext4 io hang] buffered write io hang in balance_dirty_pages

On Fri, Apr 28, 2023 at 11:47:26AM +0800, Baokun Li wrote:
> Ext4 just detects I/O Error and remounts it as read-only, it doesn't know
> if the current disk is dead or not.
> 
> I asked Yu Kuai and he said that disk_live() can be used to determine
> whether
> a disk has been removed based on the status of the inode corresponding to
> the block device, but this is generally not done in file systems.

What really needs to happen is that del_gendisk() needs to inform file
systems that the disk is gone, so that the file system can shutdown
the file system and tear everything down.

disk_live() is relatively new; it was added in August 2021.  Back in
2015, I had added the following in fs/ext4/super.c:

/*
 * The del_gendisk() function uninitializes the disk-specific data
 * structures, including the bdi structure, without telling anyone
 * else.  Once this happens, any attempt to call mark_buffer_dirty()
 * (for example, by ext4_commit_super), will cause a kernel OOPS.
 * This is a kludge to prevent these oops until we can put in a proper
 * hook in del_gendisk() to inform the VFS and file system layers.
 */
static int block_device_ejected(struct super_block *sb)
{
	struct inode *bd_inode = sb->s_bdev->bd_inode;
	struct backing_dev_info *bdi = inode_to_bdi(bd_inode);

	return bdi->dev == NULL;
}

As the comment states, it's rather awkward to have the file system
check to see if the block device is dead in various places; the real
problem is that the block device shouldn't just *vanish*, with the
block device structures egetting partially de-initialized, without the
block layer being polite enough to let the file system know.

> Those dirty pages that are already there are piling up and can't be
> written back, which I think is a real problem. Can the block layer
> clear those dirty pages when it detects that the disk is deleted?

Well, the dirty pages belong to the file system, and so it needs to be
up to the file system to clear out the dirty pages.  But I'll also
what the right thing to do when a disk gets removed is not necessarily
obvious.

For example, suppose some process has a file mmap'ed into its address
space, and that file is on the disk which the user has rudely yanked
out from their laptop; what is the right thing to do?  Do we kill the
process?  Do we let the process write to the mmap'ed region, and
silently let the modified data go *poof* when the process exits?  What
if there is an executable file on the removable disk, and there are
one or more processes running that executable when the device
disappears?  Do we kill the process?  Do we let the process run unti
it tries to access a page which hasn't been paged in and then kill the
process?

We should design a proper solution for What Should Happen when a
removable disk gets removed unceremoniously without unmounting the
file system first.  It's not just a matter of making some tests go
green....

						- Ted