Re: [PATCH rcu 14/16] rxrpc: Use call_rcu_hurry() instead of call_rcu()

[David Howells <dhowells@redhat.com>]

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=rxrpc-next&id=450b00011290660127c2d76f5c5ed264126eb229
> >
> > which should render it unnecessary.  It's a little ahead of yours in the
> > 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 used 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 no
longer will after the patches I submitted.

> After your patch, you are still doing a wake up in your call_rcu() callback:
>
> - ASSERTCMP(refcount_read(&conn->ref), ==, 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 the
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