John Garry <john.g.garry@xxxxxxxxxx> writes: > On 25/10/2024 10:31, Ritesh Harjani (IBM) wrote: >>>> >>>> - if (atomic && length != fs_block_size) >>>> + if (atomic && length != iter->len) >>>> return -EINVAL; >>> Here you expect just one iter for an atomic write always. >> Here we are lifting the limitation of iomap to support entire iter->len >> rather than just 1 fsblock. > > Sure > >> >>> In 6/6, you are saying that iomap does not allow an atomic write which >>> covers unwritten and written extents, right? >> No, it's not that. If FS does not provide a full mapping to iomap in >> ->iomap_begin then the writes will get split. > > but why would it provide multiple mapping? > >> For atomic writes this >> should not happen, hence the check in iomap above to return -EINVAL if >> mapped length does not match iter->len. >> >>> But for writing a single fs block atomically, we don't mandate it to be >>> in unwritten state. So there is a difference in behavior in writing a >>> single FS block vs multiple FS blocks atomically. >> Same as mentioned above. We can't have atomic writes to get split. >> This patch is just lifting the restriction of iomap to allow more than >> blocksize but the mapped length should still meet iter->len, as >> otherwise the writes can get split. > > Sure, I get this. But I wonder why would we be getting multiple > mappings? Why cannot the FS always provide a single mapping? FS can decide to split the mappings when it couldn't allocate a single large mapping of the requested length. Could be due to - - already allocated extent followed by EOF, - already allocated extent followed by a hole - already mapped extent followed by an extent of different type (e.g. written followed by unwritten or unwritten followed by written) - delalloc (not delalloc since we invalidate respective page cache pages before doing DIO). - fragmentation or ENOSPC - For ext4 bigalloc this will not happen since we reserve the entire cluster. So we know there should be space. But I am not sure how other filesystems might end up implementing this functionality. Thanks! -ritesh
