Review: fix truncate vs null files issues
When we change the file size via xfs_setattr(), we log the new, in-memory file size in the transaction. We do this without having flushed any dirty data so if we have previously extended the file we change the on disk file size without having written the data first.
This is a problem for both growing the file and truncating the file. The truncate case is partially hidden by the VTRUNCATE code, but if the file has not been closed before a crash has occurred we can still get NULLs appearing in files.
The following patch fixes this problem by flushing the range between the on-disk filesize and the new file size as long as the new file size is larger than the on disk file size.
Comments?
Cheers,
Dave.
Signature · 44 lines
Dave Chinner
Principal Engineer
SGI Australian Software Group
---
fs/xfs/xfs_vnodeops.c | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
Index: 2.6.x-xfs-new/fs/xfs/xfs_vnodeops.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/xfs_vnodeops.c 2007-06-20 17:59:35.050768978 +1000
+++ 2.6.x-xfs-new/fs/xfs/xfs_vnodeops.c 2007-06-21 15:18:17.187347809 +1000
@@ -595,7 +595,30 @@ xfs_setattr(
code = xfs_igrow_start(ip, vap->va_size, credp);
}
xfs_iunlock(ip, XFS_ILOCK_EXCL);
- vn_iowait(vp); /* wait for the completion of any pending DIOs */
+
+ /*
+ * We are going to log the inode size change in this
+ * transaction so any previous writes that are beyond the on
+ * disk EOF and the new EOF that have not been written out need
+ * to be written here. If we do not write the data out, we
+ * expose ourselves to the null files problem.
+ *
+ * Only flush from the on disk size to the smaller of the in
+ * memory file size or the new size as that's the range we
+ * really care about here and prevents waiting for other data
+ * not within the range we care about here.
+ */
+ if (!code &&
+ (ip->i_size != ip->i_d.di_size) &&
+ (vap->va_size > ip->i_d.di_size)) {
+ code = bhv_vop_flush_pages(XFS_ITOV(ip),
+ ip->i_d.di_size, vap->va_size,
+ XFS_B_ASYNC, FI_NONE);
+ }
+
+ /* wait for all I/O to complete */
+ vn_iowait(vp);
+
if (!code)
code = xfs_itruncate_data(ip, vap->va_size);
if (code) {Quoted 12 lines · David Chinner
When we change the file size via
xfs_setattr(), we log the new, in-memory file size in the transaction. We do this without having flushed any dirty data so if we have previously extended the file we change the on disk file size without having written the data first.This is a problem for both growing the file and truncating the file. The truncate case is partially hidden by the VTRUNCATE code, but if the file has not been closed before a crash has occurred we can still get NULLs appearing in files.
Quoted 5 lines
The following patch fixes this problem by flushing the range between the on-disk filesize and the new file size as long as the new file size is larger than the on disk file size.
Comments?
Looks good.
Review
Quoted 2 lines
(quoted diff) +2
-0+ (ip->i_size != ip->i_d.di_size) && + (vap->va_size > ip->i_d.di_size)) {
And if I'd really want to get picky I'd say please remove all the superflous braces here..