[Top] [All Lists]

Re: [PATCH 5/8] xfs: log timestamp updates

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 5/8] xfs: log timestamp updates
From: Mark Tinguely <tinguely@xxxxxxx>
Date: Thu, 01 Mar 2012 13:41:14 -0600
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20120229095508.983470279@xxxxxxxxxxxxxxxxxxxxxx>
References: <20120229095347.009884687@xxxxxxxxxxxxxxxxxxxxxx> <20120229095508.983470279@xxxxxxxxxxxxxxxxxxxxxx>
User-agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:9.0) Gecko/20120122 Thunderbird/9.0
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

<Prev in Thread] Current Thread [Next in Thread>