Re: [PATCH] sparse fix: add many lock annotations

[Alexey Dobriyan]

On Wed, Nov 22, 2006 at 12:11:46AM -0800, Ira Snyder wrote:
> This patch adds many lock annotations to the kernel source to quiet
> warnings from sparse. In almost every case, it quiets the warning caused
> by locks that are intentionally grabbed in one function and released in
> another.
>
> In the other cases, __acquire() and __release() are used to make sparse
> believe that a lock was grabbed (even though it was not), in order to
> make all exit points have equal lock counts. These follow the style in
> kernel/sched.c.

> --- a/arch/i386/kernel/smp.c
> +++ b/arch/i386/kernel/smp.c
> @@ -507,11 +507,13 @@ struct call_data_struct {
>  };
>
>  void lock_ipi_call_lock(void)
> +__acquires(call_lock)
>  {
>  	spin_lock_irq(&call_lock);
>  }
>
>  void unlock_ipi_call_lock(void)
> +__releases(call_lock)
>  {
>  	spin_unlock_irq(&call_lock);
>  }

Wrong place. Prototypes should be marked instead. How else would you
know about:

	lock_ipi_call_lock();
	if (foo)
		return -E;
	lock_ipi_call_lock();

on another compilation unit?

> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -1004,6 +1004,7 @@ EXPORT_SYMBOL(max_cstate);
>   */
>
>  acpi_cpu_flags acpi_os_acquire_lock(acpi_spinlock lockp)
> +__acquires(lockp)
>  {
>  	acpi_cpu_flags flags;
>  	spin_lock_irqsave(lockp, flags);
> @@ -1015,6 +1016,7 @@ acpi_cpu_flags acpi_os_acquire_lock(acpi
>   */
>
>  void acpi_os_release_lock(acpi_spinlock lockp, acpi_cpu_flags flags)
> +__releases(lockp)
>  {
>  	spin_unlock_irqrestore(lockp, flags);
>  }

Again, wrong. IMO, sparse should deduce itself that lock is grabbed in such
trivial cases.

-
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/