From: Eiichi Tsukata <[email protected]>
Need to call rxrpc_put_peer() for bundle candidate before kfree() as it
holds a ref to rxrpc_peer.
[DH: v2: Changed to abstract out the bundle freeing code into a function]
Fixes: 245500d853e9 ("rxrpc: Rewrite the client connection manager")
Signed-off-by: Eiichi Tsukata <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: Marc Dionne <[email protected]>
cc: [email protected]
Link: https://lore.kernel.org/r/[email protected]/ # v1
---
net/rxrpc/conn_client.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/net/rxrpc/conn_client.c b/net/rxrpc/conn_client.c
index dbea0bfee48e..8120138dac01 100644
--- a/net/rxrpc/conn_client.c
+++ b/net/rxrpc/conn_client.c
@@ -135,16 +135,20 @@ struct rxrpc_bundle *rxrpc_get_bundle(struct rxrpc_bundle *bundle)
return bundle;
}
+static void rxrpc_free_bundle(struct rxrpc_bundle *bundle)
+{
+ rxrpc_put_peer(bundle->params.peer);
+ kfree(bundle);
+}
+
void rxrpc_put_bundle(struct rxrpc_bundle *bundle)
{
unsigned int d = bundle->debug_id;
unsigned int u = atomic_dec_return(&bundle->usage);
_debug("PUT B=%x %u", d, u);
- if (u == 0) {
- rxrpc_put_peer(bundle->params.peer);
- kfree(bundle);
- }
+ if (u == 0)
+ rxrpc_free_bundle(bundle);
}
/*
@@ -328,7 +332,7 @@ static struct rxrpc_bundle *rxrpc_look_up_bundle(struct rxrpc_conn_parameters *c
return candidate;
found_bundle_free:
- kfree(candidate);
+ rxrpc_free_bundle(candidate);
found_bundle:
rxrpc_get_bundle(bundle);
spin_unlock(&local->client_bundles_lock);
From: Eiichi Tsukata <[email protected]>
Need to call rxrpc_put_local() for peer candidate before kfree() as it
holds a ref to rxrpc_local.
[DH: v2: Changed to abstract the peer freeing code out into a function]
Fixes: 9ebeddef58c4 ("rxrpc: rxrpc_peer needs to hold a ref on the rxrpc_local record")
Signed-off-by: Eiichi Tsukata <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: Marc Dionne <[email protected]>
cc: [email protected]
Link: https://lore.kernel.org/all/[email protected]/ # v1
---
net/rxrpc/peer_object.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/net/rxrpc/peer_object.c b/net/rxrpc/peer_object.c
index 68396d052052..0298fe2ad6d3 100644
--- a/net/rxrpc/peer_object.c
+++ b/net/rxrpc/peer_object.c
@@ -299,6 +299,12 @@ static struct rxrpc_peer *rxrpc_create_peer(struct rxrpc_sock *rx,
return peer;
}
+static void rxrpc_free_peer(struct rxrpc_peer *peer)
+{
+ rxrpc_put_local(peer->local);
+ kfree_rcu(peer, rcu);
+}
+
/*
* Set up a new incoming peer. There shouldn't be any other matching peers
* since we've already done a search in the list from the non-reentrant context
@@ -365,7 +371,7 @@ struct rxrpc_peer *rxrpc_lookup_peer(struct rxrpc_sock *rx,
spin_unlock_bh(&rxnet->peer_hash_lock);
if (peer)
- kfree(candidate);
+ rxrpc_free_peer(candidate);
else
peer = candidate;
}
@@ -420,8 +426,7 @@ static void __rxrpc_put_peer(struct rxrpc_peer *peer)
list_del_init(&peer->keepalive_link);
spin_unlock_bh(&rxnet->peer_hash_lock);
- rxrpc_put_local(peer->local);
- kfree_rcu(peer, rcu);
+ rxrpc_free_peer(peer);
}
/*
@@ -457,8 +462,7 @@ void rxrpc_put_peer_locked(struct rxrpc_peer *peer)
if (n == 0) {
hash_del_rcu(&peer->hash_link);
list_del_init(&peer->keepalive_link);
- rxrpc_put_local(peer->local);
- kfree_rcu(peer, rcu);
+ rxrpc_free_peer(peer);
}
}
On Wed, 24 Nov 2021 14:37:33 +0000 David Howells wrote:
> From: Eiichi Tsukata <[email protected]>
>
> Need to call rxrpc_put_peer() for bundle candidate before kfree() as it
> holds a ref to rxrpc_peer.
>
> [DH: v2: Changed to abstract out the bundle freeing code into a function]
>
> Fixes: 245500d853e9 ("rxrpc: Rewrite the client connection manager")
> Signed-off-by: Eiichi Tsukata <[email protected]>
> Signed-off-by: David Howells <[email protected]>
> cc: Marc Dionne <[email protected]>
> cc: [email protected]
> Link: https://lore.kernel.org/r/[email protected]/ # v1
Are these supposed to go to net? They are addressed To: the author.
Jakub Kicinski <[email protected]> wrote:
> Are these supposed to go to net? They are addressed To: the author.
I'm hoping the author rechecks/reviews them. I commented on his original
submission that I thought they could be done slightly differently.
David
> On Nov 26, 2021, at 17:05, David Howells <[email protected]> wrote:
>
> Jakub Kicinski <[email protected]> wrote:
>
>> Are these supposed to go to net? They are addressed To: the author.
>
> I'm hoping the author rechecks/reviews them. I commented on his original
> submission that I thought they could be done slightly differently.
>
Thanks, I’ve tested them with my environment. Looks good.
Eiichi
On Wed, Nov 24, 2021 at 10:37 AM David Howells <[email protected]> wrote:
>
> From: Eiichi Tsukata <[email protected]>
>
> Need to call rxrpc_put_peer() for bundle candidate before kfree() as it
> holds a ref to rxrpc_peer.
>
> [DH: v2: Changed to abstract out the bundle freeing code into a function]
>
> Fixes: 245500d853e9 ("rxrpc: Rewrite the client connection manager")
> Signed-off-by: Eiichi Tsukata <[email protected]>
> Signed-off-by: David Howells <[email protected]>
> cc: Marc Dionne <[email protected]>
> cc: [email protected]
> Link: https://lore.kernel.org/r/[email protected]/ # v1
> ---
>
> net/rxrpc/conn_client.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/net/rxrpc/conn_client.c b/net/rxrpc/conn_client.c
> index dbea0bfee48e..8120138dac01 100644
> --- a/net/rxrpc/conn_client.c
> +++ b/net/rxrpc/conn_client.c
> @@ -135,16 +135,20 @@ struct rxrpc_bundle *rxrpc_get_bundle(struct rxrpc_bundle *bundle)
> return bundle;
> }
>
> +static void rxrpc_free_bundle(struct rxrpc_bundle *bundle)
> +{
> + rxrpc_put_peer(bundle->params.peer);
> + kfree(bundle);
> +}
> +
> void rxrpc_put_bundle(struct rxrpc_bundle *bundle)
> {
> unsigned int d = bundle->debug_id;
> unsigned int u = atomic_dec_return(&bundle->usage);
>
> _debug("PUT B=%x %u", d, u);
> - if (u == 0) {
> - rxrpc_put_peer(bundle->params.peer);
> - kfree(bundle);
> - }
> + if (u == 0)
> + rxrpc_free_bundle(bundle);
> }
>
> /*
> @@ -328,7 +332,7 @@ static struct rxrpc_bundle *rxrpc_look_up_bundle(struct rxrpc_conn_parameters *c
> return candidate;
>
> found_bundle_free:
> - kfree(candidate);
> + rxrpc_free_bundle(candidate);
> found_bundle:
> rxrpc_get_bundle(bundle);
> spin_unlock(&local->client_bundles_lock);
Reviewed-by: Marc Dionne <[email protected]>
Marc
On Wed, Nov 24, 2021 at 10:38 AM David Howells <[email protected]> wrote:
>
> From: Eiichi Tsukata <[email protected]>
>
> Need to call rxrpc_put_local() for peer candidate before kfree() as it
> holds a ref to rxrpc_local.
>
> [DH: v2: Changed to abstract the peer freeing code out into a function]
>
> Fixes: 9ebeddef58c4 ("rxrpc: rxrpc_peer needs to hold a ref on the rxrpc_local record")
> Signed-off-by: Eiichi Tsukata <[email protected]>
> Signed-off-by: David Howells <[email protected]>
> cc: Marc Dionne <[email protected]>
> cc: [email protected]
> Link: https://lore.kernel.org/all/[email protected]/ # v1
> ---
>
> net/rxrpc/peer_object.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/net/rxrpc/peer_object.c b/net/rxrpc/peer_object.c
> index 68396d052052..0298fe2ad6d3 100644
> --- a/net/rxrpc/peer_object.c
> +++ b/net/rxrpc/peer_object.c
> @@ -299,6 +299,12 @@ static struct rxrpc_peer *rxrpc_create_peer(struct rxrpc_sock *rx,
> return peer;
> }
>
> +static void rxrpc_free_peer(struct rxrpc_peer *peer)
> +{
> + rxrpc_put_local(peer->local);
> + kfree_rcu(peer, rcu);
> +}
> +
> /*
> * Set up a new incoming peer. There shouldn't be any other matching peers
> * since we've already done a search in the list from the non-reentrant context
> @@ -365,7 +371,7 @@ struct rxrpc_peer *rxrpc_lookup_peer(struct rxrpc_sock *rx,
> spin_unlock_bh(&rxnet->peer_hash_lock);
>
> if (peer)
> - kfree(candidate);
> + rxrpc_free_peer(candidate);
> else
> peer = candidate;
> }
> @@ -420,8 +426,7 @@ static void __rxrpc_put_peer(struct rxrpc_peer *peer)
> list_del_init(&peer->keepalive_link);
> spin_unlock_bh(&rxnet->peer_hash_lock);
>
> - rxrpc_put_local(peer->local);
> - kfree_rcu(peer, rcu);
> + rxrpc_free_peer(peer);
> }
>
> /*
> @@ -457,8 +462,7 @@ void rxrpc_put_peer_locked(struct rxrpc_peer *peer)
> if (n == 0) {
> hash_del_rcu(&peer->hash_link);
> list_del_init(&peer->keepalive_link);
> - rxrpc_put_local(peer->local);
> - kfree_rcu(peer, rcu);
> + rxrpc_free_peer(peer);
> }
> }
Reviewed-by: Marc Dionne <[email protected]>
Marc
Eiichi Tsukata <[email protected]> wrote:
> Thanks, I’ve tested them with my environment. Looks good.
Thanks.
David
Hi Jakub,
> Are these supposed to go to net? They are addressed To: the author.
I've posted a new set to netdev that has the Acks from Marc added for you to
pick up.
David