atimes not updated over NFS
Subsystems (4)
After upgrading an NFS server from kernel 2.6.12 to 2.6.23, the atime of files is not updated any more when clients read them :
client$ ll -u --time-style +%Y-%m-%d_%H:%M:%S /nfsmnt/zz -rw-r--r-- 1 xxx xxx 0 2007-12-21_14:58:11 /nfsmnt/zz client$ md5sum /nfsmnt/zz d41d8cd98f00b204e9800998ecf8427e /nfsmnt/zz client$ date +%Y-%m-%d_%H:%M:%S 2007-12-21_15:00:28 client$ ll -u --time-style +%Y-%m-%d_%H:%M:%S /nfsmnt/zz -rw-r--r-- 1 xxx xxx 0 2007-12-21_14:58:11 /nfsmnt/zz
Reading the file locally, i.e. on the NFS server, does update the atime :
server$ ll -u --time-style +%Y-%m-%d_%H:%M:%S /nfsmnt/zz -rw-r--r-- 1 xxx xxx 0 2007-12-21_14:58:11 /nfsmnt/zz serveur$ md5sum /nfsmnt/zz d41d8cd98f00b204e9800998ecf8427e /nfsmnt/zz serveur$ date +%Y-%m-%d_%H:%M:%S 2007-12-21_15:04:00 serveur$ ll -u --time-style +%Y-%m-%d_%H:%M:%S /nfsmnt/zz -rw-r--r-- 1 xxx xxx 0 2007-12-21_15:03:57 /nfsmnt/zz
/nfsmnt is mounted with options :
- server : rw,noexec,nosuid,nodev,nodiratime
- client : rw,user=3Droot,nosuid,nodev,nodiratime,intr,rsize=3D8192,wsi= ze=3D8192
Any ideas ?
--=20
Andr=E9 Majorel <URL:http://www.teaser.fr/~amajorel/> Do not use this account for regular correspondence. See the URL above for contact information.
A message that is not in the archive (sent off-list or not yet received): 20080109101313.GG9212-956IwFboN44acnK+F/[email protected]
Quoted 24 lines · Andre Majorel
After upgrading an NFS server from kernel 2.6.12 to 2.6.23, the atime of files is not updated any more when clients read them :
client$ ll -u --time-style +%Y-%m-%d_%H:%M:%S /nfsmnt/zz -rw-r--r-- 1 xxx xxx 0 2007-12-21_14:58:11 /nfsmnt/zz client$ md5sum /nfsmnt/zz d41d8cd98f00b204e9800998ecf8427e /nfsmnt/zz client$ date +%Y-%m-%d_%H:%M:%S 2007-12-21_15:00:28 client$ ll -u --time-style +%Y-%m-%d_%H:%M:%S /nfsmnt/zz -rw-r--r-- 1 xxx xxx 0 2007-12-21_14:58:11 /nfsmnt/zzReading the file locally, i.e. on the NFS server, does update the atime :
server$ ll -u --time-style +%Y-%m-%d_%H:%M:%S /nfsmnt/zz -rw-r--r-- 1 xxx xxx 0 2007-12-21_14:58:11 /nfsmnt/zz serveur$ md5sum /nfsmnt/zz d41d8cd98f00b204e9800998ecf8427e /nfsmnt/zz serveur$ date +%Y-%m-%d_%H:%M:%S 2007-12-21_15:04:00 serveur$ ll -u --time-style +%Y-%m-%d_%H:%M:%S /nfsmnt/zz -rw-r--r-- 1 xxx xxx 0 2007-12-21_15:03:57 /nfsmnt/zz
Quoted 5 lines
/nfsmnt is mounted with options :
- server : rw,noexec,nosuid,nodev,nodiratime
- client : rw,user=root,nosuid,nodev,nodiratime,intr,rsize=8192,wsize=8192Any ideas ?
The file is probably entirely in cache on the client. When this happens, no READ rpc calls need to be made to the server and so the atime will not change either.
This has always been the case, BTW...
Trond
A message that is not in the archive (sent off-list or not yet received): 1199892437.11141.2.camel-rJ7iovZKK19ZJLDQqaL3InhyD016LWXt@public.gmane.org
Quoted 21 lines · Trond Myklebust
Quoted 19 lines · Andre Majorel
After upgrading an NFS server from kernel 2.6.12 to 2.6.23, the atime of files is not updated any more when clients read them :
=20
client$ ll -u --time-style +%Y-%m-%d_%H:%M:%S /nfsmnt/zz -rw-r--r-- 1 xxx xxx 0 2007-12-21_14:58:11 /nfsmnt/zz client$ md5sum /nfsmnt/zzd41d8cd98f00b204e9800998ecf8427e/nfsmnt/zz
client$ date +%Y-%m-%d_%H:%M:%S
2007-12-21_15:00:28
client$ ll -u --time-style +%Y-%m-%d_%H:%M:%S /nfsmnt/zz -rw-r--r-- 1 xxx xxx 0 2007-12-21_14:58:11 /nfsmnt/zz
=20
Reading the file locally, i.e. on the NFS server, does update the atime :
=20
server$ ll -u --time-style +%Y-%m-%d_%H:%M:%S /nfsmnt/zz -rw-r--r-- 1 xxx xxx 0 2007-12-21_14:58:11 /nfsmnt/zz serveur$ md5sum /nfsmnt/zzd41d8cd98f00b204e9800998ecf8427e/nfsmnt/zz
Quoted 8 lines
Quoted 8 lines
serveur$ date +%Y-%m-%d_%H:%M:%S
2007-12-21_15:04:00
serveur$ ll -u --time-style +%Y-%m-%d_%H:%M:%S /nfsmnt/zz -rw-r--r-- 1 xxx xxx 0 2007-12-21_15:03:57 /nfsmnt/zz
=20
/nfsmnt is mounted with options :
- server : rw,noexec,nosuid,nodev,nodiratime
- client : rw,user=3Droot,nosuid,nodev,nodiratime,intr,rsize=3D8192=
,wsize=3D8192
Quoted 4 lines
Quoted 2 lines
=20
Any ideas ?=20
The file is probably entirely in cache on the client. When this happe=
ns,
Quoted 2 lines
no READ rpc calls need to be made to the server and so the atime will not change either.
The client has 512 MB of RAM. A 513-MB file cannot possibly be cached so let's try with that.
atc5:/aym/etc$ time dd if=3D/dev/zero bs=3D1M count=3D513 >/nfsmnt/zz 513+0 records in 513+0 records out
real 0m50.412s
user 0m0.004s
sys 0m8.905s
atc5:/aym/etc$ ls -l --time-style +%Y-%m-%d_%H:%M:%S -u /nfsmnt/zz
-rw-r--r-- 1 aym aym 537919488 2007-12-21_13:46:54 /nfsmnt/zz
atc5:/aym/etc$ time wc /nfsmnt/zz
0 0 537919488 /nfsmnt/zzreal 1m30.743s user 0m47.091s sys 0m22.369s atc5:/aym/etc$ date +%Y-%m-%d_%H:%M:%S 2008-01-09_23:21:06 atc5:/aym/etc$ ls -l --time-style +%Y-%m-%d_%H:%M:%S -u /nfsmnt/zz -rw-r--r-- 1 aym aym 537919488 2007-12-21_13:46:54 /nfsmnt/zz
As the times show, the file is not in cache. And yet the atime is still not updated.
Quoted 1 line
This has always been the case, BTW...
I'll take your word for it. However, let me assure you that atimes somehow used to work over NFS, even for files small enough to fit in the cache. Had they been broken, I would have known. I use Mutt.
Thanks.
--=20
Andr=E9 Majorel <URL:http://www.teaser.fr/~amajorel/> Do not use this account for regular correspondence. See the URL above for contact information.
A message that is not in the archive (sent off-list or not yet received): 20080109223748.GH9212-956IwFboN44acnK+F/[email protected]
Quoted 3 lines · Andre Majorel
I'll take your word for it. However, let me assure you that atimes somehow used to work over NFS, even for files small enough to fit in the cache. Had they been broken, I would have known. I use Mutt.
How would the server know to update the atime? If the file is in cache we don't READ.
Trond
A message that is not in the archive (sent off-list or not yet received): 1199920996.7638.3.camel-rJ7iovZKK19ZJLDQqaL3InhyD016LWXt@public.gmane.org
Quoted 7 lines · Trond Myklebust
Quoted 3 lines · Andre Majorel
I'll take your word for it. However, let me assure you that atimes somehow used to work over NFS, even for files small enough to fit in the cache. Had they been broken, I would have known. I use Mutt.
How would the server know to update the atime? If the file is in cache we don't READ.
Put differently. Have you checked on the _server_ that the atime is actually changing when you do these tests on the client?
Trond
Quoted 7 lines · Trond Myklebust
Quoted 3 lines · Andre Majorel
I'll take your word for it. However, let me assure you that atimes somehow used to work over NFS, even for files small enough to fit in the cache. Had they been broken, I would have known. I use Mutt.
=20
How would the server know to update the atime? If the file is in cach=
e
Quoted 1 line
we don't READ.
You tell me. All I can tell you is that Mutt used to work so atimes were updated one way or another.
As of 2.6.23, access times are never updated, even for files that are too big to fit in the cache. I'm willing to run tests if that helps.
--=20
Andr=E9 Majorel <URL:http://www.teaser.fr/~amajorel/> Do not use this account for regular correspondence. See the URL above for contact information.
A message that is not in the archive (sent off-list or not yet received): 20080109235153.GI9212-956IwFboN44acnK+F/[email protected]
Quoted 7 lines · Andre Majorel
Quoted 6 lines · Trond Myklebust
Quoted 3 lines · Andre Majorel
I'll take your word for it. However, let me assure you that atimes somehow used to work over NFS, even for files small enough to fit in the cache. Had they been broken, I would have known. I use Mutt.
Quoted 5 lines
Quoted 2 lines
How would the server know to update the atime? If the file is in cache we don't READ.
You tell me. All I can tell you is that Mutt used to work so atimes were updated one way or another.
I think 2.6.22 introduced this regression. On 2.6.22.10, reading a file on NFS does not affect atime according to stat(), not on the client and not on the server. This breaks several programs.
Commands to reproduce this problem on the client:
dd </dev/zero >testfile count=1 ls -l --time=atime --full-time testfile sleep 2 wc testfile ls -l --time=atime --full-time testfile (same atime, not good)
Signature · 1 line
Frank
Quoted 18 lines · Frank van Maarseveen
Quoted 7 lines · Andre Majorel
Quoted 6 lines · Trond Myklebust
Quoted 3 lines · Andre Majorel
I'll take your word for it. However, let me assure you that atimes somehow used to work over NFS, even for files small enough to fit in the cache. Had they been broken, I would have known. I use Mutt.
Quoted 5 lines
Quoted 2 lines
How would the server know to update the atime? If the file is in cache we don't READ.
You tell me. All I can tell you is that Mutt used to work so atimes were updated one way or another.
I think 2.6.22 introduced this regression. On 2.6.22.10, reading a file on NFS does not affect atime according to
stat(), not on the client and not on the server. This breaks several programs.
Quoted 8 lines
Commands to reproduce this problem on the client:
dd </dev/zero >testfile count=1 ls -l --time=atime --full-time testfile sleep 2 wc testfile ls -l --time=atime --full-time testfile (same atime, not good)
Can you confirm that it does not change on the server? If so, then we need to look at the server for a fix. The client should only be mirroring the server's idea of the correct atime.
Cheers
Trond
A message that is not in the archive (sent off-list or not yet received): 1200325393.7470.6.camel-rJ7iovZKK19ZJLDQqaL3InhyD016LWXt@public.gmane.org
Quoted 29 lines · Trond Myklebust
Quoted 18 lines · Frank van Maarseveen
Quoted 7 lines · Andre Majorel
Quoted 6 lines · Trond Myklebust
Quoted 3 lines · Andre Majorel
I'll take your word for it. However, let me assure you that atimes somehow used to work over NFS, even for files small enough to fit in the cache. Had they been broken, I would have known. I use Mutt.
Quoted 5 lines
Quoted 2 lines
How would the server know to update the atime? If the file is in cache we don't READ.
You tell me. All I can tell you is that Mutt used to work so atimes were updated one way or another.
I think 2.6.22 introduced this regression. On 2.6.22.10, reading a file on NFS does not affect atime according to
stat(), not on the client and not on the server. This breaks several programs.Quoted 8 lines
Commands to reproduce this problem on the client:
dd </dev/zero >testfile count=1 ls -l --time=atime --full-time testfile sleep 2 wc testfile ls -l --time=atime --full-time testfile (same atime, not good)
Quoted 3 lines
Can you confirm that it does not change on the server? If so, then we need to look at the server for a fix. The client should only be mirroring the server's idea of the correct atime.
It doesn't change on the server (2.6.23.12)
Signature · 1 line
Frank
Quoted 30 lines · Frank van Maarseveen
Quoted 29 lines · Trond Myklebust
Quoted 26 lines · Frank van Maarseveen
Quoted 7 lines · Andre Majorel
Quoted 6 lines · Trond Myklebust
Quoted 3 lines · Andre Majorel
I'll take your word for it. However, let me assure you that atimes somehow used to work over NFS, even for files small enough to fit in the cache. Had they been broken, I would have known. I use Mutt.
Quoted 5 lines
Quoted 2 lines
How would the server know to update the atime? If the file is in cache we don't READ.
You tell me. All I can tell you is that Mutt used to work so atimes were updated one way or another.
I think 2.6.22 introduced this regression. On 2.6.22.10, reading a file on NFS does not affect atime according to
stat(), not on the client and not on the server. This breaks several programs.Commands to reproduce this problem on the client:
dd </dev/zero >testfile count=1 ls -l --time=atime --full-time testfile sleep 2 wc testfile ls -l --time=atime --full-time testfile (same atime, not good)
Quoted 5 lines
Quoted 3 lines
Can you confirm that it does not change on the server? If so, then we need to look at the server for a fix. The client should only be mirroring the server's idea of the correct atime.
It doesn't change on the server (2.6.23.12)
That still leaves open the question as to whether this is due to changes in the client that are causing it not to issue a read to the server when it would have before, or whether the server is just refusing to update the atime on read for some reason....
--b.
Quoted 13 lines · J. Bruce Fields
Quoted 13 lines
Quoted 12 lines
Quoted 8 lines
Commands to reproduce this problem on the client:
dd </dev/zero >testfile count=1 ls -l --time=atime --full-time testfile sleep 2 wc testfile ls -l --time=atime --full-time testfile (same atime, not good)Can you confirm that it does not change on the server? If so, then we need to look at the server for a fix. The client should only be mirroring the server's idea of the correct atime.
Quoted 6 lines
Quoted 1 line
It doesn't change on the server (2.6.23.12)
That still leaves open the question as to whether this is due to changes in the client that are causing it not to issue a read to the server when it would have before, or whether the server is just refusing to update the atime on read for some reason....
A script of the form
ssh server 'dd </dev/zero >testfile count=1; ls -l --time=atime --full-time testfile'
cat testfile >/dev/null
ssh server 'ls -l --time=atime --full-time testfile'
ls -l --time=atime --full-time testfileshould normally force the client to read the file, and should tell if the atime is being updates correctly on both server and client.
Cheers
Trond
A message that is not in the archive (sent off-list or not yet received): 1200943865.25562.56.camel-rJ7iovZKK19ZJLDQqaL3InhyD016LWXt@public.gmane.org
Quoted 15 lines · Trond Myklebust
Quoted 13 lines · J. Bruce Fields
Quoted 13 lines
Quoted 12 lines
Quoted 8 lines
Commands to reproduce this problem on the client:
dd </dev/zero >testfile count=1 ls -l --time=atime --full-time testfile sleep 2 wc testfile ls -l --time=atime --full-time testfile (same atime, not good)Can you confirm that it does not change on the server? If so, then we need to look at the server for a fix. The client should only be mirroring the server's idea of the correct atime.
Quoted 8 lines
Quoted 6 lines
Quoted 1 line
It doesn't change on the server (2.6.23.12)
That still leaves open the question as to whether this is due to changes in the client that are causing it not to issue a read to the server when it would have before, or whether the server is just refusing to update the atime on read for some reason....
A script of the form
"rm -f testfile" and at least one "touch ." in advance to get deterministic behavior.
Quoted 2 lines
ssh server 'dd </dev/zero >testfile count=1; ls -l --time=atime --full-time testfile'put a sleep 2 here.
Quoted 2 lines
cat testfile >/dev/null
put a sleep 2 here just in case we would race with something in flight
Quoted 4 lines
ssh server 'ls -l --time=atime --full-time testfile'
ls -l --time=atime --full-time testfileI tried some kernels (same on client and server). There are three atimes after each experiment.
2.6.21.7:
t t + 2 t + 2
This seems to work but it doesn't: only the first "cat testfile" will update the atime. Subsequent cat commands will not update atime unless "cat" is executed on the server.
2.6.22.10:
t t + 2 t + 2
(same behavior)
2.6.23.12:
t
t
t
definately not good. "cat" on the server updates atime again.
Trying a different combination of kernels:
server 2.6.23.12, client 2.6.22.10:
t
t
t
server 2.6.22.10, client 2.6.23.12:
t t + 2 t + 2
so, there are 2 phenomena and one has been introduced with 2.6.23 on the server side. Filesystem on the server is ext3.
Signature · 1 line
Frank
Quoted 45 lines · Frank van Maarseveen
Quoted 15 lines · Trond Myklebust
Quoted 13 lines · J. Bruce Fields
Quoted 13 lines
Quoted 12 lines
Quoted 8 lines
Commands to reproduce this problem on the client:
dd </dev/zero >testfile count=1 ls -l --time=atime --full-time testfile sleep 2 wc testfile ls -l --time=atime --full-time testfile (same atime, not good)Can you confirm that it does not change on the server? If so, then we need to look at the server for a fix. The client should only be mirroring the server's idea of the correct atime.
Quoted 8 lines
Quoted 6 lines
Quoted 1 line
It doesn't change on the server (2.6.23.12)
That still leaves open the question as to whether this is due to changes in the client that are causing it not to issue a read to the server when it would have before, or whether the server is just refusing to update the atime on read for some reason....
A script of the form
"rm -f testfile" and at least one "touch ." in advance to get deterministic behavior.
Quoted 2 lines
ssh server 'dd </dev/zero >testfile count=1; ls -l --time=atime --full-time testfile'put a sleep 2 here.
Quoted 2 lines
cat testfile >/dev/null
put a sleep 2 here just in case we would race with something in flight
Quoted 4 lines
ssh server 'ls -l --time=atime --full-time testfile' ls -l --time=atime --full-time testfileI tried some kernels (same on client and server). There are three atimes after each experiment.
Quoted 8 lines
2.6.21.7:
t t + 2 t + 2This seems to work but it doesn't: only the first "cat testfile" will update the atime. Subsequent cat commands will not update atime unless "cat" is executed on the server.
I think that's just the expected result of client caching, so all you can do in that case is add "set check_mbox_size=yes" to your .muttrc or something.....
Quoted 6 lines
2.6.22.10:
t t + 2 t + 2
Quoted 8 lines
(same behavior)
2.6.23.12:
t
t
tdefinately not good. "cat" on the server updates atime again.
Yes, that looks like a server bug, and this:...
Quoted 9 lines
Trying a different combination of kernels:
server 2.6.23.12, client 2.6.22.10:
t
t
t
Quoted 4 lines
server 2.6.22.10, client 2.6.23.12:
t t + 2 t + 2
...confirms that since the results appear to depend only on the server version, not on the client version.
--b.
Quoted 6 lines
so, there are 2 phenomena and one has been introduced with 2.6.23 on the server side. Filesystem on the server is ext3.
Signature · 1 line
Frank
Quoted 60 lines · bfields
Quoted 20 lines · Frank van Maarseveen
"rm -f testfile" and at least one "touch ." in advance to get deterministic behavior.
Quoted 2 lines
ssh server 'dd </dev/zero >testfile count=1; ls -l --time=atime --full-time testfile'put a sleep 2 here.
Quoted 2 lines
cat testfile >/dev/null
put a sleep 2 here just in case we would race with something in flight
Quoted 4 lines
ssh server 'ls -l --time=atime --full-time testfile' ls -l --time=atime --full-time testfileI tried some kernels (same on client and server). There are three atimes after each experiment.
Quoted 8 lines
2.6.21.7:
t t + 2 t + 2This seems to work but it doesn't: only the first "cat testfile" will update the atime. Subsequent cat commands will not update atime unless "cat" is executed on the server.
I think that's just the expected result of client caching, so all you can do in that case is add "set check_mbox_size=yes" to your .muttrc or something.....
Quoted 6 lines
2.6.22.10:
t t + 2 t + 2Quoted 8 lines
(same behavior)
2.6.23.12:
t
t
tdefinately not good. "cat" on the server updates atime again.
Yes, that looks like a server bug, and this:...
Quoted 9 lines
Trying a different combination of kernels:
server 2.6.23.12, client 2.6.22.10:
t
t
t
Quoted 7 lines
Quoted 4 lines
server 2.6.22.10, client 2.6.23.12:
t t + 2 t + 2...confirms that since the results appear to depend only on the server version, not on the client version.
And I can confirm this here on 2.6.24-rc8 (+ a few patches). Unfortunately, I don't have any suggestion better right now than bisecting....
--b.
J. Bruce Fields
Acked-by Trond MyklebustQuoted 32 lines · bfields
Quoted 31 lines · bfields
Quoted 5 lines · Frank van Maarseveen
2.6.22.10:
t t + 2 t + 2Quoted 8 lines
(same behavior)
2.6.23.12:
t
t
tdefinately not good. "cat" on the server updates atime again.
Yes, that looks like a server bug, and this:...
Quoted 13 lines
Trying a different combination of kernels:
server 2.6.23.12, client 2.6.22.10:
t
t
tserver 2.6.22.10, client 2.6.23.12:
t t + 2 t + 2
Quoted 6 lines
Quoted 2 lines
...confirms that since the results appear to depend only on the server version, not on the client version.
And I can confirm this here on 2.6.24-rc8 (+ a few patches). Unfortunately, I don't have any suggestion better right now than bisecting....
It looks like this happened in the switch from sendfile to sparse. Jens, any advice? What happened was nfsd reads stopped updating the atime after the following commit.
--b.
commit f0930fffa99e7fe0a0c4b6c7d9a244dc88288c27
Author: Jens Axboe <[email protected]> Date: Fri Jun 1 11:51:43 2007 +0200
sendfile: convert nfs to using splice_read()
fs/nfs/file.c +9 -6
@@ -41,7 +41,9 @@ static int nfs_file_open(struct inode *, struct file *);static int nfs_file_release (struct inode *,struct file *);static loff_t nfs_file_llseek (struct file *file,loff_t offset,int origin);static int nfs_file_mmap (struct file *,struct vm_area_struct *); -static ssize_t nfs_file_sendfile (struct file *,loff_t *,size_t ,read_actor_t ,void *); +static ssize_t nfs_file_splice_read (struct file *filp,loff_t *ppos, +struct pipe_inode_info *pipe, +size_t count,unsigned int flags);static ssize_t nfs_file_read (struct kiocb *,const struct iovec *iov,unsigned long nr_segs,loff_t pos); static ssize_t nfs_file_write(struct kiocb *, const struct iovec *iov,
@@ -65,7 +67,7 @@ const struct file_operations nfs_file_operations = { .fsync = nfs_fsync, .lock = nfs_lock, .flock = nfs_flock, - .sendfile = nfs_file_sendfile, + .splice_read = nfs_file_splice_read, .check_flags = nfs_check_flags, };
@@ -224,20 +226,21 @@ nfs_file_read(struct kiocb *iocb, const struct iovec *iov, }static ssize_t -nfs_file_sendfile (struct file *filp,loff_t *ppos,size_t count, -read_actor_t actor,void *target) +nfs_file_splice_read (struct file *filp,loff_t *ppos, +struct pipe_inode_info *pipe,size_t count, +unsigned int flags) {struct dentry *dentry = filp->f_path.dentry;struct inode *inode = dentry->d_inode;ssize_t res; -dfprintk (VFS ,"nfs: sendfile(%s/%s, %lu@%Lu)\n" , +dfprintk (VFS ,"nfs: splice_read(%s/%s, %lu@%Lu)\n" , dentry->d_parent->d_name.name, dentry->d_name.name, (unsigned long ) count, (unsigned long long ) *ppos); res =nfs_revalidate_mapping (inode, filp->f_mapping);if (!res) - res =generic_file_sendfile (filp, ppos, count, actor, target); + res =generic_file_splice_read (filp, ppos, pipe, count, flags);return res; }
Quoted 33 lines · bfields
Quoted 32 lines · bfields
Quoted 31 lines · bfields
Quoted 5 lines · Frank van Maarseveen
2.6.22.10:
t t + 2 t + 2Quoted 8 lines
(same behavior)
2.6.23.12:
t
t
tdefinately not good. "cat" on the server updates atime again.
Yes, that looks like a server bug, and this:...
Quoted 13 lines
Trying a different combination of kernels:
server 2.6.23.12, client 2.6.22.10:
t
t
tserver 2.6.22.10, client 2.6.23.12:
t t + 2 t + 2
Quoted 8 lines
Quoted 6 lines
Quoted 2 lines
...confirms that since the results appear to depend only on the server version, not on the client version.
And I can confirm this here on 2.6.24-rc8 (+ a few patches). Unfortunately, I don't have any suggestion better right now than bisecting....
It looks like this happened in the switch from sendfile to sparse.
^^^^^^ err, splice
Quoted 2 lines
Jens, any advice? What happened was nfsd reads stopped updating the atime after the following commit.
Erm, sorry, wrong commit--it's the following one that touches nfsd, below.
--b.
commit cf8208d0eabd1d5d2625ec02a175a294c3f30d36
Author: Jens Axboe <[email protected]> Date: Tue Jun 12 21:22:14 2007 +0200
sendfile: convert nfsd to splice_direct_to_actor()
fs/nfsd/vfs.c +31 -16
@@ -23,7 +23,7 @@#include <linux/file.h> #include <linux/mount.h> #include <linux/major.h> -#include <linux/ext2_fs.h> +#include <linux/pipe_fs_i.h> #include <linux/proc_fs.h> #include <linux/stat.h> #include <linux/fcntl.h>
@@ -801,26 +801,32 @@ found: }/* -* Grab and keep cached pages assosiated with a file in the svc_rqst -* so that they can be passed to the netowork sendmsg/sendpage routines -* directrly. They will be released after the sending has completed. +* Grab and keep cached pages associated with a file in the svc_rqst +* so that they can be passed to the network sendmsg/sendpage routines +* directly. They will be released after the sending has completed. */ static int -nfsd_read_actor (read_descriptor_t *desc,struct page *page,unsigned long offset ,unsigned long size) +nfsd_splice_actor (struct pipe_inode_info *pipe,struct pipe_buffer *buf, +struct splice_desc *sd) { -unsigned long count = desc->count; -struct svc_rqst *rqstp = desc->arg.data; +struct svc_rqst *rqstp = sd->u.data;struct page **pp = rqstp->rq_respages + rqstp->rq_resused; +struct page *page = buf->page; +size_t size; +int ret; + + ret = buf->ops->pin (pipe, buf); +if (unlikely (ret)) +return ret; -if (size > count) - size = count; + size = sd->len;if (rqstp->rq_res.page_len ==0 ) {get_page (page);put_page (*pp); *pp = page; rqstp->rq_resused++; - rqstp->rq_res.page_base = offset; + rqstp->rq_res.page_base = buf->offset; rqstp->rq_res.page_len = size; }else if (page != pp[-1 ]) {get_page (page);
@@ -832,11 +838,15 @@ nfsd_read_actor(read_descriptor_t *desc, struct page *page, unsigned long offset }else rqstp->rq_res.page_len += size; - desc->count = count - size; - desc->written += size;return size; } +static int nfsd_direct_splice_actor (struct pipe_inode_info *pipe, +struct splice_desc *sd) +{ +return __splice_from_pipe (pipe, sd, nfsd_splice_actor); +} +static __be32 nfsd_vfs_read (struct svc_rqst *rqstp,struct svc_fh *fhp,struct file *file,loff_t offset,struct kvec *vec,int vlen,unsigned long *count)
@@ -861,10 +871,15 @@ nfsd_vfs_read(struct svc_rqst *rqstp, struct svc_fh *fhp, struct file *file,if (ra && ra->p_set) file->f_ra = ra->p_ra; -if (file->f_op->sendfile && rqstp->rq_sendfile_ok) { - rqstp->rq_resused =1 ; - host_err = file->f_op->sendfile (file, &offset, *count, - nfsd_read_actor, rqstp); +if (file->f_op->splice_read && rqstp->rq_splice_ok) { +struct splice_desc sd = { + .len =0 , + .total_len = *count, + .pos = offset, + .u.data = rqstp, + }; + + host_err =splice_direct_to_actor (file, &sd, nfsd_direct_splice_actor); }else { oldfs =get_fs ();set_fs (KERNEL_DS );
include/linux/sunrpc/svc.h +1 -1
@@ -253,7 +253,7 @@ struct svc_rqst {* determine what device number * to report (real or virtual) */ -int rq_sendfile_ok;/* turned off in gss privacy +int rq_splice_ok;/* turned off in gss privacy * to prevent encrypting page * cache pages */ wait_queue_head_t rq_wait;/* synchronization */
net/sunrpc/auth_gss/svcauth_gss.c +1 -1
@@ -853,7 +853,7 @@ unwrap_priv_data(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq, struct gsu32 priv_len, maj_stat;int pad, saved_len, remaining_len, offset; - rqstp->rq_sendfile_ok =0 ; + rqstp->rq_splice_ok =0 ; priv_len =svc_getnl (&buf->head[0 ]);if (rqstp->rq_deferred) {
net/sunrpc/svc.c +1 -1
@@ -814,7 +814,7 @@ svc_process(struct svc_rqst *rqstp) rqstp->rq_res.tail[0 ].iov_base =NULL ; rqstp->rq_res.tail[0 ].iov_len =0 ;/* Will be turned off only in gss privacy case: */ - rqstp->rq_sendfile_ok =1 ; + rqstp->rq_splice_ok =1 ;/* tcp needs a space for the record length... */ if (rqstp->rq_prot ==IPPROTO_TCP )svc_putnl (resv,0 );
Quoted 42 lines · J. Bruce Fields
Quoted 33 lines · bfields
Quoted 32 lines · bfields
Quoted 31 lines · bfields
Quoted 5 lines · Frank van Maarseveen
2.6.22.10:
t t + 2 t + 2Quoted 8 lines
(same behavior)
2.6.23.12:
t
t
tdefinately not good. "cat" on the server updates atime again.
Yes, that looks like a server bug, and this:...
Quoted 13 lines
Trying a different combination of kernels:
server 2.6.23.12, client 2.6.22.10:
t
t
tserver 2.6.22.10, client 2.6.23.12:
t t + 2 t + 2Quoted 8 lines
Quoted 6 lines
Quoted 2 lines
...confirms that since the results appear to depend only on the server version, not on the client version.
And I can confirm this here on 2.6.24-rc8 (+ a few patches). Unfortunately, I don't have any suggestion better right now than bisecting....
It looks like this happened in the switch from sendfile to sparse.
Quoted 8 lines
^^^^^^ err, spliceQuoted 2 lines
Jens, any advice? What happened was nfsd reads stopped updating the atime after the following commit.
Erm, sorry, wrong commit--it's the following one that touches nfsd, below.
Probably because do_generic_mapping_read() does a file_accessed() on the input file. Does this fixup current -git?
fs/splice.c +3 -1
@@ -1090,8 +1090,10 @@ long do_splice_direct(struct file *in, loff_t *ppos, struct file *out,long ret; ret =splice_direct_to_actor (in, &sd, direct_splice_actor); -if (ret >0 ) +if (ret >0 ) { *ppos += ret; +file_accessed (in); + }return ret; }
Signature · 1 line
Jens Axboe
Quoted 46 lines · Jens Axboe
Quoted 45 lines · J. Bruce Fields
Quoted 33 lines · bfields
Quoted 32 lines · bfields
Quoted 31 lines · bfields
Quoted 5 lines · Frank van Maarseveen
2.6.22.10:
t t + 2 t + 2Quoted 8 lines
(same behavior)
2.6.23.12:
t
t
tdefinately not good. "cat" on the server updates atime again.
Yes, that looks like a server bug, and this:...
Quoted 13 lines
Trying a different combination of kernels:
server 2.6.23.12, client 2.6.22.10:
t
t
tserver 2.6.22.10, client 2.6.23.12:
t t + 2 t + 2Quoted 8 lines
Quoted 6 lines
Quoted 2 lines
...confirms that since the results appear to depend only on the server version, not on the client version.
And I can confirm this here on 2.6.24-rc8 (+ a few patches). Unfortunately, I don't have any suggestion better right now than bisecting....
It looks like this happened in the switch from sendfile to sparse.
^^^^^^ err, splice
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
Jens, any advice? What happened was nfsd reads stopped updating the atime after the following commit.
Erm, sorry, wrong commit--it's the following one that touches nfsd, below.
Probably because
do_generic_mapping_read()does afile_accessed()on the input file. Does this fixup current -git?
No, nfsd is calling splice_direct_to_actor(), not do_splice_direct().
--b.
Quoted 21 lines
fs/splice.c +3
-1@@ -1090,8 +1090,10 @@ long do_splice_direct(struct file *in, loff_t *ppos, struct file *out,long ret; ret =splice_direct_to_actor (in, &sd, direct_splice_actor); -if (ret >0 ) +if (ret >0 ) { *ppos += ret; +file_accessed (in); + }return ret; }Signature · 2 lines
-- Jens Axboe
Quoted 50 lines · J. Bruce Fields
Quoted 49 lines · Jens Axboe
Quoted 48 lines · J. Bruce Fields
Quoted 33 lines · bfields
Quoted 32 lines · bfields
Quoted 31 lines · bfields
Quoted 5 lines · Frank van Maarseveen
2.6.22.10:
t t + 2 t + 2Quoted 8 lines
(same behavior)
2.6.23.12:
t
t
tdefinately not good. "cat" on the server updates atime again.
Yes, that looks like a server bug, and this:...
Quoted 13 lines
Trying a different combination of kernels:
server 2.6.23.12, client 2.6.22.10:
t
t
tserver 2.6.22.10, client 2.6.23.12:
t t + 2 t + 2Quoted 8 lines
Quoted 6 lines
Quoted 2 lines
...confirms that since the results appear to depend only on the server version, not on the client version.
And I can confirm this here on 2.6.24-rc8 (+ a few patches). Unfortunately, I don't have any suggestion better right now than bisecting....
It looks like this happened in the switch from sendfile to sparse.
^^^^^^ err, spliceQuoted 2 lines
Jens, any advice? What happened was nfsd reads stopped updating the atime after the following commit.
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
Erm, sorry, wrong commit--it's the following one that touches nfsd, below.
Probably because
do_generic_mapping_read()does afile_accessed()on the input file. Does this fixup current -git?No, nfsd is calling
splice_direct_to_actor(), notdo_splice_direct().
Ah doh, this instead then. Should work for both types.
fs/splice.c +3 -1
@@ -1052,8 +1052,10 @@ out_release:/* * If we transferred some data, return the number of bytes: */ -if (bytes >0 ) +if (bytes >0 ) { +file_accessed (in);return bytes; + }return ret;
Signature · 1 line
Jens Axboe
Quoted 66 lines · Jens Axboe
Quoted 50 lines · J. Bruce Fields
Quoted 49 lines · Jens Axboe
Quoted 48 lines · J. Bruce Fields
Quoted 33 lines · bfields
Quoted 32 lines · bfields
Quoted 31 lines · bfields
Quoted 5 lines · Frank van Maarseveen
2.6.22.10:
t t + 2 t + 2Quoted 8 lines
(same behavior)
2.6.23.12:
t
t
tdefinately not good. "cat" on the server updates atime again.
Yes, that looks like a server bug, and this:...
Quoted 13 lines
Trying a different combination of kernels:
server 2.6.23.12, client 2.6.22.10:
t
t
tserver 2.6.22.10, client 2.6.23.12:
t t + 2 t + 2Quoted 8 lines
Quoted 6 lines
Quoted 2 lines
...confirms that since the results appear to depend only on the server version, not on the client version.
And I can confirm this here on 2.6.24-rc8 (+ a few patches). Unfortunately, I don't have any suggestion better right now than bisecting....
It looks like this happened in the switch from sendfile to sparse.
^^^^^^ err, spliceQuoted 2 lines
Jens, any advice? What happened was nfsd reads stopped updating the atime after the following commit.
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
Erm, sorry, wrong commit--it's the following one that touches nfsd, below.
Probably because
do_generic_mapping_read()does afile_accessed()on the input file. Does this fixup current -git?No, nfsd is calling
splice_direct_to_actor(), notdo_splice_direct().Ah doh, this instead then. Should work for both types.
fs/splice.c +0
-0
fs/splice.c out_release: · line 1060
Quoted 10 lines
fs/splice.c +3
-1@@ -1052,8 +1052,10 @@ out_release:/* * If we transferred some data, return the number of bytes: */ -if (bytes >0 ) +if (bytes >0 ) { +file_accessed (in);return bytes; + }return ret;
Hm. It's still missing a case. I've confirmed that the following fixes the problem. (Or maybe it would be better to have them "goto" a common out with the file_accessed() check?)
--b.
fs/splice.c +5 -1
@@ -1033,6 +1033,8 @@ ssize_t splice_direct_to_actor(struct file *in, struct splice_desc *sd, goto out_release; } +if (bytes) +file_accessed (in); pipe->nrbufs = pipe->curbuf =0 ;return bytes;
@@ -1054,8 +1056,10 @@ out_release:/* * If we transferred some data, return the number of bytes: */ -if (bytes >0 ) +if (bytes >0 ) { +file_accessed (in);return bytes; + }return ret;
Quoted 76 lines · J. Bruce Fields
Quoted 75 lines · Jens Axboe
Quoted 50 lines · J. Bruce Fields
Quoted 49 lines · Jens Axboe
Quoted 48 lines · J. Bruce Fields
Quoted 33 lines · bfields
Quoted 32 lines · bfields
Quoted 31 lines · bfields
Quoted 5 lines · Frank van Maarseveen
2.6.22.10:
t t + 2 t + 2Quoted 8 lines
(same behavior)
2.6.23.12:
t
t
tdefinately not good. "cat" on the server updates atime again.
Yes, that looks like a server bug, and this:...
Quoted 13 lines
Trying a different combination of kernels:
server 2.6.23.12, client 2.6.22.10:
t
t
tserver 2.6.22.10, client 2.6.23.12:
t t + 2 t + 2Quoted 8 lines
Quoted 6 lines
Quoted 2 lines
...confirms that since the results appear to depend only on the server version, not on the client version.
And I can confirm this here on 2.6.24-rc8 (+ a few patches). Unfortunately, I don't have any suggestion better right now than bisecting....
It looks like this happened in the switch from sendfile to sparse.
^^^^^^ err, spliceQuoted 2 lines
Jens, any advice? What happened was nfsd reads stopped updating the atime after the following commit.
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
Erm, sorry, wrong commit--it's the following one that touches nfsd, below.
Probably because
do_generic_mapping_read()does afile_accessed()on the input file. Does this fixup current -git?No, nfsd is calling
splice_direct_to_actor(), notdo_splice_direct().Ah doh, this instead then. Should work for both types.
fs/splice.c +3
-1@@ -1052,8 +1052,10 @@ out_release:/* * If we transferred some data, return the number of bytes: */ -if (bytes >0 ) +if (bytes >0 ) { +file_accessed (in);return bytes; + }
Quoted 5 lines
fs/splice.c out_release: · line 1060
Quoted 1 line
fs/splice.c +0
-0return ret;Hm. It's still missing a case. I've confirmed that the following fixes the problem. (Or maybe it would be better to have them "goto" a common out with the
file_accessed()check?)
Indeed it is, insert standard disclaimer here on the evil of multiple returns. So can we agree that this then fixes both cases?
fs/splice.c +7 -8
@@ -1031,7 +1031,11 @@ ssize_t splice_direct_to_actor(struct file *in, struct splice_desc *sd, goto out_release; } +done: pipe->nrbufs = pipe->curbuf =0 ; +if (bytes >0 ) +file_accessed (in); +return bytes; out_release:
@@ -1047,16 +1051,11 @@ out_release: buf->ops =NULL ; } } - pipe->nrbufs = pipe->curbuf =0 ; - -/* -* If we transferred some data, return the number of bytes: -*/ -if (bytes >0 ) -return bytes; -return ret; +if (!bytes) + bytes = ret; + goto done; }EXPORT_SYMBOL (splice_direct_to_actor);
Signature · 1 line
Jens Axboe
Quoted 7 lines · Jens Axboe
Quoted 3 lines · J. Bruce Fields
Hm. It's still missing a case. I've confirmed that the following fixes the problem. (Or maybe it would be better to have them "goto" a common out with the
file_accessed()check?)Indeed it is, insert standard disclaimer here on the evil of multiple returns. So can we agree that this then fixes both cases?
Yep, thanks! And I ran one more test just to make sure. Looks fine.
--b.
Quoted 41 lines
fs/splice.c +7
-8@@ -1031,7 +1031,11 @@ ssize_t splice_direct_to_actor(struct file *in, struct splice_desc *sd, goto out_release; } +done: pipe->nrbufs = pipe->curbuf =0 ; +if (bytes >0 ) +file_accessed (in); +return bytes; out_release:@@ -1047,16 +1051,11 @@ out_release: buf->ops =NULL ; } } - pipe->nrbufs = pipe->curbuf =0 ; - -/* -* If we transferred some data, return the number of bytes: -*/ -if (bytes >0 ) -return bytes; -return ret; +if (!bytes) + bytes = ret; + goto done; }EXPORT_SYMBOL (splice_direct_to_actor);Signature · 2 lines
-- Jens Axboe
Quoted 10 lines · J. Bruce Fields
Quoted 7 lines · Jens Axboe
Quoted 3 lines · J. Bruce Fields
Hm. It's still missing a case. I've confirmed that the following fixes the problem. (Or maybe it would be better to have them "goto" a common out with the
file_accessed()check?)Indeed it is, insert standard disclaimer here on the evil of multiple returns. So can we agree that this then fixes both cases?
Yep, thanks! And I ran one more test just to make sure. Looks fine.
Super, thanks a lot for testing and confirming! Patch is committed.
Signature · 1 line
Jens Axboe
Quoted 13 lines · Jens Axboe
fs/splice.c +3
-1@@ -1052,8 +1052,10 @@ out_release:/* * If we transferred some data, return the number of bytes: */ -if (bytes >0 ) +if (bytes >0 ) { +file_accessed (in);return bytes; + }
Quoted 2 lines
=20
return ret;
Does the (bytes > 0) test mean that read(2) on an empty file will not update the atime ? Not a big deal for me personally, but that would be inconsistent with local file system behaviour.
--=20
Andr=E9 Majorel <URL:http://www.teaser.fr/~amajorel/> Do not use this account for regular correspondence. See the URL above for contact information.
A message that is not in the archive (sent off-list or not yet received): 20080129203713.GS17213-956IwFboN44acnK+F/[email protected]
Quoted 16 lines · Andre Majorel
Quoted 14 lines · Jens Axboe
fs/splice.c +3
-1@@ -1052,8 +1052,10 @@ out_release:/* * If we transferred some data, return the number of bytes: */ -if (bytes >0 ) +if (bytes >0 ) { +file_accessed (in);return bytes; + }
Quoted 5 lines
fs/splice.c out_release: · line 1060
Quoted 1 line
fs/splice.c +0
-0return ret;Does the (bytes > 0) test mean that read(2) on an empty file will not update the atime ? Not a big deal for me personally, but that would be inconsistent with local file system behaviour.
Good point, I'll commit a fixup for that patch.
Signature · 1 line
Jens Axboe
Quoted 10 lines · Jens Axboe
Quoted 9 lines · Andre Majorel
On 2008-01-29 19:30 +0100, Jens Axboe wrote:
=20fs/splice.c
Quoted 3 lines
fs/splice.c +0
-1--- a/fs/splice.c=20
Does the (bytes > 0) test mean that read(2) on an empty file will not update the atime ? Not a big deal for me personally, but that would be inconsistent with local file system behaviour.
Quoted 2 lines
=20
Good point, I'll commit a fixup for that patch.
Excellent. Thanks to everyone involved.
--=20
Andr=E9 Majorel <URL:http://www.teaser.fr/~amajorel/> Do not use this account for regular correspondence. See the URL above for contact information.