On Fri, May 31, 2024 at 06:31:36AM -0700, Christoph Hellwig wrote: > > + write_back = newsize > ip->i_disk_size && oldsize != ip->i_disk_size; > > Maybe need_writeback would be a better name for the variable? Also no > need to initialize it to false at declaration time if it is > unconditionally set here. This variable captures whether or not we need to write dirty file tail data because we're extending the ondisk EOF, right? I don't really like long names like any good 1980s C programmer, but maybe we should name this something like "extending_ondisk_eof"? if (newsize > ip->i_disk_size && oldsize != ip->i_disk_size) extending_ondisk_eof = true; ... if (did_zeroing || extending_ondisk_eof) filemap_write_and_wait_range(...); Hm? > > + /* > > + * Updating i_size after writing back to make sure the zeroed > > + * blocks could been written out, and drop all the page cache > > + * range that beyond blocksize aligned new EOF block. > > + * > > + * We've already locked out new page faults, so now we can > > + * safely remove pages from the page cache knowing they won't > > + * get refaulted until we drop the XFS_MMAP_EXCL lock after the And can we correct the comment here too? "...until we drop XFS_MMAPLOCK_EXCL after the extent manipulations..." --D > > + * extent manipulations are complete. > > + */ > > + i_size_write(inode, newsize); > > + truncate_pagecache(inode, roundup_64(newsize, blocksize)); > > Any reason this open codes truncate_setsize()? > >
