Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   "Benjamin Coddington" <bcodding@redhat.com>
To:     "Trond Myklebust" <trondmy@hammerspace.com>
Cc:     trondmy@kernel.org, linux-nfs@vger.kernel.org
Subject: Re: [PATCH v7 04/21] NFS: Calculate page offsets algorithmically
Date:   Fri, 25 Feb 2022 06:28:43 -0500
Message-ID: <80E3965C-3338-4C44-99BA-08FB0B5728A8@redhat.com>
In-Reply-To: <e2ee4448d8deff22d3949b4828d5334a72542701.camel@hammerspace.com>
References: <20220223211305.296816-1-trondmy@kernel.org>
 <20220223211305.296816-2-trondmy@kernel.org>
 <20220223211305.296816-3-trondmy@kernel.org>
 <20220223211305.296816-4-trondmy@kernel.org>
 <20220223211305.296816-5-trondmy@kernel.org>
 <EA90387C-B33D-4C81-BB80-8C0AB3251E5E@redhat.com>
 <e2ee4448d8deff22d3949b4828d5334a72542701.camel@hammerspace.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
Precedence: bulk

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.

Ben