netdev - Re: 2.6.25rc7 lockdep trace

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 12 Jun 2008 06:13:34 +0000
From:	Jarek Poplawski <jarkao2@...il.com>
To:	David Miller <davem@...emloft.net>
Cc:	johannes@...solutions.net, davej@...emonkey.org.uk,
	netdev@...r.kernel.org
Subject: Re: 2.6.25rc7 lockdep trace

On 11-06-2008 07:40, David Miller wrote:
...
> diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
> index 4b46e68..48ed410 100644
> --- a/drivers/net/bnx2.c
> +++ b/drivers/net/bnx2.c
> @@ -5907,12 +5907,7 @@ bnx2_close(struct net_device *dev)
>  	struct bnx2 *bp = netdev_priv(dev);
>  	u32 reset_code;
>  
> -	/* Calling flush_scheduled_work() may deadlock because
> -	 * linkwatch_event() may be on the workqueue and it will try to get
> -	 * the rtnl_lock which we are holding.
> -	 */
> -	while (bp->in_reset_task)
> -		msleep(1);
> +	cancel_work_sync(&bp->reset_task);

It seems after this change bp->in_reset_task could be removed totally
from the code.

...
> diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c
> index 4e28002..5c71098 100644
> --- a/drivers/net/smc911x.c
> +++ b/drivers/net/smc911x.c
> @@ -1531,16 +1531,8 @@ static int smc911x_close(struct net_device *dev)
>  	if (lp->phy_type != 0) {
>  		/* We need to ensure that no calls to
>  		 * smc911x_phy_configure are pending.
> -
> -		 * flush_scheduled_work() cannot be called because we
> -		 * are running with the netlink semaphore held (from
> -		 * devinet_ioctl()) and the pending work queue
> -		 * contains linkwatch_event() (scheduled by
> -		 * netif_carrier_off() above). linkwatch_event() also
> -		 * wants the netlink semaphore.
>  		 */
> -		while (lp->work_pending)
> -			schedule();
> +		cancel_work_sync(&lp->phy_configure);

... likewise lp->work_pending,


>  		smc911x_phy_powerdown(dev, lp->mii.phy_id);
>  	}
>  
> diff --git a/drivers/net/smc91x.c b/drivers/net/smc91x.c
> index a188e33..5dbe4d3 100644
> --- a/drivers/net/smc91x.c
> +++ b/drivers/net/smc91x.c
> @@ -1016,15 +1016,8 @@ static void smc_phy_powerdown(struct net_device *dev)
>  
>  	/* We need to ensure that no calls to smc_phy_configure are
>  	   pending.
> -
> -	   flush_scheduled_work() cannot be called because we are
> -	   running with the netlink semaphore held (from
> -	   devinet_ioctl()) and the pending work queue contains
> -	   linkwatch_event() (scheduled by netif_carrier_off()
> -	   above). linkwatch_event() also wants the netlink semaphore.
>  	*/
> -	while(lp->work_pending)
> -		yield();
> +	cancel_work_sync(&lp->phy_configure);

... likewise lp->work_pending.

...
> diff --git a/drivers/net/wireless/hostap/hostap_main.c b/drivers/net/wireless/hostap/hostap_main.c
> index 20d387f..57b800f 100644
> --- a/drivers/net/wireless/hostap/hostap_main.c
> +++ b/drivers/net/wireless/hostap/hostap_main.c
> @@ -682,7 +682,11 @@ static int prism2_close(struct net_device *dev)
>  		netif_device_detach(dev);
>  	}
>  
> -	flush_scheduled_work();
> +	cancel_work_sync(&local->reset_queue);
> +	cancel_work_sync(&local->set_multicast_list_queue);
> +	cancel_work_sync(&local->set_tim_queue);
> +	cancel_work_sync(&local->info_queue);

local->info_queue probably needs some #ifndef like in hostap_info_init()
or could be skipped here at all (waiting for remove).

> +	cancel_work_sync(&local->comms_qual_update);
>  
>  	module_put(local->hw_module);

Regards,
Jarek P.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html