Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 20 Feb 2007 09:06:08 -0800
From:      Sam Leffler <sam@errno.com>
To:        Luigi Rizzo <luigi@FreeBSD.org>
Cc:        cvs-src@FreeBSD.org, src-committers@FreeBSD.org, cvs-all@FreeBSD.org
Subject:   Re: cvs commit: src/sys/dev/iwi if_iwi.c if_iwivar.h
Message-ID:  <45DB2A80.1060404@errno.com>
In-Reply-To: <200702201545.l1KFjxF8052100@repoman.freebsd.org>
References:  <200702201545.l1KFjxF8052100@repoman.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
Luigi Rizzo wrote:

>   + add debugging code IWI_LOCK_ASSERT() to detect missing locks.
>     These only do a printf, and should go away once we figure out why
>     the driver sometimes freezes the system due to a (yet unidentified)
>     race condition.

LOCK_ASSERT should do a mtx_assert and not printf.  If you want to
diverge from existing convention in all other drivers please use a
different name; e.g. IWI_LOCK_CHECK.

>   
>   + add a device_printf() in iwi_ioctl() in certain conditions
>     (see comment in the code).  This helps preventing the race condition
>     mentioned above, and makes the system survive. This printf will
>     also go away once fixing this bug is completed.

Please don't commit debug stuff like this; we've discussed this
privately.  This driver has been in use for many months w/o complaints
of the sort you are seeing.

>   
>   + change iwi_getfw() to return 0 on success, 1 on error, consistently
>     with other functions.
>   
>   + fix the argument of a sizeof() in iwi_get_firmware()
>   
>   + use le32toh() to access little-endian fields
>   
>   + simplify error handling in iwi_load_firmware() and iwi_init_locked()
>   
>   The bugs fixed by this commit (the freezing one especially) are serious
>   enough to call for a quick MFC

See above.  Don't pollute stable w/ debug stuff.  Fix the problem first
in  HEAD.

	Sam



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?45DB2A80.1060404>