From: Trond Myklebust <[email protected]>
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 <[email protected]>
---
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);
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;
--
2.28.0
On Wed, Nov 4, 2020 at 11:28 AM <[email protected]> wrote:
>
> From: Trond Myklebust <[email protected]>
>
> 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 <[email protected]>
> ---
> 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 <[email protected]> wrote:
>
> On Wed, Nov 4, 2020 at 11:28 AM <[email protected]> wrote:
>>
>> From: Trond Myklebust <[email protected]>
>>
>> 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 <[email protected]>
>> ---
>> 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.
_________________________________
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]
On Wed, Nov 4, 2020 at 4:32 PM Trond Myklebust <[email protected]> wrote:
>
>
>
> > On Nov 4, 2020, at 16:01, David Wysochanski <[email protected]> wrote:
> >
> > On Wed, Nov 4, 2020 at 11:28 AM <[email protected]> wrote:
> >>
> >> From: Trond Myklebust <[email protected]>
> >>
> >> 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 <[email protected]>
> >> ---
> >> 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!