Makito SHIOKAWA wrote, On 01/18/2008 02:43 PM:
>> Hmm... I'm not sure I understand your point, but it seems both
>> bonding_store_arp_interval() and bonding_store_miimon() where this
>> field could be changed, currently use cancel_delayed_work() with
>> flush_workqueue(), so I presume, there is no rtnl_lock() nor
>> write_lock(&bond->lock) held, so cancel_delayed_work_sync() could
>> be used, which doesn't require this additional check.
> I see. I rewrited the patch as below. How about this?
> (But, it may be just a matter to change 'if (new_value < 0)' to 'if (new_value
> <= 0)' in bonding_store_miimon() and bonding_store_arp_interval()...)
Yes, looks fine to me.
(But new_value = 0 seems needed - just like from module_param()?)
Maybe only a few slight doubts, yet:
- maybe before this cancel IFF_UP test would be useful as well,
- maybe to test if the value has changed at all,
- maybe it's OK, I don't know, but it seems both monitors could be
turned off now.
Thanks,
Jarek P.
>
> ---
> drivers/net/bonding/bond_sysfs.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> --- a/drivers/net/bonding/bond_sysfs.c
> +++ b/drivers/net/bonding/bond_sysfs.c
> @@ -997,6 +997,8 @@ static ssize_t bonding_store_miimon(stru
> ": %s: Setting MII monitoring interval to %d.\n",
> bond->dev->name, new_value);
> bond->params.miimon = new_value;
> + if (bond->params.miimon == 0)
> + cancel_delayed_work_sync(&bond->mii_work);
> if(bond->params.updelay)
> printk(KERN_INFO DRV_NAME
> ": %s: Note: Updating updelay (to %d) "
> @@ -1026,7 +1028,7 @@ static ssize_t bonding_store_miimon(stru
> cancel_delayed_work_sync(&bond->lb_arp_work);
> }
>
> - if (bond->dev->flags & IFF_UP) {
> + if (bond->params.miimon && (bond->dev->flags & IFF_UP)) {
> /* If the interface is up, we may need to fire off
> * the MII timer. If the interface is down, the
> * timer will get fired off when the open function
>
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
