On 2024/5/31 23:27, Darrick J. Wong wrote: > 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? Sure, this name looks better. > >>> + /* >>> + * 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..." > Sure, > --D > >>> + * extent manipulations are complete. >>> + */ >>> + i_size_write(inode, newsize); >>> + truncate_pagecache(inode, roundup_64(newsize, blocksize)); >> >> Any reason this open codes truncate_setsize()? >> It's not equal to open codes truncate_setsize(), please look the truncate start pos is aligned to rtextsize for realtime inode, we only drop page cache that beyond the new aligned EOF block. Thanks, Yi.
