Received-SPF: pass (google.com: domain of linux-nfs+bounces-1285-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
From: Benjamin Coddington <bcodding@redhat.com>
To: samasth.norway.ananda@oracle.com
Cc: Chuck Lever <chuck.lever@oracle.com>, linux-nfs@vger.kernel.org
Subject: Re: [External] : Re: [PATCH 1/1] NFSv4.1: Assign retries to
 timeout.to_retries instead of timeout.to_initval
Date: Mon, 22 Jan 2024 19:47:01 -0500
Message-ID: <3AFA6113-0445-4EC0-83DC-96396C8FDAD7@redhat.com>
In-Reply-To: <324d4e4e-4685-4341-a7b7-c73de473c57e@oracle.com>
References: <20240122172353.2859254-1-samasth.norway.ananda@oracle.com>
 <995F0BB9-C709-4C3A-92F3-5EB9710A47F5@redhat.com>
 <bdd760d9-9900-43ef-8000-0e9ce0aa8b3e@oracle.com>
 <8B835117-498D-484E-B1DE-912FBBBCC74E@redhat.com>
 <Za7x2aYNRqc3nIbl@tissot.1015granger.net>
 <8DCCEB9D-5504-4DD4-A042-61C99D22A462@redhat.com>
 <324d4e4e-4685-4341-a7b7-c73de473c57e@oracle.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

On 22 Jan 2024, at 18:07, samasth.norway.ananda@oracle.com wrote:

> On 1/22/24 3:03 PM, Benjamin Coddington wrote:
>> On 22 Jan 2024, at 17:53, Chuck Lever wrote:
>>
>>> On Mon, Jan 22, 2024 at 05:46:56PM -0500, Benjamin Coddington wrote:
>>>> On 22 Jan 2024, at 17:44, samasth.norway.ananda@oracle.com wrote:
>>>>
>>>>> On 1/22/24 2:41 PM, Benjamin Coddington wrote:
>>>>>> On 22 Jan 2024, at 12:23, Samasth Norway Ananda wrote:
>>>>>>
>>>>>>> In the else block we are assigning the req->rq_xprt->timeout->to_=
retries
>>>>>>> value to timeout.to_initval, whereas it should have been assigned=
 to
>>>>>>> timeout.to_retries instead.
>>>>>>>
>>>>>>> Fixes: 57331a59ac0d (=E2=80=9CNFSv4.1: Use the nfs_client's rpc t=
imeouts for backchannel")
>>>>>>> Signed-off-by: Samasth Norway Ananda <samasth.norway.ananda@oracl=
e.com>
>>>>>>> ---
>>>>>>> Hi,
>>>>>>>
>>>>>>> I came across the patch 57331a59ac0d (=E2=80=9CNFSv4.1: Use the n=
fs_client's rpc
>>>>>>> timeouts for backchannel") which assigns value to same variable i=
n the
>>>>>>> else block.  Can I please get your input on the patch?
>>>>>>
>>>>>> Oh yes, this a good fix.  Usually the maintainers won't pick up a =
patch
>>>>>> that's only sent to the list, rather the patch should be addressed=
 to them
>>>>>> directly and copied to the list.  Can you re-send this patch to:
>>>>>>
>>>>>> Trond Myklebust <trond.myklebust@hammerspace.com>,Anna Schumaker <=
anna@kernel.org>
>>>>>>
>>>>>> and copy linux-nfs@vger.kernel.org?  You can also add my:
>>>>>>
>>>>>> Reviewed-by: Benjamin Coddington <bcodding@redhat.com>
>>>>>
>>>>> Sure, I will do that. Thanks for the review.
>>>>
>>>> Can you also fixup the block above the hunk you posted?  Its backwar=
ds there too!
>>>
>>> It's backwards in a different way.
>>
>> Its an artifact of my text editing..
>>
>>> And should you set to_maxval in both places as well?
>>
>> IIRC it does not need to be set there.
>
> Ah okay. So it should be like this right?

Yes!  Good catch.  Thank you!

Ben