Re: [PATCH] 1/2 Maple: Update bus driver to allow support of VMU device

On Thu, Mar 20, 2008 at 10:52:27PM +0000, Adrian McMenamin wrote:

On Thu, 2008-03-20 at 15:39 -0700, Andrew Morton wrote:
On Thu, 20 Mar 2008 22:23:17 +0000
Adrian McMenamin <adrian@xxxxxxxxxxxxxxxxxxxxxxxx> wrote:


urgh, down_trylock(). And a secret, undocumented one too.

A trylock is always an exceptional thing. How is *any* reader of this code
supposed to work out what the heck it's doing there? Convert it into
down(), run the code and decrypt the lockdep warnings, I suspect.

<looks>

Nope, I can't see any other lock being held when we call this function.

The trylocks are an utter mystery to me. Please don't write mysterious
code.


OK, I am sure this is my problem but I have no idea why you are
describing down_trylock as undocumented

I'm describing your use of it! I'm sitting here trying to work out why on
earth this code is using the highly unusual (and highly suspicious) trylock
idiom and this is far from clear.


OK, I understand your point now. Some comments wouldn't go amiss, for
sure.

Other than the fact the locking is bizarre and utterly incomprehensible,
why are you using a semaphore here at all? You're effectively using it as
a mutex, and you really should be using that API instead, especially for
the debugging capabilities. This patch set looks like an excellent stress
test for the mutex debugging code.
--
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/