| Message ID | 20201104161638.300324-17-trond.myklebust@hammerspace.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Readdir enhancements | expand |
On Wed, Nov 4, 2020 at 11:28 AM <trondmy@gmail.com> wrote: > > From: Trond Myklebust <trond.myklebust@hammerspace.com> > > If the server insists on using the readdir verifiers in order to allow > cookies to expire, then we should ensure that we cache the verifier > with the cookie, so that we can return an error if the application > tries to use the expired cookie. > > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> > --- > fs/nfs/dir.c | 35 +++++++++++++++++++++++------------ > fs/nfs/inode.c | 7 ------- > include/linux/nfs_fs.h | 8 +++++++- > 3 files changed, 30 insertions(+), 20 deletions(-) > > diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c > index 3b44bef3a1b4..454377228167 100644 > --- a/fs/nfs/dir.c > +++ b/fs/nfs/dir.c > @@ -155,6 +155,7 @@ struct nfs_readdir_descriptor { > loff_t current_index; > loff_t prev_index; > > + __be32 verf[NFS_DIR_VERIFIER_SIZE]; > unsigned long dir_verifier; > unsigned long timestamp; > unsigned long gencount; > @@ -466,15 +467,15 @@ static int nfs_readdir_search_array(struct nfs_readdir_descriptor *desc) > > /* Fill a page with xdr information before transferring to the cache page */ > static int nfs_readdir_xdr_filler(struct nfs_readdir_descriptor *desc, > - u64 cookie, struct page **pages, > - size_t bufsize) > + __be32 *verf, u64 cookie, > + struct page **pages, size_t bufsize, > + __be32 *verf_res) > { > struct inode *inode = file_inode(desc->file); > - __be32 verf_res[2]; > struct nfs_readdir_arg arg = { > .dentry = file_dentry(desc->file), > .cred = desc->file->f_cred, > - .verf = NFS_I(inode)->cookieverf, > + .verf = verf, > .cookie = cookie, > .pages = pages, > .page_len = bufsize, > @@ -503,8 +504,6 @@ static int nfs_readdir_xdr_filler(struct nfs_readdir_descriptor *desc, > } > desc->timestamp = timestamp; > desc->gencount = gencount; > - memcpy(NFS_I(inode)->cookieverf, res.verf, > - sizeof(NFS_I(inode)->cookieverf)); > error: > return error; > } > @@ -770,11 +769,13 @@ static struct page **nfs_readdir_alloc_pages(size_t npages) > } > > static int nfs_readdir_xdr_to_array(struct nfs_readdir_descriptor *desc, > - struct page *page, struct inode *inode) > + struct page *page, __be32 *verf_arg, > + __be32 *verf_res) > { > struct page **pages; > struct nfs_entry *entry; > size_t array_size; > + struct inode *inode = file_inode(desc->file); > size_t dtsize = NFS_SERVER(inode)->dtsize; > int status = -ENOMEM; > > @@ -801,8 +802,9 @@ static int nfs_readdir_xdr_to_array(struct nfs_readdir_descriptor *desc, > > do { > unsigned int pglen; > - status = nfs_readdir_xdr_filler(desc, entry->cookie, > - pages, dtsize); > + status = nfs_readdir_xdr_filler(desc, verf_arg, entry->cookie, > + pages, dtsize, > + verf_res); > if (status < 0) > break; > > @@ -854,13 +856,15 @@ static int find_and_lock_cache_page(struct nfs_readdir_descriptor *desc) > { > struct inode *inode = file_inode(desc->file); > struct nfs_inode *nfsi = NFS_I(inode); > + __be32 verf[NFS_DIR_VERIFIER_SIZE]; > int res; > > desc->page = nfs_readdir_page_get_cached(desc); > if (!desc->page) > return -ENOMEM; > if (nfs_readdir_page_needs_filling(desc->page)) { > - res = nfs_readdir_xdr_to_array(desc, desc->page, inode); > + res = nfs_readdir_xdr_to_array(desc, desc->page, > + nfsi->cookieverf, verf); > if (res < 0) { > nfs_readdir_page_unlock_and_put_cached(desc); > if (res == -EBADCOOKIE || res == -ENOTSYNC) { > @@ -870,6 +874,7 @@ static int find_and_lock_cache_page(struct nfs_readdir_descriptor *desc) > } > return res; > } > + memcpy(nfsi->cookieverf, verf, sizeof(nfsi->cookieverf)); > } > res = nfs_readdir_search_array(desc); > if (res == 0) { > @@ -902,6 +907,7 @@ static int readdir_search_pagecache(struct nfs_readdir_descriptor *desc) > static void nfs_do_filldir(struct nfs_readdir_descriptor *desc) > { > struct file *file = desc->file; > + struct nfs_inode *nfsi = NFS_I(file_inode(file)); > struct nfs_cache_array *array; > unsigned int i = 0; > > @@ -915,6 +921,7 @@ static void nfs_do_filldir(struct nfs_readdir_descriptor *desc) > desc->eof = true; > break; > } > + memcpy(desc->verf, nfsi->cookieverf, sizeof(desc->verf)); > if (i < (array->size-1)) > desc->dir_cookie = array->array[i+1].cookie; > else > @@ -949,8 +956,8 @@ static void nfs_do_filldir(struct nfs_readdir_descriptor *desc) > static int uncached_readdir(struct nfs_readdir_descriptor *desc) > { > struct page *page = NULL; > + __be32 verf[NFS_DIR_VERIFIER_SIZE]; > int status; > - struct inode *inode = file_inode(desc->file); > > dfprintk(DIRCACHE, "NFS: uncached_readdir() searching for cookie %Lu\n", > (unsigned long long)desc->dir_cookie); > @@ -967,7 +974,7 @@ static int uncached_readdir(struct nfs_readdir_descriptor *desc) > desc->duped = 0; > > nfs_readdir_page_init_array(page, desc->dir_cookie); > - status = nfs_readdir_xdr_to_array(desc, page, inode); > + status = nfs_readdir_xdr_to_array(desc, page, desc->verf, verf); > if (status < 0) > goto out_release; > > @@ -1023,6 +1030,7 @@ static int nfs_readdir(struct file *file, struct dir_context *ctx) > desc->dup_cookie = dir_ctx->dup_cookie; > desc->duped = dir_ctx->duped; > desc->attr_gencount = dir_ctx->attr_gencount; > + memcpy(desc->verf, dir_ctx->verf, sizeof(desc->verf)); > spin_unlock(&file->f_lock); > > do { > @@ -1061,6 +1069,7 @@ static int nfs_readdir(struct file *file, struct dir_context *ctx) > dir_ctx->dup_cookie = desc->dup_cookie; > dir_ctx->duped = desc->duped; > dir_ctx->attr_gencount = desc->attr_gencount; > + memcpy(dir_ctx->verf, desc->verf, sizeof(dir_ctx->verf)); > spin_unlock(&file->f_lock); > > kfree(desc); > @@ -1101,6 +1110,8 @@ static loff_t nfs_llseek_dir(struct file *filp, loff_t offset, int whence) > dir_ctx->dir_cookie = offset; > else > dir_ctx->dir_cookie = 0; > + if (offset == 0) > + memset(dir_ctx->verf, 0, sizeof(dir_ctx->verf)); > dir_ctx->duped = 0; > } > spin_unlock(&filp->f_lock); Thanks for doing these patches! For some reason this patch does not apply but I get a problem at this hunk. Is there a fixup or hunk or two missing from 01/17 ? I'm starting at 3cea11cd5e3b (Linux 5.10-rc2). Problem looks like it's at the spin_unlock - here's what the hunk looks like for me: fs/nfs/dir.c 1092 inode_lock(inode); 1093 offset += filp->f_pos; 1094 if (offset < 0) { 1095 inode_unlock(inode); 1096 return -EINVAL; 1097 } 1098 } 1099 if (offset != filp->f_pos) { 1100 filp->f_pos = offset; 1101 if (nfs_readdir_use_cookie(filp)) 1102 dir_ctx->dir_cookie = offset; 1103 else 1104 dir_ctx->dir_cookie = 0; 1105 dir_ctx->duped = 0; 1106 } 1107 inode_unlock(inode); 1108 return offset; 1109 } $ git reset --hard 3cea11cd5e3b HEAD is now at 3cea11cd5e3b Linux 5.10-rc2 $ for f in ~/Downloads/trond-nfs-readdir/v3/*; do echo applying $(basename "$f"); git am "$f"; done applying [PATCH v3 01_17] NFS_ Ensure contents of struct nfs_open_dir_context are consistent.eml Applying: NFS: Ensure contents of struct nfs_open_dir_context are consistent applying [PATCH v3 02_17] NFS_ Clean up readdir struct nfs_cache_array.eml Applying: NFS: Clean up readdir struct nfs_cache_array applying [PATCH v3 03_17] NFS_ Clean up nfs_readdir_page_filler().eml Applying: NFS: Clean up nfs_readdir_page_filler() applying [PATCH v3 04_17] NFS_ Clean up directory array handling.eml Applying: NFS: Clean up directory array handling applying [PATCH v3 05_17] NFS_ Don't discard readdir results.eml Applying: NFS: Don't discard readdir results applying [PATCH v3 06_17] NFS_ Remove unnecessary kmap in nfs_readdir_xdr_to_array().eml Applying: NFS: Remove unnecessary kmap in nfs_readdir_xdr_to_array() applying [PATCH v3 07_17] NFS_ Replace kmap() with kmap_atomic() in nfs_readdir_search_array().eml Applying: NFS: Replace kmap() with kmap_atomic() in nfs_readdir_search_array() applying [PATCH v3 08_17] NFS_ Simplify struct nfs_cache_array_entry.eml Applying: NFS: Simplify struct nfs_cache_array_entry applying [PATCH v3 09_17] NFS_ Support larger readdir buffers.eml Applying: NFS: Support larger readdir buffers applying [PATCH v3 10_17] NFS_ More readdir cleanups.eml Applying: NFS: More readdir cleanups applying [PATCH v3 11_17] NFS_ nfs_do_filldir() does not return a value.eml Applying: NFS: nfs_do_filldir() does not return a value applying [PATCH v3 12_17] NFS_ Reduce readdir stack usage.eml Applying: NFS: Reduce readdir stack usage applying [PATCH v3 13_17] NFS_ Cleanup to remove nfs_readdir_descriptor_t typedef.eml Applying: NFS: Cleanup to remove nfs_readdir_descriptor_t typedef applying [PATCH v3 14_17] NFS_ Allow the NFS generic code to pass in a verifier to readdir.eml Applying: NFS: Allow the NFS generic code to pass in a verifier to readdir applying [PATCH v3 15_17] NFS_ Handle NFS4ERR_NOT_SAME and NFSERR_BADCOOKIE from readdir calls.eml Applying: NFS: Handle NFS4ERR_NOT_SAME and NFSERR_BADCOOKIE from readdir calls applying [PATCH v3 16_17] NFS_ Improve handling of directory verifiers.eml Applying: NFS: Improve handling of directory verifiers error: patch failed: fs/nfs/dir.c:1101 error: fs/nfs/dir.c: patch does not apply Patch failed at 0001 NFS: Improve handling of directory verifiers The copy of the patch that failed is found in: /home/dwysocha/git/kernel/.git/rebase-apply/patch When you have resolved this problem, run "git am --resolved". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". applying [PATCH v3 17_17] NFS_ Optimisations for monotonically increasing readdir cookies.eml previous rebase directory /home/dwysocha/git/kernel/.git/rebase-apply still exists but mbox given.
> On Nov 4, 2020, at 16:01, David Wysochanski <dwysocha@redhat.com> wrote: > > On Wed, Nov 4, 2020 at 11:28 AM <trondmy@gmail.com> wrote: >> >> From: Trond Myklebust <trond.myklebust@hammerspace.com> >> >> If the server insists on using the readdir verifiers in order to allow >> cookies to expire, then we should ensure that we cache the verifier >> with the cookie, so that we can return an error if the application >> tries to use the expired cookie. >> >> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> >> --- >> fs/nfs/dir.c | 35 +++++++++++++++++++++++------------ >> fs/nfs/inode.c | 7 ------- >> include/linux/nfs_fs.h | 8 +++++++- >> 3 files changed, 30 insertions(+), 20 deletions(-) >> >> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c >> index 3b44bef3a1b4..454377228167 100644 >> --- a/fs/nfs/dir.c >> +++ b/fs/nfs/dir.c >> @@ -155,6 +155,7 @@ struct nfs_readdir_descriptor { >> loff_t current_index; >> loff_t prev_index; >> >> + __be32 verf[NFS_DIR_VERIFIER_SIZE]; >> unsigned long dir_verifier; >> unsigned long timestamp; >> unsigned long gencount; >> @@ -466,15 +467,15 @@ static int nfs_readdir_search_array(struct nfs_readdir_descriptor *desc) >> >> /* Fill a page with xdr information before transferring to the cache page */ >> static int nfs_readdir_xdr_filler(struct nfs_readdir_descriptor *desc, >> - u64 cookie, struct page **pages, >> - size_t bufsize) >> + __be32 *verf, u64 cookie, >> + struct page **pages, size_t bufsize, >> + __be32 *verf_res) >> { >> struct inode *inode = file_inode(desc->file); >> - __be32 verf_res[2]; >> struct nfs_readdir_arg arg = { >> .dentry = file_dentry(desc->file), >> .cred = desc->file->f_cred, >> - .verf = NFS_I(inode)->cookieverf, >> + .verf = verf, >> .cookie = cookie, >> .pages = pages, >> .page_len = bufsize, >> @@ -503,8 +504,6 @@ static int nfs_readdir_xdr_filler(struct nfs_readdir_descriptor *desc, >> } >> desc->timestamp = timestamp; >> desc->gencount = gencount; >> - memcpy(NFS_I(inode)->cookieverf, res.verf, >> - sizeof(NFS_I(inode)->cookieverf)); >> error: >> return error; >> } >> @@ -770,11 +769,13 @@ static struct page **nfs_readdir_alloc_pages(size_t npages) >> } >> >> static int nfs_readdir_xdr_to_array(struct nfs_readdir_descriptor *desc, >> - struct page *page, struct inode *inode) >> + struct page *page, __be32 *verf_arg, >> + __be32 *verf_res) >> { >> struct page **pages; >> struct nfs_entry *entry; >> size_t array_size; >> + struct inode *inode = file_inode(desc->file); >> size_t dtsize = NFS_SERVER(inode)->dtsize; >> int status = -ENOMEM; >> >> @@ -801,8 +802,9 @@ static int nfs_readdir_xdr_to_array(struct nfs_readdir_descriptor *desc, >> >> do { >> unsigned int pglen; >> - status = nfs_readdir_xdr_filler(desc, entry->cookie, >> - pages, dtsize); >> + status = nfs_readdir_xdr_filler(desc, verf_arg, entry->cookie, >> + pages, dtsize, >> + verf_res); >> if (status < 0) >> break; >> >> @@ -854,13 +856,15 @@ static int find_and_lock_cache_page(struct nfs_readdir_descriptor *desc) >> { >> struct inode *inode = file_inode(desc->file); >> struct nfs_inode *nfsi = NFS_I(inode); >> + __be32 verf[NFS_DIR_VERIFIER_SIZE]; >> int res; >> >> desc->page = nfs_readdir_page_get_cached(desc); >> if (!desc->page) >> return -ENOMEM; >> if (nfs_readdir_page_needs_filling(desc->page)) { >> - res = nfs_readdir_xdr_to_array(desc, desc->page, inode); >> + res = nfs_readdir_xdr_to_array(desc, desc->page, >> + nfsi->cookieverf, verf); >> if (res < 0) { >> nfs_readdir_page_unlock_and_put_cached(desc); >> if (res == -EBADCOOKIE || res == -ENOTSYNC) { >> @@ -870,6 +874,7 @@ static int find_and_lock_cache_page(struct nfs_readdir_descriptor *desc) >> } >> return res; >> } >> + memcpy(nfsi->cookieverf, verf, sizeof(nfsi->cookieverf)); >> } >> res = nfs_readdir_search_array(desc); >> if (res == 0) { >> @@ -902,6 +907,7 @@ static int readdir_search_pagecache(struct nfs_readdir_descriptor *desc) >> static void nfs_do_filldir(struct nfs_readdir_descriptor *desc) >> { >> struct file *file = desc->file; >> + struct nfs_inode *nfsi = NFS_I(file_inode(file)); >> struct nfs_cache_array *array; >> unsigned int i = 0; >> >> @@ -915,6 +921,7 @@ static void nfs_do_filldir(struct nfs_readdir_descriptor *desc) >> desc->eof = true; >> break; >> } >> + memcpy(desc->verf, nfsi->cookieverf, sizeof(desc->verf)); >> if (i < (array->size-1)) >> desc->dir_cookie = array->array[i+1].cookie; >> else >> @@ -949,8 +956,8 @@ static void nfs_do_filldir(struct nfs_readdir_descriptor *desc) >> static int uncached_readdir(struct nfs_readdir_descriptor *desc) >> { >> struct page *page = NULL; >> + __be32 verf[NFS_DIR_VERIFIER_SIZE]; >> int status; >> - struct inode *inode = file_inode(desc->file); >> >> dfprintk(DIRCACHE, "NFS: uncached_readdir() searching for cookie %Lu\n", >> (unsigned long long)desc->dir_cookie); >> @@ -967,7 +974,7 @@ static int uncached_readdir(struct nfs_readdir_descriptor *desc) >> desc->duped = 0; >> >> nfs_readdir_page_init_array(page, desc->dir_cookie); >> - status = nfs_readdir_xdr_to_array(desc, page, inode); >> + status = nfs_readdir_xdr_to_array(desc, page, desc->verf, verf); >> if (status < 0) >> goto out_release; >> >> @@ -1023,6 +1030,7 @@ static int nfs_readdir(struct file *file, struct dir_context *ctx) >> desc->dup_cookie = dir_ctx->dup_cookie; >> desc->duped = dir_ctx->duped; >> desc->attr_gencount = dir_ctx->attr_gencount; >> + memcpy(desc->verf, dir_ctx->verf, sizeof(desc->verf)); >> spin_unlock(&file->f_lock); >> >> do { >> @@ -1061,6 +1069,7 @@ static int nfs_readdir(struct file *file, struct dir_context *ctx) >> dir_ctx->dup_cookie = desc->dup_cookie; >> dir_ctx->duped = desc->duped; >> dir_ctx->attr_gencount = desc->attr_gencount; >> + memcpy(dir_ctx->verf, desc->verf, sizeof(dir_ctx->verf)); >> spin_unlock(&file->f_lock); >> >> kfree(desc); >> @@ -1101,6 +1110,8 @@ static loff_t nfs_llseek_dir(struct file *filp, loff_t offset, int whence) >> dir_ctx->dir_cookie = offset; >> else >> dir_ctx->dir_cookie = 0; >> + if (offset == 0) >> + memset(dir_ctx->verf, 0, sizeof(dir_ctx->verf)); >> dir_ctx->duped = 0; >> } >> spin_unlock(&filp->f_lock); > > Thanks for doing these patches! > > For some reason this patch does not apply but I get a problem at this hunk. > Is there a fixup or hunk or two missing from 01/17 ? > I'm starting at 3cea11cd5e3b (Linux 5.10-rc2). > > Problem looks like it's at the spin_unlock - here's what the hunk > looks like for me: > fs/nfs/dir.c > 1092 inode_lock(inode); > 1093 offset += filp->f_pos; > 1094 if (offset < 0) { > 1095 inode_unlock(inode); > 1096 return -EINVAL; > 1097 } > 1098 } > 1099 if (offset != filp->f_pos) { > 1100 filp->f_pos = offset; > 1101 if (nfs_readdir_use_cookie(filp)) > 1102 dir_ctx->dir_cookie = offset; > 1103 else > 1104 dir_ctx->dir_cookie = 0; > 1105 dir_ctx->duped = 0; > 1106 } > 1107 inode_unlock(inode); > 1108 return offset; > 1109 } > Yes, it depends on the patch "[PATCH 1/2] NFS: Remove unnecessary inode locking in nfs_llseek_dir()” that I sent out to the list on Oct 30. I can include that in future updates of this patchset.
On Wed, Nov 4, 2020 at 4:32 PM Trond Myklebust <trondmy@hammerspace.com> wrote: > > > > > On Nov 4, 2020, at 16:01, David Wysochanski <dwysocha@redhat.com> wrote: > > > > On Wed, Nov 4, 2020 at 11:28 AM <trondmy@gmail.com> wrote: > >> > >> From: Trond Myklebust <trond.myklebust@hammerspace.com> > >> > >> If the server insists on using the readdir verifiers in order to allow > >> cookies to expire, then we should ensure that we cache the verifier > >> with the cookie, so that we can return an error if the application > >> tries to use the expired cookie. > >> > >> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> > >> --- > >> fs/nfs/dir.c | 35 +++++++++++++++++++++++------------ > >> fs/nfs/inode.c | 7 ------- > >> include/linux/nfs_fs.h | 8 +++++++- > >> 3 files changed, 30 insertions(+), 20 deletions(-) > >> > >> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c > >> index 3b44bef3a1b4..454377228167 100644 > >> --- a/fs/nfs/dir.c > >> +++ b/fs/nfs/dir.c > >> @@ -155,6 +155,7 @@ struct nfs_readdir_descriptor { > >> loff_t current_index; > >> loff_t prev_index; > >> > >> + __be32 verf[NFS_DIR_VERIFIER_SIZE]; > >> unsigned long dir_verifier; > >> unsigned long timestamp; > >> unsigned long gencount; > >> @@ -466,15 +467,15 @@ static int nfs_readdir_search_array(struct nfs_readdir_descriptor *desc) > >> > >> /* Fill a page with xdr information before transferring to the cache page */ > >> static int nfs_readdir_xdr_filler(struct nfs_readdir_descriptor *desc, > >> - u64 cookie, struct page **pages, > >> - size_t bufsize) > >> + __be32 *verf, u64 cookie, > >> + struct page **pages, size_t bufsize, > >> + __be32 *verf_res) > >> { > >> struct inode *inode = file_inode(desc->file); > >> - __be32 verf_res[2]; > >> struct nfs_readdir_arg arg = { > >> .dentry = file_dentry(desc->file), > >> .cred = desc->file->f_cred, > >> - .verf = NFS_I(inode)->cookieverf, > >> + .verf = verf, > >> .cookie = cookie, > >> .pages = pages, > >> .page_len = bufsize, > >> @@ -503,8 +504,6 @@ static int nfs_readdir_xdr_filler(struct nfs_readdir_descriptor *desc, > >> } > >> desc->timestamp = timestamp; > >> desc->gencount = gencount; > >> - memcpy(NFS_I(inode)->cookieverf, res.verf, > >> - sizeof(NFS_I(inode)->cookieverf)); > >> error: > >> return error; > >> } > >> @@ -770,11 +769,13 @@ static struct page **nfs_readdir_alloc_pages(size_t npages) > >> } > >> > >> static int nfs_readdir_xdr_to_array(struct nfs_readdir_descriptor *desc, > >> - struct page *page, struct inode *inode) > >> + struct page *page, __be32 *verf_arg, > >> + __be32 *verf_res) > >> { > >> struct page **pages; > >> struct nfs_entry *entry; > >> size_t array_size; > >> + struct inode *inode = file_inode(desc->file); > >> size_t dtsize = NFS_SERVER(inode)->dtsize; > >> int status = -ENOMEM; > >> > >> @@ -801,8 +802,9 @@ static int nfs_readdir_xdr_to_array(struct nfs_readdir_descriptor *desc, > >> > >> do { > >> unsigned int pglen; > >> - status = nfs_readdir_xdr_filler(desc, entry->cookie, > >> - pages, dtsize); > >> + status = nfs_readdir_xdr_filler(desc, verf_arg, entry->cookie, > >> + pages, dtsize, > >> + verf_res); > >> if (status < 0) > >> break; > >> > >> @@ -854,13 +856,15 @@ static int find_and_lock_cache_page(struct nfs_readdir_descriptor *desc) > >> { > >> struct inode *inode = file_inode(desc->file); > >> struct nfs_inode *nfsi = NFS_I(inode); > >> + __be32 verf[NFS_DIR_VERIFIER_SIZE]; > >> int res; > >> > >> desc->page = nfs_readdir_page_get_cached(desc); > >> if (!desc->page) > >> return -ENOMEM; > >> if (nfs_readdir_page_needs_filling(desc->page)) { > >> - res = nfs_readdir_xdr_to_array(desc, desc->page, inode); > >> + res = nfs_readdir_xdr_to_array(desc, desc->page, > >> + nfsi->cookieverf, verf); > >> if (res < 0) { > >> nfs_readdir_page_unlock_and_put_cached(desc); > >> if (res == -EBADCOOKIE || res == -ENOTSYNC) { > >> @@ -870,6 +874,7 @@ static int find_and_lock_cache_page(struct nfs_readdir_descriptor *desc) > >> } > >> return res; > >> } > >> + memcpy(nfsi->cookieverf, verf, sizeof(nfsi->cookieverf)); > >> } > >> res = nfs_readdir_search_array(desc); > >> if (res == 0) { > >> @@ -902,6 +907,7 @@ static int readdir_search_pagecache(struct nfs_readdir_descriptor *desc) > >> static void nfs_do_filldir(struct nfs_readdir_descriptor *desc) > >> { > >> struct file *file = desc->file; > >> + struct nfs_inode *nfsi = NFS_I(file_inode(file)); > >> struct nfs_cache_array *array; > >> unsigned int i = 0; > >> > >> @@ -915,6 +921,7 @@ static void nfs_do_filldir(struct nfs_readdir_descriptor *desc) > >> desc->eof = true; > >> break; > >> } > >> + memcpy(desc->verf, nfsi->cookieverf, sizeof(desc->verf)); > >> if (i < (array->size-1)) > >> desc->dir_cookie = array->array[i+1].cookie; > >> else > >> @@ -949,8 +956,8 @@ static void nfs_do_filldir(struct nfs_readdir_descriptor *desc) > >> static int uncached_readdir(struct nfs_readdir_descriptor *desc) > >> { > >> struct page *page = NULL; > >> + __be32 verf[NFS_DIR_VERIFIER_SIZE]; > >> int status; > >> - struct inode *inode = file_inode(desc->file); > >> > >> dfprintk(DIRCACHE, "NFS: uncached_readdir() searching for cookie %Lu\n", > >> (unsigned long long)desc->dir_cookie); > >> @@ -967,7 +974,7 @@ static int uncached_readdir(struct nfs_readdir_descriptor *desc) > >> desc->duped = 0; > >> > >> nfs_readdir_page_init_array(page, desc->dir_cookie); > >> - status = nfs_readdir_xdr_to_array(desc, page, inode); > >> + status = nfs_readdir_xdr_to_array(desc, page, desc->verf, verf); > >> if (status < 0) > >> goto out_release; > >> > >> @@ -1023,6 +1030,7 @@ static int nfs_readdir(struct file *file, struct dir_context *ctx) > >> desc->dup_cookie = dir_ctx->dup_cookie; > >> desc->duped = dir_ctx->duped; > >> desc->attr_gencount = dir_ctx->attr_gencount; > >> + memcpy(desc->verf, dir_ctx->verf, sizeof(desc->verf)); > >> spin_unlock(&file->f_lock); > >> > >> do { > >> @@ -1061,6 +1069,7 @@ static int nfs_readdir(struct file *file, struct dir_context *ctx) > >> dir_ctx->dup_cookie = desc->dup_cookie; > >> dir_ctx->duped = desc->duped; > >> dir_ctx->attr_gencount = desc->attr_gencount; > >> + memcpy(dir_ctx->verf, desc->verf, sizeof(dir_ctx->verf)); > >> spin_unlock(&file->f_lock); > >> > >> kfree(desc); > >> @@ -1101,6 +1110,8 @@ static loff_t nfs_llseek_dir(struct file *filp, loff_t offset, int whence) > >> dir_ctx->dir_cookie = offset; > >> else > >> dir_ctx->dir_cookie = 0; > >> + if (offset == 0) > >> + memset(dir_ctx->verf, 0, sizeof(dir_ctx->verf)); > >> dir_ctx->duped = 0; > >> } > >> spin_unlock(&filp->f_lock); > > > > Thanks for doing these patches! > > > > For some reason this patch does not apply but I get a problem at this hunk. > > Is there a fixup or hunk or two missing from 01/17 ? > > I'm starting at 3cea11cd5e3b (Linux 5.10-rc2). > > > > Problem looks like it's at the spin_unlock - here's what the hunk > > looks like for me: > > fs/nfs/dir.c > > 1092 inode_lock(inode); > > 1093 offset += filp->f_pos; > > 1094 if (offset < 0) { > > 1095 inode_unlock(inode); > > 1096 return -EINVAL; > > 1097 } > > 1098 } > > 1099 if (offset != filp->f_pos) { > > 1100 filp->f_pos = offset; > > 1101 if (nfs_readdir_use_cookie(filp)) > > 1102 dir_ctx->dir_cookie = offset; > > 1103 else > > 1104 dir_ctx->dir_cookie = 0; > > 1105 dir_ctx->duped = 0; > > 1106 } > > 1107 inode_unlock(inode); > > 1108 return offset; > > 1109 } > > > > Yes, it depends on the patch "[PATCH 1/2] NFS: Remove unnecessary inode locking in nfs_llseek_dir()” that I sent out to the list on Oct 30. > I can include that in future updates of this patchset. > FWIW, the statement of the dependency is fine with me - I grabbed the other two patches and now applies cleanly. Thanks!
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c index 3b44bef3a1b4..454377228167 100644 --- a/fs/nfs/dir.c +++ b/fs/nfs/dir.c @@ -155,6 +155,7 @@ struct nfs_readdir_descriptor { loff_t current_index; loff_t prev_index; + __be32 verf[NFS_DIR_VERIFIER_SIZE]; unsigned long dir_verifier; unsigned long timestamp; unsigned long gencount; @@ -466,15 +467,15 @@ static int nfs_readdir_search_array(struct nfs_readdir_descriptor *desc) /* Fill a page with xdr information before transferring to the cache page */ static int nfs_readdir_xdr_filler(struct nfs_readdir_descriptor *desc, - u64 cookie, struct page **pages, - size_t bufsize) + __be32 *verf, u64 cookie, + struct page **pages, size_t bufsize, + __be32 *verf_res) { struct inode *inode = file_inode(desc->file); - __be32 verf_res[2]; struct nfs_readdir_arg arg = { .dentry = file_dentry(desc->file), .cred = desc->file->f_cred, - .verf = NFS_I(inode)->cookieverf, + .verf = verf, .cookie = cookie, .pages = pages, .page_len = bufsize, @@ -503,8 +504,6 @@ static int nfs_readdir_xdr_filler(struct nfs_readdir_descriptor *desc, } desc->timestamp = timestamp; desc->gencount = gencount; - memcpy(NFS_I(inode)->cookieverf, res.verf, - sizeof(NFS_I(inode)->cookieverf)); error: return error; } @@ -770,11 +769,13 @@ static struct page **nfs_readdir_alloc_pages(size_t npages) } static int nfs_readdir_xdr_to_array(struct nfs_readdir_descriptor *desc, - struct page *page, struct inode *inode) + struct page *page, __be32 *verf_arg, + __be32 *verf_res) { struct page **pages; struct nfs_entry *entry; size_t array_size; + struct inode *inode = file_inode(desc->file); size_t dtsize = NFS_SERVER(inode)->dtsize; int status = -ENOMEM; @@ -801,8 +802,9 @@ static int nfs_readdir_xdr_to_array(struct nfs_readdir_descriptor *desc, do { unsigned int pglen; - status = nfs_readdir_xdr_filler(desc, entry->cookie, - pages, dtsize); + status = nfs_readdir_xdr_filler(desc, verf_arg, entry->cookie, + pages, dtsize, + verf_res); if (status < 0) break; @@ -854,13 +856,15 @@ static int find_and_lock_cache_page(struct nfs_readdir_descriptor *desc) { struct inode *inode = file_inode(desc->file); struct nfs_inode *nfsi = NFS_I(inode); + __be32 verf[NFS_DIR_VERIFIER_SIZE]; int res; desc->page = nfs_readdir_page_get_cached(desc); if (!desc->page) return -ENOMEM; if (nfs_readdir_page_needs_filling(desc->page)) { - res = nfs_readdir_xdr_to_array(desc, desc->page, inode); + res = nfs_readdir_xdr_to_array(desc, desc->page, + nfsi->cookieverf, verf); if (res < 0) { nfs_readdir_page_unlock_and_put_cached(desc); if (res == -EBADCOOKIE || res == -ENOTSYNC) { @@ -870,6 +874,7 @@ static int find_and_lock_cache_page(struct nfs_readdir_descriptor *desc) } return res; } + memcpy(nfsi->cookieverf, verf, sizeof(nfsi->cookieverf)); } res = nfs_readdir_search_array(desc); if (res == 0) { @@ -902,6 +907,7 @@ static int readdir_search_pagecache(struct nfs_readdir_descriptor *desc) static void nfs_do_filldir(struct nfs_readdir_descriptor *desc) { struct file *file = desc->file; + struct nfs_inode *nfsi = NFS_I(file_inode(file)); struct nfs_cache_array *array; unsigned int i = 0; @@ -915,6 +921,7 @@ static void nfs_do_filldir(struct nfs_readdir_descriptor *desc) desc->eof = true; break; } + memcpy(desc->verf, nfsi->cookieverf, sizeof(desc->verf)); if (i < (array->size-1)) desc->dir_cookie = array->array[i+1].cookie; else @@ -949,8 +956,8 @@ static void nfs_do_filldir(struct nfs_readdir_descriptor *desc) static int uncached_readdir(struct nfs_readdir_descriptor *desc) { struct page *page = NULL; + __be32 verf[NFS_DIR_VERIFIER_SIZE]; int status; - struct inode *inode = file_inode(desc->file); dfprintk(DIRCACHE, "NFS: uncached_readdir() searching for cookie %Lu\n", (unsigned long long)desc->dir_cookie); @@ -967,7 +974,7 @@ static int uncached_readdir(struct nfs_readdir_descriptor *desc) desc->duped = 0; nfs_readdir_page_init_array(page, desc->dir_cookie); - status = nfs_readdir_xdr_to_array(desc, page, inode); + status = nfs_readdir_xdr_to_array(desc, page, desc->verf, verf); if (status < 0) goto out_release; @@ -1023,6 +1030,7 @@ static int nfs_readdir(struct file *file, struct dir_context *ctx) desc->dup_cookie = dir_ctx->dup_cookie; desc->duped = dir_ctx->duped; desc->attr_gencount = dir_ctx->attr_gencount; + memcpy(desc->verf, dir_ctx->verf, sizeof(desc->verf)); spin_unlock(&file->f_lock); do { @@ -1061,6 +1069,7 @@ static int nfs_readdir(struct file *file, struct dir_context *ctx) dir_ctx->dup_cookie = desc->dup_cookie; dir_ctx->duped = desc->duped; dir_ctx->attr_gencount = desc->attr_gencount; + memcpy(dir_ctx->verf, desc->verf, sizeof(dir_ctx->verf)); spin_unlock(&file->f_lock); kfree(desc); @@ -1101,6 +1110,8 @@ static loff_t nfs_llseek_dir(struct file *filp, loff_t offset, int whence) dir_ctx->dir_cookie = offset; else dir_ctx->dir_cookie = 0; + if (offset == 0) + memset(dir_ctx->verf, 0, sizeof(dir_ctx->verf)); dir_ctx->duped = 0; } spin_unlock(&filp->f_lock); diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c index aa6493905bbe..9b765a900b28 100644 --- a/fs/nfs/inode.c +++ b/fs/nfs/inode.c @@ -229,7 +229,6 @@ static void nfs_zap_caches_locked(struct inode *inode) nfsi->attrtimeo = NFS_MINATTRTIMEO(inode); nfsi->attrtimeo_timestamp = jiffies; - memset(NFS_I(inode)->cookieverf, 0, sizeof(NFS_I(inode)->cookieverf)); if (S_ISREG(mode) || S_ISDIR(mode) || S_ISLNK(mode)) { nfs_set_cache_invalid(inode, NFS_INO_INVALID_ATTR | NFS_INO_INVALID_DATA @@ -1237,7 +1236,6 @@ EXPORT_SYMBOL_GPL(nfs_revalidate_inode); static int nfs_invalidate_mapping(struct inode *inode, struct address_space *mapping) { - struct nfs_inode *nfsi = NFS_I(inode); int ret; if (mapping->nrpages != 0) { @@ -1250,11 +1248,6 @@ static int nfs_invalidate_mapping(struct inode *inode, struct address_space *map if (ret < 0) return ret; } - if (S_ISDIR(inode->i_mode)) { - spin_lock(&inode->i_lock); - memset(nfsi->cookieverf, 0, sizeof(nfsi->cookieverf)); - spin_unlock(&inode->i_lock); - } nfs_inc_stats(inode, NFSIOS_DATAINVALIDATE); nfs_fscache_wait_on_invalidate(inode); diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h index dd6b463dda80..681ed98e4ba8 100644 --- a/include/linux/nfs_fs.h +++ b/include/linux/nfs_fs.h @@ -45,6 +45,11 @@ */ #define NFS_RPC_SWAPFLAGS (RPC_TASK_SWAPPER|RPC_TASK_ROOTCREDS) +/* + * Size of the NFS directory verifier + */ +#define NFS_DIR_VERIFIER_SIZE 2 + /* * NFSv3/v4 Access mode cache entry */ @@ -89,6 +94,7 @@ struct nfs_open_context { struct nfs_open_dir_context { struct list_head list; unsigned long attr_gencount; + __be32 verf[NFS_DIR_VERIFIER_SIZE]; __u64 dir_cookie; __u64 dup_cookie; signed char duped; @@ -156,7 +162,7 @@ struct nfs_inode { * This is the cookie verifier used for NFSv3 readdir * operations */ - __be32 cookieverf[2]; + __be32 cookieverf[NFS_DIR_VERIFIER_SIZE]; atomic_long_t nrequests; struct nfs_mds_commit_info commit_info;