Re: [RFC 05/12] HID: wiimote: Synchronize wiimote input and hid event handling

Am Donnerstag, 16. Juni 2011, 12:29:40 schrieb David Herrmann:

On Thu, Jun 16, 2011 at 12:18 PM, Oliver Neukum [off-list ref] wrote:

quoted

Am Donnerstag, 16. Juni 2011, 11:20:18 schrieb Jiri Kosina:

quoted

quoted

Here an smp_wmb()
It is redundant because input_register_device takes a spinlock which
implies a barrier, but you'll make the automated checking tools happier.

Yeah, spinlock implies SMP barrier implicitly.

I don't think adding barriers just to make some buggy automatic checker
happy is worth it.

Fine by me as long as the problem is understood.

Does that mean that I can omit the smp_rmb(), too? I do not like
relying on input_register_device to use a spinlock. It makes reading
the code hard, so I added both barriers. Should I remove them again or
keep them?

The full explanation is in Documentation/atomic-ops.txt
Basically, whenever you do

if (!descriptor->flag)
	return -ERR;

descriptor->pointer->member = expression; //or anything else with the pointer

you need smp_rmb() in between

If you do

if (!descriptor->flag)
	return -ERR;

// no use of pointer here

spin_lock(&lock);

descriptor->pointer->member = expression; //or anything else with the pointer

You are save and can do without the smp_rmb()

But, you still need smp_rmb() if you do:

if (!descriptor->flag)
	return -ERR;

// no use of pointer here

spin_lock(&descriptor->pointer->lock);

All clear? I must say I stopped listing and checking all the places you use
the flag, but I think some were buggy.

	Regards
		Oliver