On Thu, May 08, 2025 at 08:05:27PM +0530, Ritesh Harjani wrote: > John Garry <john.g.garry@xxxxxxxxxx> writes: > > > On 30/04/2025 06:20, Ritesh Harjani (IBM) wrote: > >> This is still an early preview (RFC v2) of multi-fsblock atomic write. Since the > >> core design of the feature looks ready, wanted to post this for some early > >> feedback. We will break this into more smaller and meaningful patches in later > >> revision. However to simplify the review of the core design changes, this > >> version is limited to just two patches. Individual patches might have more > >> details in the commit msg. > >> > >> Note: This overall needs more careful review (other than the core design) which > >> I will be doing in parallel. However it would be helpful if one can provide any > >> feedback on the core design changes. Specially around ext4_iomap_alloc() > >> changes, ->end_io() changes and a new get block flag > >> EXT4_GET_BLOCKS_QUERY_LEAF_BLOCKS. > > > > I gave this a try and it looks ok, specifically atomic writing mixed > > mappings. > > > > Thanks John for taking a look. > > > I'll try to look closer that the implementation details. > > We are in the process of sending v3 (hopefully by tonight) which is an > improved version w.r.t error handling, journal credits and few other > changes. Although nothing has changed w.r.t the design aspect. > > > But I do note > > that you use blkdev_issue_zeroout() to pre-zero any unwritten range > > which is being atomically written. > > Yes, that is how internally ext4_map_blocks() with > EXT4_GET_BLOCKS_CREATE_ZERO will return us the allocated blocks. During > block allocation, on mixed mapping range, we ensure that the entire range > becomes a contiguous mapped extent before starting any data writes. > That means calling ext4_map_blocks() in a loop with > EXT4_GET_BLOCKS_CREATE_ZERO, so that it can zero out any unwritten > extents in the requested region. > I assume writing over a mixed mapping region is not a performance > critical path. > > Do you forsee any problems with the approach (since you said "But I do note...")? It's a little dumb to write zeroes just so you can atomicwrite a block. However, ext4 lacks an out of place write handler, so I don't think there's much else that can be done easily. --D > -ritesh