On 2024/5/11 16:19, Dave Chinner wrote: > On Sat, May 11, 2024 at 03:43:17PM +0800, Zhang Yi wrote: >> On 2024/5/11 11:45, Dave Chinner wrote: >>> On Sat, May 11, 2024 at 11:11:32AM +0800, Zhang Yi wrote: >>>> On 2024/5/8 17:01, Chandan Babu R wrote: >>> It might actually be easiest to pass the block size for zeroing into >>> iomap_truncate_page() rather than relying on being able to extract >>> the zeroing range from the inode via i_blocksize(). We can't use >>> i_blocksize() for RT files, because inode->i_blkbits and hence >>> i_blocksize() only supports power of 2 block sizes. Changing that is >>> a *much* bigger job, so fixing xfs_truncate_page() is likely the >>> best thing to do right now... >>> >> >> Thanks for the explanation and suggestion, I agree with you. However, >> why do you recommend to pass block size for zeroing in to >> iomap_truncate_page()? It's looks like we could fix xfs_truncate_page() >> by using iomap_zero_range() and dax_zero_range() instead and don't use >> iomap_truncate_page() and dax_truncate_page(). > > Fair question. Yes, we could just fix it in XFS. > > But then any other filesystem that uses iomap might have the same > problem where the allocation block size (and hence the range that > needs zeroing) is different to the filesystem block size (e.g. ext4 > with bigalloc?). At that point, those filesystem developers need to: > > a) be aware of the issue; and > b) implement their own iomap_zero_range() wrapper for the > same function. > > If the iomap infrastructure doesn't assume block sizes are always > i_blocksize() (which is clearly a bad assumption!), then the API > itself informs the filesystem developers of the fact they really > have to care about using the correct allocation block sizes during > EOF zeroing. > > At the moment, only XFS uses iomap_truncate_page(), and only ext2 > and XFS use dax_truncate_page(). It seems pretty simple to change > the infrastructure and document why we don't use i_blocksize() at > this point. Especially considering we have forced alignment stuff in > XFS coming up which further decouples allocation unit (and hence > zeroing range) from the filesystem block size.... > ext4 with bigalloc still use block size (not cluster size) to manage allocation extents, so it aligned to the block size when truncate and doesn't have this problem now. But it doesn't matter, your suggestion make sense to me, pass block size to iomap_truncate_page() and dax_truncate_page() could make them to have better compatibility. Thanks, Yi.
