lkml.org 
[lkml]   [2019]   [Dec]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v2 10/12] nfsd: use boottime for lease expiry alculation
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

\
 
 \ /
  Last update: 2019-12-13 22:20    [W:0.038 / U:0.716 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site