| Message ID | 20150516131628.GA981@gondor.apana.org.au |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Herbert Xu <herbert@gondor.apana.org.au> Date: Sat, 16 May 2015 21:16:28 +0800 > On Fri, May 15, 2015 at 01:02:57PM -0400, David Miller wrote: >> From: Herbert Xu <herbert@gondor.apana.org.au> >> Date: Thu, 14 May 2015 13:58:24 +0800 >> >> > The current rhashtable rehash code is buggy and can't deal with >> > parallel insertions/removals without corrupting the hash table. >> > >> > This patch disables it by partially reverting >> > c5adde9468b0714a051eac7f9666f23eb10b61f7 ("netlink: eliminate >> > nl_sk_hash_lock"). >> > >> > This patch also removes a bogus socket lock introduced by that >> > very same patch. >> > >> > Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> >> >> Herbert, if you agree with me in the other thread that the lock_sock() >> or something like it has to remain, you'll need to respin this. > > Actually I think this one is OK because I'm replacing it with the > hash table mutex which is just like the previous global lock except > that it is per-family. As you cannot change the family on a netlink > socket this should be good enough. > > But the changelog message is wrong so here is an updated version. > > Cheers, > > ---8<--- > The current rhashtable rehash code is buggy and can't deal with > parallel insertions/removals without corrupting the hash table. > > This patch disables it by partially reverting > c5adde9468b0714a051eac7f9666f23eb10b61f7 ("netlink: eliminate > nl_sk_hash_lock"). > > Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> Ok, I've queued this up for -stable, thanks Herbert. -- 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
On Sat, May 16, 2015 at 05:10:19PM -0400, David Miller wrote: > From: Herbert Xu <herbert@gondor.apana.org.au> > Date: Sat, 16 May 2015 21:16:28 +0800 > > > On Fri, May 15, 2015 at 01:02:57PM -0400, David Miller wrote: > >> From: Herbert Xu <herbert@gondor.apana.org.au> > >> Date: Thu, 14 May 2015 13:58:24 +0800 > >> > >> > The current rhashtable rehash code is buggy and can't deal with > >> > parallel insertions/removals without corrupting the hash table. > >> > > >> > This patch disables it by partially reverting > >> > c5adde9468b0714a051eac7f9666f23eb10b61f7 ("netlink: eliminate > >> > nl_sk_hash_lock"). > >> > > >> > This patch also removes a bogus socket lock introduced by that > >> > very same patch. > >> > > >> > Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> > >> > >> Herbert, if you agree with me in the other thread that the lock_sock() > >> or something like it has to remain, you'll need to respin this. > > > > Actually I think this one is OK because I'm replacing it with the > > hash table mutex which is just like the previous global lock except > > that it is per-family. As you cannot change the family on a netlink > > socket this should be good enough. > > > > But the changelog message is wrong so here is an updated version. > > > > Cheers, > > > > ---8<--- > > The current rhashtable rehash code is buggy and can't deal with > > parallel insertions/removals without corrupting the hash table. > > > > This patch disables it by partially reverting > > c5adde9468b0714a051eac7f9666f23eb10b61f7 ("netlink: eliminate > > nl_sk_hash_lock"). > > > > Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> > > Ok, I've queued this up for -stable, thanks Herbert. > Hi David, sorry for bothering you - I don't see this patch in any of your trees, and it is marked as "changes requested" in patchwork. Did I look at the wrong places, do you still plan to apply the patch as-is, or do you expect some changes ? As side info, I have been trying to track down the getaddrinfo hang problem observed by others, which we see in 3.19.4 and 4.0.4. Thanks, Guenter -- 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
From: Guenter Roeck <linux@roeck-us.net> Date: Thu, 4 Jun 2015 09:27:27 -0700 > sorry for bothering you - I don't see this patch in any of your trees, > and it is marked as "changes requested" in patchwork. Did I look > at the wrong places, do you still plan to apply the patch as-is, > or do you expect some changes ? > > As side info, I have been trying to track down the getaddrinfo > hang problem observed by others, which we see in 3.19.4 and 4.0.4. Changes-requested, amazingly, means that someone gave you feedback and changes along with a fresh resubmission is expected of you. -- 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
On Thu, 2015-06-04 at 11:59 -0700, David Miller wrote: > From: Guenter Roeck <linux@roeck-us.net> > Date: Thu, 4 Jun 2015 09:27:27 -0700 > > > sorry for bothering you - I don't see this patch in any of your trees, > > and it is marked as "changes requested" in patchwork. Did I look > > at the wrong places, do you still plan to apply the patch as-is, > > or do you expect some changes ? > > > > As side info, I have been trying to track down the getaddrinfo > > hang problem observed by others, which we see in 3.19.4 and 4.0.4. > > Changes-requested, amazingly, means that someone gave you feedback > and changes along with a fresh resubmission is expected of you. Arg. I hope Herbert can sort out this soon enough, I thought patch was already queued. 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
On 06/04/2015 11:59 AM, David Miller wrote: > From: Guenter Roeck <linux@roeck-us.net> > Date: Thu, 4 Jun 2015 09:27:27 -0700 > >> sorry for bothering you - I don't see this patch in any of your trees, >> and it is marked as "changes requested" in patchwork. Did I look >> at the wrong places, do you still plan to apply the patch as-is, >> or do you expect some changes ? >> >> As side info, I have been trying to track down the getaddrinfo >> hang problem observed by others, which we see in 3.19.4 and 4.0.4. > > Changes-requested, amazingly, means that someone gave you feedback > and changes along with a fresh resubmission is expected of you. > Yes, I understand. My problem is/was that your last e-mail on the subject suggested that you would apply the patch, while at the same time patchwork suggests that you expect to see changes. Sorry that I was - and still am - unable to correlate those two pieces of information. Guenter -- 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
On Thu, Jun 04, 2015 at 09:27:27AM -0700, Guenter Roeck wrote: > > sorry for bothering you - I don't see this patch in any of your trees, > and it is marked as "changes requested" in patchwork. Did I look > at the wrong places, do you still plan to apply the patch as-is, > or do you expect some changes ? I just looked up the patchwork entry and it actually says "not applicable" which is correct: https://patchwork.ozlabs.org/patch/473041/ Because the patch only applies to stable and is not needed in either net or net-next. Cheers,
On 06/04/2015 08:52 PM, Herbert Xu wrote: > On Thu, Jun 04, 2015 at 09:27:27AM -0700, Guenter Roeck wrote: >> >> sorry for bothering you - I don't see this patch in any of your trees, >> and it is marked as "changes requested" in patchwork. Did I look >> at the wrong places, do you still plan to apply the patch as-is, >> or do you expect some changes ? > > I just looked up the patchwork entry and it actually says "not > applicable" which is correct: > > https://patchwork.ozlabs.org/patch/473041/ > > Because the patch only applies to stable and is not needed in either > net or net-next. > Ah, so I was looking at the wrong branch after all. Thanks a lot for the clarification, Guenter -- 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
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c index 05919bf..322fe03 100644 --- a/net/netlink/af_netlink.c +++ b/net/netlink/af_netlink.c @@ -1052,7 +1052,7 @@ static int netlink_insert(struct sock *sk, u32 portid) struct netlink_table *table = &nl_table[sk->sk_protocol]; int err; - lock_sock(sk); + mutex_lock(&table->hash.mutex); err = -EBUSY; if (nlk_sk(sk)->portid) @@ -1073,7 +1073,7 @@ static int netlink_insert(struct sock *sk, u32 portid) } err: - release_sock(sk); + mutex_unlock(&table->hash.mutex); return err; } @@ -1082,10 +1082,12 @@ static void netlink_remove(struct sock *sk) struct netlink_table *table; table = &nl_table[sk->sk_protocol]; + mutex_lock(&table->hash.mutex); if (rhashtable_remove(&table->hash, &nlk_sk(sk)->node)) { WARN_ON(atomic_read(&sk->sk_refcnt) == 1); __sock_put(sk); } + mutex_unlock(&table->hash.mutex); netlink_table_grab(); if (nlk_sk(sk)->subscriptions) {