diff mbox series

[net-next,07/17] rxrpc: Don't take spinlocks in the RCU callback functions

Message ID 166919852169.1258552.10370784990641295051.stgit@warthog.procyon.org.uk (mailing list archive)
State RFC
Delegated to: Netdev Maintainers
Headers show
Series rxrpc: Increasing SACK size and moving away from softirq, part 3 | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/apply fail Patch does not apply to net-next

Commit Message

David Howells Nov. 23, 2022, 10:15 a.m. UTC
Don't take spinlocks in the RCU callback functions as these are run in
softirq context - which then requires all other takers to use _bh-marked
locks.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: linux-afs@lists.infradead.org
---

 net/rxrpc/call_object.c |   30 +++++++-----------------------
 net/rxrpc/conn_object.c |   18 +++++++++---------
 2 files changed, 16 insertions(+), 32 deletions(-)

Comments

Marc Dionne Nov. 24, 2022, 6:29 p.m. UTC | #1
On Wed, Nov 23, 2022 at 6:15 AM David Howells <dhowells@redhat.com> wrote:
>
> Don't take spinlocks in the RCU callback functions as these are run in
> softirq context - which then requires all other takers to use _bh-marked
> locks.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Marc Dionne <marc.dionne@auristor.com>
> cc: linux-afs@lists.infradead.org
> ---
>
>  net/rxrpc/call_object.c |   30 +++++++-----------------------
>  net/rxrpc/conn_object.c |   18 +++++++++---------
>  2 files changed, 16 insertions(+), 32 deletions(-)
>
> diff --git a/net/rxrpc/call_object.c b/net/rxrpc/call_object.c
> index 01ffe99516b8..d77b65bf3273 100644
> --- a/net/rxrpc/call_object.c
> +++ b/net/rxrpc/call_object.c
> @@ -613,36 +613,16 @@ void rxrpc_put_call(struct rxrpc_call *call, enum rxrpc_call_trace why)
>  /*
>   * Final call destruction - but must be done in process context.
>   */
> -static void rxrpc_destroy_call(struct work_struct *work)
> +static void rxrpc_destroy_call(struct rcu_head *rcu)
>  {
> -       struct rxrpc_call *call = container_of(work, struct rxrpc_call, processor);
> +       struct rxrpc_call *call = container_of(rcu, struct rxrpc_call, rcu);
>         struct rxrpc_net *rxnet = call->rxnet;
>
> -       rxrpc_delete_call_timer(call);
> -
> -       rxrpc_put_connection(call->conn, rxrpc_conn_put_call);
> -       rxrpc_put_peer(call->peer, rxrpc_peer_put_call);
>         kmem_cache_free(rxrpc_call_jar, call);
>         if (atomic_dec_and_test(&rxnet->nr_calls))
>                 wake_up_var(&rxnet->nr_calls);
>  }
>
> -/*
> - * Final call destruction under RCU.
> - */
> -static void rxrpc_rcu_destroy_call(struct rcu_head *rcu)
> -{
> -       struct rxrpc_call *call = container_of(rcu, struct rxrpc_call, rcu);
> -
> -       if (rcu_read_lock_held()) {
> -               INIT_WORK(&call->processor, rxrpc_destroy_call);
> -               if (!rxrpc_queue_work(&call->processor))
> -                       BUG();
> -       } else {
> -               rxrpc_destroy_call(&call->processor);
> -       }
> -}
> -
>  /*
>   * clean up a call
>   */
> @@ -663,8 +643,12 @@ void rxrpc_cleanup_call(struct rxrpc_call *call)
>         }
>         rxrpc_put_txbuf(call->tx_pending, rxrpc_txbuf_put_cleaned);
>         rxrpc_free_skb(call->acks_soft_tbl, rxrpc_skb_put_ack);
> +       rxrpc_delete_call_timer(call);

This is not safe to call directly here, since rxrpc_cleanup_call can
be called from the timer function, so the del_timer_sync may wait
forever.

> +
> +       rxrpc_put_connection(call->conn, rxrpc_conn_put_call);
> +       rxrpc_put_peer(call->peer, rxrpc_peer_put_call);
>
> -       call_rcu(&call->rcu, rxrpc_rcu_destroy_call);
> +       call_rcu(&call->rcu, rxrpc_destroy_call);
>  }
>
>  /*
> diff --git a/net/rxrpc/conn_object.c b/net/rxrpc/conn_object.c
> index f7c271a740ed..54821c2f6d89 100644
> --- a/net/rxrpc/conn_object.c
> +++ b/net/rxrpc/conn_object.c
> @@ -249,6 +249,15 @@ void rxrpc_kill_connection(struct rxrpc_connection *conn)
>          */
>         rxrpc_purge_queue(&conn->rx_queue);
>
> +       del_timer_sync(&conn->timer);
> +       rxrpc_purge_queue(&conn->rx_queue);
> +
> +       conn->security->clear(conn);
> +       key_put(conn->key);
> +       rxrpc_put_bundle(conn->bundle, rxrpc_bundle_put_conn);
> +       rxrpc_put_peer(conn->peer, rxrpc_peer_put_conn);
> +       rxrpc_put_local(conn->local, rxrpc_local_put_kill_conn);
> +
>         /* Leave final destruction to RCU.  The connection processor work item
>          * must carry a ref on the connection to prevent us getting here whilst
>          * it is queued or running.
> @@ -358,17 +367,8 @@ static void rxrpc_destroy_connection(struct rcu_head *rcu)
>
>         ASSERTCMP(refcount_read(&conn->ref), ==, 0);
>
> -       del_timer_sync(&conn->timer);
> -       rxrpc_purge_queue(&conn->rx_queue);
> -
> -       conn->security->clear(conn);
> -       key_put(conn->key);
> -       rxrpc_put_bundle(conn->bundle, rxrpc_bundle_put_conn);
> -       rxrpc_put_peer(conn->peer, rxrpc_peer_put_conn);
> -
>         if (atomic_dec_and_test(&conn->local->rxnet->nr_conns))
>                 wake_up_var(&conn->local->rxnet->nr_conns);
> -       rxrpc_put_local(conn->local, rxrpc_local_put_kill_conn);
>
>         kfree(conn);
>         _leave("");
diff mbox series

Patch

diff --git a/net/rxrpc/call_object.c b/net/rxrpc/call_object.c
index 01ffe99516b8..d77b65bf3273 100644
--- a/net/rxrpc/call_object.c
+++ b/net/rxrpc/call_object.c
@@ -613,36 +613,16 @@  void rxrpc_put_call(struct rxrpc_call *call, enum rxrpc_call_trace why)
 /*
  * Final call destruction - but must be done in process context.
  */
-static void rxrpc_destroy_call(struct work_struct *work)
+static void rxrpc_destroy_call(struct rcu_head *rcu)
 {
-	struct rxrpc_call *call = container_of(work, struct rxrpc_call, processor);
+	struct rxrpc_call *call = container_of(rcu, struct rxrpc_call, rcu);
 	struct rxrpc_net *rxnet = call->rxnet;
 
-	rxrpc_delete_call_timer(call);
-
-	rxrpc_put_connection(call->conn, rxrpc_conn_put_call);
-	rxrpc_put_peer(call->peer, rxrpc_peer_put_call);
 	kmem_cache_free(rxrpc_call_jar, call);
 	if (atomic_dec_and_test(&rxnet->nr_calls))
 		wake_up_var(&rxnet->nr_calls);
 }
 
-/*
- * Final call destruction under RCU.
- */
-static void rxrpc_rcu_destroy_call(struct rcu_head *rcu)
-{
-	struct rxrpc_call *call = container_of(rcu, struct rxrpc_call, rcu);
-
-	if (rcu_read_lock_held()) {
-		INIT_WORK(&call->processor, rxrpc_destroy_call);
-		if (!rxrpc_queue_work(&call->processor))
-			BUG();
-	} else {
-		rxrpc_destroy_call(&call->processor);
-	}
-}
-
 /*
  * clean up a call
  */
@@ -663,8 +643,12 @@  void rxrpc_cleanup_call(struct rxrpc_call *call)
 	}
 	rxrpc_put_txbuf(call->tx_pending, rxrpc_txbuf_put_cleaned);
 	rxrpc_free_skb(call->acks_soft_tbl, rxrpc_skb_put_ack);
+	rxrpc_delete_call_timer(call);
+
+	rxrpc_put_connection(call->conn, rxrpc_conn_put_call);
+	rxrpc_put_peer(call->peer, rxrpc_peer_put_call);
 
-	call_rcu(&call->rcu, rxrpc_rcu_destroy_call);
+	call_rcu(&call->rcu, rxrpc_destroy_call);
 }
 
 /*
diff --git a/net/rxrpc/conn_object.c b/net/rxrpc/conn_object.c
index f7c271a740ed..54821c2f6d89 100644
--- a/net/rxrpc/conn_object.c
+++ b/net/rxrpc/conn_object.c
@@ -249,6 +249,15 @@  void rxrpc_kill_connection(struct rxrpc_connection *conn)
 	 */
 	rxrpc_purge_queue(&conn->rx_queue);
 
+	del_timer_sync(&conn->timer);
+	rxrpc_purge_queue(&conn->rx_queue);
+
+	conn->security->clear(conn);
+	key_put(conn->key);
+	rxrpc_put_bundle(conn->bundle, rxrpc_bundle_put_conn);
+	rxrpc_put_peer(conn->peer, rxrpc_peer_put_conn);
+	rxrpc_put_local(conn->local, rxrpc_local_put_kill_conn);
+
 	/* Leave final destruction to RCU.  The connection processor work item
 	 * must carry a ref on the connection to prevent us getting here whilst
 	 * it is queued or running.
@@ -358,17 +367,8 @@  static void rxrpc_destroy_connection(struct rcu_head *rcu)
 
 	ASSERTCMP(refcount_read(&conn->ref), ==, 0);
 
-	del_timer_sync(&conn->timer);
-	rxrpc_purge_queue(&conn->rx_queue);
-
-	conn->security->clear(conn);
-	key_put(conn->key);
-	rxrpc_put_bundle(conn->bundle, rxrpc_bundle_put_conn);
-	rxrpc_put_peer(conn->peer, rxrpc_peer_put_conn);
-
 	if (atomic_dec_and_test(&conn->local->rxnet->nr_conns))
 		wake_up_var(&conn->local->rxnet->nr_conns);
-	rxrpc_put_local(conn->local, rxrpc_local_put_kill_conn);
 
 	kfree(conn);
 	_leave("");