Hi Trond, Unfortunately, this change doesn't fix another issue introduced with be4c2d4723a4: en extra READDIR request after server have sent all entries and indicated it with eol=true: https://www.spinics.net/lists/linux-nfs/msg73399.html Tigran. ----- Original Message ----- > From: "Trond Myklebust" <trondmy@xxxxxxxxx> > To: "linux-nfs" <linux-nfs@xxxxxxxxxxxxxxx> > Cc: "zhangliguang" <zhangliguang@xxxxxxxxxxxxxxxxx> > Sent: Saturday, July 6, 2019 8:52:50 PM > Subject: [PATCH 1/3] NFS: Fix off-by-one errors in nfs_readdir > In C, the array size and the maximum index are not the same. In this > case, the field desc->pvec.nr is being used as a cursor but is > occasionally being treated as if it the array size. > This patch renames it to desc->pvec.cursor in order to make clear that > it is tracking an index. > > Fixes: be4c2d4723a4 ("NFS: readdirplus optimization by cache mechanism") > Cc: Liguang Zhang <zhangliguang@xxxxxxxxxxxxxxxxx> > Cc: stable@xxxxxxxxxxxxxxx # v5.1+ > Signed-off-by: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx> > --- > fs/nfs/dir.c | 53 +++++++++++++++++++++++++--------------------------- > 1 file changed, 25 insertions(+), 28 deletions(-) > > diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c > index 57b6a45576ad..e44f3c9fad5b 100644 > --- a/fs/nfs/dir.c > +++ b/fs/nfs/dir.c > @@ -134,14 +134,14 @@ struct nfs_cache_array_entry { > }; > > struct nfs_cache_array { > - int size; > - int eof_index; > u64 last_cookie; > + unsigned int size; > + bool eof; > struct nfs_cache_array_entry array[0]; > }; > > struct readdirvec { > - unsigned long nr; > + unsigned long cursor; > unsigned long index; > struct page *pages[NFS_MAX_READDIR_RAPAGES]; > }; > @@ -172,7 +172,7 @@ static > void nfs_readdir_clear_array(struct page *page) > { > struct nfs_cache_array *array; > - int i; > + unsigned int i; > > array = kmap_atomic(page); > for (i = 0; i < array->size; i++) > @@ -224,7 +224,7 @@ int nfs_readdir_add_to_array(struct nfs_entry *entry, struct > page *page) > array->last_cookie = entry->cookie; > array->size++; > if (entry->eof != 0) > - array->eof_index = array->size; > + array->eof = true; > out: > kunmap(page); > return ret; > @@ -239,7 +239,7 @@ int nfs_readdir_search_for_pos(struct nfs_cache_array > *array, nfs_readdir_descri > if (diff < 0) > goto out_eof; > if (diff >= array->size) { > - if (array->eof_index >= 0) > + if (array->eof) > goto out_eof; > return -EAGAIN; > } > @@ -265,7 +265,7 @@ nfs_readdir_inode_mapping_valid(struct nfs_inode *nfsi) > static > int nfs_readdir_search_for_cookie(struct nfs_cache_array *array, > nfs_readdir_descriptor_t *desc) > { > - int i; > + unsigned int i; > loff_t new_pos; > int status = -EAGAIN; > > @@ -300,7 +300,7 @@ int nfs_readdir_search_for_cookie(struct nfs_cache_array > *array, nfs_readdir_des > return 0; > } > } > - if (array->eof_index >= 0) { > + if (array->eof) { > status = -EBADCOOKIE; > if (*desc->dir_cookie == array->last_cookie) > desc->eof = true; > @@ -532,10 +532,9 @@ int nfs_readdir_page_filler(nfs_readdir_descriptor_t *desc, > struct nfs_entry *en > struct nfs_cache_array *array; > unsigned int count = 0; > int status; > - int max_rapages = NFS_MAX_READDIR_RAPAGES; > > desc->pvec.index = desc->page_index; > - desc->pvec.nr = 0; > + desc->pvec.cursor = 0; > > scratch = alloc_page(GFP_KERNEL); > if (scratch == NULL) > @@ -560,12 +559,12 @@ int nfs_readdir_page_filler(nfs_readdir_descriptor_t > *desc, struct nfs_entry *en > if (desc->plus) > nfs_prime_dcache(file_dentry(desc->file), entry); > > - status = nfs_readdir_add_to_array(entry, desc->pvec.pages[desc->pvec.nr]); > + status = nfs_readdir_add_to_array(entry, > desc->pvec.pages[desc->pvec.cursor]); > if (status == -ENOSPC) { > - desc->pvec.nr++; > - if (desc->pvec.nr == max_rapages) > + if (desc->pvec.cursor == ARRAY_SIZE(desc->pvec.pages) - 1) > break; > - status = nfs_readdir_add_to_array(entry, desc->pvec.pages[desc->pvec.nr]); > + desc->pvec.cursor++; > + status = nfs_readdir_add_to_array(entry, > desc->pvec.pages[desc->pvec.cursor]); > } > if (status != 0) > break; > @@ -579,8 +578,8 @@ int nfs_readdir_page_filler(nfs_readdir_descriptor_t *desc, > struct nfs_entry *en > > out_nopages: > if (count == 0 || (status == -EBADCOOKIE && entry->eof != 0)) { > - array = kmap_atomic(desc->pvec.pages[desc->pvec.nr]); > - array->eof_index = array->size; > + array = kmap_atomic(desc->pvec.pages[desc->pvec.cursor]); > + array->eof = true; > status = 0; > kunmap_atomic(array); > } > @@ -588,11 +587,11 @@ int nfs_readdir_page_filler(nfs_readdir_descriptor_t > *desc, struct nfs_entry *en > put_page(scratch); > > /* > - * desc->pvec.nr > 0 means at least one page was completely filled, > + * desc->pvec.cursor > 0 means at least one page was completely filled, > * we should return -ENOSPC. Otherwise function > * nfs_readdir_xdr_to_array will enter infinite loop. > */ > - if (desc->pvec.nr > 0) > + if (desc->pvec.cursor > 0) > return -ENOSPC; > return status; > } > @@ -634,13 +633,12 @@ static > void nfs_readdir_rapages_init(nfs_readdir_descriptor_t *desc) > { > struct nfs_cache_array *array; > - int max_rapages = NFS_MAX_READDIR_RAPAGES; > - int index; > + unsigned int max_rapages = NFS_MAX_READDIR_RAPAGES; > + unsigned int index; > > for (index = 0; index < max_rapages; index++) { > array = kmap_atomic(desc->pvec.pages[index]); > memset(array, 0, sizeof(struct nfs_cache_array)); > - array->eof_index = -1; > kunmap_atomic(array); > } > } > @@ -678,7 +676,6 @@ int nfs_readdir_xdr_to_array(nfs_readdir_descriptor_t *desc, > struct page *page, > > array = kmap(page); > memset(array, 0, sizeof(struct nfs_cache_array)); > - array->eof_index = -1; > > status = nfs_readdir_alloc_pages(pages, array_size); > if (status < 0) > @@ -696,7 +693,7 @@ int nfs_readdir_xdr_to_array(nfs_readdir_descriptor_t *desc, > struct page *page, > status = 0; > break; > } > - } while (array->eof_index < 0); > + } while (!array->eof); > > nfs_readdir_free_pages(pages, array_size); > out_release_array: > @@ -723,10 +720,10 @@ int nfs_readdir_filler(void *data, struct page* page) > > /* > * If desc->page_index in range desc->pvec.index and > - * desc->pvec.index + desc->pvec.nr, we get readdir cache hit. > + * desc->pvec.index + desc->pvec.cursor, we get readdir cache hit. > */ > if (desc->page_index >= desc->pvec.index && > - desc->page_index < (desc->pvec.index + desc->pvec.nr)) { > + desc->page_index <= (desc->pvec.index + desc->pvec.cursor)) { > /* > * page and desc->pvec.pages[x] are valid, don't need to check > * whether or not to be NULL. > @@ -809,7 +806,7 @@ static > int nfs_do_filldir(nfs_readdir_descriptor_t *desc) > { > struct file *file = desc->file; > - int i = 0; > + unsigned int i = 0; > int res = 0; > struct nfs_cache_array *array = NULL; > struct nfs_open_dir_context *ctx = file->private_data; > @@ -832,7 +829,7 @@ int nfs_do_filldir(nfs_readdir_descriptor_t *desc) > if (ctx->duped != 0) > ctx->duped = 1; > } > - if (array->eof_index >= 0) > + if (array->eof) > desc->eof = true; > > kunmap(desc->page); > @@ -903,7 +900,7 @@ static int nfs_readdir(struct file *file, struct dir_context > *ctx) > *desc = &my_desc; > struct nfs_open_dir_context *dir_ctx = file->private_data; > int res = 0; > - int max_rapages = NFS_MAX_READDIR_RAPAGES; > + unsigned int max_rapages = NFS_MAX_READDIR_RAPAGES; > > dfprintk(FILE, "NFS: readdir(%pD2) starting at cookie %llu\n", > file, (long long)ctx->pos); > -- > 2.21.0
