Skip to content

Commit

Permalink
Fix truncate(2) mtime and ctime handling
Browse files Browse the repository at this point in the history
On Linux, ftruncate(2) always changes the file timestamps, even if the
file size is not changed. However, in case of a successfull
truncate(2), the timestamps are updated only if the file size changes.
This translates to the VFS calling the ZFS Posix Layer "setattr"
function (zpl_setattr) with ATTR_MTIME and ATTR_CTIME unconditionally
set on the iattr mask only when doing a ftruncate(2), while the
truncate(2) is left to the filesystem implementation to be dealt with.

This behaviour is consistent with POSIX:2004/SUSv3 specifications
where there's no explicit requirement for file size changes to update
the timestamps only for ftruncate(2):

http://pubs.opengroup.org/onlinepubs/009695399/functions/truncate.html
http://pubs.opengroup.org/onlinepubs/009695399/functions/ftruncate.html

This has been later updated in POSIX:2008/SUSv4 where, for both
truncate(2)/ftruncate(2), there's no mention of this size change
requirement:

http://austingroupbugs.net/view.php?id=489
http://pubs.opengroup.org/onlinepubs/9699919799/functions/truncate.html
http://pubs.opengroup.org/onlinepubs/9699919799/functions/ftruncate.html

Unfortunately the Linux VFS is still calling into the ZPL without
ATTR_MTIME/ATTR_CTIME set in the truncate(2) case: we fix this by
explicitly updating the timestamps when detecting the ATTR_SIZE bit,
which is always set in do_truncate(), on the iattr mask.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: loli10K <[email protected]>
Closes openzfs#6811 
Closes openzfs#6819
  • Loading branch information
loli10K authored and behlendorf committed Nov 13, 2017
1 parent 13042a6 commit 99834d1
Show file tree
Hide file tree
Showing 7 changed files with 221 additions and 4 deletions.
4 changes: 2 additions & 2 deletions module/zfs/zfs_vnops.c
Original file line number Diff line number Diff line change
Expand Up @@ -3155,7 +3155,7 @@ zfs_setattr(struct inode *ip, vattr_t *vap, int flags, cred_t *cr)
&atime, sizeof (atime));
}

if (mask & ATTR_MTIME) {
if (mask & (ATTR_MTIME | ATTR_SIZE)) {
ZFS_TIME_ENCODE(&vap->va_mtime, mtime);
ZTOI(zp)->i_mtime = timespec_trunc(vap->va_mtime,
ZTOI(zp)->i_sb->s_time_gran);
Expand All @@ -3164,7 +3164,7 @@ zfs_setattr(struct inode *ip, vattr_t *vap, int flags, cred_t *cr)
mtime, sizeof (mtime));
}

if (mask & ATTR_CTIME) {
if (mask & (ATTR_CTIME | ATTR_SIZE)) {
ZFS_TIME_ENCODE(&vap->va_ctime, ctime);
ZTOI(zp)->i_ctime = timespec_trunc(vap->va_ctime,
ZTOI(zp)->i_sb->s_time_gran);
Expand Down
2 changes: 1 addition & 1 deletion tests/runfiles/linux.run
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,7 @@ tests = ['tmpfile_001_pos', 'tmpfile_002_pos', 'tmpfile_003_pos']
tags = ['functional', 'tmpfile']

[tests/functional/truncate]
tests = ['truncate_001_pos', 'truncate_002_pos']
tests = ['truncate_001_pos', 'truncate_002_pos', 'truncate_timestamps']
tags = ['functional', 'truncate']

[tests/functional/upgrade]
Expand Down
30 changes: 30 additions & 0 deletions tests/zfs-tests/include/math.shlib
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,33 @@ function to_bytes

return 0
}

#
# Verify $a is equal to $b, otherwise raise an error specifying
# the $type of values being compared
#
function verify_eq # <a> <b> <type>
{
typeset a=$1
typeset b=$2
typeset type=$3

if [[ $a -ne $b ]]; then
log_fail "Compared $type should be equal: $a != $b"
fi
}

#
# Verify $a is not equal to $b, otherwise raise an error specifying
# the $type of values being compared
#
function verify_ne # <a> <b> <type>
{
typeset a=$1
typeset b=$2
typeset type=$3

if [[ $a -eq $b ]]; then
log_fail "Compared $type should be not equal: $a == $b"
fi
}
1 change: 1 addition & 0 deletions tests/zfs-tests/tests/functional/truncate/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/truncate_test
11 changes: 10 additions & 1 deletion tests/zfs-tests/tests/functional/truncate/Makefile.am
Original file line number Diff line number Diff line change
@@ -1,7 +1,16 @@
include $(top_srcdir)/config/Rules.am

pkgdatadir = $(datadir)/@PACKAGE@/zfs-tests/tests/functional/truncate

dist_pkgdata_SCRIPTS = \
setup.ksh \
cleanup.ksh \
truncate.cfg \
truncate_001_pos.ksh \
truncate_002_pos.ksh
truncate_002_pos.ksh \
truncate_timestamps.ksh

pkgexecdir = $(datadir)/@PACKAGE@/zfs-tests/tests/functional/truncate

pkgexec_PROGRAMS = truncate_test
truncate_test_SOURCES = truncate_test.c
103 changes: 103 additions & 0 deletions tests/zfs-tests/tests/functional/truncate/truncate_test.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
/*
* This file and its contents are supplied under the terms of the
* Common Development and Distribution License ("CDDL"), version 1.0.
* You may only use this file in accordance with the terms of version
* 1.0 of the CDDL.
*
* A full copy of the text of the CDDL should have accompanied this
* source. A copy of the CDDL is also available via the Internet at
* http://www.illumos.org/license/CDDL.
*/

/*
* Copyright (c) 2012, 2014 by Delphix. All rights reserved.
* Copyright 2017, loli10K <[email protected]>. All rights reserved.
*/

#include <fcntl.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <unistd.h>
#include <errno.h>
#include <stdio.h>
#include <stdlib.h>

#define FSIZE 256*1024*1024

static long fsize = FSIZE;
static int errflag = 0;
static char *filename = NULL;
static int ftruncflag = 0;

static void parse_options(int argc, char *argv[]);

static void
usage(char *execname)
{
(void) fprintf(stderr,
"usage: %s [-s filesize] [-f] /path/to/file\n", execname);
(void) exit(1);
}

int
main(int argc, char *argv[])
{
int fd;

parse_options(argc, argv);

if (ftruncflag) {
fd = open(filename, O_RDWR|O_CREAT, 0666);
if (fd < 0) {
perror("open");
return (1);
}
if (ftruncate(fd, fsize) < 0) {
perror("ftruncate");
return (1);
}
if (close(fd)) {
perror("close");
return (1);
}
} else {
if (truncate(filename, fsize) < 0) {
perror("truncate");
return (1);
}
}
return (0);
}

static void
parse_options(int argc, char *argv[])
{
int c;
extern char *optarg;
extern int optind, optopt;

while ((c = getopt(argc, argv, "s:f")) != -1) {
switch (c) {
case 's':
fsize = atoi(optarg);
break;
case 'f':
ftruncflag++;
break;
case ':':
(void) fprintf(stderr,
"Option -%c requires an operand\n", optopt);
errflag++;
break;
}
if (errflag) {
(void) usage(argv[0]);
}
}

if (argc <= optind) {
(void) fprintf(stderr, "No filename specified\n");
usage(argv[0]);
}
filename = argv[optind];
}
74 changes: 74 additions & 0 deletions tests/zfs-tests/tests/functional/truncate/truncate_timestamps.ksh
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
#!/bin/ksh -p
#
# This file and its contents are supplied under the terms of the
# Common Development and Distribution License ("CDDL"), version 1.0.
# You may only use this file in accordance with the terms of version
# 1.0 of the CDDL.
#
# A full copy of the text of the CDDL should have accompanied this
# source. A copy of the CDDL is also available via the Internet at
# http://www.illumos.org/license/CDDL.
#

#
# Copyright 2017, loli10K <[email protected]>. All rights reserved.
#

. $STF_SUITE/tests/functional/truncate/truncate.cfg
. $STF_SUITE/include/libtest.shlib
. $STF_SUITE/include/math.shlib

#
# DESCRIPTION:
# Ensure both truncate(2)/ftruncate(2) update target file mtime/ctime attributes
#
# STRATEGY:
# 1. Create a file
# 2. Truncate the file
# 3. Verify both mtime/ctime are updated
# 4. Rinse and repeat for both truncate(2) and ftruncate(2) with various sizes
#

verify_runnable "both"

function verify_truncate # <filename> <filesize> <option>
{
typeset filename="$1"
typeset -i size="$2"
typeset option="$3"

log_must mkfile $sizeavg $filename # always start with $sizeavg
typeset -i timestm="$(stat -c %Y $filename)"
typeset -i timestc="$(stat -c %Z $filename)"
log_must sleep 1
log_must $STF_SUITE/tests/functional/truncate/truncate_test -s $size $filename $option
verify_eq $size "$(stat -c %s $filename)" "size"
verify_ne $timestm "$(stat -c %Y $filename)" "mtime"
verify_ne $timestc "$(stat -c %Z $filename)" "ctime"
log_must rm -f $filename
}

function cleanup
{
[[ -f $truncfile ]] && rm -f $truncfile
}

log_assert "Ensure both truncate(2)/ftruncate(2) update target file timestamps"
log_onexit cleanup

truncfile="$TESTDIR/truncate.$$"
sizemin="123"
sizeavg="$((256*1024))"
sizemax="$((1024*1024))"

# truncate(2)
verify_truncate $truncfile $sizemin ""
verify_truncate $truncfile $sizeavg ""
verify_truncate $truncfile $sizemax ""

# ftruncate(2)
verify_truncate $truncfile $sizemin "-f"
verify_truncate $truncfile $sizeavg "-f"
verify_truncate $truncfile $sizemax "-f"

log_pass "Successful truncation correctly update timestamps"

0 comments on commit 99834d1

Please sign in to comment.