[PATCH 5/8] xfs: log timestamp updates

[Mark Tinguely]

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 at redhat.com>
>> Signed-off-by: Christoph Hellwig<hch at lst.de>
>>
>> ---
> ...
>
>> @@ -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 at oss.sgi.com
> 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 at sgi.com