On Fri, Mar 15, 2019 at 10:08:41AM +1100, Dave Chinner wrote: > On Thu, Mar 14, 2019 at 02:29:06PM -0700, Darrick J. Wong wrote: > > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx> > > > > When writing to a delalloc region in the data fork, commit the new > > allocations (of the da reservation) as unwritten so that the mappings > > are only marked written once writeback completes successfully. This > > fixes the problem of stale data exposure if the system goes down during > > targeted writeback of a specific region of a file, as tested by > > generic/042. > > So what does this do to buffered sequential and random write > performance? Whacks it quite a bit -- 10-20% depending on how fast the storage is in the first place. I barely noticed on my usb thumb drive, however. :P That said, shovelling that many writes through the completion workqueues creates a thundering herd problem on the ILOCK so maybe it wouldn't be so bad if we simply dumped the completions on a per-inode queue and only had one thread handling the completions. (As we've been discussing on IRC.) > Next, if the entire delalloc extent being allocated is beyond the > on-disk EOF (typical for extending sequential buffered writes), why > do we want those to be allocated as unwritten? i.e. isn't "allocate > as unwritten" only necessary for delalloc extent allocation > inside EOF? I wasn't sure on this point -- for delalloc extents that start at or above EOF, we can continue go straight to real like we do today. We still have to use the setfilesize transaction to update isize on disk, so it probably doesn't make that big of a difference. If the delalloc extent crosses EOF then I think it makes sense to map it in as unwritten, issue the write, and convert the extent to real during io completion, and not split it up just to avoid having unwritten extents past EOF. IOWS, there wasn't any particular intentionality behind having the code not set PREALLOC if the extent is totally beyond EOF; this was just the bare minimum to get a discussion started. :) --D > Cheers, > > Dave. > -- > Dave Chinner > david@xxxxxxxxxxxxx