> 在 2023年11月28日，06:26，Olga Kornievskaia <aglo@xxxxxxxxx> 写道： > > On Mon, Nov 27, 2023 at 11:31 AM NeilBrown <neilb@xxxxxxx> wrote: >> >>> 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@xxxxxxx. 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. > > Thanks Neil for pointing it out. We need the "find_active", otherwise > as Trond pointed out it breaks the offline function. But I do believe > I missed the "!" in the logic. I believe the reason this hasn't caused > problems is because for the offline transports we never use the > xprt_iter_xprt(). We only iterate thru the get_next when we iterate > offline transports. But I should fix the function that adds the "!". > return a xprt active state EQUAL find_active maybe “&&” “||” “!”logic not need >> >> 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@xxxxxxxxxxxxxxx >>> >>> >>> >> >>
