On 08.02.21 18:59, Taehee Yoo wrote:
> MLD module's context is atomic although most logic is called from
> control-path, not data path. Only a few functions are called from
> datapath, most of the functions are called from the control-path.
> Furthermore, MLD's response is not processed immediately because
> MLD protocol is using delayed response.
> It means that If a query is received, the node should have a delay
> in response At this point, it could change the context.
> It means most of the functions can be implemented as the sleepable
> context so that mld functions can use sleepable functions.
>
> Most resources are protected by spinlock and rwlock so the context
> of mld functions is atomic. So, in order to change context, locking
> scenario should be changed.
> It switches from spinlock/rwlock to mutex and rcu.
>
> Some locks are deleted and added.
> 1. ipv6->mc_socklist->sflock is deleted
> This is rwlock and it is unnecessary.
> Because it protects ipv6_mc_socklist-sflist but it is now protected
> by rtnl_lock().
>
> 2. ifmcaddr6->mca_work_lock is added.
> This lock protects ifmcaddr6->mca_work.
> This workqueue can be used by both control-path and data-path.
> It means mutex can't be used.
> So mca_work_lock(spinlock) is added.
>
> 3. inet6_dev->mc_tomb_lock is deleted
> This lock protects inet6_dev->mc_bom_list.
> But it is protected by rtnl_lock().
>
> 4. inet6_dev->lock is used for protecting workqueues.
> inet6_dev has its own workqueues(mc_gq_work, mc_ifc_work, mc_delrec_work)
> and it can be started and stop by both control-path and data-path.
> So, mutex can't be used.
>
> Suggested-by: Cong Wang <xiyou.wangc...@gmail.com>
> Signed-off-by: Taehee Yoo <ap420...@gmail.com>
> ---
> drivers/s390/net/qeth_l3_main.c | 6 +-
> include/net/if_inet6.h | 29 +-
> net/batman-adv/multicast.c | 4 +-
> net/ipv6/addrconf.c | 4 +-
> net/ipv6/mcast.c | 785 ++++++++++++++++----------------
> 5 files changed, 411 insertions(+), 417 deletions(-)
>
> diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
> index e49abdeff69c..085afb24482e 100644
> --- a/drivers/s390/net/qeth_l3_main.c
> +++ b/drivers/s390/net/qeth_l3_main.c
> @@ -1098,8 +1098,8 @@ static int qeth_l3_add_mcast_rtnl(struct net_device
> *dev, int vid, void *arg)
> tmp.disp_flag = QETH_DISP_ADDR_ADD;
> tmp.is_multicast = 1;
>
> - read_lock_bh(&in6_dev->lock);
> - list_for_each_entry(im6, in6_dev->mc_list, list) {
> + rcu_read_lock();
> + list_for_each_entry_rcu(im6, in6_dev->mc_list, list) {
No need for the rcu_read_lock(), we're called under rtnl.
So if there's a v2, please just make this
list_for_each_entry_rcu(im6, in6_dev->mc_list, list,
lockdep_rtnl_is_held())
> tmp.u.a6.addr = im6->mca_addr;
>
> ipm = qeth_l3_find_addr_by_ip(card, &tmp);
> @@ -1117,7 +1117,7 @@ static int qeth_l3_add_mcast_rtnl(struct net_device
> *dev, int vid, void *arg)
> qeth_l3_ipaddr_hash(ipm));
>
> }
> - read_unlock_bh(&in6_dev->lock);
> + rcu_read_unlock();
>
> out:
> return 0;
