On Wed, 28 Feb 2007 10:12:01 +0100 (CET) Jaroslav Kysela <[EMAIL PROTECTED]> wrote:
> Hi, > > please, review and apply to mm tree for further testing. The patch > is also available at > ftp://ftp.alsa-project.org/pub/kernel-patches/bonding-workqueue.patch . Please cc netdev@vger.kernel.org on net-related patches, thanks. > Thank you, > Jaroslav > > ================== > bonding: replace system timer with work queue > > This patch replaces system timer with work queue in monitor functions. > The reason for this change is that bonding handlers calls various > sleeping functions from the timer handler which is not allowed. Which sleeping functions? I'd have expected the kernel to spew runtime warnings when this happens, but I don't recall any such reports. > Because we cannot share the main workqueue threads (rtnl_lock is used > also in linkwatch_event) - new bond workqueue thread is created. > > Signed-off-by: Jaroslav Kysela <[EMAIL PROTECTED]> > > diff -rupN linux-2.6.20.orig/drivers/net/bonding/bond_3ad.c > linux-2.6.20/drivers/net/bonding/bond_3ad.c > --- linux-2.6.20.orig/drivers/net/bonding/bond_3ad.c 2007-02-04 > 19:44:54.000000000 +0100 > +++ linux-2.6.20/drivers/net/bonding/bond_3ad.c 2007-02-28 > 09:19:43.831369202 +0100 > @@ -2097,8 +2097,10 @@ void bond_3ad_unbind_slave(struct slave > * times out, and it selects an aggregator for the ports that are yet not > * related to any aggregator, and selects the active aggregator for a bond. > */ > -void bond_3ad_state_machine_handler(struct bonding *bond) > +void bond_3ad_state_machine_handler(struct work_struct *work) > { > + struct ad_bond_info *ad_info = container_of(work, struct ad_bond_info, > ad_work.work); > + struct bonding *bond = (struct bonding *)((char *)ad_info - > offsetof(struct bonding, ad_info)); We can use containers_of here too? > -void bond_alb_monitor(struct bonding *bond) > +void bond_alb_monitor(struct work_struct *work) > { > - struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond)); > + struct alb_bond_info *bond_info = container_of(work, struct > alb_bond_info, alb_work.work); > + struct bonding *bond = (struct bonding *)((char *)bond_info - > offsetof(struct bonding, alb_info)); And here. > + cancel_rearming_delayed_workqueue(bond_wq, > &(BOND_AD_INFO(bond).ad_work)); > break; > case BOND_MODE_TLB: > case BOND_MODE_ALB: > - del_timer_sync(&(BOND_ALB_INFO(bond).alb_timer)); > + cancel_rearming_delayed_workqueue(bond_wq, > &(BOND_ALB_INFO(bond).alb_work)); > break; > default: > break; > @@ -4289,6 +4272,14 @@ static int bond_init(struct net_device * > rwlock_init(&bond->lock); > rwlock_init(&bond->curr_slave_lock); > > + /* initialize work */ > + INIT_DELAYED_WORK(&bond->mii_work, (void *)&bond_mii_monitor); > + if (params->mode == BOND_MODE_ACTIVEBACKUP) { > + INIT_DELAYED_WORK(&bond->arp_work, (void > *)&bond_activebackup_arp_mon); > + } else { > + INIT_DELAYED_WORK(&bond->arp_work, (void > *)&bond_loadbalance_arp_mon); > + } Can we lose the unneeded braces, the unneeded typecasts and fit the code into 80 cols? <does all that> yup. > bond->params = *params; /* copy params struct */ > > /* Initialize pointers */ > @@ -4782,6 +4773,12 @@ static int __init bonding_init(void) > goto err; > } > > + bond_wq = create_singlethread_workqueue("bond"); > + if (bond_wq == NULL) { > + res = -ENOMEM; > + goto err; > + } > + > res = bond_create_sysfs(); > if (res) > goto err; > @@ -4807,6 +4804,7 @@ static void __exit bonding_exit(void) > > rtnl_lock(); > bond_free_all(); > + destroy_workqueue(bond_wq); > bond_destroy_sysfs(); > rtnl_unlock(); Are you sure that all pending delayed works have been cancelled when we destroy this workqueue? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/