Change in NFS client behavior

Change in NFS client behavior

[Rob Sims]

We have noticed when changing from kernel 2.4.23 to 2.6.8 that
timestamps of files are not changed if opened for a write and nothing is
written.  When using 2.4.23 timestamps are changed.  When using a local
filesystem (reiserfs) with either kernel, timestamps are changed.
Symptoms vary with the client, not the server.  See the script below.

When run on a 2.4.23 machine in an NFS mounted directory, output is
"Good."  When run on a 2.6.8 or 2.6.12-rc4 machine in an NFS directory,
output is "Error."

Is this a bug?  How do we revert to the 2.4/local fs behavior?  

Thanks,
Rob

#!/bin/sh

if [ -n "$1" ]; then
  if [ -e "$1" ]; then
    printf "%s exists - please specify a new file name.\n" "$1"
  else
    touch $1
    origtime=`stat -c '%X %Y %Z' "$1"`
    sleep 5
    cat /dev/null > "$1"
    newtime=`stat -c '%X %Y %Z' "$1"`
    rm "$1"

    printf "%s\n%s\n" "$origtime" "$newtime"
    if [ "$origtime" = "$newtime" ]; then
      printf "Error - timestamps not modified\n"
    else
      printf "Good - timestamps modified\n"
    fi
  fi
else
  printf "Please specify a file name.\n"
fi

Re: Change in NFS client behavior

[Trond Myklebust]

[-- Attachment #1: Type: text/plain, Size: 1028 bytes --]

on den 31.08.2005 Klokka 08:55 (-0600) skreiv Rob Sims:
> We have noticed when changing from kernel 2.4.23 to 2.6.8 that
> timestamps of files are not changed if opened for a write and nothing is
> written.  When using 2.4.23 timestamps are changed.  When using a local
> filesystem (reiserfs) with either kernel, timestamps are changed.
> Symptoms vary with the client, not the server.  See the script below.
> 
> When run on a 2.4.23 machine in an NFS mounted directory, output is
> "Good."  When run on a 2.6.8 or 2.6.12-rc4 machine in an NFS directory,
> output is "Error."
> 
> Is this a bug?  How do we revert to the 2.4/local fs behavior?  

This is a consequence of 2.6 NFS clients optimising away unnecessary
truncate calls. Whereas this is correct behaviour for truncate(), it
appears to be incorrect for open(O_TRUNC).

In fact, local filesystems like xfs and ext3 appear to have the opposite
problem: they change ctime if you call ftruncate(0) on the zero-length
file, as the attached test shows.

Cheers,
  Trond



[-- Attachment #2: test.c --]
[-- Type: text/x-csrc, Size: 1447 bytes --]

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

int main(int argc, char *argv[])
{
	struct stat buf1, buf2, buf3;
	int fd;

	if (argc != 2) {
		printf("syntax: %s filename\n", argv[0]);
		exit(1);
	}
	fd = open(argv[1], O_CREAT|O_EXCL|O_WRONLY, 0644);
	if (fd == -1) {
		perror("open(%s, O_CREAT|O_EXCL|O_WRONLY) failed\n", argv[1]);
		exit(1);
	}
	if (fstat(fd, &buf1) == -1) {
		perror("fstat() failed\n");
		exit(1);
	}
	printf("File: %s, st_size = %lu, st_ctime = %s\n", argv[1],
			buf1.st_size,
			asctime(localtime(&buf1.st_ctime)));
	close(fd);
	sleep(2);
	fd = open(argv[1], O_TRUNC|O_WRONLY);
	if (fd == -1) {
		perror("open(%s, O_TRUNC|O_WRONLY) failed\n", argv[1]);
		exit(1);
	}
	if (fstat(fd, &buf2) == -1) {
		perror("fstat() failed\n");
		exit(1);
	}
	printf("File: %s, st_size = %lu, st_ctime = %s\n", argv[1],
			buf2.st_size,
			asctime(localtime(&buf2.st_ctime)));
	if (buf1.st_ctime == buf2.st_ctime)
		printf("Bad behaviour in open(%s, O_TRUNC)!\n", argv[1]);
	sleep(2);
	if (ftruncate(fd, 0) == -1) {
		perror("ftruncate(0) failed\n");
		exit(1);
	}
	if (fstat(fd, &buf3) == -1) {
		perror("fstat() failed\n");
		exit(1);
	}
	printf("File: %s, st_size = %lu, st_ctime = %s\n", argv[1],
			buf3.st_size,
			asctime(localtime(&buf3.st_ctime)));
	if (buf2.st_ctime != buf3.st_ctime)
		printf("Bad behaviour in ftruncate(0)!\n");
	close(fd);
	exit(0);
}

Re: Change in NFS client behavior

[Trond Myklebust]

[-- Attachment #1: Type: text/plain, Size: 560 bytes --]

to den 01.09.2005 Klokka 19:38 (-0400) skreiv Trond Myklebust:
> This is a consequence of 2.6 NFS clients optimising away unnecessary
> truncate calls. Whereas this is correct behaviour for truncate(), it
> appears to be incorrect for open(O_TRUNC).
> 
> In fact, local filesystems like xfs and ext3 appear to have the opposite
> problem: they change ctime if you call ftruncate(0) on the zero-length
> file, as the attached test shows.

Rob,

Could you please check if the following patch fixes NFS (and also the
local filesystems) for you?

Cheers,
  Trond


[-- Attachment #2: linux-2.6.13-37-fix_create_truncate.dif --]
[-- Type: text/plain, Size: 2198 bytes --]

VFS/NFS: Fix up behaviour w.r.t. truncate() and open(O_TRUNC)

 POSIX and the SUSv3 specify that open(O_TRUNC) should always bump ctime/mtime
 whereas truncate() should only do so if the file size actually changes.

 Fix the behaviour of NFS, which currently is broken w.r.t. open(), and fix
 the VFS truncate() so that it no enforces the POSIX rules.

 Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 nfs/inode.c |    5 -----
 open.c      |   12 ++++++++++--
 2 files changed, 10 insertions(+), 7 deletions(-)

Index: linux-2.6.13/fs/nfs/inode.c
===================================================================
--- linux-2.6.13.orig/fs/nfs/inode.c
+++ linux-2.6.13/fs/nfs/inode.c
@@ -833,11 +833,6 @@ nfs_setattr(struct dentry *dentry, struc
 	struct nfs_fattr fattr;
 	int error;
 
-	if (attr->ia_valid & ATTR_SIZE) {
-		if (!S_ISREG(inode->i_mode) || attr->ia_size == i_size_read(inode))
-			attr->ia_valid &= ~ATTR_SIZE;
-	}
-
 	/* Optimization: if the end result is no change, don't RPC */
 	attr->ia_valid &= NFS_VALID_ATTRS;
 	if (attr->ia_valid == 0)
Index: linux-2.6.13/fs/open.c
===================================================================
--- linux-2.6.13.orig/fs/open.c
+++ linux-2.6.13/fs/open.c
@@ -211,6 +211,14 @@ int do_truncate(struct dentry *dentry, l
 	return err;
 }
 
+static inline int do_posix_truncate(struct dentry *dentry, loff_t length)
+{
+	/* In SuS/Posix lore, truncate to the current file size is a no-op */
+	if (length == i_size_read(dentry->d_inode))
+		return 0;
+	return do_truncate(dentry, length);
+}
+
 static inline long do_sys_truncate(const char __user * path, loff_t length)
 {
 	struct nameidata nd;
@@ -261,7 +269,7 @@ static inline long do_sys_truncate(const
 	error = locks_verify_truncate(inode, NULL, length);
 	if (!error) {
 		DQUOT_INIT(inode);
-		error = do_truncate(nd.dentry, length);
+		error = do_posix_truncate(nd.dentry, length);
 	}
 	put_write_access(inode);
 
@@ -313,7 +321,7 @@ static inline long do_sys_ftruncate(unsi
 
 	error = locks_verify_truncate(inode, file, length);
 	if (!error)
-		error = do_truncate(dentry, length);
+		error = do_posix_truncate(dentry, length);
 out_putf:
 	fput(file);
 out:

Re: Change in NFS client behavior

[Andrew Morton]

Trond Myklebust <trond.myklebust@fys.uio.no> wrote:
>
>  +static inline int do_posix_truncate(struct dentry *dentry, loff_t length)
>  +{
>  +	/* In SuS/Posix lore, truncate to the current file size is a no-op */
>  +	if (length == i_size_read(dentry->d_inode))
>  +		return 0;
>  +	return do_truncate(dentry, length);
>  +}

We have the same optimisation in inode_setattr()...

Re: Change in NFS client behavior

[Trond Myklebust]

to den 01.09.2005 Klokka 20:45 (-0700) skreiv Andrew Morton:
> Trond Myklebust <trond.myklebust@fys.uio.no> wrote:
> >
> >  +static inline int do_posix_truncate(struct dentry *dentry, loff_t length)
> >  +{
> >  +	/* In SuS/Posix lore, truncate to the current file size is a no-op */
> >  +	if (length == i_size_read(dentry->d_inode))
> >  +		return 0;
> >  +	return do_truncate(dentry, length);
> >  +}
> 
> We have the same optimisation in inode_setattr()...

Look again. The two are NOT the same.

The code in inode_setattr() will cause truncate() to erroneously update
the ctime/mtime.

Cheers,
  Trond

Re: Change in NFS client behavior

[Andrew Morton]

Trond Myklebust <trond.myklebust@fys.uio.no> wrote:
>
> to den 01.09.2005 Klokka 20:45 (-0700) skreiv Andrew Morton:
> > Trond Myklebust <trond.myklebust@fys.uio.no> wrote:
> > >
> > >  +static inline int do_posix_truncate(struct dentry *dentry, loff_t length)
> > >  +{
> > >  +	/* In SuS/Posix lore, truncate to the current file size is a no-op */
> > >  +	if (length == i_size_read(dentry->d_inode))
> > >  +		return 0;
> > >  +	return do_truncate(dentry, length);
> > >  +}
> > 
> > We have the same optimisation in inode_setattr()...
> 
> Look again. The two are NOT the same.
> 
> The code in inode_setattr() will cause truncate() to erroneously update
> the ctime/mtime.

Of course.  But with your patch, the optimisation in inode_setattr() is
redundant (except for O_TRUNC, perhaps).

Re: Change in NFS client behavior

[Trond Myklebust]

[-- Attachment #1: Type: text/plain, Size: 321 bytes --]

to den 01.09.2005 Klokka 21:07 (-0700) skreiv Andrew Morton:

> Of course.  But with your patch, the optimisation in inode_setattr() is
> redundant (except for O_TRUNC, perhaps).

Sure. The other problem is that the test is made before the i_sem is
grabbed. OK, so how about the appended patch instead?

Cheers,
  Trond


[-- Attachment #2: linux-2.6.13-37-fix_create_truncate.dif --]
[-- Type: text/plain, Size: 3467 bytes --]

VFS/NFS: Fix up behaviour w.r.t. truncate() and open(O_TRUNC)

 POSIX and the SUSv3 specify that open(O_TRUNC) should always bump ctime/mtime
 whereas truncate() should only do so if the file size actually changes.

 Fix the behaviour of NFS, which currently is broken w.r.t. open(), and fix
 the VFS truncate() so that it no enforces the POSIX rules.

 Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 attr.c      |   14 +++-----------
 nfs/inode.c |    5 -----
 open.c      |   25 +++++++++++++++++++++++--
 3 files changed, 26 insertions(+), 18 deletions(-)

Index: linux-2.6.13/fs/nfs/inode.c
===================================================================
--- linux-2.6.13.orig/fs/nfs/inode.c
+++ linux-2.6.13/fs/nfs/inode.c
@@ -833,11 +833,6 @@ nfs_setattr(struct dentry *dentry, struc
 	struct nfs_fattr fattr;
 	int error;
 
-	if (attr->ia_valid & ATTR_SIZE) {
-		if (!S_ISREG(inode->i_mode) || attr->ia_size == i_size_read(inode))
-			attr->ia_valid &= ~ATTR_SIZE;
-	}
-
 	/* Optimization: if the end result is no change, don't RPC */
 	attr->ia_valid &= NFS_VALID_ATTRS;
 	if (attr->ia_valid == 0)
Index: linux-2.6.13/fs/open.c
===================================================================
--- linux-2.6.13.orig/fs/open.c
+++ linux-2.6.13/fs/open.c
@@ -206,11 +206,32 @@ int do_truncate(struct dentry *dentry, l
 	newattrs.ia_valid = ATTR_SIZE | ATTR_CTIME;
 
 	down(&dentry->d_inode->i_sem);
+	/* This should be used for open(O_TRUNC) and functions that need to
+	 * set mtime/ctime whether or not the size changes
+	 */
+	if (length == i_size_read(dentry->d_inode))
+		newattrs.ia_valid &= ~ATTR_SIZE;
 	err = notify_change(dentry, &newattrs);
 	up(&dentry->d_inode->i_sem);
 	return err;
 }
 
+static int do_posix_truncate(struct dentry *dentry, loff_t length)
+{
+	int err = 0;
+	struct iattr newattrs;
+
+	newattrs.ia_size = length;
+	newattrs.ia_valid = ATTR_SIZE;
+
+	down(&dentry->d_inode->i_sem);
+	/* In SuS/Posix lore, truncate to the current file size is a no-op */
+	if (length != i_size_read(dentry->d_inode))
+		err = do_truncate(dentry, length);
+	up(&dentry->d_inode->i_sem);
+	return err;
+}
+
 static inline long do_sys_truncate(const char __user * path, loff_t length)
 {
 	struct nameidata nd;
@@ -261,7 +282,7 @@ static inline long do_sys_truncate(const
 	error = locks_verify_truncate(inode, NULL, length);
 	if (!error) {
 		DQUOT_INIT(inode);
-		error = do_truncate(nd.dentry, length);
+		error = do_posix_truncate(nd.dentry, length);
 	}
 	put_write_access(inode);
 
@@ -313,7 +334,7 @@ static inline long do_sys_ftruncate(unsi
 
 	error = locks_verify_truncate(inode, file, length);
 	if (!error)
-		error = do_truncate(dentry, length);
+		error = do_posix_truncate(dentry, length);
 out_putf:
 	fput(file);
 out:
Index: linux-2.6.13/fs/attr.c
===================================================================
--- linux-2.6.13.orig/fs/attr.c
+++ linux-2.6.13/fs/attr.c
@@ -70,17 +70,9 @@ int inode_setattr(struct inode * inode, 
 	int error = 0;
 
 	if (ia_valid & ATTR_SIZE) {
-		if (attr->ia_size != i_size_read(inode)) {
-			error = vmtruncate(inode, attr->ia_size);
-			if (error || (ia_valid == ATTR_SIZE))
-				goto out;
-		} else {
-			/*
-			 * We skipped the truncate but must still update
-			 * timestamps
-			 */
-			ia_valid |= ATTR_MTIME|ATTR_CTIME;
-		}
+		error = vmtruncate(inode, attr->ia_size);
+		if (error || (ia_valid == ATTR_SIZE))
+			goto out;
 	}
 
 	if (ia_valid & ATTR_UID)

Re: Change in NFS client behavior

[Trond Myklebust]

[-- Attachment #1: Type: text/plain, Size: 204 bytes --]

fr den 02.09.2005 Klokka 00:15 (-0400) skreiv Trond Myklebust:

> Sure. The other problem is that the test is made before the i_sem is
> grabbed. OK, so how about the appended patch instead?

Doh!

Trond

[-- Attachment #2: linux-2.6.13-37-fix_create_truncate.dif --]
[-- Type: text/plain, Size: 3472 bytes --]

VFS/NFS: Fix up behaviour w.r.t. truncate() and open(O_TRUNC)

 POSIX and the SUSv3 specify that open(O_TRUNC) should always bump ctime/mtime
 whereas truncate() should only do so if the file size actually changes.

 Fix the behaviour of NFS, which currently is broken w.r.t. open(), and fix
 the VFS truncate() so that it no enforces the POSIX rules.

 Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 attr.c      |   14 +++-----------
 nfs/inode.c |    5 -----
 open.c      |   25 +++++++++++++++++++++++--
 3 files changed, 26 insertions(+), 18 deletions(-)

Index: linux-2.6.13/fs/nfs/inode.c
===================================================================
--- linux-2.6.13.orig/fs/nfs/inode.c
+++ linux-2.6.13/fs/nfs/inode.c
@@ -833,11 +833,6 @@ nfs_setattr(struct dentry *dentry, struc
 	struct nfs_fattr fattr;
 	int error;
 
-	if (attr->ia_valid & ATTR_SIZE) {
-		if (!S_ISREG(inode->i_mode) || attr->ia_size == i_size_read(inode))
-			attr->ia_valid &= ~ATTR_SIZE;
-	}
-
 	/* Optimization: if the end result is no change, don't RPC */
 	attr->ia_valid &= NFS_VALID_ATTRS;
 	if (attr->ia_valid == 0)
Index: linux-2.6.13/fs/open.c
===================================================================
--- linux-2.6.13.orig/fs/open.c
+++ linux-2.6.13/fs/open.c
@@ -206,11 +206,32 @@ int do_truncate(struct dentry *dentry, l
 	newattrs.ia_valid = ATTR_SIZE | ATTR_CTIME;
 
 	down(&dentry->d_inode->i_sem);
+	/* This should be used for open(O_TRUNC) and functions that need to
+	 * set mtime/ctime whether or not the size changes
+	 */
+	if (length == i_size_read(dentry->d_inode))
+		newattrs.ia_valid &= ~ATTR_SIZE;
 	err = notify_change(dentry, &newattrs);
 	up(&dentry->d_inode->i_sem);
 	return err;
 }
 
+static int do_posix_truncate(struct dentry *dentry, loff_t length)
+{
+	int err = 0;
+	struct iattr newattrs;
+
+	newattrs.ia_size = length;
+	newattrs.ia_valid = ATTR_SIZE;
+
+	down(&dentry->d_inode->i_sem);
+	/* In SuS/Posix lore, truncate to the current file size is a no-op */
+	if (length != i_size_read(dentry->d_inode))
+		err = notify_change(dentry, &newattrs);
+	up(&dentry->d_inode->i_sem);
+	return err;
+}
+
 static inline long do_sys_truncate(const char __user * path, loff_t length)
 {
 	struct nameidata nd;
@@ -261,7 +282,7 @@ static inline long do_sys_truncate(const
 	error = locks_verify_truncate(inode, NULL, length);
 	if (!error) {
 		DQUOT_INIT(inode);
-		error = do_truncate(nd.dentry, length);
+		error = do_posix_truncate(nd.dentry, length);
 	}
 	put_write_access(inode);
 
@@ -313,7 +334,7 @@ static inline long do_sys_ftruncate(unsi
 
 	error = locks_verify_truncate(inode, file, length);
 	if (!error)
-		error = do_truncate(dentry, length);
+		error = do_posix_truncate(dentry, length);
 out_putf:
 	fput(file);
 out:
Index: linux-2.6.13/fs/attr.c
===================================================================
--- linux-2.6.13.orig/fs/attr.c
+++ linux-2.6.13/fs/attr.c
@@ -70,17 +70,9 @@ int inode_setattr(struct inode * inode, 
 	int error = 0;
 
 	if (ia_valid & ATTR_SIZE) {
-		if (attr->ia_size != i_size_read(inode)) {
-			error = vmtruncate(inode, attr->ia_size);
-			if (error || (ia_valid == ATTR_SIZE))
-				goto out;
-		} else {
-			/*
-			 * We skipped the truncate but must still update
-			 * timestamps
-			 */
-			ia_valid |= ATTR_MTIME|ATTR_CTIME;
-		}
+		error = vmtruncate(inode, attr->ia_size);
+		if (error || (ia_valid == ATTR_SIZE))
+			goto out;
 	}
 
 	if (ia_valid & ATTR_UID)

Re: Change in NFS client behavior

[Andrew Morton]

Trond Myklebust <trond.myklebust@fys.uio.no> wrote:
>

Looks good from a quick scan.

> +static int do_posix_truncate(struct dentry *dentry, loff_t length)
>  +{
>  +	int err = 0;
>  +	struct iattr newattrs;
>  +
>  +	newattrs.ia_size = length;
>  +	newattrs.ia_valid = ATTR_SIZE;
>  +
>  +	down(&dentry->d_inode->i_sem);
>  +	/* In SuS/Posix lore, truncate to the current file size is a no-op */
>  +	if (length != i_size_read(dentry->d_inode))
>  +		err = notify_change(dentry, &newattrs);
>  +	up(&dentry->d_inode->i_sem);
>  +	return err;
>  +}

I'm not sure that we really need to bother putting the i_size test inside
i_sem.  If somebody is changing the file size in parallel with truncate
then they'll get unpredictable results anyway.  Needs deep thought.

Drop NFS speed on move from kernel 2.4 to 2.6 (was: Change in NFS client behavior)

[Tomasz Kłoczko]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1938 bytes --]

On Wed, 31 Aug 2005, Rob Sims wrote:

> We have noticed when changing from kernel 2.4.23 to 2.6.8 that
> timestamps of files are not changed if opened for a write and nothing is
> written.  When using 2.4.23 timestamps are changed.  When using a local
> filesystem (reiserfs) with either kernel, timestamps are changed.
> Symptoms vary with the client, not the server.  See the script below.
>
> When run on a 2.4.23 machine in an NFS mounted directory, output is
> "Good."  When run on a 2.6.8 or 2.6.12-rc4 machine in an NFS directory,
> output is "Error."
>
> Is this a bug?  How do we revert to the 2.4/local fs behavior?

I have another (IMO strange) observation on move NFS client from kernel 
2.4. to 2.6: drop NFS speed by *~1/3*.

In attachemt is small giff generated by mrtg from data sucked from 
switch SNMP agent. Each day (at night) I perform dayly backups (via NFS) 
and on this plots it visable by dayly high network activity.
Aprox four weeks ago I'm switch on host connected to monitored switch port 
from kernel 2.4 to 2.6. Result can be observer on attached picture by 
reduce maximum bandwitch data transferred on this port by ~1/3 (?!?).

Next smaller drob down (~two weeks ago) which can be observed on this plot 
it is result switchung from UDP to TCP (performed specialy for produce 
some visable data for compare).

~30% drob down NFS client speed on this host isn't critical but probably 
this is kind of top iceberg with some other problems which now exist in 
kernel 2.6. I'm not expert in this area but my frient (Marcin Dalecki) 
says this drop down may have some roots in worse memory management in 
kernel 2.6. Is it can be true or not ?

kloczek
-- 
-----------------------------------------------------------
*Ludzie nie mają problemów, tylko sobie sami je stwarzają*
-----------------------------------------------------------
Tomasz Kłoczko, sys adm @zie.pg.gda.pl|*e-mail: kloczek@rudy.mif.pg.gda.pl*

[-- Attachment #2: Type: APPLICATION/octet-stream, Size: 2705 bytes --]

Re: Change in NFS client behavior

[Rob Sims]

On Thu, Sep 01, 2005 at 11:43:17PM -0400, Trond Myklebust wrote:
> to den 01.09.2005 Klokka 19:38 (-0400) skreiv Trond Myklebust:
> > This is a consequence of 2.6 NFS clients optimising away unnecessary
> > truncate calls. Whereas this is correct behaviour for truncate(), it
> > appears to be incorrect for open(O_TRUNC).

> > In fact, local filesystems like xfs and ext3 appear to have the opposite
> > problem: they change ctime if you call ftruncate(0) on the zero-length
> > file, as the attached test shows.
 
The following patch fixes the problem, at least when applied against
2.6.8:

--- inode.c.orig        2005-08-31 16:54:27.000000000 -0600
+++ inode.c     2005-08-31 17:06:52.000000000 -0600
@@ -756,7 +756,7 @@
 	int error;
 
 	if (attr->ia_valid & ATTR_SIZE) {
-		if (!S_ISREG(inode->i_mode) || attr->ia_size == i_size_read(inode))
+		if (!S_ISREG(inode->i_mode))
 	attr->ia_valid &= ~ATTR_SIZE;
 	}

> Could you please check if the following patch fixes NFS (and also the
> local filesystems) for you?
 
I'll try the latest in the flood today.  Thanks for all the help!
-- 
Rob

Re: Change in NFS client behavior

[Rob Sims]

On Fri, Sep 02, 2005 at 12:19:07AM -0400, Trond Myklebust wrote:
> fr den 02.09.2005 Klokka 00:15 (-0400) skreiv Trond Myklebust:
> 
> > Sure. The other problem is that the test is made before the i_sem is
> > grabbed. OK, so how about the appended patch instead?
> 
> Doh!
> 
> Trond

> VFS/NFS: Fix up behaviour w.r.t. truncate() and open(O_TRUNC)
> 
>  POSIX and the SUSv3 specify that open(O_TRUNC) should always bump ctime/mtime
>  whereas truncate() should only do so if the file size actually changes.
> 
>  Fix the behaviour of NFS, which currently is broken w.r.t. open(), and fix
>  the VFS truncate() so that it no enforces the POSIX rules.
> 
>  Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> ---
>  attr.c      |   14 +++-----------
>  nfs/inode.c |    5 -----
>  open.c      |   25 +++++++++++++++++++++++--
>  3 files changed, 26 insertions(+), 18 deletions(-)

This patch does not fix the original issue - timestamps are not updated
as expected.
-- 
Rob