On Thu, Mar 14, 2019 at 08:13:03PM -0700, Darrick J. Wong wrote: > 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 I figured that would be the case, with random writes being much worse... > 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.) *nod* > > 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. We have to keep the setfilesize completion code, anyway (think partial last block file extensions), but the setfilesize stuff is much, much lower overhead than unwritten extent conversion so i think we want to avoid unwritten extents where-ever they are unnecessary. > 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. Yup, makes sense. For a sequential write, they should always be beyond EOF. For anything else, we use unwritten. > 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. :) *nod* Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx
