On Friday 31 August 2007, Larry Finger wrote:
> This patch fixes a problem with work queues during shutdown. The bug
> seems to be responsible for the boot-time crashes reported in
> http://bugzilla.kernel.org/show_bug.cgi?id=8937.
>
> Signed-off-by: Larry Finger <[EMAIL PROTECTED]>
> ---
>
> Index: linux-2.6/drivers/net/wireless/bcm43xx/bcm43xx_main.c
> ===================================================================
> --- linux-2.6.orig/drivers/net/wireless/bcm43xx/bcm43xx_main.c
> +++ linux-2.6/drivers/net/wireless/bcm43xx/bcm43xx_main.c
> @@ -3174,6 +3174,9 @@ static void bcm43xx_periodic_every1sec(s
>
> static void do_periodic_work(struct bcm43xx_private *bcm)
> {
> + /* keep from rearming periodic work if shutting down */
> + if (bcm43xx_status(bcm) == BCM43xx_STAT_UNINIT)
> + return;
I think this should be put at the beginning of bcm43xx_periodic_work_handler()
after the mutex locking.
> if (bcm->periodic_state % 120 == 0)
> bcm43xx_periodic_every120sec(bcm);
> if (bcm->periodic_state % 60 == 0)
> @@ -3245,11 +3248,6 @@ static void bcm43xx_periodic_work_handle
> mutex_unlock(&bcm->mutex);
> }
>
> -void bcm43xx_periodic_tasks_delete(struct bcm43xx_private *bcm)
> -{
> - cancel_rearming_delayed_work(&bcm->periodic_work);
> -}
> -
> void bcm43xx_periodic_tasks_setup(struct bcm43xx_private *bcm)
> {
> struct delayed_work *work = &bcm->periodic_work;
> @@ -3335,9 +3333,14 @@ static void bcm43xx_free_board(struct bc
> {
> bcm43xx_rng_exit(bcm);
> bcm43xx_sysfs_unregister(bcm);
> - bcm43xx_periodic_tasks_delete(bcm);
> +
> + /* next 2 steps must be unlocked, else they may deadlock */
> + cancel_work_sync(&bcm->restart_work);
> + cancel_delayed_work_sync(&bcm->periodic_work);
> mutex_lock(&(bcm)->mutex);
> + bcm43xx_set_status(bcm, BCM43xx_STAT_UNINIT);
> +
This status change must be moved above the cancel calls.
> bcm43xx_shutdown_all_wireless_cores(bcm);
> bcm43xx_pctl_set_crystal(bcm, 0);
> mutex_unlock(&(bcm)->mutex);
> @@ -4030,7 +4033,6 @@ static int bcm43xx_net_stop(struct net_d
> err = bcm43xx_disable_interrupts_sync(bcm);
> assert(!err);
> bcm43xx_free_board(bcm);
> - flush_scheduled_work();
>
> return 0;
> }
> @@ -4164,7 +4166,6 @@ static void bcm43xx_chip_reset(struct wo
>
> mutex_lock(&(bcm)->mutex);
> if (bcm43xx_status(bcm) == BCM43xx_STAT_INITIALIZED) {
> - bcm43xx_periodic_tasks_delete(bcm);
Look at the comment below. That also applies here.
> phy = bcm43xx_current_phy(bcm);
> err = bcm43xx_select_wireless_core(bcm, phy->type);
> if (!err)
> Index: linux-2.6/drivers/net/wireless/bcm43xx/bcm43xx_main.h
> ===================================================================
> --- linux-2.6.orig/drivers/net/wireless/bcm43xx/bcm43xx_main.h
> +++ linux-2.6/drivers/net/wireless/bcm43xx/bcm43xx_main.h
> @@ -122,7 +122,6 @@ void bcm43xx_wireless_core_reset(struct
> void bcm43xx_mac_suspend(struct bcm43xx_private *bcm);
> void bcm43xx_mac_enable(struct bcm43xx_private *bcm);
>
> -void bcm43xx_periodic_tasks_delete(struct bcm43xx_private *bcm);
> void bcm43xx_periodic_tasks_setup(struct bcm43xx_private *bcm);
>
> void bcm43xx_controller_restart(struct bcm43xx_private *bcm, const char
> *reason);
> Index: linux-2.6/drivers/net/wireless/bcm43xx/bcm43xx_sysfs.c
> ===================================================================
> --- linux-2.6.orig/drivers/net/wireless/bcm43xx/bcm43xx_sysfs.c
> +++ linux-2.6/drivers/net/wireless/bcm43xx/bcm43xx_sysfs.c
> @@ -327,7 +327,6 @@ static ssize_t bcm43xx_attr_phymode_stor
> goto out;
> }
>
> - bcm43xx_periodic_tasks_delete(bcm);
Well, this is tricky crap code.
This breaks it.
Look at the !err case below. We re-enable pwork there.
We only do it for !err. So when core select failed, we
end up without pwork, which is what we want.
I think it might be OK to simply remove the delete _and_
the setup below, as select core sets status to UNINIT when
it failed. So pwork will bail out early (if you change your
patch above as I suggested).
> mutex_lock(&(bcm)->mutex);
> err = bcm43xx_select_wireless_core(bcm, phytype);
> if (!err)
>
>
PS: Yes, this is crap code. :) All these cornercases.
_______________________________________________
Bcm43xx-dev mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev
