Re: cvs commit: src/sys/dev/iwi if_iwi.c if_iwivar.h

From: Sam Leffler <sam_at_errno.com>
Date: Tue, 20 Feb 2007 09:06:08 -0800
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
Received on Tue Feb 20 2007 - 17:06:09 UTC