Re: [PATCH v7 04/21] NFS: Calculate page offsets algorithmically

[Trond Myklebust <trondmy@hammerspace.com>]

On Fri, 2022-02-25 at 06:28 -0500, Benjamin Coddington wrote:
> On 24 Feb 2022, at 21:11, Trond Myklebust wrote:
> 
> > On Thu, 2022-02-24 at 09:15 -0500, Benjamin Coddington wrote:
> > > On 23 Feb 2022, at 16:12, trondmy@kernel.org wrote:
> > > 
> > > > From: Trond Myklebust <trond.myklebust@hammerspace.com>
> > > > 
> > > > Instead of relying on counting the page offsets as we walk
> > > > through
> > > > the
> > > > page cache, switch to calculating them algorithmically.
> > > > 
> > > > Signed-off-by: Trond Myklebust
> > > > <trond.myklebust@hammerspace.com>
> > > > ---
> > > >  fs/nfs/dir.c | 18 +++++++++++++-----
> > > >  1 file changed, 13 insertions(+), 5 deletions(-)
> > > > 
> > > > diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> > > > index 8f17aaebcd77..f2258e926df2 100644
> > > > --- a/fs/nfs/dir.c
> > > > +++ b/fs/nfs/dir.c
> > > > @@ -248,17 +248,20 @@ static const char
> > > > *nfs_readdir_copy_name(const
> > > > char *name, unsigned int len)
> > > >         return ret;
> > > >  }
> > > > 
> > > > +static size_t nfs_readdir_array_maxentries(void)
> > > > +{
> > > > +       return (PAGE_SIZE - sizeof(struct nfs_cache_array)) /
> > > > +              sizeof(struct nfs_cache_array_entry);
> > > > +}
> > > > +
> > > 
> > > Why the choice to use a runtime function call rather than the
> > > compiler's
> > > calculation?  I suspect that the end result is the same, as the
> > > compiler
> > > will optimize it away, but I'm curious if there's a good reason
> > > for
> > > this.
> > > 
> > 
> > The comparison is more efficient because no pointer arithmetic is
> > needed. As you said, the above function always evaluates to a
> > constant,
> > and the array->size has been pre-calculated.
> 
> Comparisons are more efficient than using something like this?:
> 
> static const int nfs_readdir_array_maxentries =
>         (PAGE_SIZE - sizeof(struct nfs_cache_array)) /
>         sizeof(struct nfs_cache_array_entry);
> 
> I don't understand why, I must admit.   I'm not saying it should be
> changed,
> I'm just trying to figure out the reason for the function declaration
> when
> the value is a constant, and I thought there was a hole in my head.
> 

Unless we're talking about a compiler from the 1960s, there is little
difference between the two proposals. Any modern C compiler worth its
salt will know to inline the numeric value into the comparison (and
will optimise away the storage of your variable).

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com