On Tue, Jul 24, 2007 at 06:59:53PM -0700, Michael Nishimoto wrote:
> 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)
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.
> The code isn't exactly the same, but it appears to do the same work.
Locking is the key difference.
> 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.
--
Dave Chinner
Principal Engineer
SGI Australian Software Group
|