From: Nikolay Aleksandrov <nikolay@redhat.com>
To: Veaceslav Falico <vfalico@redhat.com>
Cc: netdev@vger.kernel.org, fubar@us.ibm.com, andy@greyhouse.net,
davem@davemloft.net
Subject: Re: [PATCH] bonding: remove sysfs before removing devices
Date: Sat, 06 Apr 2013 01:29:48 +0200 [thread overview]
Message-ID: <515F5E6C.2030009@redhat.com> (raw)
In-Reply-To: <20130405232133.GA19437@redhat.com>
On 04/06/2013 01:21 AM, Veaceslav Falico wrote:
> On Sat, Apr 06, 2013 at 12:15:11AM +0200, Nikolay Aleksandrov wrote:
>> Hi,
>> Sorry for the late reply but I was travelling this week. In my
>> opinion this
>> fix is wrong because in bond_uninit() (called by rtnl_link_unregister)
>> you have:
>> list_del(&bond->bond_list);
>> which is linked in the bond_net dev_list which is freed by
>> unregister_pernet_subsys.
>
> Yep, you're right, I've hit it recently with the patch applied, and now
> working on it. However, I still think that the idea of the patch is
> correct
> - i.e. to first disable sysfs (especially bonding_masters) and only
> afterwards to start removing everything else. Or, obviously, to finally
> add normal locking to sysfs functions.
>
> Anyway, this corruption is really rare, so I'll wait for your fix next
> week.
>
Well, there's no need for that because you'll have to iterate over all
"net"s to do it properly. Since we already have code that does it
(unregister_pernet_subsys), my fix is to kill off any "left" bond
devices in bond_net_exit() _after_ destroying the bonding_masters
sysfs entry for that net. This way we preserve the code structure and avoid
duplicating a loop over all nets.
This is the fix we discussed a week ago.
I'd be happy to hear any comments on it before posting :-)
Of course stopping the whole bonding sysfs handling is also an alternative.
next prev parent reply other threads:[~2013-04-05 23:32 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-04-03 15:46 [PATCH] bonding: remove sysfs before removing devices Veaceslav Falico
2013-04-05 4:50 ` David Miller
2013-04-05 22:15 ` Nikolay Aleksandrov
2013-04-05 23:21 ` Veaceslav Falico
2013-04-05 23:29 ` Nikolay Aleksandrov [this message]
2013-04-06 1:49 ` Veaceslav Falico
2013-04-06 7:38 ` Nikolay Aleksandrov
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=515F5E6C.2030009@redhat.com \
--to=nikolay@redhat.com \
--cc=andy@greyhouse.net \
--cc=davem@davemloft.net \
--cc=fubar@us.ibm.com \
--cc=netdev@vger.kernel.org \
--cc=vfalico@redhat.com \
/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.