[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAF2d9jgj-nMnXD2V++Ug5UmkoCpa_EtMrqYnaSCb6Ncq-vezQA@mail.gmail.com>
Date: Thu, 2 Oct 2014 22:49:54 +0530
From: Mahesh Bandewar <maheshb@...gle.com>
To: Cong Wang <cwang@...pensource.com>
Cc: Jay Vosburgh <j.vosburgh@...il.com>,
Veaceslav Falico <vfalico@...hat.com>,
Andy Gospodarek <andy@...yhouse.net>,
David Miller <davem@...emloft.net>,
netdev <netdev@...r.kernel.org>,
Eric Dumazet <edumazet@...gle.com>,
Maciej Zenczykowski <maze@...gle.com>
Subject: Re: [PATCH net-next v6 2/2] bonding: Simplify the xmit function for
modes that use xmit_hash
On Thu, Oct 2, 2014 at 10:10 AM, Cong Wang <cwang@...pensource.com> wrote:
> On Wed, Oct 1, 2014 at 1:38 AM, Mahesh Bandewar <maheshb@...gle.com> wrote:
>> +int bond_update_slave_arr(struct bonding *bond, struct slave *skipslave)
>> +{
>> + struct slave *slave;
>> + struct list_head *iter;
>> + struct bond_up_slave *new_arr, *old_arr;
>> + int slaves_in_agg;
>> + int agg_id = 0;
>> + int ret = 0;
>> +
>> +#ifdef CONFIG_LOCKDEP
>> + WARN_ON(lockdep_is_held(&bond->mode_lock));
>> +#endif
>
>
> I think you can use lockdep_is_held().
>
>> +
>> + new_arr = kzalloc(offsetof(struct bond_up_slave, arr[bond->slave_cnt]),
>> + GFP_KERNEL);
>> + if (!new_arr) {
>> + ret = -ENOMEM;
>> + pr_err("Failed to build slave-array.\n");
>> + goto out;
>> + }
>
>
> No need to print an error message for OOM, it is already noisy. :)
>
Agreed OOM condition is noisy but failing silently would not help
debugging hence adding the message.
>
>> + if (BOND_MODE(bond) == BOND_MODE_8023AD) {
>> + struct ad_info ad_info;
>> +
>> + if (bond_3ad_get_active_agg_info(bond, &ad_info)) {
>> + pr_debug("bond_3ad_get_active_agg_info failed\n");
>
>
> I suspect how useful this debug info is since your patch is almost ready
> to merge.
>
It could be useful for someone else too :)
>> + kfree_rcu(new_arr, rcu);
>> + /* No active aggragator means its not safe to use
>
> s/its/it's/
>
thanks
>> + * the previous array.
>> + */
>> + old_arr = rtnl_dereference(bond->slave_arr);
>> + if (old_arr) {
>> + RCU_INIT_POINTER(bond->slave_arr, NULL);
>> + kfree_rcu(old_arr, rcu);
>> + }
>> + goto out;
>> + }
>> + slaves_in_agg = ad_info.ports;
>> + agg_id = ad_info.aggregator_id;
>> + }
>
>
>
> Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists