Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Organization: Red Hat UK Ltd. Registered Address: Red Hat UK Ltd, Amberley
        Place, 107-111 Peascod Street, Windsor, Berkshire, SI4 1TE, United
        Kingdom.
        Registered in England and Wales under Company Registration No. 3798903
From:   David Howells <dhowells@redhat.com>
In-Reply-To: <CAEXW_YSd3dyxHxnU1EuER+xyBGGatONzPovphFX5K9seSbkdkg@mail.gmail.com>
References: <CAEXW_YSd3dyxHxnU1EuER+xyBGGatONzPovphFX5K9seSbkdkg@mail.gmail.com> <20221130181316.GA1012431@paulmck-ThinkPad-P17-Gen-1> <20221130181325.1012760-14-paulmck@kernel.org> <CAEXW_YS1nfsV_ohXDaB1i2em=+0KP1DofktS24oGFa4wPAbiiw@mail.gmail.com> <639433.1669835344@warthog.procyon.org.uk>
To:     Joel Fernandes <joel@joelfernandes.org>
Cc:     dhowells@redhat.com, "Paul E. McKenney" <paulmck@kernel.org>,
        rcu@vger.kernel.org, linux-kernel@vger.kernel.org,
        kernel-team@meta.com, rostedt@goodmis.org,
        Marc Dionne <marc.dionne@auristor.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>, linux-afs@lists.infradead.org,
        netdev@vger.kernel.org
Subject: Re: [PATCH rcu 14/16] rxrpc: Use call_rcu_hurry() instead of call_rcu()
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <658623.1669849522.1@warthog.procyon.org.uk>
Content-Transfer-Encoding: quoted-printable
Date:   Wed, 30 Nov 2022 23:05:22 +0000
Message-ID: <658624.1669849522@warthog.procyon.org.uk>
Precedence: bulk

Joel Fernandes <joel@joelfernandes.org> wrote:

> > Note that this conflicts with my patch:
> >
> >         rxrpc: Don't hold a ref for connection workqueue
> >         https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux=
-fs.git/commit/?h=3Drxrpc-next&id=3D450b00011290660127c2d76f5c5ed264126eb2=
29
> >
> > which should render it unnecessary.  It's a little ahead of yours in t=
he
> > net-next queue, if that means anything.
> =

> Could you clarify why it is unnecessary?

Rather than tearing down parts of the connection it only logs a trace line=
,
frees the memory and decrements the counter on the namespace.  This it use=
d to
account that all the pieces of memory allocated in that namespace are gone
before the namespace is removed to check for leaks.  The RCU cleanup used =
to
use some other stuff (such as the peer hash) in the rxrpc_net struct but n=
o
longer will after the patches I submitted.

> After your patch, you are still doing a wake up in your call_rcu() callb=
ack:
>
> - ASSERTCMP(refcount_read(&conn->ref), =3D=3D, 0);
> + if (atomic_dec_and_test(&rxnet->nr_conns))
> +    wake_up_var(&rxnet->nr_conns);
> +}
> =

> Are you saying the code can now tolerate delays? What if the RCU
> callback is invoked after arbitrarily long delays making the sleeping
> process to wait?

True.  But that now only holds up the destruction of a net namespace and t=
he
removal of the rxrpc module.

> If you agree, you can convert the call_rcu() to call_rcu_hurry() in
> your patch itself. Would you be willing to do that? If not, that's
> totally OK and I can send a patch later once yours is in (after
> further testing).

I can add it to part 4 (see my rxrpc-ringless-5 branch) if it is necessary=
.

David