| Submitter | Eric Dumazet |
|---|---|
| Date | Nov. 30, 2008, 4:36 p.m. |
| Message ID | <4932C128.6040401@cosmosbay.com> |
| Download | mbox | patch |
| Permalink | /patch/11472/ |
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
Comments
From: Eric Dumazet <dada1@cosmosbay.com> Date: Sun, 30 Nov 2008 17:36:56 +0100 > [PATCH] net: percpu_counter_inc() should not be called in BH-disabled section > > I checked all per_cpu_counter_xxx() usages in network tree, and I think > all call sites are BH enabled except one in inet_csk_listen_stop(). > > commit dd24c00191d5e4a1ae896aafe33c6b8095ab4bd1 > (net: Use a percpu_counter for orphan_count) > replaced atomic_t orphan_count to a percpu_counter. > > atomic_inc()/atomic_dec() can be called from any context, while percpu_counter_xxx() > should be called from a consistent state. > > For orphan_count, this context can be the BH-enabled one. > > Signed-off-by: Eric Dumazet <dada1@cosmosbay.com> I applied this preemptively even though Alexey hasn't given test feedback yet, and I also added a mention of his report in the commit message. Thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
David Miller a écrit : > From: Eric Dumazet <dada1@cosmosbay.com> > Date: Sun, 30 Nov 2008 17:36:56 +0100 > >> [PATCH] net: percpu_counter_inc() should not be called in BH-disabled section >> >> I checked all per_cpu_counter_xxx() usages in network tree, and I think >> all call sites are BH enabled except one in inet_csk_listen_stop(). >> >> commit dd24c00191d5e4a1ae896aafe33c6b8095ab4bd1 >> (net: Use a percpu_counter for orphan_count) >> replaced atomic_t orphan_count to a percpu_counter. >> >> atomic_inc()/atomic_dec() can be called from any context, while percpu_counter_xxx() >> should be called from a consistent state. >> >> For orphan_count, this context can be the BH-enabled one. >> >> Signed-off-by: Eric Dumazet <dada1@cosmosbay.com> > > I applied this preemptively even though Alexey hasn't given > test feedback yet, and I also added a mention of his report > in the commit message. Hum, this patch was necessary but wont help Alexy case that was not related, if I undertsnad well its oops. Its oops was about nr_files and a network percpu_counter, one always called in BH-enabled context, one always in BD-disabled context. We need a core change here, so that lockdep dont assume all percpu_counter have the same class. I know nothing about lockdep so a fix could take me some time, one can beat me easily ;) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Patch
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c index 1ccdbba..fe32255 100644 --- a/net/ipv4/inet_connection_sock.c +++ b/net/ipv4/inet_connection_sock.c @@ -632,6 +632,8 @@ void inet_csk_listen_stop(struct sock *sk) acc_req = req->dl_next; + percpu_counter_inc(sk->sk_prot->orphan_count); + local_bh_disable(); bh_lock_sock(child); WARN_ON(sock_owned_by_user(child)); @@ -641,8 +643,6 @@ void inet_csk_listen_stop(struct sock *sk) sock_orphan(child); - percpu_counter_inc(sk->sk_prot->orphan_count); - inet_csk_destroy_sock(child); bh_unlock_sock(child);