On Fri, Dec 13, 2019 at 10:13 PM Bruce Fields <bfie...@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 <a...@arndb.de> wrote:
> > > On Fri, Dec 13, 2019 at 5:26 PM Chuck Lever <chuck.le...@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
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038
