| Message ID | 67cfa1f7f028229bfd4b1ac2619bc85b9e34ef7f.1472059583.git.bcodding@redhat.com (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Delegated to: | Trond Myklebust |
| Headers | show |
> On Aug 24, 2016, at 13:27, Benjamin Coddington <bcodding@redhat.com> wrote: > > Commit 311324ad1713666a6e803aecf0d4e1a136a5b34a ("NFS: Be more aggressive > in using readdirplus for 'ls -l' situations") removed the optimization > added by commit 07b5ce8ef2d87f1914054804720d6facbaa3f4ce ("NFS: Make nfs_readdir > revalidate less often") to bypass the revalidation of the directory mapping > on subsequent calls into nfs_readdir(). Add that optimization back here. > > A directory modified once every second and containing 40k entries takes my > system around 80 seconds to list. With this patch, that time is reduced to > 7 seconds. > > Signed-off-by: Benjamin Coddington <bcodding@redhat.com> > --- > fs/nfs/dir.c | 13 +------------ > 1 file changed, 1 insertion(+), 12 deletions(-) > > diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c > index 19d93d0cd400..9a036dc6caa3 100644 > --- a/fs/nfs/dir.c > +++ b/fs/nfs/dir.c > @@ -872,17 +872,6 @@ int uncached_readdir(nfs_readdir_descriptor_t *desc) > goto out; > } > > -static bool nfs_dir_mapping_need_revalidate(struct inode *dir) > -{ > - struct nfs_inode *nfsi = NFS_I(dir); > - > - if (nfs_attribute_cache_expired(dir)) > - return true; > - if (nfsi->cache_validity & NFS_INO_INVALID_DATA) > - return true; > - return false; > -} > - > /* The file offset position represents the dirent entry number. A > last cookie cache takes care of the common case of reading the > whole directory. > @@ -914,7 +903,7 @@ static int nfs_readdir(struct file *file, struct dir_context *ctx) > desc->decode = NFS_PROTO(inode)->decode_dirent; > desc->plus = nfs_use_readdirplus(inode, ctx) ? 1 : 0; > > - if (ctx->pos == 0 || nfs_dir_mapping_need_revalidate(inode)) > + if (ctx->pos == 0 || nfs_attribute_cache_expired(inode)) > res = nfs_revalidate_mapping(inode, file->f_mapping); > if (res < 0) > goto out; > -- > 2.5.5 > NACK. As already discussed, this patch will break the functionality introduced in commit 311324ad1713. You need at least to ensure that we respect nfs_force_use_readdirplus(). -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>> On Aug 24, 2016, at 13:27, Benjamin Coddington <bcodding@redhat.com> >> wrote: >> >> Commit 311324ad1713666a6e803aecf0d4e1a136a5b34a ("NFS: Be more >> aggressive >> in using readdirplus for 'ls -l' situations") removed the >> optimization >> added by commit 07b5ce8ef2d87f1914054804720d6facbaa3f4ce ("NFS: Make >> nfs_readdir >> revalidate less often") to bypass the revalidation of the directory >> mapping >> on subsequent calls into nfs_readdir(). Add that optimization back >> here. >> >> A directory modified once every second and containing 40k entries >> takes my >> system around 80 seconds to list. With this patch, that time is >> reduced to >> 7 seconds. >> >> Signed-off-by: Benjamin Coddington <bcodding@redhat.com> >> --- >> fs/nfs/dir.c | 13 +------------ >> 1 file changed, 1 insertion(+), 12 deletions(-) >> >> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c >> index 19d93d0cd400..9a036dc6caa3 100644 >> --- a/fs/nfs/dir.c >> +++ b/fs/nfs/dir.c >> @@ -872,17 +872,6 @@ int uncached_readdir(nfs_readdir_descriptor_t >> *desc) >> goto out; >> } >> >> -static bool nfs_dir_mapping_need_revalidate(struct inode *dir) >> -{ >> - struct nfs_inode *nfsi = NFS_I(dir); >> - >> - if (nfs_attribute_cache_expired(dir)) >> - return true; >> - if (nfsi->cache_validity & NFS_INO_INVALID_DATA) >> - return true; >> - return false; >> -} >> - >> /* The file offset position represents the dirent entry number. A >> last cookie cache takes care of the common case of reading the >> whole directory. >> @@ -914,7 +903,7 @@ static int nfs_readdir(struct file *file, struct >> dir_context *ctx) >> desc->decode = NFS_PROTO(inode)->decode_dirent; >> desc->plus = nfs_use_readdirplus(inode, ctx) ? 1 : 0; >> >> - if (ctx->pos == 0 || nfs_dir_mapping_need_revalidate(inode)) >> + if (ctx->pos == 0 || nfs_attribute_cache_expired(inode)) >> res = nfs_revalidate_mapping(inode, file->f_mapping); >> if (res < 0) >> goto out; >> -- >> 2.5.5 >> > > NACK. > > As already discussed, this patch will break the functionality > introduced > in commit 311324ad1713. You need at least to ensure that we respect > nfs_force_use_readdirplus(). Hi Trond, I did not correctly understand the case for commit 311324ad1713, but re-reading the original posting thread has fixed me up. I'll send a v2 that handles both cases. Ben -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c index 19d93d0cd400..9a036dc6caa3 100644 --- a/fs/nfs/dir.c +++ b/fs/nfs/dir.c @@ -872,17 +872,6 @@ int uncached_readdir(nfs_readdir_descriptor_t *desc) goto out; } -static bool nfs_dir_mapping_need_revalidate(struct inode *dir) -{ - struct nfs_inode *nfsi = NFS_I(dir); - - if (nfs_attribute_cache_expired(dir)) - return true; - if (nfsi->cache_validity & NFS_INO_INVALID_DATA) - return true; - return false; -} - /* The file offset position represents the dirent entry number. A last cookie cache takes care of the common case of reading the whole directory. @@ -914,7 +903,7 @@ static int nfs_readdir(struct file *file, struct dir_context *ctx) desc->decode = NFS_PROTO(inode)->decode_dirent; desc->plus = nfs_use_readdirplus(inode, ctx) ? 1 : 0; - if (ctx->pos == 0 || nfs_dir_mapping_need_revalidate(inode)) + if (ctx->pos == 0 || nfs_attribute_cache_expired(inode)) res = nfs_revalidate_mapping(inode, file->f_mapping); if (res < 0) goto out;
Commit 311324ad1713666a6e803aecf0d4e1a136a5b34a ("NFS: Be more aggressive in using readdirplus for 'ls -l' situations") removed the optimization added by commit 07b5ce8ef2d87f1914054804720d6facbaa3f4ce ("NFS: Make nfs_readdir revalidate less often") to bypass the revalidation of the directory mapping on subsequent calls into nfs_readdir(). Add that optimization back here. A directory modified once every second and containing 40k entries takes my system around 80 seconds to list. With this patch, that time is reduced to 7 seconds. Signed-off-by: Benjamin Coddington <bcodding@redhat.com> --- fs/nfs/dir.c | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-)