All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Rusty Russell <rusty@rustcorp.com.au>,
	Nick Piggin <npiggin@kernel.dk>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Andi Kleen <ak@linux.intel.com>,
	"Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com>,
	linux-fsdevel@vger.kernel.org,
	Peter Zijlstra <a.p.zijlstra@chello.nl>
Subject: Re: [PATCH] cpumask: fix lg_lock/br_lock.
Date: Wed, 29 Feb 2012 09:29:44 +0100	[thread overview]
Message-ID: <20120229082944.GA10425@elte.hu> (raw)
In-Reply-To: <20120228132719.f375071a.akpm@linux-foundation.org>


* Andrew Morton <akpm@linux-foundation.org> wrote:

> On Tue, 28 Feb 2012 09:43:59 +0100
> Ingo Molnar <mingo@elte.hu> wrote:
> 
> > This patch should also probably go upstream through the 
> > locking/lockdep tree? Mind sending it us once you think it's 
> > ready?
> 
> Oh goody, that means you own 
> http://marc.info/?l=linux-kernel&m=131419353511653&w=2.
>
> I do think the brlock code is sick, but Nick has turned into 
> an ex-Nick and nobody works on it.

The main problem, highlighted by the above soft lockup report, 
is that lglocks and brlocks go outside the regular spinlock 
facilities and thus avoid quite a bit of generic lock debugging 
for no good reason. Those patches should never have gone 
upstream in their incomplete form via the VFS tree.

As part of any cleanup they should first be converted from 
arch_spinlock_t to regular spinlock_t - I bet if that is done 
then that not only simplifies the wrappers massively, it also 
turns the above soft lockup report into a nice, actionable 
lockdep splat.

Andi's deficient patch does not address that main shortcoming of 
brlocks/lglocks, it just addresses a symptom - while introducing 
a handful of new symptoms of suckage.

I'll review and process resubmitted patches from Andi if he 
wants to submit a proper, complete series - but there's a 
quality threshold and in this case I'd rather keep 1970's 
preprocessor code that sucks very visibly and possibly attracts 
someone with a clue than have a sloppy patch hiding the deeper 
design problems done by a clueless person who is also openly 
hostile towards the concept of producing quality code.

Thanks,

	Ingo

  parent reply	other threads:[~2012-02-29  8:30 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-27 23:22 [PATCH] cpumask: fix lg_lock/br_lock Rusty Russell
2012-02-27 23:53 ` Andrew Morton
2012-02-27 23:53   ` Andrew Morton
2012-02-28  8:43   ` Ingo Molnar
2012-02-28 11:25     ` Andi Kleen
2012-02-28 12:51       ` Ingo Molnar
2012-02-28 21:27     ` Andrew Morton
2012-02-29  5:44       ` Srivatsa S. Bhat
2012-02-29  9:17         ` Ingo Molnar
2012-02-29 11:12           ` Srivatsa S. Bhat
2012-03-01  7:38             ` Ingo Molnar
2012-03-01  9:15               ` Srivatsa S. Bhat
2012-03-01  9:45                 ` Ingo Molnar
2012-03-01  9:56                   ` Srivatsa S. Bhat
2012-03-01  8:12             ` Srivatsa S. Bhat
2012-03-01  8:24               ` Srivatsa S. Bhat
2012-03-01  8:12               ` Srivatsa S. Bhat
2012-03-01  8:15               ` [PATCH 1/3] CPU hotplug: Fix issues with callback registration Srivatsa S. Bhat
2012-03-01  8:27                 ` Srivatsa S. Bhat
2012-03-01  8:15                 ` Srivatsa S. Bhat
2012-03-01  8:16               ` [PATCH 2/3] CPU hotplug, arch/powerpc: Fix CPU hotplug " Srivatsa S. Bhat
2012-03-01  8:28                 ` Srivatsa S. Bhat
2012-03-01  8:16                 ` Srivatsa S. Bhat
2012-03-01  8:18               ` [PATCH 3/3] CPU hotplug, arch/sparc: " Srivatsa S. Bhat
2012-03-01  8:30                 ` Srivatsa S. Bhat
2012-03-01  8:18                 ` Srivatsa S. Bhat
2012-02-29  8:29       ` Ingo Molnar [this message]
2012-02-29  8:58         ` [PATCH] cpumask: fix lg_lock/br_lock Peter Zijlstra
2012-02-29  9:32           ` Ingo Molnar
2012-02-28 11:24   ` Andi Kleen
2012-03-05  7:02     ` Rusty Russell
2012-03-05  7:03     ` [PATCH 1/3] lglock: remove online variants of lock Rusty Russell
2012-04-20 11:12       ` Nick Piggin
2012-03-05  7:04     ` [PATCH 2/3] brlocks/lglocks: API cleanups Rusty Russell
2012-03-05  7:05     ` [PATCH 3/3] brlocks/lglocks: turn into functions Rusty Russell
2012-04-20 11:21       ` Nick Piggin
2012-05-07  3:39         ` Rusty Russell
2012-05-07  5:46           ` Al Viro
2012-05-08  3:59             ` [PATCH 1/3] lglock: remove online variants of lock Rusty Russell
2012-05-08  4:50               ` Al Viro
2012-05-08  6:12                 ` Rusty Russell
2012-05-08  4:02             ` [PATCH 2/3] brlocks/lglocks: API cleanups Rusty Russell
2012-05-08  4:02             ` [PATCH 3/3] brlocks/lglocks: turn into functions Rusty Russell
2012-05-09  7:35           ` Nick Piggin
2012-05-09  7:35             ` Nick Piggin

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=20120229082944.GA10425@elte.hu \
    --to=mingo@elte.hu \
    --cc=a.p.zijlstra@chello.nl \
    --cc=ak@linux.intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=npiggin@kernel.dk \
    --cc=rusty@rustcorp.com.au \
    --cc=srivatsa.bhat@linux.vnet.ibm.com \
    --cc=viro@zeniv.linux.org.uk \
    /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.