| Message ID | 20180403154122.00d76d61@canb.auug.org.au |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | linux-next: build failure after merge of the tip tree | expand |
On Tue, Apr 03, 2018 at 03:41:22PM +1000, Stephen Rothwell wrote: > Caused by commit > > 9b8cce52c4b5 ("sched/wait: Remove the wait_on_atomic_t() API") > > interacting with commits > > d3be4d244330 ("xrpc: Fix potential call vs socket/net destruction race") > 31f5f9a1691e ("rxrpc: Fix apparent leak of rxrpc_local objects") > > from the net-next tree. > > Haven't we figured out how to remove/change APIs yet? :-( I figured that since there were only a handful of users it wasn't a popular API, also David very much knew of those patches changing it so could easily have pulled in the special tip/sched/wait branch :/
Peter Zijlstra <peterz@infradead.org> wrote: > I figured that since there were only a handful of users it wasn't a > popular API, also David very much knew of those patches changing it so > could easily have pulled in the special tip/sched/wait branch :/ I'm not sure I could, since I have to base on net-next. I'm not sure what DaveM's policy on that is. Also, it might've been better not to simply erase the atomic_t wait API immediately, but substitute wrappers for it to be removed one iteration hence. David
Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> + wait_var_event(&rxnet->nr_calls, !atomic_read(&rxnet->nr_calls));
I would prefer == 0 to ! as it's not really a true/false value.
But apart from that, it's looks okay and you can add my Reviewed-by.
David
On Tue, Apr 03, 2018 at 01:39:08PM +0100, David Howells wrote: > Peter Zijlstra <peterz@infradead.org> wrote: > > > I figured that since there were only a handful of users it wasn't a > > popular API, also David very much knew of those patches changing it so > > could easily have pulled in the special tip/sched/wait branch :/ > > I'm not sure I could, since I have to base on net-next. I'm not sure what > DaveM's policy on that is. > > Also, it might've been better not to simply erase the atomic_t wait API > immediately, but substitute wrappers for it to be removed one iteration hence. Yeah, I know, but I really wasn't expecting new users of this thing, it seemed like quite an exotic API with very limited users. A well..
diff --git a/net/rxrpc/call_accept.c b/net/rxrpc/call_accept.c index f67017dcb25e..a9a9be5519b9 100644 --- a/net/rxrpc/call_accept.c +++ b/net/rxrpc/call_accept.c @@ -220,7 +220,7 @@ void rxrpc_discard_prealloc(struct rxrpc_sock *rx) write_unlock(&rxnet->conn_lock); kfree(conn); if (atomic_dec_and_test(&rxnet->nr_conns)) - wake_up_atomic_t(&rxnet->nr_conns); + wake_up_var(&rxnet->nr_conns); tail = (tail + 1) & (size - 1); } diff --git a/net/rxrpc/call_object.c b/net/rxrpc/call_object.c index f721c2b7e234..f6734d8cb01a 100644 --- a/net/rxrpc/call_object.c +++ b/net/rxrpc/call_object.c @@ -658,7 +658,7 @@ static void rxrpc_rcu_destroy_call(struct rcu_head *rcu) kfree(call->rxtx_annotations); kmem_cache_free(rxrpc_call_jar, call); if (atomic_dec_and_test(&rxnet->nr_calls)) - wake_up_atomic_t(&rxnet->nr_calls); + wake_up_var(&rxnet->nr_calls); } /* @@ -725,5 +725,5 @@ void rxrpc_destroy_all_calls(struct rxrpc_net *rxnet) write_unlock(&rxnet->call_lock); atomic_dec(&rxnet->nr_calls); - wait_on_atomic_t(&rxnet->nr_calls, atomic_t_wait, TASK_UNINTERRUPTIBLE); + wait_var_event(&rxnet->nr_calls, !atomic_read(&rxnet->nr_calls)); } diff --git a/net/rxrpc/conn_object.c b/net/rxrpc/conn_object.c index 0950ee3d26f5..4c77a78a252a 100644 --- a/net/rxrpc/conn_object.c +++ b/net/rxrpc/conn_object.c @@ -367,7 +367,7 @@ static void rxrpc_destroy_connection(struct rcu_head *rcu) rxrpc_put_peer(conn->params.peer); if (atomic_dec_and_test(&conn->params.local->rxnet->nr_conns)) - wake_up_atomic_t(&conn->params.local->rxnet->nr_conns); + wake_up_var(&conn->params.local->rxnet->nr_conns); rxrpc_put_local(conn->params.local); kfree(conn); @@ -482,6 +482,6 @@ void rxrpc_destroy_all_connections(struct rxrpc_net *rxnet) /* We need to wait for the connections to be destroyed by RCU as they * pin things that we still need to get rid of. */ - wait_on_atomic_t(&rxnet->nr_conns, atomic_t_wait, TASK_UNINTERRUPTIBLE); + wait_var_event(&rxnet->nr_conns, !atomic_read(&rxnet->nr_conns)); _leave(""); }
Hi all, After merging the tip tree, today's linux-next build (x86_64 allmodconfig) failed like this: net/rxrpc/call_object.c: In function 'rxrpc_rcu_destroy_call': net/rxrpc/call_object.c:661:3: error: implicit declaration of function 'wake_up_atomic_t'; did you mean 'wake_up_bit'? [-Werror=implicit-function-declaration] wake_up_atomic_t(&rxnet->nr_calls); ^~~~~~~~~~~~~~~~ wake_up_bit net/rxrpc/call_object.c: In function 'rxrpc_destroy_all_calls': net/rxrpc/call_object.c:728:2: error: implicit declaration of function 'wait_on_atomic_t'; did you mean 'wait_on_bit'? [-Werror=implicit-function-declaration] wait_on_atomic_t(&rxnet->nr_calls, atomic_t_wait, TASK_UNINTERRUPTIBLE); ^~~~~~~~~~~~~~~~ wait_on_bit net/rxrpc/call_object.c:728:37: error: 'atomic_t_wait' undeclared (first use in this function); did you mean 'atomic_long_t'? wait_on_atomic_t(&rxnet->nr_calls, atomic_t_wait, TASK_UNINTERRUPTIBLE); ^~~~~~~~~~~~~ atomic_long_t net/rxrpc/call_object.c:728:37: note: each undeclared identifier is reported only once for each function it appears in net/rxrpc/call_accept.c: In function 'rxrpc_discard_prealloc': net/rxrpc/call_accept.c:223:4: error: implicit declaration of function 'wake_up_atomic_t'; did you mean 'wake_up_bit'? [-Werror=implicit-function-declaration] wake_up_atomic_t(&rxnet->nr_conns); ^~~~~~~~~~~~~~~~ wake_up_bit Caused by commit 9b8cce52c4b5 ("sched/wait: Remove the wait_on_atomic_t() API") interacting with commits d3be4d244330 ("xrpc: Fix potential call vs socket/net destruction race") 31f5f9a1691e ("rxrpc: Fix apparent leak of rxrpc_local objects") from the net-next tree. Haven't we figured out how to remove/change APIs yet? :-( That tip tree commit is now in Linus' tree (merged since I started this morning) so the net-next tree will need the below patch (or something similar when it is merged. I have applied the following merge fix patch (this may need more work): From: Stephen Rothwell <sfr@canb.auug.org.au> Date: Tue, 3 Apr 2018 15:34:48 +1000 Subject: [PATCH] sched/wait: merge fix up for wait_on_atomic() API removal Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> --- net/rxrpc/call_accept.c | 2 +- net/rxrpc/call_object.c | 4 ++-- net/rxrpc/conn_object.c | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-)