cvs commit: src/sys/pci if_sis.c

Nate Lawson nate at root.org
Tue May 6 13:57:48 PDT 2003


On Mon, 5 May 2003, Olivier Houchard wrote:
>   Modified files:
>     sys/pci              if_sis.c
>   Log:
>   Don't call timeout() in sis_tick(), this is done earlier by mii_tick(), and it
>   leads to a panic at unload time, as we own 2 instances of callout and
>   untimeout() only one.
>   Will I'm there, remove a call to callout_handler_init(), one is enough.
>
>   Reviewed by:    wpaul

Comments below.

> --- src/sys/pci/if_sis.c:1.74	Mon Apr 21 11:34:04 2003
> +++ src/sys/pci/if_sis.c	Mon May  5 19:00:01 2003
> @@ -29,7 +29,7 @@
>   * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
>   * THE POSSIBILITY OF SUCH DAMAGE.
>   *
> - * $FreeBSD: /repoman/r/ncvs/src/sys/pci/if_sis.c,v 1.74 2003/04/21 18:34:04 imp Exp $
> + * $FreeBSD: /repoman/r/ncvs/src/sys/pci/if_sis.c,v 1.75 2003/05/06 02:00:01 cognet Exp $
>   */
>
>  /*
> @@ -58,7 +58,7 @@
>   */
>
>  #include <sys/cdefs.h>
> -__FBSDID("$FreeBSD: /repoman/r/ncvs/src/sys/pci/if_sis.c,v 1.74 2003/04/21 18:34:04 imp Exp $");
> +__FBSDID("$FreeBSD: /repoman/r/ncvs/src/sys/pci/if_sis.c,v 1.75 2003/05/06 02:00:01 cognet Exp $");
>
>  #include <sys/param.h>
>  #include <sys/systm.h>

Why do we have both the RCS id and FBSDID?  I thought only one was enough.

> @@ -1374,8 +1374,6 @@
>  		goto fail;
>  	}
>
> -	callout_handle_init(&sc->sis_stat_ch);
> -
>  	/*
>  	 * Call MI attach routine.
>  	 */

I would rather you deleted the first call to callout_handle_init() instead
(after the "Inform the world" comment).  Reasons include poor style for
the first one and uniformity with other sys/pci/if_* drivers which do the
init right before ether_ifattach().

> @@ -1747,8 +1745,6 @@
>  		if (ifp->if_snd.ifq_head != NULL)
>  			sis_start(ifp);
>  	}
> -
> -	sc->sis_stat_ch = timeout(sis_tick, sc, hz);
>
>  	SIS_UNLOCK(sc);
>

Can you check the other sys/pci/if_* devices to see if they have the same
bug?  I think they all re-add the timeout at the end of *_tick().

-Nate


More information about the cvs-all mailing list