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;
Subject: Re: [PATCH nfs-utils 1/2] Remove 'force' arg from cache_flush()
To:     NeilBrown <neilb@suse.de>
Cc:     Petr Vorel <pvorel@suse.cz>,
        "J . Bruce Fields" <bfields@fieldses.org>,
        linux-nfs@vger.kernel.org, Chuck Lever <chuck.lever@oracle.com>,
        Alexey Kodanev <alexey.kodanev@oracle.com>
References: <20210422191803.31511-1-pvorel@suse.cz>
 <20210422202334.GB25415@fieldses.org> <YILQip3nAxhpXP9+@pevik>
 <162035212343.24322.12361160756597283121@noble.neil.brown.name>
 <162122673178.19062.96081788305923933@noble.neil.brown.name>
 <289c5819-917a-39a7-9aa4-2a27ae7248c0@RedHat.com>
 <162156113063.19062.9406037279407040033@noble.neil.brown.name>
 <162156122215.19062.11710239266795260824@noble.neil.brown.name>
From:   Steve Dickson <SteveD@RedHat.com>
Message-ID: <9a370e4b-6cdc-66a4-6a08-715fc4e4983a@RedHat.com>
Date:   Sun, 23 May 2021 14:30:17 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.10.1
MIME-Version: 1.0
In-Reply-To: <162156122215.19062.11710239266795260824@noble.neil.brown.name>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 5/20/21 9:40 PM, NeilBrown wrote:
> 
> Since v4.17 the timestamp written to 'flush' is ignored,
> so there isn't much point choosing too precisely.
> 
> For kernels since v4.3-rc3-13-g778620364ef5 it is safe
> to write 1 second beyond the current time.
> 
> For earlier kernels, nothing is really safe (even the current
> behaviour), but writing one second beyond the current time isn't too bad
> in the unlikely case the people use a new nfs-utils on a 5 year old
> kernel.
> 
> This remove a dependency for libnfs.a on 'etab' being declare,
> so svcgssd no longer needs to declare it.
> 
> Signed-off-by: NeilBrown <neilb@suse.de>
Committed... (tag: nfs-utils-2-5-4-rc4)

steved.
> ---
>  support/export/auth.c     |  2 +-
>  support/include/nfslib.h  |  2 +-
>  support/nfs/cacheio.c     | 17 ++++++++---------
>  utils/exportfs/exportfs.c |  4 ++--
>  utils/gssd/svcgssd.c      |  1 -
>  5 files changed, 12 insertions(+), 14 deletions(-)
> 
> diff --git a/support/export/auth.c b/support/export/auth.c
> index cea376300d01..17bdfc83748e 100644
> --- a/support/export/auth.c
> +++ b/support/export/auth.c
> @@ -80,7 +80,7 @@ check_useipaddr(void)
>  		use_ipaddr = 0;
>  
>  	if (use_ipaddr != old_use_ipaddr)
> -		cache_flush(1);
> +		cache_flush();
>  }
>  
>  unsigned int
> diff --git a/support/include/nfslib.h b/support/include/nfslib.h
> index 84d8270b330f..58eeb3382fcc 100644
> --- a/support/include/nfslib.h
> +++ b/support/include/nfslib.h
> @@ -132,7 +132,7 @@ int			wildmat(char *text, char *pattern);
>  
>  int qword_get(char **bpp, char *dest, int bufsize);
>  int qword_get_int(char **bpp, int *anint);
> -void cache_flush(int force);
> +void cache_flush(void);
>  void qword_add(char **bpp, int *lp, char *str);
>  void qword_addhex(char **bpp, int *lp, char *buf, int blen);
>  void qword_addint(char **bpp, int *lp, int n);
> diff --git a/support/nfs/cacheio.c b/support/nfs/cacheio.c
> index 70ead94d64f0..73f4be4af9f9 100644
> --- a/support/nfs/cacheio.c
> +++ b/support/nfs/cacheio.c
> @@ -32,8 +32,6 @@
>  #include <time.h>
>  #include <errno.h>
>  
> -extern struct state_paths etab;
> -
>  void qword_add(char **bpp, int *lp, char *str)
>  {
>  	char *bp = *bpp;
> @@ -213,7 +211,7 @@ int qword_get_uint(char **bpp, unsigned int *anint)
>   */
>  
>  void
> -cache_flush(int force)
> +cache_flush(void)
>  {
>  	struct stat stb;
>  	int c;
> @@ -234,12 +232,13 @@ cache_flush(int force)
>  		NULL
>  	};
>  	now = time(0);
> -	if (force ||
> -	    stat(etab.statefn, &stb) != 0 ||
> -	    stb.st_mtime > now)
> -		stb.st_mtime = time(0);
> -	
> -	sprintf(stime, "%" PRId64 "\n", (int64_t)stb.st_mtime);
> +
> +	/* Since v4.16-rc2-3-g3b68e6ee3cbd the timestamp written is ignored.
> +	 * It is safest always to flush caches if there is any doubt.
> +	 * For earlier kernels, writing the next second from now is
> +	 * the best we can do.
> +	 */
> +	sprintf(stime, "%" PRId64 "\n", (int64_t)now+1);
>  	for (c=0; cachelist[c]; c++) {
>  		int fd;
>  		sprintf(path, "/proc/net/rpc/%s/flush", cachelist[c]);
> diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
> index bc76aaaf8714..d586296796a9 100644
> --- a/utils/exportfs/exportfs.c
> +++ b/utils/exportfs/exportfs.c
> @@ -188,7 +188,7 @@ main(int argc, char **argv)
>  
>  	if (optind == argc && ! f_all) {
>  		if (force_flush) {
> -			cache_flush(1);
> +			cache_flush();
>  			free_state_path_names(&etab);
>  			return 0;
>  		} else {
> @@ -235,7 +235,7 @@ main(int argc, char **argv)
>  				unexportfs(argv[i], f_verbose);
>  	}
>  	xtab_export_write();
> -	cache_flush(force_flush);
> +	cache_flush();
>  	free_state_path_names(&etab);
>  	export_freeall();
>  
> diff --git a/utils/gssd/svcgssd.c b/utils/gssd/svcgssd.c
> index 3ab2100b66bb..881207b3e8a2 100644
> --- a/utils/gssd/svcgssd.c
> +++ b/utils/gssd/svcgssd.c
> @@ -67,7 +67,6 @@
>  #include "misc.h"
>  #include "svcgssd_krb5.h"
>  
> -struct state_paths etab; /* from cacheio.c */
>  static bool signal_received = false;
>  static struct event_base *evbase = NULL;
>  static int nullrpc_fd = -1;
>