All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ding Tianhong <dingtianhong@huawei.com>
To: Nikolay Aleksandrov <nikolay@redhat.com>
Cc: Jay Vosburgh <fubar@us.ibm.com>,
	Andy Gospodarek <andy@greyhouse.net>,
	"David S. Miller" <davem@davemloft.net>,
	Veaceslav Falico <vfalico@redhat.com>,
	Netdev <netdev@vger.kernel.org>
Subject: Re: [PATCH 4/5] bonding: restructure and simplify bond_for_each_slave_next()
Date: Fri, 30 Aug 2013 11:37:12 +0800	[thread overview]
Message-ID: <52201368.4070200@huawei.com> (raw)
In-Reply-To: <521F59EE.1080408@redhat.com>


>>  			slave->link = BOND_LINK_BACK;
>>  			bond_set_slave_active_flags(slave);
>> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
>> index ecb5d1d..7670584 100644
>> --- a/drivers/net/bonding/bonding.h
>> +++ b/drivers/net/bonding/bonding.h
>> @@ -119,14 +119,25 @@
>>   * bond_for_each_slave_from - iterate the slaves list from a starting point
>>   * @bond:	the bond holding this list.
>>   * @pos:	current slave.
>> - * @cnt:	counter for max number of moves
>>   * @start:	starting point.
>>   *
>>   * Caller must hold bond->lock
>>   */
>> -#define bond_for_each_slave_from(bond, pos, cnt, start) \
>> -	for (cnt = 0, pos = start; pos && cnt < (bond)->slave_cnt; \
>> -	     cnt++, pos = bond_next_slave(bond, pos))
>> +#define bond_for_each_slave_from(bond, pos, start) \
>> +	for (pos = bond_next_slave(bond, start); pos && pos != start; \
>> +			pos = bond_next_slave(bond, pos))
>> +
>> +/**
>> + * bond_for_each_slave_from_rcu - iterate the slaves list from a starting point
>> + * @bond:	the bond holding this list.
>> + * @pos:	current slave.
>> + * @start:	starting point.
>> + *
>> + * Caller must hold rcu_read_lock
>> + */
>> +#define bond_for_each_slave_from_rcu(bond, pos, start) \
>> +	for (pos = bond_next_slave_rcu(bond, start); pos && pos != start; \
>> +			pos = bond_next_slave_rcu(bond, pos))
>>  
> One question here: what if "start" gets deleted while we're traversing the list,
> could this lead to an infinite loop ?
> 

agree, so the start should not be delete while traversing the list, the func should not run
without protection. 

>>  /**
>>   * bond_for_each_slave - iterate over all slaves
>>
> 
> 
> .
> 

      reply	other threads:[~2013-08-30  3:37 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-28  4:21 [PATCH 4/5] bonding: restructure and simplify bond_for_each_slave_next() Ding Tianhong
2013-08-29 14:25 ` Nikolay Aleksandrov
2013-08-30  3:37   ` Ding Tianhong [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=52201368.4070200@huawei.com \
    --to=dingtianhong@huawei.com \
    --cc=andy@greyhouse.net \
    --cc=davem@davemloft.net \
    --cc=fubar@us.ibm.com \
    --cc=netdev@vger.kernel.org \
    --cc=nikolay@redhat.com \
    --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.