lkml.org 
[lkml]   [2007]   [Mar]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] bonding: replace system timer with work queue
    On Wed, 28 Feb 2007 10:12:01 +0100 (CET) Jaroslav Kysela <perex@suse.cz> 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 <perex@suse.cz>
    >
    > 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 majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2007-03-01 08:39    [W:0.039 / U:59.956 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site