[ofa-general] Incorrect atomic usage in ipath driver

Benjamin Herrenschmidt benh at kernel.crashing.org
Mon May 7 20:21:56 PDT 2007


Hi !

So I see this construct:

	/* There is already a thread processing this queue. */
	if (test_and_set_bit(0, &dd->ipath_rcv_pending))
		goto bail;

	.../...

done:
	clear_bit(0, &dd->ipath_rcv_pending);
	smp_mb__after_clear_bit();

So that's basically an attempt at doing a spinlock. The problem is your
barrier is wrong at the end. Better would be:


done:
	smp_mb__before_clear_bit();
	clear_bit(0, &dd->ipath_rcv_pending);


Though it's still less optimal that doing:

	if (!spin_trylock(...))
		goto bail;

	.../...

done:
	spin_unlock(...)

If you really want to stick to bitops, then you may want to look at
Nick's upcoming patches adding some bitops with appropriate lock
semantics.

Cheers,
Ben.





More information about the general mailing list