From: Christoph Hellwig <hch@infradead.org>
To: Andi Kleen <andi@firstfloor.org>
Cc: Dave Chinner <david@fromorbit.com>,
Christoph Hellwig <hch@infradead.org>,
akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
Andi Kleen <ak@linux.intel.com>,
xfs-masters@oss.sgi.com
Subject: Re: [PATCH 05/13] XFS: Fix lock ASSERT on UP
Date: Fri, 30 Mar 2012 10:10:59 -0400 [thread overview]
Message-ID: <20120330141058.GA19986@infradead.org> (raw)
In-Reply-To: <20120330140457.GH17822@one.firstfloor.org>
On Fri, Mar 30, 2012 at 04:04:57PM +0200, Andi Kleen wrote:
> It's really interesting how much you guys argue for your buggy construct
> which you clearly never tested on a UP build...
spin_is_locked always return 0 on UP builds, and given that XFS only
has !spin_is_locked asserts things will work just fine on UP builds
(not the !CONFIG_SMP and !CONFIG_XFS_DEBUG would be a common
combination).
In fact if you check the archives the one use of assert_spin_locked was
added exactly because some still tripped over the spin_is_locked
behaviour on UP relatively soon.
> Not sure if that is a hot path, but on highly contended locks every cache line
> fetch is quite expensive on larger systems.
It's not an overly contended lock.
> also I doubt the thing really catches bugs, and if it did you would be
> probably better off with a sparse notation or so.
This one probably doesn't - the first occurance is just after a blocking
allocation, and the second one just before taking the lock. So they
probably could as well just be removed.
The point that still stands is that lockdep_assert_held isn't actually
a useful replacement.
next prev parent reply other threads:[~2012-03-30 14:11 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-03-28 0:47 spin-is-locked is evil patchkit v2 Andi Kleen
2012-03-28 0:47 ` [PATCH 01/13] block: use lockdep_assert_held for queue locking Andi Kleen
2012-03-30 10:33 ` Jens Axboe
2012-03-28 0:47 ` [PATCH 02/13] sgi-xp: Use lockdep_assert_held Andi Kleen
2012-03-28 0:47 ` [PATCH 03/13] ada152x: Remove broken usage of spin_is_locked Andi Kleen
2012-03-28 0:47 ` [PATCH 04/13] staging/zmem: Use lockdep_assert_held instead " Andi Kleen
2012-03-28 0:47 ` [PATCH 05/13] XFS: Fix lock ASSERT on UP Andi Kleen
2012-03-29 23:21 ` Christoph Hellwig
2012-03-29 23:52 ` Andi Kleen
2012-03-30 4:13 ` Dave Chinner
2012-03-30 14:04 ` Andi Kleen
2012-03-30 14:10 ` Christoph Hellwig [this message]
2012-04-19 21:31 ` Andrew Morton
2012-03-28 0:47 ` [PATCH 06/13] huge-memory: Use lockdep_assert_held Andi Kleen
2012-03-28 0:47 ` [PATCH 07/13] futex: Use lockdep_assert_held() for lock checking Andi Kleen
2012-03-28 0:47 ` [PATCH 08/13] irda: remove spin_is_locked Andi Kleen
2012-03-28 1:32 ` David Miller
2012-03-28 0:47 ` [PATCH 09/13] usb: gadget: f_fs: Remove lock is held before freeing checks Andi Kleen
2012-04-10 10:42 ` Felipe Balbi
2012-03-28 0:47 ` [PATCH 10/13] smsc911x: Use lockdep_assert_held instead of home grown buggy construct Andi Kleen
2012-03-28 1:32 ` David Miller
2012-03-28 0:47 ` [PATCH 11/13] Add a discussion on why spin_is_locked() is bad to spinlocks.txt Andi Kleen
2012-03-28 8:48 ` Wolfram Sang
2012-03-28 9:07 ` Andi Kleen
2012-03-28 0:47 ` [PATCH 12/13] Add a kerneldoc comment to spin_is_locked() that discourages its usage Andi Kleen
2012-03-28 8:49 ` Wolfram Sang
2012-03-28 0:47 ` [PATCH 13/13] checkpatch: Check for spin_is_locked Andi Kleen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20120330141058.GA19986@infradead.org \
--to=hch@infradead.org \
--cc=ak@linux.intel.com \
--cc=akpm@linux-foundation.org \
--cc=andi@firstfloor.org \
--cc=david@fromorbit.com \
--cc=linux-kernel@vger.kernel.org \
--cc=xfs-masters@oss.sgi.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.