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@xxxxxxxxxx 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 (“NFSv4.1: Use the nfs_client's rpc timeouts for backchannel")
Signed-off-by: Samasth Norway Ananda <samasth.norway.ananda@xxxxxxxxxx>
---
Hi,
I came across the patch 57331a59ac0d (“NFSv4.1: Use the nfs_client's rpc
timeouts for backchannel") which assigns value to same variable in 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@xxxxxxxxxxxxxxx>,Anna Schumaker <anna@xxxxxxxxxx>
and copy linux-nfs@xxxxxxxxxxxxxxx? You can also add my:
Reviewed-by: Benjamin Coddington <bcodding@xxxxxxxxxx>
Sure, I will do that. Thanks for the review.
Can you also fixup the block above the hunk you posted? Its backwards 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?
if (rqstp->bc_to_initval > 0) {
timeout.to_initval = rqstp->bc_to_initval;
timeout.to_retries = rqstp->bc_to_retries;
}
else {
timeout.to_initval = req->rq_xprt->timeout->to_initval;
timeout.to_retries = req->rq_xprt->timeout->to_retries;
}
Thanks,
Samasth.
Ben