From: Nikolay Aleksandrov <nikolay@redhat.com>
To: Jay Vosburgh <fubar@us.ibm.com>
Cc: netdev@vger.kernel.org, andy@greyhouse.net
Subject: Re: [PATCH net-next] bonding: fix system hang due to fast igmp timer rescheduling
Date: Wed, 31 Jul 2013 01:25:52 +0200 [thread overview]
Message-ID: <51F84B80.7070106@redhat.com> (raw)
In-Reply-To: <7289.1375209640@death.nxdomain>
On 07/30/2013 08:40 PM, Jay Vosburgh wrote:
> Nikolay Aleksandrov <nikolay@redhat.com> wrote:
>
>> From: Nikolay Aleksandrov <Nikolay Aleksandrov nikolay@redhat.com>
>>
>> After commit 4aa5dee4d9 ("net: convert resend IGMP to notifier event")
>> we try to acquire rtnl in bond_resend_igmp_join_requests but it can be
>> scheduled with rtnl already held (e.g. when bond_change_active_slave is
>> called with rtnl) causing a loop of immediate reschedules + calls because
>> rtnl_trylock fails each time since it's being already held.
>> For me this issue leads to system hangs very easy:
>> modprobe bonding; ifconfig bond0 up; ifenslave bond0 eth0; rmmod
>> bonding;
>
> I believe that bond_change_active_slave is always called with
> rtnl held, and it is the only caller of bond_resend_igmp_join_requests
> (well, "caller" in the sense that it queues the delayed work for
> mcast_work that runs the function, currently with delay of 0).
>
>> The fix is to introduce a small (1 jiffy) delay which is enough for the
>> sections holding rtnl to finish without putting any strain on the system.
>
> Should the delay also be in the bond_change_active_slave queue
> work call as well, to eliminate one loop of the "rtnl_trylock failing ->
> queue_delayed_work" sequence in bond_resend_igmp_join_requests?
>
> -J
>
Hi Jay,
I actually think there's one way to call bond_change_active_slave without rtnl:
bond_select_active_slave (and thus bond_change_active_slave) called from
bond_loadbalance_arp_mon with read_lock(&bond->lock) &
write_lock_bh(&bond->curr_slave_lock) only.
But you're right that most of the time (i.e. all other callers) it's called
with rtnl held, I will adjust its timer as well and send a v2.
Since I've prepared the initial RCU conversion of the bonding which I think
to submit tomorrow I don't want to make this patch a part of that series,
so I will submit it alone (and thus apart).
If this is not the protocol in such cases, or if there's anything else, I
can wait for it to go in and submit the series then, just let me know.
Thanks,
Nik
prev parent reply other threads:[~2013-07-30 23:25 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-30 17:37 [PATCH net-next] bonding: fix system hang due to fast igmp timer rescheduling Nikolay Aleksandrov
2013-07-30 18:40 ` Jay Vosburgh
2013-07-30 23:25 ` Nikolay Aleksandrov [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=51F84B80.7070106@redhat.com \
--to=nikolay@redhat.com \
--cc=andy@greyhouse.net \
--cc=fubar@us.ibm.com \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.