linux-kernel - Re: [PATCH 05/13] XFS: Fix lock ASSERT on UP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Thu, 19 Apr 2012 14:31:31 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Dave Chinner <david@...morbit.com>
Cc:	Andi Kleen <andi@...stfloor.org>,
	Christoph Hellwig <hch@...radead.org>,
	linux-kernel@...r.kernel.org, Andi Kleen <ak@...ux.intel.com>,
	xfs-masters@....sgi.com
Subject: Re: [PATCH 05/13] XFS: Fix lock ASSERT on UP

On Fri, 30 Mar 2012 15:13:48 +1100
Dave Chinner <david@...morbit.com> wrote:

> On Fri, Mar 30, 2012 at 01:52:01AM +0200, Andi Kleen wrote:
> > On Thu, Mar 29, 2012 at 07:21:14PM -0400, Christoph Hellwig wrote:
> > > On Tue, Mar 27, 2012 at 05:47:09PM -0700, Andi Kleen wrote:
> > > > From: Andi Kleen <ak@...ux.intel.com>
> > > > 
> > > > ASSERT(!spin_is_locked()) doesn't work on UP builds. Replace with a standard
> > > > lockdep_assert_held()
> > > 
> > > The "standard" is assert_spin_locked() - which not only is much cheaper
> > > but also has the advantage of working in non-lockdep builds.
> > 
> > But then you have it unconditional, not just on debug builds.
> 
> And the problem with that is what? There is so little overhead to the
> check it doesn't matter that it is enabled in production kernels...
> 

(old thread)

Perhaps assert_spin_locked() would be better - any advantages of
lockdep_assert_held() don't seem to outweigh the cost of using a new
interface.

And I'm not sure that I buy the performance argument - if an assertion
is in such a hot path, just remove the dang thing.

OTOH, one argument in favour of using lockdep_assert_held() is that
(afaict) it applies to spinlocks and to mutexes and to rw_semaphores. 
Not sure about rwlocks?

Now, having an API which can apply to different types is a bit
unpleasant - this ain't C++.  I think we should overlay
lockdep_assert_held() with a properly-typed API written in C and slap
the lockdep guys.  But the fact that this mechanism applies to all(?)
lock types is attractive.

Anyway, these patches are still floating around in my tree so please
let's finish this off one way or another.

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