Christoph Hellwig <hch@xxxxxxxxxxxxx> writes: > 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. I can write a fstest to test the behavior. >> Sure, make sense. Thanks! >> I can try and check if the the wrapper helps. > > Let's wait until we have a few more conversions. > Sure. >> > 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. Sure, I will update the commit log. -ritesh