From: Pavel Machek <pavel@ucw.cz>
To: Nick Piggin <nickpiggin@yahoo.com.au>
Cc: Andrew Morton <akpm@linux-foundation.org>, linux-kernel@vger.kernel.org
Subject: Re: [patch 1/5] wait: use lock bitops for __wait_on_bit_lock
Date: Thu, 11 Oct 2007 20:56:42 +0000 [thread overview]
Message-ID: <20071011205641.GA3864@ucw.cz> (raw)
In-Reply-To: <200710251217.10704.nickpiggin@yahoo.com.au>
Hi!
> > Sorry, I'm just not going to apply a patch like that.
> >
> > I mean, how the heck is anyone else supposed to understand what you're up
> > to?
>
> Hmm, I might just withdraw this patch 1/5. This is generally a slowpath,
> and it's hard to guarantee that any exported user doesn't rely on the
> full barrier here (not that they would know whether they do or not, let
> alone document the fact).
>
> I think all the others are pretty clear, though? (read on if no)
...
> > There's a bit of documentation in Documentation/atomic_ops.txt
> > (probably enough, I guess) but it is totally unobvious to 98.3% of kernel
> > developers when they should use test_and_set_bit() versus
> > test_and_set_bit_lock() and it is far too much work to work out why it was
> > used in __wait_on_bit_lock(), whether it is correct, what advantages it
> > brings and whether and where others should emulate it.
>
> If you set a bit for the purpose of opening a critical section, then
> you can use this. And clear_bit_unlock to close it.
>
> The advantages are that it is faster, and the hapless driver writer
> doesn't have to butcher or forget about doing the correct
> smp_mb__before_clear_bit(), or have reviewers wondering exactly WTF
> that barrier is for, etc.
Actually, I'd expect *_lock() ro be slower than *()...
> Basically, it is faster, harder to get wrong, and more self-docuemnting.
So I'd not call it self-documenting.
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
next prev parent reply other threads:[~2007-11-01 14:34 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-10-24 8:13 [patch 0/5] lock bitops patches npiggin
2007-10-24 8:13 ` [patch 1/5] wait: use lock bitops for __wait_on_bit_lock npiggin
2007-10-25 1:14 ` Andrew Morton
2007-10-25 2:17 ` Nick Piggin
2007-10-11 20:56 ` Pavel Machek [this message]
2007-10-24 8:13 ` [patch 2/5] tasklet: use lock bitops for tasklet lock npiggin
2007-10-24 8:13 ` [patch 3/5] mm: rename page lock npiggin
2007-10-24 8:13 ` [patch 4/5] mm: use lock bitops for " npiggin
2007-10-24 8:13 ` [patch 5/5] fs: use lock bitops for the buffer lock npiggin
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=20071011205641.GA3864@ucw.cz \
--to=pavel@ucw.cz \
--cc=akpm@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=nickpiggin@yahoo.com.au \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox