John Garry <john.g.garry@xxxxxxxxxx> writes: > On 20/10/2024 09:21, Ritesh Harjani (IBM) wrote: >>> -293,7 +295,8 @@ static loff_t iomap_dio_bio_iter(const struct iomap_iter *iter, >>> const struct iomap *iomap = &iter->iomap; >>> struct inode *inode = iter->inode; >>> unsigned int fs_block_size = i_blocksize(inode), pad; >>> - loff_t length = iomap_length(iter); >>> + const loff_t length = iomap_length(iter); >>> + bool atomic = iter->flags & IOMAP_ATOMIC; >>> loff_t pos = iter->pos; >>> blk_opf_t bio_opf; >>> struct bio *bio; >>> @@ -303,6 +306,9 @@ static loff_t iomap_dio_bio_iter(const struct iomap_iter *iter, >>> size_t copied = 0; >>> size_t orig_count; >>> >>> + if (atomic && length != fs_block_size) >>> + return -EINVAL; >> We anyway mandate iov_iter_count() write should be same as sb_blocksize >> in xfs_file_write_iter() for atomic writes. >> This comparison here is not required. I believe we do plan to lift this >> restriction maybe when we are going to add forcealign support right? > > Yes, we would lift this restriction if and when forcealign is added. Or > when bigalloc is leveraged for ext4 atomic writes. > > But I think that today it is proper to add this check, as we are saying > that iomap DIO path does not support anything else than fs_block_size. > > For forcealign, we were introducing support for atomic writes spanning > mixed unwritten and written extents in [0]. We don't have that support > here, so it is prudent to say that we just support fs_block_size. > > [0] > https://lore.kernel.org/linux-xfs/20240607143919.2622319-4-john.g.garry@xxxxxxxxxx/ > Sure. >> >> And similarly this needs to be lifted when ext4 adds support for atomic >> write even with bigalloc. I hope we can do so when we add such support, right? > > Right > Thanks for confirming that. The patch looks good to me. Please feel free to add - Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@xxxxxxxxx>
