Received-SPF: pass (google.com: best guess record for domain of linux-nfs-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Content-Type: text/plain;
        charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.11\))
Subject: Re: [PATCH] SUNRPC/cache: don't allow invalid entries to be flushed
From:   Chuck Lever <chuck.lever@oracle.com>
In-Reply-To: <20200327155309.GA135601@pick.fieldses.org>
Date:   Fri, 27 Mar 2020 12:15:18 -0400
Cc:     Trond Myklebust <trondmy@hammerspace.com>,
        Bruce Fields <bfields@fieldses.org>,
        "kinglongmee@gmail.com" <kinglongmee@gmail.com>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <413957A3-FDF3-4201-8131-7BBAFDDFD88D@oracle.com>
References: <20200114165738.922961-1-trond.myklebust@hammerspace.com>
 <20200206163322.GB2244@fieldses.org>
 <8dc1ed17de98e4b59fb9e408692c152456863a20.camel@hammerspace.com>
 <20200207181817.GC17036@fieldses.org> <20200326204001.GA25053@fieldses.org>
 <1a0ce8bb1150835f7a25126df2524e8a8fb0e112.camel@hammerspace.com>
 <20200327015012.GA107036@pick.fieldses.org>
 <80c83f5543d7d758a165be167d3bf0b2175e57f8.camel@hammerspace.com>
 <20200327155309.GA135601@pick.fieldses.org>
To:     Bruce Fields <bfields@redhat.com>
Sender: linux-nfs-owner@vger.kernel.org
Precedence: bulk


> On Mar 27, 2020, at 11:53 AM, J. Bruce Fields <bfields@redhat.com> =
wrote:
>=20
> From: "J. Bruce Fields" <bfields@redhat.com>
>=20
> Trond points out in 277f27e2f277 that we allow invalid cache entries =
to
> persist indefinitely.  That fix, however, reintroduces the problem =
fixed
> by Kinglong Mee's d6fc8821c2d2 "SUNRPC/Cache: Always treat the invalid
> cache as unexpired", where an invalid cache entry is immediately =
removed
> by a flush before mountd responds to it.  The result is that the =
server
> thread that should be waiting for mountd to fill in that entry instead
> gets an -ETIMEDOUT return from cache_check().  Symptoms are the server
> becoming unresponsive after a restart, reproduceable by running pynfs
> 4.1 test REBT5.
>=20
> Instead, take a compromise approach: allow invalid cache entries to be
> removed after they expire, but not to be removed by a cache flush.
>=20
> Fixes: 277f27e2f277 "SUNRPC/cache: Allow garbage collection..."
> Signed-off-by: J. Bruce Fields <bfields@redhat.com>

Thanks, Bruce. checkpatch.pl is complaining loudly about the style of
the short commit descriptions. I'll fix those up before applying it to
nfsd-5.7.


> ---
> include/linux/sunrpc/cache.h | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>=20
> diff --git a/include/linux/sunrpc/cache.h =
b/include/linux/sunrpc/cache.h
> index 532cdbda43da..10891b70fc7b 100644
> --- a/include/linux/sunrpc/cache.h
> +++ b/include/linux/sunrpc/cache.h
> @@ -209,8 +209,11 @@ static inline void cache_put(struct cache_head =
*h, struct cache_detail *cd)
>=20
> static inline bool cache_is_expired(struct cache_detail *detail, =
struct cache_head *h)
> {
> -	return  (h->expiry_time < seconds_since_boot()) ||
> -		(detail->flush_time >=3D h->last_refresh);
> +	if (h->expiry_time < seconds_since_boot())
> +		return true;
> +	if (!test_bit(CACHE_VALID, &h->flags))
> +		return false;
> +	return detail->flush_time >=3D h->last_refresh;
> }
>=20
> extern int cache_check(struct cache_detail *detail,
> --=20
> 2.25.1
>=20

--
Chuck Lever