On 03/01/12 13:41, Mark Tinguely wrote:
On 02/29/12 03:53, Christoph Hellwig wrote:
Timestamps on regular files are the last metadata that XFS does not
update
transactionally. Now that we use the delaylog mode exclusively and made
the log scode scale extremly well there is no need to bypass that code
for
timestamp updates. Logging all updates allows to drop a lot of code, and
will allow for further performance improvements later on.
Note that this patch drops optimized handling of fdatasync - it will be
added back in a separate commit.
Reviewed-by: Dave Chinner<dchinner@xxxxxxxxxx>
Signed-off-by: Christoph Hellwig<hch@xxxxxx>
---
...
@@ -659,9 +598,6 @@ restart:
return error;
}
- if (likely(!(file->f_mode& FMODE_NOCMTIME)))
- file_update_time(file);
-
/*
* If the offset is beyond the size of the file, we need to zero any
* blocks that fall between the existing EOF and the start of this
@@ -685,6 +621,15 @@ restart:
return error;
/*
+ * Updating the timestamps will grab the ilock again from
+ * xfs_fs_dirty_inode, so we have to call it after dropping the
+ * lock above. Eventually we should look into a way to avoid
+ * the pointless lock roundtrip.
+ */
+ if (likely(!(file->f_mode& FMODE_NOCMTIME)))
+ file_update_time(file);
+
I see the inode timestamp update was taken out of the dio path. Are
those being updated in xfs_trans_ichgtime()?
Reviewed-by: Mark Tinguely
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
It is still in the common xfs_file_aio_write_checks() routine. For some
reason, I thought it was removed from the xfs_file_aio_write_checks()
and placed only in the xfs_file_buffered_aio_write() routine and I could
not figure out how the dio case was handled.
Sorry for the noise.
--Mark Tinguely.
tinguely@xxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
