Differential D11969 Diff 31905 sys/dev/e1000/e1000_80003es2lan.c

Changeset View

Standalone View

sys/dev/e1000/e1000_80003es2lan.c

	Show First 20 Lines • Show All 402 Lines • ▼ Show 20 Lines
	{			{
	u32 swfw_sync;			u32 swfw_sync;
	u32 swmask = mask;			u32 swmask = mask;
	u32 fwmask = mask << 16;			u32 fwmask = mask << 16;
	s32 i = 0;			s32 i = 0;
	s32 timeout = 50;			s32 timeout = 50;

	DEBUGFUNC("e1000_acquire_swfw_sync_80003es2lan");			DEBUGFUNC("e1000_acquire_swfw_sync_80003es2lan");
				ASSERT_NO_LOCKS();
				jtlUnsubmitted Not Done Inline Actions I'm not sure what this check accomplishes. There are numerous problems with these hand-rolled locks. First, precisely because they are NOT enclosed in OS-level locks, you can end up with priority inversions that keep the thread that has the lock from making progress while a higher-priority thread is spinning to try to acquire the lock. Second, I think it is possible for two different threads on two different CPUs to run this code simultaneously and both think they end up owning the lock. (There may be more problems...) I think that what we are acquiring here is a lock that is shared by both the operating system and the firmware on the NIC. If so, it seems to me that the answer is to wrap any of these shared HW/SW locks in OS-level locks. That makes sure that the OS side of things behaves roughly as expected (priority propagation, avoiding races, etc.). jtl: I'm not sure what this check accomplishes. There are numerous problems with these hand-rolled…
				kmacyAuthorUnsubmitted Not Done Inline Actions This checks that non-sleepable locks aren't held across this function. It's alerted me to a number of issues already. I agree with the need for a more complete fix. However, this is incremental. Any real fix will build on this. kmacy: This checks that non-sleepable locks aren't held across this function. It's alerted me to a…
				gallatinUnsubmitted Not Done Inline Actions Are all these horrific swfw_sync routines now guaranteed to be called from the same taskq context? gallatin: Are all these horrific swfw_sync routines now guaranteed to be called from the same taskq…
				kmacyAuthorUnsubmitted Not Done Inline Actions No. They still also need to be called from init, which has to be synchronous - hence the changes. kmacy: No. They still also need to be called from init, which has to be synchronous - hence the…
	while (i < timeout) {			while (i < timeout) {
	if (e1000_get_hw_semaphore_generic(hw))			if (e1000_get_hw_semaphore_generic(hw))
	return -E1000_ERR_SWFW_SYNC;			return -E1000_ERR_SWFW_SYNC;

	swfw_sync = E1000_READ_REG(hw, E1000_SW_FW_SYNC);			swfw_sync = E1000_READ_REG(hw, E1000_SW_FW_SYNC);
	if (!(swfw_sync & (fwmask \| swmask)))			if (!(swfw_sync & (fwmask \| swmask)))
	break;			break;

	▲ Show 20 Lines • Show All 991 Lines • Show Last 20 Lines