All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Jiri Kosina <jkosina@suse.cz>
Cc: Jeremy Fitzhardinge <jeremy@goop.org>,
	Jeremy Fitzhardinge <jeremy@xensource.com>,
	linux-kernel@vger.kernel.org,
	Thomas Gleixner <tglx@linutronix.de>,
	"H. Peter Anvin" <hpa@zytor.com>
Subject: Re: [PATCH] x86: remove byte locks
Date: Thu, 15 Jan 2009 11:58:08 +0100	[thread overview]
Message-ID: <20090115105808.GA16253@elte.hu> (raw)
In-Reply-To: <alpine.LNX.1.10.0901151147420.5377@jikos.suse.cz>


* Jiri Kosina <jkosina@suse.cz> wrote:

> On Tue, 13 Jan 2009, Jeremy Fitzhardinge wrote:
> 
> > > Why can't this just be somewhere in documentation? (possibly even with 
> > > the byte locks code as a reference).
> > Because Ingo's compil-o-matic will never fail on a documentation error.
> 
> Hmm, I have always considered the "we don't accept any code that would 
> have zero in-kernel users" rule as a quite reasonable one, at least in 
> order to prevent from bloat and code getting confusing.
> But apparently it's not the intention here.
> 
> > > It is IMHO just totally confusing to have a spinlock implementation that is
> > > not used at all in the tree. It took me quite some time to go through this
> > > until I finally figured out that this code is actually never used.
> > > Currently, on first sight it might seem that byte locks are used whenever
> > > CONFIG_PARAVIRT is set, which is not true.
> > Well, a comment next to the code explaining the rationale probably 
> > wouldn't go astray.
> 
> I still strongly feel that if the only purpose of the code in kernel is 
> "to provide example", then it belongs to documentation.
> 
> > > And apparently even Linus got confused by this, which also tells us
> > > something by itself, see [1].
> > > [1] http://marc.info/?l=linux-kernel&m=123144211719754&w=2
> > It tells us that Linus couldn't give a rat's arse about virtualization, 
> > which is just something we have to cope with ;)
> 
> I am afraid this has nothing to do with virtualization. It's simply 
> confusing when looking at the code.

i'd tend to agree, that area of code is quite complex already.

	Ingo

  reply	other threads:[~2009-01-15 10:58 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-12 11:53 [PATCH] x86: remove byte locks Jiri Kosina
2009-01-12 12:07 ` Ingo Molnar
2009-01-12 12:36   ` Jiri Kosina
2009-01-12 12:44     ` Ingo Molnar
2009-01-13 23:10       ` Jeremy Fitzhardinge
2009-01-13 23:22         ` Jiri Kosina
2009-01-13 23:52           ` Jeremy Fitzhardinge
2009-01-15 10:55             ` Jiri Kosina
2009-01-15 10:58               ` Ingo Molnar [this message]
2009-01-20 16:12                 ` Jiri Kosina
2009-01-20 16:14                   ` Ingo Molnar

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=20090115105808.GA16253@elte.hu \
    --to=mingo@elte.hu \
    --cc=hpa@zytor.com \
    --cc=jeremy@goop.org \
    --cc=jeremy@xensource.com \
    --cc=jkosina@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tglx@linutronix.de \
    /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.