Received: by 2002:a05:6a10:16a7:0:0:0:0 with SMTP id gp39csp1186661pxb; Wed, 4 Nov 2020 02:13:33 -0800 (PST) X-Google-Smtp-Source: ABdhPJy4AIN3uK2Bvn3zCS1hC2U7Kel5QpKOR+LQX26kpRJpMT1TrHeMzkhRiWFw3jLyUzoqg2nQ X-Received: by 2002:a50:bb25:: with SMTP id y34mr25490406ede.249.1604484813079; Wed, 04 Nov 2020 02:13:33 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1604484813; cv=none; d=google.com; s=arc-20160816; b=FtLDNqbFP3oC3vrtMVHCc7c+LKD4UI8CDNnuAku1dw7MCZQf3xpMO3f67aRDTKVEbn 7oBPwBnp8gnc8dRtIe0attEZvh4KNMeCfgI2M6GT96YAuEfKNO8J8hgKcjgKLC6ntdMk 7a9arEzyQT4n5RdrMCGDF/B1zchTocN3+myB6+4MhBRl5QaOtm3EgCi67x+NFuMqUeDa wr6oVN/Tv7jhOJg3nrSwUQD7zv33EBOcE13iPqaAjJTTO3ljxNAJaGWNz1PaNo/OQfMG LldmJji7RGIK3HrFwUW8yQ5tZMuPvmbLUOITR2E9UcruRgzMt3IOEqOepwWPiBG3o+mN iWIA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:thread-index:thread-topic :content-transfer-encoding:mime-version:subject:references :in-reply-to:message-id:cc:to:from:date:dkim-signature:dkim-filter; bh=80L72VLbhQkYn+bohZhheH5RpGTsGN+3DGhaCsShsdw=; b=Icu0DvVBBFulPCUMp7tChqDcBtH0q+sIG2ThTKeGisKcUzfhV6IBj2acf98f2S7NLI iSpjy2AezbP/m5lOCmbJP0H3dIIAbVwkhRyvWdRdkyEHp/OjZqbj/nRo29S3dantVi9d VqQ2nbSJCfPc220JCAj7jLt+MKPHStx6sr0KMCQe12P005ZIqsoCTZxBwX7+EUz9W+Le NppnClPx56wq2Eiw0nvEZ3RP2iVYunEzGPBul62vpq8YfN9noqCbHVmU2TmLM1Gynb4l VXVmdNAWMzjmDrfkpoe9ReIArbVbs0Gd6EFbINRIDJUpIm9MnOD6DeaPOFkPGKUecKht N8HQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@desy.de header.s=default header.b=j0Zvl+Hj; spf=pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-nfs-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id p13si1048377ejx.702.2020.11.04.02.12.58; Wed, 04 Nov 2020 02:13:33 -0800 (PST) Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@desy.de header.s=default header.b=j0Zvl+Hj; spf=pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-nfs-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727923AbgKDKMz (ORCPT + 99 others); Wed, 4 Nov 2020 05:12:55 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33180 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727389AbgKDKMy (ORCPT ); Wed, 4 Nov 2020 05:12:54 -0500 X-Greylist: delayed 153246 seconds by postgrey-1.37 at lindbergh.monkeyblade.net; Wed, 04 Nov 2020 02:12:53 PST Received: from smtp-o-1.desy.de (smtp-o-1.desy.de [IPv6:2001:638:700:1038::1:9a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E9580C0613D3 for ; Wed, 4 Nov 2020 02:12:53 -0800 (PST) Received: from smtp-buf-1.desy.de (smtp-buf-1.desy.de [131.169.56.164]) by smtp-o-1.desy.de (Postfix) with ESMTP id 8A07EE04DE for ; Wed, 4 Nov 2020 11:12:50 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp-o-1.desy.de 8A07EE04DE DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=desy.de; s=default; t=1604484770; bh=80L72VLbhQkYn+bohZhheH5RpGTsGN+3DGhaCsShsdw=; h=Date:From:To:Cc:In-Reply-To:References:Subject:From; b=j0Zvl+Hj7QZIXvzh3dtij5v18X3pOVpt6a4+jIdw4DBT0sXKoZDotDTD6qRPYoiwI LwBS5f5JW5Pho8Dgg0ezU4CqaXf+a+kDXCpnFH/KlaPjw2oDwO3F7dDrpb7e+shSIq tV01osqqbEHasPYd2+6Vgg77+8zQ0QW1Q5I80DoY= Received: from smtp-m-1.desy.de (smtp-m-1.desy.de [IPv6:2001:638:700:1038::1:81]) by smtp-buf-1.desy.de (Postfix) with ESMTP id 8621F120906; Wed, 4 Nov 2020 11:12:50 +0100 (CET) X-Virus-Scanned: amavisd-new at desy.de Received: from z-mbx-2.desy.de (z-mbx-2.desy.de [131.169.55.140]) by smtp-intra-1.desy.de (Postfix) with ESMTP id 5F84FC0177; Wed, 4 Nov 2020 11:12:50 +0100 (CET) Date: Wed, 4 Nov 2020 11:12:49 +0100 (CET) From: "Mkrtchyan, Tigran" To: trondmy@kernel.org Cc: linux-nfs Message-ID: <1868756897.5941283.1604484769947.JavaMail.zimbra@desy.de> In-Reply-To: <20201103153329.531942-17-trondmy@kernel.org> References: <20201103153329.531942-1-trondmy@kernel.org> <20201103153329.531942-11-trondmy@kernel.org> <20201103153329.531942-12-trondmy@kernel.org> <20201103153329.531942-13-trondmy@kernel.org> <20201103153329.531942-14-trondmy@kernel.org> <20201103153329.531942-15-trondmy@kernel.org> <20201103153329.531942-16-trondmy@kernel.org> <20201103153329.531942-17-trondmy@kernel.org> Subject: Re: [PATCH v2 16/16] NFS: Improve handling of directory verifiers MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Mailer: Zimbra 8.8.15_GA_3959 (ZimbraWebClient - FF82 (Linux)/8.8.15_GA_3953) Thread-Topic: Improve handling of directory verifiers Thread-Index: dueZGGPnoh+hPttb4hAaTQh+G+LniA== Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org ----- Original Message ----- > From: trondmy@kernel.org > To: "linux-nfs" > Sent: Tuesday, 3 November, 2020 16:33:29 > Subject: [PATCH v2 16/16] NFS: Improve handling of directory verifiers > From: Trond Myklebust > > 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 > --- > 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 1c5a5f9cb228..0bd9cc625bdb 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; > @@ -467,15 +468,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, > @@ -504,8 +505,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; > } > @@ -771,11 +770,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; > > @@ -802,8 +803,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; > > @@ -855,13 +857,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) { > @@ -871,6 +875,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) { > @@ -903,6 +908,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; > > @@ -916,6 +922,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 > @@ -950,8 +957,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); > @@ -968,7 +975,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; > > @@ -1024,6 +1031,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 { > @@ -1062,6 +1070,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); > @@ -1102,6 +1111,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]; Just for my education. Why we use 2x32 bit BE encoded ints instead of raw 8 bytes? And if it's treaded as a number, as spec sometimes does ("The request's cookieverf field should be set to 0"), then why it's not then __be64? Tigran. > > atomic_long_t nrequests; > struct nfs_mds_commit_info commit_info; > -- > 2.28.0