Couple of code comments
Hi,
I've got a couple of code comments. In xfs_write, there is a call to bhv_vop_flushinval_pages(). It appears that this function call is not needed because the call to generic_file_direct_write() done later in xfs_write does the same work.
generic_file_direct_write -> generic_file_direct_IO -> (filemap_write_and_wait + invalidate_inode_pages2_range)
bhv_vop_flushinval_pages -> fs_flushinval_pages -> (filemap_write_and_wait + truncate_inode_pages)
The code isn't exactly the same, but it appears to do the same work.
Also, within xfs_swap_extents() the code fixes up on-disk inode block counts, but doesn't change the block counts for the Linux inode.
Comments?
Michael
Quoted 7 lines · Michael Nishimoto
Hi,
I've got a couple of code comments. In
xfs_write, there is a call tobhv_vop_flushinval_pages(). It appears that this function call is not needed because the call togeneric_file_direct_write()done later inxfs_writedoes the same work.
Quoted 5 lines
generic_file_direct_write->generic_file_direct_IO-> (filemap_write_and_wait+invalidate_inode_pages2_range)
bhv_vop_flushinval_pages->fs_flushinval_pages-> (filemap_write_and_wait+truncate_inode_pages)
Similar, but seeing we call generic_file_direct_write() without the i_mutex held and we don't use the generic direct I/O path locking or flushes (i.e in __blockdev_direct_IO()), we've got to do this flush somewhere with the i_mutex held....
That being said, xfs_write() is an utter mess and could do with a major cleanup.
Quoted 1 line
The code isn't exactly the same, but it appears to do the same work.
Locking is the key difference.
Quoted 2 lines
Also, within
xfs_swap_extents()the code fixes up on-disk inode block counts, but doesn't change the block counts for the Linux inode.
Patch, please. ;)
Cheers,
Dave.
Signature · 3 lines
Dave Chinner Principal Engineer SGI Australian Software Group