Messages in this thread | ![/](/images/icornerl.gif) | | Date | Fri, 23 Jun 2006 12:18:40 +0200 | From | Ingo Molnar <> | Subject | Re: [patch 36/61] lock validator: special locking: serial |
| |
* Andrew Morton <akpm@osdl.org> wrote:
> btw, I was looking at this change:
> @@ -1003,6 +1003,7 @@ unsigned ata_exec_internal(struct ata_de > unsigned int err_mask; > int rc; > > + init_completion(&wait); > spin_lock_irqsave(ap->lock, flags); > > /* no internal command while frozen */ > > That local was already initialised with DEFINE_COMPLETION(). Am > surprised that an init_completion() also was needed?
That's a fundamental problem of DECLARE_COMPLETION() done on the kernel stack - it does build-time initialization with no opportunity to inject any runtime logic. (which lockdep would need. Maybe i missed some clever way to add a runtime callback into the initialization? [*])
Btw., there is no danger from missing the initialization of a wait structure: lockdep will detect "uninitialized" on-stack locks and will complain about it and turn itself off. [this happened a few times during development - that's how those init_completion() calls got added]
But at a minimum these initializations need to become lockdep-specific key-reinits - otherwise there will be impact to non-lockdep kernels too.
Ingo
[*] the only solution i can see is to introduce DECLARE_COMPLETION_ONSTACK(), which could call a function with &wait passed in, where that function would return with a structure. The macro magic would resolve to something like:
struct completion wait = lockdep_init_completion(&wait);
and thus the structure would be initialized. But this method cannot be used for static scope uses of DECLARE_COMPLETION, because it's not a constant initializer. So we'd definitely have to make a distinction in terms of _ONSTACK(). Is there really no compiler feature that could help us out here? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| ![\](/images/icornerr.gif) |