On Fri, Dec 13, 2019 at 10:13 PM Bruce Fields bfields@fieldses.org wrote:
On Fri, Dec 13, 2019 at 01:23:08PM -0500, Chuck Lever wrote:
On Dec 13, 2019, at 11:40 AM, Arnd Bergmann arnd@arndb.de wrote: On Fri, Dec 13, 2019 at 5:26 PM Chuck Lever chuck.lever@oracle.com wrote:
/* nfsd4_lease is set to at most one hour */if (WARN_ON_ONCE(nn->nfsd4_lease > 3600))return 360 * HZ;Why is the WARN_ON_ONCE added here? Is it really necessary?
This is to ensure the kernel doesn't change to a larger limit that requires a 64-bit division on a 32-bit architecture.
With the old code, dividing by 10 was always fast as nn->nfsd4_lease was the size of an integer register. Now it is 64 bit wide, and I check that truncating it to 32 bit again is safe.
OK. That comment should state this reason rather than just repeating what the code does. ;-)
Note that __nfsd4_write_time() already limits nfsd4_lease to 3600.
We could just use a smaller type for nfsd4_lease if that'd help.
I think it's generally clearer to have only one type to store the lease time, and time64_t is the most sensible one, even if the range is a bit excessive.
I've seen too many time related bugs from mixing integer types incorrectly.
Arnd