| Message ID | CAHSpA5-ivVaQnQU7_ikt=BV-EAwFJFaA6ErpQ1Jn1HmPmXZapg@mail.gmail.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | ipv6: multicast: In mld_send_cr function moving read lock to second for loop | expand |
From: Guruswamy Basavaiah <guru2018@gmail.com> Date: Fri, 17 Aug 2018 18:01:41 +0530 > @@ -1860,7 +1860,6 @@ static void mld_send_cr(struct inet6_dev *idev) > struct sk_buff *skb = NULL; > int type, dtype; > > - read_lock_bh(&idev->lock); > spin_lock(&idev->mc_lock); > > /* deleted MCA's */ This will lead to deadlocks, idev->mc_lock must be taken with _bh(). I have zero confidence in this change, did you do any stress testing with lockdep enabled? It would have caught this quickly.
> This will lead to deadlocks, idev->mc_lock must be taken with _bh(). Modified the existing spin_lock to spin_lock_bh > I have zero confidence in this change, did you do any stress testing > with lockdep enabled? It would have caught this quickly. With LOCKDEP enabled ran LTP multicast stress with the below new patch. Test case is successful and LOCKDEP did not catch any dead lock issues. --- From 789840a6c6f783311ea7dfd832787c27d5b8359f Mon Sep 17 00:00:00 2001 From: Guruswamy Basavaiah <guru2018@gmail.com> Date: Tue, 18 Sep 2018 18:40:21 +0530 Subject: [PATCH] ipv6: multicast: In mld_send_cr function moving read lock to second for loop In function mld_send_cr, the first loop is already protected by idev->mc_lock, it dont need idev->lock read lock, hence moving it only to second for loop. And converting the existing spin_lock to spin_lock_bh Signed-off-by: Guruswamy Basavaiah <guru2018@gmail.com> --- net/ipv6/mcast.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c index 4ae54aaca373..751e580eb0ed 100644 --- a/net/ipv6/mcast.c +++ b/net/ipv6/mcast.c @@ -1912,8 +1912,7 @@ static void mld_send_cr(struct inet6_dev *idev) struct sk_buff *skb = NULL; int type, dtype; - read_lock_bh(&idev->lock); - spin_lock(&idev->mc_lock); + spin_lock_bh(&idev->mc_lock); /* deleted MCA's */ pmc_prev = NULL; @@ -1947,8 +1946,9 @@ static void mld_send_cr(struct inet6_dev *idev) } else pmc_prev = pmc; } - spin_unlock(&idev->mc_lock); + spin_unlock_bh(&idev->mc_lock); + read_lock_bh(&idev->lock); /* change recs */ for (pmc = idev->mc_list; pmc; pmc = pmc->next) { spin_lock_bh(&pmc->mca_lock);
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c index d64ee7e..d8e7e15 100644 --- a/net/ipv6/mcast.c +++ b/net/ipv6/mcast.c @@ -1860,7 +1860,6 @@ static void mld_send_cr(struct inet6_dev *idev) struct sk_buff *skb = NULL; int type, dtype; - read_lock_bh(&idev->lock); spin_lock(&idev->mc_lock); /* deleted MCA's */ @@ -1897,6 +1896,7 @@ static void mld_send_cr(struct inet6_dev *idev) } spin_unlock(&idev->mc_lock); + read_lock_bh(&idev->lock); /* change recs */ for (pmc = idev->mc_list; pmc; pmc = pmc->next) { spin_lock_bh(&pmc->mca_lock);
In function mld_send_cr, the first loop is already protected by idev->mc_lock, it dont need idev->lock read lock, hence moving it only to second for loop. Signed-off-by: Guruswamy Basavaiah <guru2018@gmail.com> --- net/ipv6/mcast.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)