On Tue, 1 May 2012 18:06:23 -0400 Trond Myklebust <Trond.Myklebust@xxxxxxxxxx> wrote: > While the use of READDIRPLUS is significantly more efficient than > READDIR followed by many LOOKUP calls, it is still less efficient > than just READDIR if the attributes are not required. > > This patch tracks when lookups are attempted on the directory, > and uses that information to selectively disable READDIRPLUS > on that directory. > The first 'readdir' call is always served using READDIRPLUS. > Subsequent calls only use READDIRPLUS if there was a successful > lookup or revalidation on a child in the mean time. > > Credit for the original idea should go to Neil Brown. See: > http://www.spinics.net/lists/linux-nfs/msg19996.html > However, the implementation in this patch differs from Neil's > in that it focuses on tracking lookups rather than calls to > stat(). > > Signed-off-by: Trond Myklebust <Trond.Myklebust@xxxxxxxxxx> > Cc: Neil Brown <neilb@xxxxxxx> Thanks! Looks good. Tracking lookups rather than getattr does make sense - more general. > @@ -874,7 +897,9 @@ static int nfs_readdir(struct file *filp, void *dirent, filldir_t filldir) > desc->file = filp; > desc->dir_cookie = &dir_ctx->dir_cookie; > desc->decode = NFS_PROTO(inode)->decode_dirent; > - desc->plus = NFS_USE_READDIRPLUS(inode); > + desc->plus = 0; > + if (nfs_use_readdirplus(inode, filp)) > + desc->plus = 1; > This looks a bit odd to me. desc->plus = nfs_use_readdirplus(inode, filp): ?? Or if you think there is a type conflict: desc->plus = nfs_use_readdirplus(inode, file) ? 1 : 0; But maybe you prefer the original - your choice of course. Also, the current code will clear NFS_INO_ADVISE_RDPLUS if it gets -ETOOSMALL, and it will stay cleared until the inode falls out of cache and is reloaded. The new code will set it again a lot sooner. Is that likely to be a problem (I don't remember what conditions lead to ETOOSMALL)? In fact I think it will ignore the ETOOSMALL and always retry with readdirplus for the first read in a directory. That doesn't seem good ? Thanks, NeilBrown
Attachment:
signature.asc
Description: PGP signature
