LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Satyam Sharma" <satyam.sharma@gmail.com>
To: "Heiko Carstens" <heiko.carstens@de.ibm.com>
Cc: "Jan Glauber" <jan.glauber@de.ibm.com>,
	"David Miller" <davem@davemloft.net>,
	akpm@osdl.org, mingo@elte.hu, ak@suse.de, schwidefsky@de.ibm.com,
	linux-kernel@vger.kernel.org, "Alan Cox" <alan@redhat.com>
Subject: Re: [patch] i386/x86_64: smp_call_function locking inconsistency
Date: Thu, 7 Jun 2007 22:24:01 +0530	[thread overview]
Message-ID: <a781481a0706070954l636d752am82ef7ca9703788aa@mail.gmail.com> (raw)
In-Reply-To: <20070607162743.GA9433@osiris.ibm.com>

Hi Heiko,

On 6/7/07, Heiko Carstens <heiko.carstens@de.ibm.com> wrote:
> > Replacing the _bh variants and making smp_call_function{_single}
> > illegal from all contexts but process is fine for x86_64, as we
> > don't really have any driver that needs to use this from softirq
> > context in the x86_64 tree. This means it becomes dissimilar to
> > s390, but similar to powerpc, mips, alpha, sparc64 semantics.
> > I'll prepare and submit a patch for the same, shortly.
>
> Calling an smp_call_* function from any context but process context is
> a bug. We didn't notice this initially when we used smp_call_function
> from softirq context... until we deadlocked ;)
> So s390 is the same as any other architecture wrt this.

I'll fix the necessary patch for x86_64, in that case.

> > I don't see any CPU hotplug / preemption disabling issues here.
> > Note that both smp_call_function() and smp_call_function_single()
> > on x86_64 acquire the call_lock spinlock before using cpu_online_map
> > via num_online_cpus(). And spin_lock() does preempt_disable() on both
> > SMP and !SMP, so we're safe. [ But we're not explicitly disabling
> > preemption and depending on spin_lock() instead, so a comment would
> > be in order? ]
>
> Calling smp_call_function_single() with preemption enabled is pointless.
> You might be scheduled on the cpu you want to send an IPI to and get
> -EBUSY as return... If cpu hotplug is enabled the target cpu might even
> be gone when smp_call_function_single() gets executed.

Exactly. I was only mentioning that the smp_call_function*
of x86{_64} were safe anyway (but the smp_processor_id()
that would've preceded it need not have been, of course).

> Avi Kivity has already a patch which introduces an on_cpu() function which
> looks quite like on_each_cpu(). That way you don't have to open code this
> stuff over and over again:
>
> preempt_disable();
> if (cpu == smp_processor_id())
>         func();
> else
>         smp_call_function_single(...);
> preempt_enable();
>
> There are already quite a few of these around.

Indeed -- this was doubly problematic because the un-safeness
was because of smp_processor_id() as well as the (eventual)
access of cpu_online_map (via smp_call_function() ->
num_online_cpus()) ... thanks for letting me know about this.

Satyam

  reply	other threads:[~2007-06-07 17:05 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-02-08 20:32 Heiko Carstens
2007-02-08 20:43 ` David Miller
2007-02-09  8:42   ` Heiko Carstens
2007-02-09 12:57     ` Jan Glauber
2007-06-07 14:07       ` Satyam Sharma
2007-06-07 16:27         ` Heiko Carstens
2007-06-07 16:54           ` Satyam Sharma [this message]
2007-06-07 17:18             ` Satyam Sharma
2007-06-07 17:22               ` Avi Kivity
2007-06-07 17:33                 ` Satyam Sharma
2007-06-10  7:38                   ` Avi Kivity
2007-06-08 19:43             ` Andi Kleen
2007-06-08 19:42         ` Andi Kleen
2007-02-09  7:40 ` 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=a781481a0706070954l636d752am82ef7ca9703788aa@mail.gmail.com \
    --to=satyam.sharma@gmail.com \
    --cc=ak@suse.de \
    --cc=akpm@osdl.org \
    --cc=alan@redhat.com \
    --cc=davem@davemloft.net \
    --cc=heiko.carstens@de.ibm.com \
    --cc=jan.glauber@de.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=schwidefsky@de.ibm.com \
    --subject='Re: [patch] i386/x86_64: smp_call_function locking inconsistency' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).