Received-SPF: pass (google.com: domain of linux-nfs+bounces-109-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
From: "NeilBrown" <neilb@suse.de>
To: "Trond Myklebust" <trondmy@hammerspace.com>
Cc: "jlayton@kernel.org" <jlayton@kernel.org>,
 "thfeathers@sina.cn" <thfeathers@sina.cn>,
 "chuck.lever@oracle.com" <chuck.lever@oracle.com>,
 "tom@talpey.com" <tom@talpey.com>, "Dai.Ngo@oracle.com" <Dai.Ngo@oracle.com>,
 "kolga@netapp.com" <kolga@netapp.com>,
 "linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH] SUNRPC: _xprt_switch_find_current_entry return xprt with
 condition find_active
In-reply-to: <aa9e250a966c47782f79d258ea9818ae4fcbdbc5.camel@hammerspace.com>
References: <20231127153959.2067-1-thfeathers@sina.cn>,
 <aa9e250a966c47782f79d258ea9818ae4fcbdbc5.camel@hammerspace.com>
Date: Tue, 28 Nov 2023 08:31:22 +1100
Message-id: <170112068218.7109.1172633879607916557@noble.neil.brown.name>

On Tue, 28 Nov 2023, Trond Myklebust wrote:
> On Mon, 2023-11-27 at 23:39 +0800, jsq wrote:
> > [You don't often get email from thfeathers@sina.cn. Learn why this is
> > important at https://aka.ms/LearnAboutSenderIdentification ]
> > 
> > current function always return a active xprt or NULL no matter what
> > find_active
> 
> 
> This patch clearly breaks xprt_switch_find_current_entry_offline().

I think it actually fixes xprt_switch_find_current_entry_offline().

Looking closely at _xprt_switch_find_current_entry:

		if (found && ((find_active && xprt_is_active(pos)) ||
			      (!find_active && xprt_is_active(pos))))

and comparing with similar code in xprt_switch_find_next_entry:

		if (found && ((check_active && xprt_is_active(pos)) ||
			      (!check_active && !xprt_is_active(pos))))

There is a difference in the number of '!'.  I suspect the former is
wrong.
If the former is correct, then "find_active" is irrelevant.

NeilBrown

> Furthermore, we do not accept patches without a real name on a Signed-
> off-by: line.
> 
> So NACK on two accounts.
> 
> -- 
> Trond Myklebust
> Linux NFS client maintainer, Hammerspace
> trond.myklebust@hammerspace.com
> 
> 
>