Re: [PATCH 0/24] make atomic_read() behave consistently across allarchitectures

From: Chris Snook
Date: Thu Aug 16 2007 - 16:03:41 EST


Ilpo Järvinen wrote:
On Thu, 16 Aug 2007, Herbert Xu wrote:

We've been through that already. If it's a busy-wait it
should use cpu_relax.

I looked around a bit by using some command lines and ended up wondering if these are equal to busy-wait case (and should be fixed) or not:

./drivers/telephony/ixj.c
6674: while (atomic_read(&j->DSPWrite) > 0)
6675- atomic_dec(&j->DSPWrite);

...besides that, there are couple of more similar cases in the same file (with braces)...

atomic_dec() already has volatile behavior everywhere, so this is semantically okay, but this code (and any like it) should be calling cpu_relax() each iteration through the loop, unless there's a compelling reason not to. I'll allow that for some hardware drivers (possibly this one) such a compelling reason may exist, but hardware-independent core subsystems probably have no excuse.

If the maintainer of this code doesn't see a compelling reason not to add cpu_relax() in this loop, then it should be patched.

-- Chris
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/