linux-kernel - Re: [PATCH rcu 14/16] rxrpc: Use call_rcu_hurry() instead of call

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <658624.1669849522@warthog.procyon.org.uk>
Date:   Wed, 30 Nov 2022 23:05:22 +0000
From:   David Howells <dhowells@...hat.com>
To:     Joel Fernandes <joel@...lfernandes.org>
Cc:     dhowells@...hat.com, "Paul E. McKenney" <paulmck@...nel.org>,
        rcu@...r.kernel.org, linux-kernel@...r.kernel.org,
        kernel-team@...a.com, rostedt@...dmis.org,
        Marc Dionne <marc.dionne@...istor.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, linux-afs@...ts.infradead.org,
        netdev@...r.kernel.org
Subject: Re: [PATCH rcu 14/16] rxrpc: Use call_rcu_hurry() instead of call_rcu()

Joel Fernandes <joel@...lfernandes.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