All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: David Miller <davem@davemloft.net>
Cc: linux-kernel@vger.kernel.org, wli@holomorphy.com,
	sparclinux@vger.kernel.org, manfred@colorfullife.com,
	akpm@linux-foundation.org
Subject: Re: [PATCH] prevent sparc64 from invoking irq handlers on offline
Date: Wed, 03 Sep 2008 00:42:11 +0000	[thread overview]
Message-ID: <20080903004211.GD6748@linux.vnet.ibm.com> (raw)
In-Reply-To: <20080902.171630.193505044.davem@davemloft.net>

On Tue, Sep 02, 2008 at 05:16:30PM -0700, David Miller wrote:
> From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> Date: Sun, 31 Aug 2008 10:33:49 -0700
> 
> > Make sparc64 refrain from clearing a given to-be-offlined CPU's bit in the
> > cpu_online_mask until it has processed pending irqs.  This change
> > prevents other CPUs from being blindsided by an apparently offline CPU
> > nevertheless changing globally visible state.
> > 
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> 
> I wonder what the 'call_lock' thing protects :-)

I didn't look till now.  ;-)

> That lock is a cobweb from the sparc64 code before I switched it over
> to use the generic smp_call_function() code in kernel/smp.c
> 
> So this lock doesn't protect anything any more.

It is a static defined in arch/sparc64/kernel/smp.c, and is used only
when setting and clearing bits in cpu_online_mask.

> kernel/smp.c has a call_function_lock, which isn't marked static
> but isn't declared in any header file.

It is exported via ipi_call_lock(), ipi_call_unlock(), and friends.
A few architectures use it to exclude some of the IPI code while
setting (but not clearing) bits in cpu_online_map.  These particular
architectures have a phase during CPU offlining where they drain
pending interrupts, so perhaps that is why they only worry about
onlining?

> My instinct is that the intention is that I could use this lock
> for the synchronization previously provided by sparc64's local
> "call_lock", and it even seems the author of kernel/smp.c intended
> this kind of usage.
> 
> Anyways, if this code is still using the worthless call_lock, it
> isn't protecting against anything.

Agreed.

> So I'd like to hold off on this patch until this locking issue is
> resolved.

OK, it is your architecture.  But in the meantime, sparc64 can take
interrupts on CPUs whose cpu_online_map bits have been cleared.

							Thanx, Paul

WARNING: multiple messages have this Message-ID (diff)
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: David Miller <davem@davemloft.net>
Cc: linux-kernel@vger.kernel.org, wli@holomorphy.com,
	sparclinux@vger.kernel.org, manfred@colorfullife.com,
	akpm@linux-foundation.org
Subject: Re: [PATCH] prevent sparc64 from invoking irq handlers on offline CPUs
Date: Tue, 2 Sep 2008 17:42:11 -0700	[thread overview]
Message-ID: <20080903004211.GD6748@linux.vnet.ibm.com> (raw)
In-Reply-To: <20080902.171630.193505044.davem@davemloft.net>

On Tue, Sep 02, 2008 at 05:16:30PM -0700, David Miller wrote:
> From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> Date: Sun, 31 Aug 2008 10:33:49 -0700
> 
> > Make sparc64 refrain from clearing a given to-be-offlined CPU's bit in the
> > cpu_online_mask until it has processed pending irqs.  This change
> > prevents other CPUs from being blindsided by an apparently offline CPU
> > nevertheless changing globally visible state.
> > 
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> 
> I wonder what the 'call_lock' thing protects :-)

I didn't look till now.  ;-)

> That lock is a cobweb from the sparc64 code before I switched it over
> to use the generic smp_call_function() code in kernel/smp.c
> 
> So this lock doesn't protect anything any more.

It is a static defined in arch/sparc64/kernel/smp.c, and is used only
when setting and clearing bits in cpu_online_mask.

> kernel/smp.c has a call_function_lock, which isn't marked static
> but isn't declared in any header file.

It is exported via ipi_call_lock(), ipi_call_unlock(), and friends.
A few architectures use it to exclude some of the IPI code while
setting (but not clearing) bits in cpu_online_map.  These particular
architectures have a phase during CPU offlining where they drain
pending interrupts, so perhaps that is why they only worry about
onlining?

> My instinct is that the intention is that I could use this lock
> for the synchronization previously provided by sparc64's local
> "call_lock", and it even seems the author of kernel/smp.c intended
> this kind of usage.
> 
> Anyways, if this code is still using the worthless call_lock, it
> isn't protecting against anything.

Agreed.

> So I'd like to hold off on this patch until this locking issue is
> resolved.

OK, it is your architecture.  But in the meantime, sparc64 can take
interrupts on CPUs whose cpu_online_map bits have been cleared.

							Thanx, Paul

  reply	other threads:[~2008-09-03  0:42 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-08-31 17:33 [PATCH] prevent sparc64 from invoking irq handlers on offline CPUs Paul E. McKenney
2008-08-31 17:33 ` Paul E. McKenney
2008-09-03  0:16 ` [PATCH] prevent sparc64 from invoking irq handlers on offline David Miller
2008-09-03  0:16   ` [PATCH] prevent sparc64 from invoking irq handlers on offline CPUs David Miller
2008-09-03  0:42   ` Paul E. McKenney [this message]
2008-09-03  0:42     ` Paul E. McKenney
2008-09-03  9:21     ` [PATCH] prevent sparc64 from invoking irq handlers on offline David Miller
2008-09-03  9:21       ` [PATCH] prevent sparc64 from invoking irq handlers on offline CPUs David Miller
2008-09-03 15:42       ` [PATCH] prevent sparc64 from invoking irq handlers on offline Paul E. McKenney
2008-09-03 15:42         ` [PATCH] prevent sparc64 from invoking irq handlers on offline CPUs Paul E. McKenney
2008-09-09  0:17         ` [PATCH] prevent sparc64 from invoking irq handlers on offline David Miller
2008-09-09  0:17           ` [PATCH] prevent sparc64 from invoking irq handlers on offline CPUs David Miller
2008-09-09 14:54           ` [PATCH] prevent sparc64 from invoking irq handlers on offline Paul E. McKenney
2008-09-09 14:54             ` [PATCH] prevent sparc64 from invoking irq handlers on offline CPUs Paul E. McKenney
2008-09-09 18:49           ` [PATCH] prevent sparc64 from invoking irq handlers on offline Manfred Spraul
2008-09-09 18:49             ` [PATCH] prevent sparc64 from invoking irq handlers on offline CPUs Manfred Spraul
2008-09-09 19:57             ` [PATCH] prevent sparc64 from invoking irq handlers on offline David Miller
2008-09-09 19:57               ` [PATCH] prevent sparc64 from invoking irq handlers on offline CPUs David Miller

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=20080903004211.GD6748@linux.vnet.ibm.com \
    --to=paulmck@linux.vnet.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=manfred@colorfullife.com \
    --cc=sparclinux@vger.kernel.org \
    --cc=wli@holomorphy.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.