Received-SPF: pass (google.com: domain of linux-ext4+bounces-65-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Mon, 20 Nov 2023 22:08:31 -0800
From: Christoph Hellwig <hch@infradead.org>
To: Ritesh Harjani <ritesh.list@gmail.com>
Cc: Christoph Hellwig <hch@infradead.org>, linux-ext4@vger.kernel.org,
	Jan Kara <jack@suse.cz>, linux-fsdevel@vger.kernel.org
Subject: Re: [RFC 2/3] ext2: Convert ext2 regular file buffered I/O to use
 iomap
Message-ID: <ZVxJXxKipvxcPlSo@infradead.org>
References: <ZVw1xxNYQuHimSmx@infradead.org>
 <874jhfy7i8.fsf@doe.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <874jhfy7i8.fsf@doe.com>

On Tue, Nov 21, 2023 at 11:26:15AM +0530, Ritesh Harjani wrote:
> > instantly spot anything that relies on them - you are just a lot more
> > likely to hit an -ENOSPC from ->map_blocks now.
> 
> Which is also true with existing code no? If the block reservation is
> not done at the write fault, writeback is likely to fail due to ENOSPC?

Yes. Not saying you should change this, I just want to make sure the
iomap code handles this fine.  I think it does, but I'd rather be sure.

> Sure, make sense. Thanks!
> I can try and check if the the wrapper helps.

Let's wait until we have a few more conversions.

> > Did yo run into issues in using the iomap based aops for the other uses
> > of ext2_aops, or are just trying to address the users one at a time?
> 
> There are problems for e.g. for dir type in ext2. It uses the pagecache
> for dir. It uses buffer_heads and attaches them to folio->private.
>     ...it uses block_write_begin/block_write_end() calls.
>     Look for ext4_make_empty() -> ext4_prepare_chunk ->
>     block_write_begin(). 
> Now during sync/writeback of the dirty pages (ext4_handle_dirsync()), we
> might take a iomap writeback path (if using ext2_file_aops for dir)
> which sees folio->private assuming it is "struct iomap_folio_state".
> And bad things will happen... 

Oh, indeed, bufferheads again.

> Now we don't have an equivalent APIs in iomap for
> block_write_begin()/end() which the users can call for. Hence, Jan
> suggested to lets first convert ext2 regular file path to iomap as an RFC.

Yes, no problem.  But maybe worth documenting in the commit log.