diff options
author | Trond Myklebust <Trond.Myklebust@netapp.com> | 2008-10-14 19:16:07 -0400 |
---|---|---|
committer | Trond Myklebust <Trond.Myklebust@netapp.com> | 2008-10-14 19:23:17 -0400 |
commit | 4704f0e274829e3af00737d2d9adace2d71a9605 (patch) | |
tree | 048404f927dc210f1d7c695cb39e28e4e7d49030 /fs/nfs | |
parent | 921615f111108258820226a3258a047d9bf1d96a (diff) |
NFS: Fix the resolution problem with nfs_inode_attrs_need_update()
It appears that 'jiffies' timestamps do not have high enough resolution for
nfs_inode_attrs_need_update(). One problem is that a GETATTR can be
launched within < 1 jiffy of the last operation that updated the attribute.
Another problem is that RPC calls can take < 1 jiffy to execute.
We can fix this by switching the variables to use a simple global counter
that gets incremented every time we start another GETATTR call.
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
Diffstat (limited to 'fs/nfs')
-rw-r--r-- | fs/nfs/dir.c | 14 | ||||
-rw-r--r-- | fs/nfs/inode.c | 37 |
2 files changed, 40 insertions, 11 deletions
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c index 49d565412827..4807074ada8c 100644 --- a/fs/nfs/dir.c +++ b/fs/nfs/dir.c | |||
@@ -156,6 +156,7 @@ typedef struct { | |||
156 | decode_dirent_t decode; | 156 | decode_dirent_t decode; |
157 | int plus; | 157 | int plus; |
158 | unsigned long timestamp; | 158 | unsigned long timestamp; |
159 | unsigned long gencount; | ||
159 | int timestamp_valid; | 160 | int timestamp_valid; |
160 | } nfs_readdir_descriptor_t; | 161 | } nfs_readdir_descriptor_t; |
161 | 162 | ||
@@ -177,7 +178,7 @@ int nfs_readdir_filler(nfs_readdir_descriptor_t *desc, struct page *page) | |||
177 | struct file *file = desc->file; | 178 | struct file *file = desc->file; |
178 | struct inode *inode = file->f_path.dentry->d_inode; | 179 | struct inode *inode = file->f_path.dentry->d_inode; |
179 | struct rpc_cred *cred = nfs_file_cred(file); | 180 | struct rpc_cred *cred = nfs_file_cred(file); |
180 | unsigned long timestamp; | 181 | unsigned long timestamp, gencount; |
181 | int error; | 182 | int error; |
182 | 183 | ||
183 | dfprintk(DIRCACHE, "NFS: %s: reading cookie %Lu into page %lu\n", | 184 | dfprintk(DIRCACHE, "NFS: %s: reading cookie %Lu into page %lu\n", |
@@ -186,6 +187,7 @@ int nfs_readdir_filler(nfs_readdir_descriptor_t *desc, struct page *page) | |||
186 | 187 | ||
187 | again: | 188 | again: |
188 | timestamp = jiffies; | 189 | timestamp = jiffies; |
190 | gencount = nfs_inc_attr_generation_counter(); | ||
189 | error = NFS_PROTO(inode)->readdir(file->f_path.dentry, cred, desc->entry->cookie, page, | 191 | error = NFS_PROTO(inode)->readdir(file->f_path.dentry, cred, desc->entry->cookie, page, |
190 | NFS_SERVER(inode)->dtsize, desc->plus); | 192 | NFS_SERVER(inode)->dtsize, desc->plus); |
191 | if (error < 0) { | 193 | if (error < 0) { |
@@ -199,6 +201,7 @@ int nfs_readdir_filler(nfs_readdir_descriptor_t *desc, struct page *page) | |||
199 | goto error; | 201 | goto error; |
200 | } | 202 | } |
201 | desc->timestamp = timestamp; | 203 | desc->timestamp = timestamp; |
204 | desc->gencount = gencount; | ||
202 | desc->timestamp_valid = 1; | 205 | desc->timestamp_valid = 1; |
203 | SetPageUptodate(page); | 206 | SetPageUptodate(page); |
204 | /* Ensure consistent page alignment of the data. | 207 | /* Ensure consistent page alignment of the data. |
@@ -224,9 +227,10 @@ int dir_decode(nfs_readdir_descriptor_t *desc) | |||
224 | if (IS_ERR(p)) | 227 | if (IS_ERR(p)) |
225 | return PTR_ERR(p); | 228 | return PTR_ERR(p); |
226 | desc->ptr = p; | 229 | desc->ptr = p; |
227 | if (desc->timestamp_valid) | 230 | if (desc->timestamp_valid) { |
228 | desc->entry->fattr->time_start = desc->timestamp; | 231 | desc->entry->fattr->time_start = desc->timestamp; |
229 | else | 232 | desc->entry->fattr->gencount = desc->gencount; |
233 | } else | ||
230 | desc->entry->fattr->valid &= ~NFS_ATTR_FATTR; | 234 | desc->entry->fattr->valid &= ~NFS_ATTR_FATTR; |
231 | return 0; | 235 | return 0; |
232 | } | 236 | } |
@@ -471,7 +475,7 @@ int uncached_readdir(nfs_readdir_descriptor_t *desc, void *dirent, | |||
471 | struct rpc_cred *cred = nfs_file_cred(file); | 475 | struct rpc_cred *cred = nfs_file_cred(file); |
472 | struct page *page = NULL; | 476 | struct page *page = NULL; |
473 | int status; | 477 | int status; |
474 | unsigned long timestamp; | 478 | unsigned long timestamp, gencount; |
475 | 479 | ||
476 | dfprintk(DIRCACHE, "NFS: uncached_readdir() searching for cookie %Lu\n", | 480 | dfprintk(DIRCACHE, "NFS: uncached_readdir() searching for cookie %Lu\n", |
477 | (unsigned long long)*desc->dir_cookie); | 481 | (unsigned long long)*desc->dir_cookie); |
@@ -482,6 +486,7 @@ int uncached_readdir(nfs_readdir_descriptor_t *desc, void *dirent, | |||
482 | goto out; | 486 | goto out; |
483 | } | 487 | } |
484 | timestamp = jiffies; | 488 | timestamp = jiffies; |
489 | gencount = nfs_inc_attr_generation_counter(); | ||
485 | status = NFS_PROTO(inode)->readdir(file->f_path.dentry, cred, | 490 | status = NFS_PROTO(inode)->readdir(file->f_path.dentry, cred, |
486 | *desc->dir_cookie, page, | 491 | *desc->dir_cookie, page, |
487 | NFS_SERVER(inode)->dtsize, | 492 | NFS_SERVER(inode)->dtsize, |
@@ -490,6 +495,7 @@ int uncached_readdir(nfs_readdir_descriptor_t *desc, void *dirent, | |||
490 | desc->ptr = kmap(page); /* matching kunmap in nfs_do_filldir */ | 495 | desc->ptr = kmap(page); /* matching kunmap in nfs_do_filldir */ |
491 | if (status >= 0) { | 496 | if (status >= 0) { |
492 | desc->timestamp = timestamp; | 497 | desc->timestamp = timestamp; |
498 | desc->gencount = gencount; | ||
493 | desc->timestamp_valid = 1; | 499 | desc->timestamp_valid = 1; |
494 | if ((status = dir_decode(desc)) == 0) | 500 | if ((status = dir_decode(desc)) == 0) |
495 | desc->entry->prev_cookie = *desc->dir_cookie; | 501 | desc->entry->prev_cookie = *desc->dir_cookie; |
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c index de3f11e6234e..116a3bd2bc9b 100644 --- a/fs/nfs/inode.c +++ b/fs/nfs/inode.c | |||
@@ -305,7 +305,7 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr) | |||
305 | init_special_inode(inode, inode->i_mode, fattr->rdev); | 305 | init_special_inode(inode, inode->i_mode, fattr->rdev); |
306 | 306 | ||
307 | nfsi->read_cache_jiffies = fattr->time_start; | 307 | nfsi->read_cache_jiffies = fattr->time_start; |
308 | nfsi->last_updated = now; | 308 | nfsi->attr_gencount = fattr->gencount; |
309 | nfsi->cache_change_attribute = now; | 309 | nfsi->cache_change_attribute = now; |
310 | inode->i_atime = fattr->atime; | 310 | inode->i_atime = fattr->atime; |
311 | inode->i_mtime = fattr->mtime; | 311 | inode->i_mtime = fattr->mtime; |
@@ -909,6 +909,30 @@ static int nfs_size_need_update(const struct inode *inode, const struct nfs_fatt | |||
909 | return nfs_size_to_loff_t(fattr->size) > i_size_read(inode); | 909 | return nfs_size_to_loff_t(fattr->size) > i_size_read(inode); |
910 | } | 910 | } |
911 | 911 | ||
912 | static unsigned long nfs_attr_generation_counter; | ||
913 | |||
914 | static unsigned long nfs_read_attr_generation_counter(void) | ||
915 | { | ||
916 | smp_rmb(); | ||
917 | return nfs_attr_generation_counter; | ||
918 | } | ||
919 | |||
920 | unsigned long nfs_inc_attr_generation_counter(void) | ||
921 | { | ||
922 | unsigned long ret; | ||
923 | smp_rmb(); | ||
924 | ret = ++nfs_attr_generation_counter; | ||
925 | smp_wmb(); | ||
926 | return ret; | ||
927 | } | ||
928 | |||
929 | void nfs_fattr_init(struct nfs_fattr *fattr) | ||
930 | { | ||
931 | fattr->valid = 0; | ||
932 | fattr->time_start = jiffies; | ||
933 | fattr->gencount = nfs_inc_attr_generation_counter(); | ||
934 | } | ||
935 | |||
912 | /** | 936 | /** |
913 | * nfs_inode_attrs_need_update - check if the inode attributes need updating | 937 | * nfs_inode_attrs_need_update - check if the inode attributes need updating |
914 | * @inode - pointer to inode | 938 | * @inode - pointer to inode |
@@ -922,8 +946,7 @@ static int nfs_size_need_update(const struct inode *inode, const struct nfs_fatt | |||
922 | * catch the case where ctime either didn't change, or went backwards | 946 | * catch the case where ctime either didn't change, or went backwards |
923 | * (if someone reset the clock on the server) by looking at whether | 947 | * (if someone reset the clock on the server) by looking at whether |
924 | * or not this RPC call was started after the inode was last updated. | 948 | * or not this RPC call was started after the inode was last updated. |
925 | * Note also the check for jiffy wraparound if the last_updated timestamp | 949 | * Note also the check for wraparound of 'attr_gencount' |
926 | * is later than 'jiffies'. | ||
927 | * | 950 | * |
928 | * The function returns 'true' if it thinks the attributes in 'fattr' are | 951 | * The function returns 'true' if it thinks the attributes in 'fattr' are |
929 | * more recent than the ones cached in the inode. | 952 | * more recent than the ones cached in the inode. |
@@ -933,10 +956,10 @@ static int nfs_inode_attrs_need_update(const struct inode *inode, const struct n | |||
933 | { | 956 | { |
934 | const struct nfs_inode *nfsi = NFS_I(inode); | 957 | const struct nfs_inode *nfsi = NFS_I(inode); |
935 | 958 | ||
936 | return time_after(fattr->time_start, nfsi->last_updated) || | 959 | return ((long)fattr->gencount - (long)nfsi->attr_gencount) > 0 || |
937 | nfs_ctime_need_update(inode, fattr) || | 960 | nfs_ctime_need_update(inode, fattr) || |
938 | nfs_size_need_update(inode, fattr) || | 961 | nfs_size_need_update(inode, fattr) || |
939 | time_after(nfsi->last_updated, jiffies); | 962 | ((long)nfsi->attr_gencount - (long)nfs_read_attr_generation_counter() > 0); |
940 | } | 963 | } |
941 | 964 | ||
942 | static int nfs_refresh_inode_locked(struct inode *inode, struct nfs_fattr *fattr) | 965 | static int nfs_refresh_inode_locked(struct inode *inode, struct nfs_fattr *fattr) |
@@ -1107,7 +1130,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) | |||
1107 | } | 1130 | } |
1108 | /* If ctime has changed we should definitely clear access+acl caches */ | 1131 | /* If ctime has changed we should definitely clear access+acl caches */ |
1109 | if (!timespec_equal(&inode->i_ctime, &fattr->ctime)) | 1132 | if (!timespec_equal(&inode->i_ctime, &fattr->ctime)) |
1110 | invalid |= NFS_INO_INVALID_ACCESS|NFS_INO_INVALID_ACL; | 1133 | invalid |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_ACCESS|NFS_INO_INVALID_ACL; |
1111 | } else if (nfsi->change_attr != fattr->change_attr) { | 1134 | } else if (nfsi->change_attr != fattr->change_attr) { |
1112 | dprintk("NFS: change_attr change on server for file %s/%ld\n", | 1135 | dprintk("NFS: change_attr change on server for file %s/%ld\n", |
1113 | inode->i_sb->s_id, inode->i_ino); | 1136 | inode->i_sb->s_id, inode->i_ino); |
@@ -1163,7 +1186,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr) | |||
1163 | nfs_inc_stats(inode, NFSIOS_ATTRINVALIDATE); | 1186 | nfs_inc_stats(inode, NFSIOS_ATTRINVALIDATE); |
1164 | nfsi->attrtimeo = NFS_MINATTRTIMEO(inode); | 1187 | nfsi->attrtimeo = NFS_MINATTRTIMEO(inode); |
1165 | nfsi->attrtimeo_timestamp = now; | 1188 | nfsi->attrtimeo_timestamp = now; |
1166 | nfsi->last_updated = now; | 1189 | nfsi->attr_gencount = nfs_inc_attr_generation_counter(); |
1167 | } else { | 1190 | } else { |
1168 | if (!time_in_range(now, nfsi->attrtimeo_timestamp, nfsi->attrtimeo_timestamp + nfsi->attrtimeo)) { | 1191 | if (!time_in_range(now, nfsi->attrtimeo_timestamp, nfsi->attrtimeo_timestamp + nfsi->attrtimeo)) { |
1169 | if ((nfsi->attrtimeo <<= 1) > NFS_MAXATTRTIMEO(inode)) | 1192 | if ((nfsi->attrtimeo <<= 1) > NFS_MAXATTRTIMEO(inode)) |