Date:   Thu, 4 May 2023 11:09:42 +0800
From:   Baokun Li <libaokun1@...wei.com>
To:     Theodore Ts'o <tytso@....edu>
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 2023/4/28 13:47, Theodore Ts'o wrote:
> 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.

Yes, first of all, we need to be able to sense whether the current disk has
been removed. We're just sensing an I/O error now, so we're just making
the file system read-only.
>
> 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.

I didn't notice the block_device_ejected() function, and it's really 
awkward

for the file system to detect whether the current disk has been removed.

>> 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.
Yes, I know that! If the block layer can find and clear these dirty 
pages in a
unified manner, there is no need to do this for each file system.

The subsequent solution is to declare the interface at the VFS layer, 
which is
implemented by each file system. When the block layer detects that the disk
is deleted, the block layer invokes the common interface at the VFS layer.
This also sounds good.
>
> 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
> 						
>
Yes, we need to consider a variety of scenarios, which is not a simple 
matter.
Thank you very much for your detailed explanation.

-- 
With Best Regards,
Baokun Li
.

Powered by blists - more mailing lists