* Re: [PATCH 3/4] KVM: PPC: Add support for IOMMU in-kernel handling
From: David Gibson @ 2013-06-27 11:01 UTC (permalink / raw)
To: Alex Williamson
Cc: kvm@vger.kernel.org mailing list, Alexey Kardashevskiy,
Joerg Roedel, Rusty Russell, Alexander Graf, kvm-ppc, open list,
Paul Mackerras, linuxppc-dev
In-Reply-To: <1372048884.30572.168.camel@ul30vt.home>
[-- Attachment #1: Type: text/plain, Size: 4030 bytes --]
On Sun, Jun 23, 2013 at 10:41:24PM -0600, Alex Williamson wrote:
> On Mon, 2013-06-24 at 13:52 +1000, David Gibson wrote:
> > On Sat, Jun 22, 2013 at 08:28:06AM -0600, Alex Williamson wrote:
> > > On Sat, 2013-06-22 at 22:03 +1000, David Gibson wrote:
> > > > On Thu, Jun 20, 2013 at 08:55:13AM -0600, Alex Williamson wrote:
> > > > > On Thu, 2013-06-20 at 18:48 +1000, Alexey Kardashevskiy wrote:
> > > > > > On 06/20/2013 05:47 PM, Benjamin Herrenschmidt wrote:
> > > > > > > On Thu, 2013-06-20 at 15:28 +1000, David Gibson wrote:
> > > > > > >>> Just out of curiosity - would not get_file() and fput_atomic() on a
> > > > > > >> group's
> > > > > > >>> file* do the right job instead of vfio_group_add_external_user() and
> > > > > > >>> vfio_group_del_external_user()?
> > > > > > >>
> > > > > > >> I was thinking that too. Grabbing a file reference would certainly be
> > > > > > >> the usual way of handling this sort of thing.
> > > > > > >
> > > > > > > But that wouldn't prevent the group ownership to be returned to
> > > > > > > the kernel or another user would it ?
> > > > > >
> > > > > >
> > > > > > Holding the file pointer does not let the group->container_users counter go
> > > > > > to zero
> > > > >
> > > > > How so? Holding the file pointer means the file won't go away, which
> > > > > means the group release function won't be called. That means the group
> > > > > won't go away, but that doesn't mean it's attached to an IOMMU. A user
> > > > > could call UNSET_CONTAINER.
> > > >
> > > > Uhh... *thinks*. Ah, I see.
> > > >
> > > > I think the interface should not take the group fd, but the container
> > > > fd. Holding a reference to *that* would keep the necessary things
> > > > around. But more to the point, it's the right thing semantically:
> > > >
> > > > The container is essentially the handle on a host iommu address space,
> > > > and so that's what should be bound by the KVM call to a particular
> > > > guest iommu address space. e.g. it would make no sense to bind two
> > > > different groups to different guest iommu address spaces, if they were
> > > > in the same container - the guest thinks they are different spaces,
> > > > but if they're in the same container they must be the same space.
> > >
> > > While the container is the gateway to the iommu, what empowers the
> > > container to maintain an iommu is the group. What happens to a
> > > container when all the groups are disconnected or closed? Groups are
> > > the unit that indicates hardware access, not containers. Thanks,
> >
> > Uh... huh? I'm really not sure what you're getting at.
> >
> > The operation we're doing for KVM here is binding a guest iommu
> > address space to a particular host iommu address space. Why would we
> > not want to use the obvious handle on the host iommu address space,
> > which is the container fd?
>
> AIUI, the request isn't for an interface through which to do iommu
> mappings. The request is for an interface to show that the user has
> sufficient privileges to do mappings. Groups are what gives the user
> that ability. The iommu is also possibly associated with multiple iommu
> groups and I believe what is being asked for here is a way to hold and
> lock a single iommu group with iommu protection.
>
> >From a practical point of view, the iommu interface is de-privileged
> once the groups are disconnected or closed. Holding a reference count
> on the iommu fd won't prevent that. That means we'd have to use a
> notifier to have KVM stop the side-channel iommu access. Meanwhile
> holding the file descriptor for the group and adding an interface that
> bumps use counter allows KVM to lock itself in, just as if it had a
> device opened itself. Thanks,
Ah, good point.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH 4/8] powerpc/perf: Use existing out label in power_pmu_enable()
From: Anshuman Khandual @ 2013-06-27 11:01 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, sukadev, Paul Mackerras
In-Reply-To: <1372073336-8189-4-git-send-email-michael@ellerman.id.au>
On 06/24/2013 04:58 PM, Michael Ellerman wrote:
> In power_pmu_enable() we can use the existing out label to reduce the
> number of return paths.
>
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
Reviewed-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
> ---
> arch/powerpc/perf/core-book3s.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index 3d566ee..af4b4b1 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -919,12 +919,13 @@ static void power_pmu_enable(struct pmu *pmu)
>
> if (!ppmu)
> return;
> +
> local_irq_save(flags);
> +
> cpuhw = &__get_cpu_var(cpu_hw_events);
> - if (!cpuhw->disabled) {
> - local_irq_restore(flags);
> - return;
> - }
> + if (!cpuhw->disabled)
> + goto out;
> +
> cpuhw->disabled = 0;
>
> /*
>
^ permalink raw reply
* Re: [PATCH 7/8] powerpc/perf: Core EBB support for 64-bit book3s
From: Michael Ellerman @ 2013-06-27 11:52 UTC (permalink / raw)
To: Anshuman Khandual; +Cc: linuxppc-dev, sukadev, Paul Mackerras
In-Reply-To: <51CAA885.8060406@linux.vnet.ibm.com>
On Wed, 2013-06-26 at 14:08 +0530, Anshuman Khandual wrote:
> On 06/24/2013 04:58 PM, Michael Ellerman wrote:
> > Add support for EBB (Event Based Branches) on 64-bit book3s. See the
> > included documentation for more details.
..
> > +
> > +
> > +Terminology
> > +-----------
> > +
> > +Throughout this document we will refer to an "EBB event" or "EBB events". This
> > +just refers to a struct perf_event which has set the "EBB" flag in its
> > +attr.config. All events which can be configured on the hardware PMU are
> > +possible "EBB events".
> > +
>
> Then why we have a condition like this where the event code must have the PMC
> value inside it (in the next patch) ?
Those two things are not contradictory.
> + if (!pmc && ebb)
> + /* EBB events must specify the PMC */
> + return -1;
This does not exclude any events. It just means if you're using one of
the any-PMC events you must choose a PMC for the event before opening
it.
The reason we do this is because userspace needs to know which PMC each
event is on, in order to read the PMCs and count them correctly. There
is a mechanism for the kernel to communicate to userspace which event is
on which PMC, but it is vastly simpler for userspace if it just
specifies the PMC in the event to begin with.
We could relax this restriction in future if someone comes up with a
really good reason to, but I don't see one.
I will add it to the documentation though.
> > +
> > +Background
> > +----------
> > +
> > +When a PMU EBB occurs it is delivered to the currently running process. As such
> > +EBBs can only sensibly be used by programs for self-monitoring.
> > +
> > +It is a feature of the perf_events API that events can be created on other
> > +processes, subject to standard permission checks. This is also true of EBB
> > +events, however unless the target process enables EBBs (via mtspr(BESCR)) no
> > +EBBs will ever be delivered.
> > +
> > +This makes it possible for a process to enable EBBs for itself, but not
> > +actually configure any events. At a later time another process can come along
> > +and attach an EBB event to the process, which will then cause EBBs to be
> > +delivered to the first process. It's not clear if this is actually useful.
> > +
>
> May be useful when a "master process" wants each of the thread to collect statistics
> (about the same thread) on various dynamically configured events (as and when it
> wishes) and report it back to the master process. Just a thought about a case where
> it can be useful.
Yeah sort of.
I was thinking more of a long running process which sets up an EBB
handler and is therefore prepared to monitor itself, but you use an
external program to turn the event collection on and off.
> > +When the PMU is configured for EBBs, all PMU interrupts are delivered to the
> > +user process. This means once an EBB event is scheduled on the PMU, no non-EBB
> > +events can be configured. This means that EBB events can not be run
> > +concurrently with regular 'perf' commands.
> > +
> > +It is however safe to run 'perf' commands on a process which is using EBBs. In
> > +general the EBB event will take priority, though it depends on the exact
> > +options used on the perf_event_open() and the timing.
> > +
>
> This is confusing.
I don't think it is.
> If a process A is using EBB for itself on event "p" and gets scheduled
> on CPU X. Process B has started perf session on process A for event "q". Now the PMU of
> CPU X would to be programmed for both the events "p" and "q" on different PMCs at the same
> point of time (with a condition checking that they dont collide on the same PMC though).
No that's not correct.
> What you are saying is that when the event "p" overflows, the PMU interrupt (CPU X) would
> be delivered to the process A user space and when the event "q" overflows, the PMU interrupt
> (CPU X) would be delivered inside the perf kernel component "perf_event_interrupt()" and would
> be processed for the perf session initiated by the second process B on process A.
I'm not saying that, I don't know how you got that idea.
> But again this contradicts your previous statement that when PMU is configured for EBB, *all*
> PMU interrupts would be delivered to the user space. Could you please kindly clarify this
> scenario.
To paraphrase you above:
Process A is using EBB with event "p" on CPU x, and process B
starts running perf on process A with event "q".
The PMU of CPU x will be programmed to count event "p" _and
nothing else_. All PMU interrupts will be delivered to
userspace.
When the perf session reads its counter it will see that event
"q" was unable to run for some of the time it was enabled.
There is one caveat to this which is that the EBB event gets priority
because it is pinned and exclusive. If there is another event that is
also pinned then there is a race between which event is opened on the
process first. This is what I talk about below in the "Enabling an EBB
event" section.
In general perf the tool doesn't pin events, so I don't see this as
being a problem.
> > +
> > +Creating an EBB event
> > +---------------------
> > +
> > +To request that an event is counted using EBB, the event code should have bit
> > +63 set.
> > +
>
> This macro (defined in arch header) identifies any event as an EBB event
>
> +/*
> + * We use the event config bit 63 as a flag to request EBB.
> + */
> +#define EVENT_CONFIG_EBB_SHIFT 63
> +
>
> So any user program would have to include the arch header to be able to set EBB bit.
> Numeric 63 will not be a clean ABI.
I think you mean we should put that in an exported header and expose it
to userspace?
If so I'm not sure yet.
Doing so would mean we were guaranteeing that bit 63 would always mean
"EBB please" (on processors that support EBB). I /think/ that's probably
OK, but I want to confirm that with the HW folks first.
If we don't think we want to guarantee that then we would export the EBB
bit information using the existing format attrs in the perf code, and
userspace would have to look that up in sysfs.
> > +Enabling an EBB event
> > +---------------------
> > +
> > +Once an EBB event has been successfully opened, it must be enabled with the
> > +perf_events API. This can be achieved either via the ioctl() interface, or the
> > +prctl() interface.
> > +
> > +However, due to the design of the perf_events API, enabling an event does not
> > +guarantee that it has been scheduled on the PMU. To ensure that the EBB event
> > +has been scheduled on the PMU, you must perform a read() on the event. If the
> > +read() returns EOF, then the event has not been scheduled and EBBs are not
> > +enabled.
...
> > +EBB Handler
> > +-----------
> > +
> > +The EBB handler is just regular userspace code, however it must be written in
> > +the style of an interrupt handler. When the handler is entered all registers
> > +are live (possibly) and so must be saved somehow before the handler can invoke
> > +other code.
> > +
> > +It's up to the program how to handle this. For C programs a relatively simple
> > +option is to create an interrupt frame on the stack and save registers there.
> > +
>
> Would be a great if you could give sample framework here on how to save and restore
> registers. Moreover we could actually put the various essential parts of the EBB
> handler construct in the perf_event_server.h file, so that the user would be able
> to user them directly and only focus on core part of the event handling.
I will eventually merge some test programs and example code.
But they definitely don't belong in the kernel headers.
And as an aside perf_event_server.h isn't exported to userspace.
> > diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
> > index 48af5d7..f9a4cdc 100644
> > --- a/arch/powerpc/include/asm/processor.h
> > +++ b/arch/powerpc/include/asm/processor.h
> > @@ -287,8 +287,9 @@ struct thread_struct {
> > unsigned long siar;
> > unsigned long sdar;
> > unsigned long sier;
> > - unsigned long mmcr0;
> > unsigned long mmcr2;
> > + unsigned mmcr0;
> > + unsigned used_ebb;
> > #endif
> > };
> >
>
> Why mmrc0 has to change position here.
So the structure packs properly.
> > diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> > index 362142b..5d7d9c2 100644
> > --- a/arch/powerpc/include/asm/reg.h
> > +++ b/arch/powerpc/include/asm/reg.h
> > @@ -621,6 +621,9 @@
> > #define MMCR0_PMXE 0x04000000UL /* performance monitor exception enable */
> > #define MMCR0_FCECE 0x02000000UL /* freeze ctrs on enabled cond or event */
> > #define MMCR0_TBEE 0x00400000UL /* time base exception enable */
> > +#define MMCR0_EBE 0x00100000UL /* Event based branch enable */
> > +#define MMCR0_PMCC 0x000c0000UL /* PMC control */
> > +#define MMCR0_PMCC_U6 0x00080000UL /* PMC1-6 are R/W by user (PR) */
> > #define MMCR0_PMC1CE 0x00008000UL /* PMC1 count enable*/
> > #define MMCR0_PMCjCE 0x00004000UL /* PMCj count enable*/
> > #define MMCR0_TRIGGER 0x00002000UL /* TRIGGER enable */
> > @@ -674,6 +677,11 @@
> > #define SIER_SIAR_VALID 0x0400000 /* SIAR contents valid */
> > #define SIER_SDAR_VALID 0x0200000 /* SDAR contents valid */
> >
> > +/* When EBB is enabled, some of MMCR0/MMCR2/SIER are user accessible */
> > +#define MMCR0_USER_MASK (MMCR0_FC | MMCR0_PMXE | MMCR0_PMAO)
> > +#define MMCR2_USER_MASK 0x4020100804020000UL /* (FC1P|FC2P|FC3P|FC4P|FC5P|FC6P) */
> > +#define SIER_USER_MASK 0x7fffffUL
> > +
>
> Ohh these are the bits in SPR which are available in user space to read and write as well ?
Yes. See section 9.4.10 of PowerISA v2.07.
> Better to have macros instead of hex codes here.
We don't have macros for the other fields. If we add macros for FCxP then we can redo it then.
> > #endif /* _ASM_POWERPC_SWITCH_TO_H */
> > diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> > index 076d124..c517dbe 100644
> > --- a/arch/powerpc/kernel/process.c
> > +++ b/arch/powerpc/kernel/process.c
> > @@ -916,7 +916,11 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
> > flush_altivec_to_thread(src);
> > flush_vsx_to_thread(src);
> > flush_spe_to_thread(src);
> > +
> > *dst = *src;
> > +
> > + clear_task_ebb(dst);
> > +
> > return 0;
> > }
>
> Blank lines are not necessary here.
Blank lines are never necessary.
But they highlight parts of the code that are important and make it more
readable. In this case I'm highlighting that we do the structure
assignment and then clear the EBB info of the destination task, vs the
other routines which operate on the src.
> > +static void ebb_switch_out(unsigned long mmcr0)
> > +{
> > + if (!(mmcr0 & MMCR0_EBE))
> > + return;
> > +
> > + current->thread.siar = mfspr(SPRN_SIAR);
> > + current->thread.sier = mfspr(SPRN_SIER);
> > + current->thread.sdar = mfspr(SPRN_SDAR);
> > + current->thread.mmcr0 = mmcr0 & MMCR0_USER_MASK;
> > + current->thread.mmcr2 = mfspr(SPRN_MMCR2) & MMCR2_USER_MASK;
> > +}
> > +
>
> We also need to filter sier value for SIER_USER_MASK, right ?
We don't need to, the hardware does it for us.
We are filtering the others because we do (in the case of MMCR0) or
might (MMCR2) use those inside the perf code, and we don't want to
confuse values we've set with values the user has set.
cheers
^ permalink raw reply
* Re: [PATCH 8/8] powerpc/perf: Add power8 EBB support
From: Michael Ellerman @ 2013-06-27 11:52 UTC (permalink / raw)
To: Anshuman Khandual; +Cc: linuxppc-dev, sukadev, Paul Mackerras
In-Reply-To: <51CABB56.9010105@linux.vnet.ibm.com>
On Wed, 2013-06-26 at 15:28 +0530, Anshuman Khandual wrote:
> > @@ -117,6 +117,7 @@
> > (EVENT_UNIT_MASK << EVENT_UNIT_SHIFT) | \
> > (EVENT_COMBINE_MASK << EVENT_COMBINE_SHIFT) | \
> > (EVENT_MARKED_MASK << EVENT_MARKED_SHIFT) | \
> > + (1ull << EVENT_CONFIG_EBB_SHIFT) | \
>
> We should define this macro like EVENT_MARKED_MASK
>
> #define EVENT_EBB_MASK 0x1
>
> Numeric value of "1ull" stands out odd in the scheme.
Yeah I guess.
Would you like it in blue? :)
cheers
^ permalink raw reply
* Re: [RFC PATCH 1/3] mm/cma: Move dma contiguous changes into a seperate config
From: Marek Szyprowski @ 2013-06-27 11:42 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: linux-mm, paulus, linuxppc-dev, Michal Nazarewicz
In-Reply-To: <1372062327-7028-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com>
Hello,
On 6/24/2013 10:25 AM, Aneesh Kumar K.V wrote:
> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>
> We want to use CMA for allocating hash page table and real mode area for
> PPC64. Hence move DMA contiguous related changes into a seperate config
> so that ppc64 can enable CMA without requiring DMA contiguous.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
It's nice to see that CMA is gaining another client in mainline Linux
kernel.
I also like the idea of adding CONFIG_DMA_CMA for DMA-mapping related CMA
use to let others to use alloc_contig_range() interface.
However I noticed that You have almost copied the whole drivers/base.c for
separate use with PPC64 KVM. Is this really necessary? I think it should be
possible to isolate some common code, which plays with contiguous region
management and use it for both alloc_contig_range() clients (dma-mapping and
ppc64 kvm).
> ---
> arch/arm/configs/omap2plus_defconfig | 2 +-
> arch/arm/configs/tegra_defconfig | 2 +-
> arch/arm/include/asm/dma-contiguous.h | 2 +-
> arch/arm/mm/dma-mapping.c | 6 +++---
> drivers/base/Kconfig | 21 +++++----------------
> drivers/base/Makefile | 2 +-
> include/linux/dma-contiguous.h | 2 +-
> mm/Kconfig | 20 ++++++++++++++++++++
> 8 files changed, 33 insertions(+), 24 deletions(-)
>
> diff --git a/arch/arm/configs/omap2plus_defconfig b/arch/arm/configs/omap2plus_defconfig
> index abbe319..098268f 100644
> --- a/arch/arm/configs/omap2plus_defconfig
> +++ b/arch/arm/configs/omap2plus_defconfig
> @@ -71,7 +71,7 @@ CONFIG_MAC80211=m
> CONFIG_MAC80211_RC_PID=y
> CONFIG_MAC80211_RC_DEFAULT_PID=y
> CONFIG_UEVENT_HELPER_PATH="/sbin/hotplug"
> -CONFIG_CMA=y
> +CONFIG_DMA_CMA=y
> CONFIG_CONNECTOR=y
> CONFIG_DEVTMPFS=y
> CONFIG_DEVTMPFS_MOUNT=y
> diff --git a/arch/arm/configs/tegra_defconfig b/arch/arm/configs/tegra_defconfig
> index f7ba3161..34ae8f2 100644
> --- a/arch/arm/configs/tegra_defconfig
> +++ b/arch/arm/configs/tegra_defconfig
> @@ -79,7 +79,7 @@ CONFIG_RFKILL_GPIO=y
> CONFIG_DEVTMPFS=y
> CONFIG_DEVTMPFS_MOUNT=y
> # CONFIG_FIRMWARE_IN_KERNEL is not set
> -CONFIG_CMA=y
> +CONFIG_DMA_CMA=y
> CONFIG_MTD=y
> CONFIG_MTD_CHAR=y
> CONFIG_MTD_M25P80=y
> diff --git a/arch/arm/include/asm/dma-contiguous.h b/arch/arm/include/asm/dma-contiguous.h
> index 3ed37b4..e072bb2 100644
> --- a/arch/arm/include/asm/dma-contiguous.h
> +++ b/arch/arm/include/asm/dma-contiguous.h
> @@ -2,7 +2,7 @@
> #define ASMARM_DMA_CONTIGUOUS_H
>
> #ifdef __KERNEL__
> -#ifdef CONFIG_CMA
> +#ifdef CONFIG_DMA_CMA
>
> #include <linux/types.h>
> #include <asm-generic/dma-contiguous.h>
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index ef3e0f3..1fb40dc 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -358,7 +358,7 @@ static int __init atomic_pool_init(void)
> if (!pages)
> goto no_pages;
>
> - if (IS_ENABLED(CONFIG_CMA))
> + if (IS_ENABLED(CONFIG_DMA_CMA))
> ptr = __alloc_from_contiguous(NULL, pool->size, prot, &page,
> atomic_pool_init);
> else
> @@ -670,7 +670,7 @@ static void *__dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
> addr = __alloc_simple_buffer(dev, size, gfp, &page);
> else if (!(gfp & __GFP_WAIT))
> addr = __alloc_from_pool(size, &page);
> - else if (!IS_ENABLED(CONFIG_CMA))
> + else if (!IS_ENABLED(CONFIG_DMA_CMA))
> addr = __alloc_remap_buffer(dev, size, gfp, prot, &page, caller);
> else
> addr = __alloc_from_contiguous(dev, size, prot, &page, caller);
> @@ -759,7 +759,7 @@ static void __arm_dma_free(struct device *dev, size_t size, void *cpu_addr,
> __dma_free_buffer(page, size);
> } else if (__free_from_pool(cpu_addr, size)) {
> return;
> - } else if (!IS_ENABLED(CONFIG_CMA)) {
> + } else if (!IS_ENABLED(CONFIG_DMA_CMA)) {
> __dma_free_remap(cpu_addr, size);
> __dma_free_buffer(page, size);
> } else {
> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> index 07abd9d..74b7c98 100644
> --- a/drivers/base/Kconfig
> +++ b/drivers/base/Kconfig
> @@ -202,11 +202,10 @@ config DMA_SHARED_BUFFER
> APIs extension; the file's descriptor can then be passed on to other
> driver.
>
> -config CMA
> - bool "Contiguous Memory Allocator"
> - depends on HAVE_DMA_CONTIGUOUS && HAVE_MEMBLOCK
> - select MIGRATION
> - select MEMORY_ISOLATION
> +config DMA_CMA
> + bool "DMA Contiguous Memory Allocator"
> + depends on HAVE_DMA_CONTIGUOUS
> + select CMA
> help
> This enables the Contiguous Memory Allocator which allows drivers
> to allocate big physically-contiguous blocks of memory for use with
> @@ -215,17 +214,7 @@ config CMA
> For more information see <include/linux/dma-contiguous.h>.
> If unsure, say "n".
>
> -if CMA
> -
> -config CMA_DEBUG
> - bool "CMA debug messages (DEVELOPMENT)"
> - depends on DEBUG_KERNEL
> - help
> - Turns on debug messages in CMA. This produces KERN_DEBUG
> - messages for every CMA call as well as various messages while
> - processing calls such as dma_alloc_from_contiguous().
> - This option does not affect warning and error messages.
> -
> +if DMA_CMA
> comment "Default contiguous memory area size:"
>
> config CMA_SIZE_MBYTES
> diff --git a/drivers/base/Makefile b/drivers/base/Makefile
> index 4e22ce3..5d93bb5 100644
> --- a/drivers/base/Makefile
> +++ b/drivers/base/Makefile
> @@ -6,7 +6,7 @@ obj-y := core.o bus.o dd.o syscore.o \
> attribute_container.o transport_class.o \
> topology.o
> obj-$(CONFIG_DEVTMPFS) += devtmpfs.o
> -obj-$(CONFIG_CMA) += dma-contiguous.o
> +obj-$(CONFIG_DMA_CMA) += dma-contiguous.o
> obj-y += power/
> obj-$(CONFIG_HAS_DMA) += dma-mapping.o
> obj-$(CONFIG_HAVE_GENERIC_DMA_COHERENT) += dma-coherent.o
> diff --git a/include/linux/dma-contiguous.h b/include/linux/dma-contiguous.h
> index 01b5c84..00141d3 100644
> --- a/include/linux/dma-contiguous.h
> +++ b/include/linux/dma-contiguous.h
> @@ -57,7 +57,7 @@ struct cma;
> struct page;
> struct device;
>
> -#ifdef CONFIG_CMA
> +#ifdef CONFIG_DMA_CMA
>
> /*
> * There is always at least global CMA area and a few optional device
> diff --git a/mm/Kconfig b/mm/Kconfig
> index e742d06..b362369 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -477,3 +477,23 @@ config FRONTSWAP
> and swap data is stored as normal on the matching swap device.
>
> If unsure, say Y to enable frontswap.
> +
> +config CMA
> + bool "Contiguous Memory Allocator"
> + depends on HAVE_MEMBLOCK
> + select MIGRATION
> + select MEMORY_ISOLATION
> + help
> + This enables the Contiguous Memory Allocator which allows other
> + subsystem to allocate big physically-contiguous blocks of memory
> +
> + If unsure, say "n".
> +
> +config CMA_DEBUG
> + bool "CMA debug messages (DEVELOPMENT)"
> + depends on DEBUG_KERNEL && CMA
> + help
> + Turns on debug messages in CMA. This produces KERN_DEBUG
> + messages for every CMA call as well as various messages while
> + processing calls such as dma_alloc_from_contiguous().
> + This option does not affect warning and error messages.
Best regards
--
Marek Szyprowski
Samsung R&D Institute Poland
^ permalink raw reply
* Re: [PATCH 3/4] powerpc: Rename and flesh out the facility unavailable exception handler
From: Michael Ellerman @ 2013-06-27 14:16 UTC (permalink / raw)
To: Stephen Rothwell; +Cc: linuxppc-dev
In-Reply-To: <20130627140539.5ff63bad1e7e99e5526d0f3d@canb.auug.org.au>
On Thu, Jun 27, 2013 at 02:05:39PM +1000, Stephen Rothwell wrote:
> Hi Michael,
>
> On Tue, 25 Jun 2013 17:47:56 +1000 Michael Ellerman <michael@ellerman.id.au> wrote:
> >
> > -void tm_unavailable_exception(struct pt_regs *regs)
> > +void facility_unavailable_exception(struct pt_regs *regs)
> > {
> > + static char *facility_strings[] = {
> > + "FPU",
> > + "VMX/VSX",
> > + "DSCR",
> > + "PMU SPRs",
> > + "BHRB",
> > + "TM",
> > + "AT",
> > + "EBB",
> > + "TAR",
> > + };
>
> Are the indexes into this array defined somewhere? If not, can we do
> that. Then, can we use explicit indexed initialisers for this array?
I'm not sure I follow.
The mapping is defined by the definition of the "Interruption Cause"
field of the FSCR, section 6.2.10 of PowerISA v2.07.
cheers
^ permalink raw reply
* Re: [PATCH v2 2/2] perf tools: Make Power7 events available for perf
From: Michael Ellerman @ 2013-06-27 14:23 UTC (permalink / raw)
To: Runzhen Wang
Cc: linux-kernel, xiaoguangrong, acme, paulus, sukadev, linuxppc-dev
In-Reply-To: <1372170933-4538-3-git-send-email-runzhen@linux.vnet.ibm.com>
On Tue, Jun 25, 2013 at 10:35:33PM +0800, Runzhen Wang wrote:
> Power7 supports over 530 different perf events but only a small
> subset of these can be specified by name, for the remaining
> events, we must specify them by their raw code:
Hi Runzhen,
This is looking good. Sorry one last request below ...
> diff --git a/arch/powerpc/perf/power7-events-list.h b/arch/powerpc/perf/power7-events-list.h
> new file mode 100644
> index 0000000..a67e8a9
> --- /dev/null
> +++ b/arch/powerpc/perf/power7-events-list.h
> @@ -0,0 +1,548 @@
..
> +
> +EVENT(PM_IC_DEMAND_L2_BR_ALL, 0x4898)
> +EVENT(PM_GCT_UTIL_7_TO_10_SLOTS, 0x20a0)
> +EVENT(PM_PMC2_SAVED, 0x10022)
> +EVENT(PM_CMPLU_STALL_DFU, 0x2003c)
> +EVENT(PM_VSU0_16FLOP, 0xa0a4)
Can you add a leading zero to all the events that don't have a PMC, so
that they all line up vertically. It makes it a lot easier to scan the
list visually.
eg:
EVENT(PM_IC_DEMAND_L2_BR_ALL, 0x04898)
EVENT(PM_GCT_UTIL_7_TO_10_SLOTS, 0x020a0)
EVENT(PM_PMC2_SAVED, 0x10022)
EVENT(PM_CMPLU_STALL_DFU, 0x2003c)
EVENT(PM_VSU0_16FLOP, 0x0a0a4)
cheers
^ permalink raw reply
* Re: [PATCH v3 1/2] perf tools: fix a typo of a Power7 event name
From: Michael Ellerman @ 2013-06-27 14:26 UTC (permalink / raw)
To: Runzhen Wang
Cc: linux-kernel, xiaoguangrong, acme, paulus, sukadev, linuxppc-dev
In-Reply-To: <1372170933-4538-2-git-send-email-runzhen@linux.vnet.ibm.com>
On Tue, Jun 25, 2013 at 10:35:32PM +0800, Runzhen Wang wrote:
> In the Power7 PMU guide:
> https://www.power.org/documentation/commonly-used-metrics-for-performance-analysis/
> PM_BRU_MPRED is referred to as PM_BR_MPRED.
>
> It fixed the typo by changing the name of the event in kernel
> and documentation accordingly.
This patch fixes the typo by ...
> This patch changes the ABI, there are some reasons I think it's ok:
>
> - It is relatively new interface, specific to the Power7 platform.
>
> - No tools that we know of actually use this interface at this point
> (none are listed near the interface).
>
> - Users of this interface (eg oprofile users migrating to perf)
> would be more used to the "PM_BR_MPRED" rather than "PM_BRU_MPRED".
>
> - These are in the ABI/testing at this point rather than ABI/stable,
> so hoping we have some wiggle room.
>
> Signed-off-by: Runzhen Wang <runzhen@linux.vnet.ibm.com>
Acked-by: Michael Ellerman <michael@ellerman.id.au>
cheers
^ permalink raw reply
* Re: [PATCH v2 0/3] Nvram-to-pstore: compression support for oops data
From: Kees Cook @ 2013-06-27 15:36 UTC (permalink / raw)
To: Aruna Balakrishnaiah
Cc: jkenisto, Tony Luck, Anton Vorontsov, LKML, linuxppc-dev,
Colin Cross, paulus, Anton Blanchard, mahesh
In-Reply-To: <20130627082941.16749.67023.stgit@aruna-ThinkPad-T420>
On Thu, Jun 27, 2013 at 1:32 AM, Aruna Balakrishnaiah
<aruna@linux.vnet.ibm.com> wrote:
> Changes from v1:
> - Add header size argument in the pstore write callback
> instead of a separate API to return header size.
>
> The patchset takes care of compressing oops messages while writing to NVRAM,
> so that more oops data can be captured in the given space.
>
> big_oops_buf (2.22 * oops_data_sz) is allocated for compression.
> oops_data_sz is oops header size less of oops partition size.
>
> Pstore will internally call kmsg_dump to capture messages from printk
> buffer. While returning the data to nvram it adds is own header.
>
> For compression:
> Register pstore with big_oops_buf.
>
> In case compression fails, copy header added by pstore and
> last oops_data_sz bytes (recent messages) of big_oops_buf to
> nvram for which we need to know header size.
>
> patch 01/03 adds an additional argument for header size in pstore_write callback.
>
> pstore read callback of nvram will read the compressed data and return the
> decompressed data so that dmesg file (under /dev/pstore) is readable.
>
> In case decompression fails, instead of having the compressed data (junk) in the
> dmesg file it will skip and continue reading other partitions. This results in
> absence of dmesg file but will still have files relating to other parititons.
This seems good to me. I'm starting to ponder if we need to have some
kind of regular header structure that backends can extend, but that
doesn't need to be part of this series. :)
Acked-by: Kees Cook <keescook@chromium.org>
Thanks,
-Kees
--
Kees Cook
Chrome OS Security
^ permalink raw reply
* Re: [PATCH v2] vfio: add external user support
From: Alex Williamson @ 2013-06-27 15:44 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: kvm, linux-kernel, Paul Mackerras, linuxppc-dev, David Gibson
In-Reply-To: <1372317260-6438-1-git-send-email-aik@ozlabs.ru>
On Thu, 2013-06-27 at 17:14 +1000, Alexey Kardashevskiy wrote:
> VFIO is designed to be used via ioctls on file descriptors
> returned by VFIO.
>
> However in some situations support for an external user is required.
> The first user is KVM on PPC64 (SPAPR TCE protocol) which is going to
> use the existing VFIO groups for exclusive access in real/virtual mode
> in the host kernel to avoid passing map/unmap requests to the user
> space which would made things pretty slow.
>
> The proposed protocol includes:
>
> 1. do normal VFIO init stuff such as opening a new container, attaching
> group(s) to it, setting an IOMMU driver for a container. When IOMMU is
> set for a container, all groups in it are considered ready to use by
> an external user.
>
> 2. pass a fd of the group we want to accelerate to KVM. KVM calls
> vfio_group_iommu_id_from_file() to verify if the group is initialized
> and IOMMU is set for it. The current TCE IOMMU driver marks the whole
> IOMMU table as busy when IOMMU is set for a container what this prevents
> other DMA users from allocating from it so it is safe to pass the group
> to the user space.
>
> 3. KVM increases the container users counter via
> vfio_group_add_external_user(). This prevents the VFIO group from
> being disposed prior to exiting KVM.
>
> 4. When KVM is finished and doing cleanup, it releases the group file
> and decrements the container users counter. Everything gets released.
>
> 5. KVM also keeps the group file as otherwise its fd might have been
> closed at the moment of KVM finish so vfio_group_del_external_user()
> call will not be possible.
This is the wrong order in my mind. An external user has no business
checking or maintaining any state of a group until it calls
add_external_user(). Only after that call is successful can the user
assume the filep to group relationship is static and get the iommu_id.
Any use of the "external user" API should start with "add" and end with
"del".
> The "vfio: Limit group opens" patch is also required for the consistency.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>
> v1->v2: added definitions to vfio.h :)
> Should not compile but compiled. Hm.
>
> ---
> drivers/vfio/vfio.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/vfio.h | 7 +++++++
> 2 files changed, 61 insertions(+)
>
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index c488da5..40875d2 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -1370,6 +1370,60 @@ static const struct file_operations vfio_device_fops = {
> };
>
> /**
> + * External user API, exported by symbols to be linked dynamically.
> + */
> +
> +/* Allows an external user (for example, KVM) to lock an IOMMU group */
> +int vfio_group_add_external_user(struct file *filep)
> +{
> + struct vfio_group *group = filep->private_data;
> +
> + if (filep->f_op != &vfio_group_fops)
> + return -EINVAL;
> +
> + if (!atomic_inc_not_zero(&group->container_users))
> + return -EINVAL;
This is the place where I was suggesting we need tests to match
get_device_fd. It's not clear what the external user is holding if the
group has no iommu or is not viable here.
if (!group->container->iommu_driver || !vfio_group_viable(group)) {
vfio_group_try_dissolve_container(group);
return -EINVAL;
}
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(vfio_group_add_external_user);
> +
> +/* Allows an external user (for example, KVM) to unlock an IOMMU group */
> +void vfio_group_del_external_user(struct file *filep)
> +{
> + struct vfio_group *group = filep->private_data;
> +
> + if (WARN_ON(filep->f_op != &vfio_group_fops))
> + return;
How about we make this return int so we can return 0/-EINVAL and the
caller can decide the severity of the response?
> +
> + vfio_group_try_dissolve_container(group);
> +}
> +EXPORT_SYMBOL_GPL(vfio_group_del_external_user);
> +
> +/*
> + * Checks if a group for the specified file can be used by
> + * an external user and returns the IOMMU ID if external use is possible.
> + */
> +int vfio_group_iommu_id_from_file(struct file *filep)
Let's name this in a way that makes it clear that it's part of the
external_user API. vfio_group_external_user_iommu_id?
> +{
> + int ret;
> + struct vfio_group *group = filep->private_data;
> +
> + if (WARN_ON(filep->f_op != &vfio_group_fops))
> + return -EINVAL;
This one probably doesn't deserve a WARN_ON either, let the caller
blowup if it wants.
> +
> + if (0 == atomic_read(&group->container_users) ||
> + !group->container->iommu_driver ||
> + !vfio_group_viable(group))
> + return -EINVAL;
The above test just becomes a weak test that the caller is correctly
using the API since we should be enforcing these tests when the external
user is added. It doesn't hurt to leave them, but it's not very
convincing that the caller is the one holding anything.
> + ret = iommu_group_id(group->iommu_group);
The 'ret' variable isn't needed.
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(vfio_group_iommu_id_from_file);
> +
> +/**
> * Module/class support
> */
> static char *vfio_devnode(struct device *dev, umode_t *mode)
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index ac8d488..7ee6575 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -90,4 +90,11 @@ extern void vfio_unregister_iommu_driver(
> TYPE tmp; \
> offsetof(TYPE, MEMBER) + sizeof(tmp.MEMBER); }) \
>
> +/*
> + * External user API
> + */
> +int vfio_group_add_external_user(struct file *filep);
> +void vfio_group_del_external_user(struct file *filep);
> +int vfio_group_iommu_id_from_file(struct file *filep);
> +
As Stephen noted, let's use extern here. Also, please add some
additional text about the general purpose of this API and how to use it.
A generalized version of your commit log after fixes would be great.
Thanks,
Alex
^ permalink raw reply
* Re: [PATCH 8/8] KVM: PPC: Add hugepage support for IOMMU in-kernel handling
From: Scott Wood @ 2013-06-27 18:39 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: kvm, linux-doc, Alexey Kardashevskiy, Alexander Graf, kvm-ppc,
linux-kernel, Alex Williamson, Paul Mackerras, Paul E . McKenney,
linuxppc-dev, David Gibson
In-Reply-To: <1372309356-28320-9-git-send-email-aik@ozlabs.ru>
On 06/27/2013 12:02:36 AM, Alexey Kardashevskiy wrote:
> +/*
> + * The KVM guest can be backed with 16MB pages.
> + * In this case, we cannot do page counting from the real mode
> + * as the compound pages are used - they are linked in a list
> + * with pointers as virtual addresses which are inaccessible
> + * in real mode.
> + *
> + * The code below keeps a 16MB pages list and uses page struct
> + * in real mode if it is already locked in RAM and inserted into
> + * the list or switches to the virtual mode where it can be
> + * handled in a usual manner.
> + */
> +#define KVMPPC_HUGEPAGE_HASH(gpa) hash_32(gpa >> 24, 32)
> +
> +struct kvmppc_iommu_hugepage {
> + struct hlist_node hash_node;
> + unsigned long gpa; /* Guest physical address */
> + unsigned long hpa; /* Host physical address */
> + struct page *page; /* page struct of the very first =20
> subpage */
> + unsigned long size; /* Huge page size (always 16MB at the =20
> moment) */
> +};
Shouldn't this be namespaced to something like "book3s" or "spapr"?
-Scott=
^ permalink raw reply
* [PATCH v3 00/45] CPU hotplug: stop_machine()-free CPU hotplug, part 1
From: Srivatsa S. Bhat @ 2013-06-27 19:52 UTC (permalink / raw)
To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
walken, vincent.guittot, laijs, David.Laight
Cc: linux-arch, nikunj, zhong, linux-pm, fweisbec, linux-kernel,
rostedt, xiaoguangrong, sbw, wangyun, srivatsa.bhat, netdev,
linuxppc-dev
Hi,
This patchset is a first step towards removing stop_machine() from the
CPU hotplug offline path. It introduces a set of APIs (as a replacement to
preempt_disable()/preempt_enable()) to synchronize with CPU hotplug from
atomic contexts.
The motivation behind getting rid of stop_machine() is to avoid its
ill-effects, such as performance penalties[1] and the real-time latencies it
inflicts on the system (and also things involving stop_machine() have often
been notoriously hard to debug). And in general, getting rid of stop_machine()
from CPU hotplug also greatly enhances the elegance of the CPU hotplug design
itself.
Getting rid of stop_machine() involves building the corresponding
infrastructure in the core CPU hotplug code and converting all places which
used to depend on the semantics of stop_machine() to synchronize with CPU
hotplug.
This patchset builds a first-level base infrastructure on which tree-wide
conversions can be built upon, and also includes the conversions themselves.
We certainly need a few more careful tree-sweeps to complete the conversion,
but the goal of this patchset is to introduce the core pieces and to get the
first batch of conversions in, while covering a reasonable bulk among them.
This patchset also has a debug infrastructure to help with the conversions -
with the newly introduced CONFIG_DEBUG_HOTPLUG_CPU option turned on, it
prints warnings whenever the need for a conversion is detected. Patches 4-7
build this framework. Needless to say, I'd really appreciate if people could
test kernels with this option turned on and report omissions or better yet,
send patches to contribute to this effort.
[It is to be noted that this patchset doesn't replace stop_machine() yet,
so the immediate risk in having an unconverted (or converted) call-site
is nil, since there is no major functional change involved.]
Once the conversion gets completed, we can finalize on the design of the
stop_machine() replacement and use that in the core CPU hotplug code. We have
had some discussions in the past where we debated several different
designs[2]. We'll revisit that with more ideas once this conversion gets over.
This patchset applies on current tip:master. It is also available in the
following git branch:
git://github.com/srivatsabhat/linux.git stop-mch-free-cpuhp-part1-v3
Thank you very much!
Changes in v3:
-------------
* Added _nocheck() variants of cpu accessor functions for use in RCU
and percpu-counter code. Reworked the RCU and percpu-counter patches
accordingly.
* Dropped the patch related to kvm-vmx, since it was unnecessary.
* Added changes to r4k_on_each_cpu() in Mips, as pointed out by Ralf Baechle.
* Added Acks and rebased to current tip/master.
Changes in v2:
-------------
* Build-fix for !HOTPLUG_CPU case.
* Minor code cleanups, and added a few comments.
References:
----------
1. Performance difference between CPU Hotplug with and without
stop_machine():
http://article.gmane.org/gmane.linux.kernel/1435249
2. Links to discussions around alternative synchronization schemes to
replace stop_machine() in the CPU Hotplug code:
v6: http://lwn.net/Articles/538819/
v5: http://lwn.net/Articles/533553/
v4: https://lkml.org/lkml/2012/12/11/209
v3: http://lwn.net/Articles/528541/
v2: https://lkml.org/lkml/2012/12/5/322
v1: https://lkml.org/lkml/2012/12/4/88
http://lwn.net/Articles/527988/
3. Links to previous versions of this patchset:
v2: http://thread.gmane.org/gmane.linux.kernel/1515396
v1: http://lwn.net/Articles/556138/
--
Srivatsa S. Bhat (45):
CPU hotplug: Provide APIs to prevent CPU offline from atomic context
CPU hotplug: Clarify the usage of different synchronization APIs
Documentation, CPU hotplug: Recommend usage of get/put_online_cpus_atomic()
CPU hotplug: Add infrastructure to check lacking hotplug synchronization
CPU hotplug: Protect set_cpu_online() to avoid false-positives
CPU hotplug: Sprinkle debugging checks to catch locking bugs
CPU hotplug: Add _nocheck() variants of accessor functions
CPU hotplug: Expose the new debug config option
CPU hotplug: Convert preprocessor macros to static inline functions
smp: Use get/put_online_cpus_atomic() to prevent CPU offline
sched/core: Use get/put_online_cpus_atomic() to prevent CPU offline
migration: Use raw_spin_lock/unlock since interrupts are already disabled
sched/fair: Use get/put_online_cpus_atomic() to prevent CPU offline
timer: Use get/put_online_cpus_atomic() to prevent CPU offline
sched/rt: Use get/put_online_cpus_atomic() to prevent CPU offline
rcu: Use cpu_is_offline_nocheck() to avoid false-positive warnings
tick-broadcast: Use get/put_online_cpus_atomic() to prevent CPU offline
time/clocksource: Use get/put_online_cpus_atomic() to prevent CPU offline
softirq: Use get/put_online_cpus_atomic() to prevent CPU offline
irq: Use get/put_online_cpus_atomic() to prevent CPU offline
net: Use get/put_online_cpus_atomic() to prevent CPU offline
block: Use get/put_online_cpus_atomic() to prevent CPU offline
percpu_counter: Use _nocheck version of for_each_online_cpu()
infiniband: ehca: Use get/put_online_cpus_atomic() to prevent CPU offline
[SCSI] fcoe: Use get/put_online_cpus_atomic() to prevent CPU offline
staging/octeon: Use get/put_online_cpus_atomic() to prevent CPU offline
x86: Use get/put_online_cpus_atomic() to prevent CPU offline
perf/x86: Use get/put_online_cpus_atomic() to prevent CPU offline
KVM: Use get/put_online_cpus_atomic() to prevent CPU offline
x86/xen: Use get/put_online_cpus_atomic() to prevent CPU offline
alpha/smp: Use get/put_online_cpus_atomic() to prevent CPU offline
blackfin/smp: Use get/put_online_cpus_atomic() to prevent CPU offline
cris/smp: Use get/put_online_cpus_atomic() to prevent CPU offline
hexagon/smp: Use get/put_online_cpus_atomic() to prevent CPU offline
ia64: irq, perfmon: Use get/put_online_cpus_atomic() to prevent CPU offline
ia64: smp, tlb: Use get/put_online_cpus_atomic() to prevent CPU offline
m32r: Use get/put_online_cpus_atomic() to prevent CPU offline
MIPS: Use get/put_online_cpus_atomic() to prevent CPU offline
mn10300: Use get/put_online_cpus_atomic() to prevent CPU offline
powerpc, irq: Use GFP_ATOMIC allocations in atomic context
powerpc: Use get/put_online_cpus_atomic() to prevent CPU offline
powerpc: Use get/put_online_cpus_atomic() to avoid false-positive warning
sh: Use get/put_online_cpus_atomic() to prevent CPU offline
sparc: Use get/put_online_cpus_atomic() to prevent CPU offline
tile: Use get/put_online_cpus_atomic() to prevent CPU offline
Documentation/cpu-hotplug.txt | 20 +++-
arch/alpha/kernel/smp.c | 19 ++-
arch/blackfin/mach-common/smp.c | 4 -
arch/cris/arch-v32/kernel/smp.c | 5 +
arch/hexagon/kernel/smp.c | 3 +
arch/ia64/kernel/irq_ia64.c | 15 +++
arch/ia64/kernel/perfmon.c | 8 +
arch/ia64/kernel/smp.c | 12 +-
arch/ia64/mm/tlb.c | 4 -
arch/m32r/kernel/smp.c | 16 +--
arch/mips/kernel/cevt-smtc.c | 7 +
arch/mips/kernel/smp.c | 16 +--
arch/mips/kernel/smtc.c | 12 ++
arch/mips/mm/c-octeon.c | 4 -
arch/mips/mm/c-r4k.c | 5 +
arch/mn10300/mm/cache-smp.c | 3 +
arch/mn10300/mm/tlb-smp.c | 17 ++-
arch/powerpc/kernel/irq.c | 9 +-
arch/powerpc/kernel/machine_kexec_64.c | 4 -
arch/powerpc/kernel/smp.c | 4 +
arch/powerpc/kvm/book3s_hv.c | 5 +
arch/powerpc/mm/mmu_context_nohash.c | 3 +
arch/powerpc/oprofile/cell/spu_profiler.c | 3 +
arch/powerpc/oprofile/cell/spu_task_sync.c | 4 +
arch/powerpc/oprofile/op_model_cell.c | 6 +
arch/sh/kernel/smp.c | 12 +-
arch/sparc/kernel/smp_64.c | 12 +-
arch/tile/kernel/module.c | 3 +
arch/tile/kernel/tlb.c | 15 +++
arch/tile/mm/homecache.c | 3 +
arch/x86/kernel/apic/io_apic.c | 21 +++-
arch/x86/kernel/cpu/mcheck/therm_throt.c | 4 -
arch/x86/kernel/cpu/perf_event_intel_uncore.c | 6 +
arch/x86/mm/tlb.c | 14 +--
arch/x86/xen/mmu.c | 9 +-
block/blk-softirq.c | 3 +
drivers/infiniband/hw/ehca/ehca_irq.c | 5 +
drivers/scsi/fcoe/fcoe.c | 7 +
drivers/staging/octeon/ethernet-rx.c | 3 +
include/linux/cpu.h | 29 +++++
include/linux/cpumask.h | 118 +++++++++++++++++++++
kernel/cpu.c | 138 +++++++++++++++++++++++++
kernel/irq/manage.c | 7 +
kernel/irq/proc.c | 3 +
kernel/rcutree.c | 12 ++
kernel/sched/core.c | 27 ++++-
kernel/sched/fair.c | 14 ++-
kernel/sched/rt.c | 14 +++
kernel/smp.c | 52 +++++----
kernel/softirq.c | 3 +
kernel/time/clocksource.c | 5 +
kernel/time/tick-broadcast.c | 8 +
kernel/timer.c | 4 +
lib/Kconfig.debug | 8 +
lib/cpumask.c | 8 +
lib/percpu_counter.c | 9 +-
net/core/dev.c | 9 +-
virt/kvm/kvm_main.c | 8 +
58 files changed, 674 insertions(+), 127 deletions(-)
Regards,
Srivatsa S. Bhat
IBM Linux Technology Center
^ permalink raw reply
* [PATCH v3 01/45] CPU hotplug: Provide APIs to prevent CPU offline from atomic context
From: Srivatsa S. Bhat @ 2013-06-27 19:52 UTC (permalink / raw)
To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
walken, vincent.guittot, laijs, David.Laight
Cc: linux-arch, nikunj, zhong, linux-pm, fweisbec, linux-kernel,
rostedt, xiaoguangrong, Rafael J. Wysocki, sbw, Yasuaki Ishimatsu,
wangyun, Srivatsa S. Bhat, netdev, Tejun Heo, Thomas Gleixner,
linuxppc-dev, Andrew Morton
In-Reply-To: <20130627195136.29830.10445.stgit@srivatsabhat.in.ibm.com>
The current CPU offline code uses stop_machine() internally. And disabling
preemption prevents stop_machine() from taking effect, thus also preventing
CPUs from going offline, as a side effect.
There are places where this side-effect of preempt_disable() (or equivalent)
is used to synchronize with CPU hotplug. Typically these are in atomic
sections of code, where they can't make use of get/put_online_cpus(), because
the latter set of APIs can sleep.
Going forward, we want to get rid of stop_machine() from the CPU hotplug
offline path. And then, with stop_machine() gone, disabling preemption will
no longer prevent CPUs from going offline.
So provide a set of APIs for such atomic hotplug readers, to prevent (any)
CPUs from going offline. For now, they will default to preempt_disable()
and preempt_enable() itself, but this will help us do the tree-wide conversion,
as a preparatory step to remove stop_machine() from CPU hotplug.
(Besides, it is good documentation as well, since it clearly marks places
where we synchronize with CPU hotplug, instead of combining it subtly with
disabling preemption).
In future, when actually removing stop_machine(), we will alter the
implementation of these APIs to a suitable synchronization scheme.
Reviewed-by: Steven Rostedt <rostedt@goodmis.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---
include/linux/cpu.h | 20 ++++++++++++++++++++
kernel/cpu.c | 38 ++++++++++++++++++++++++++++++++++++++
2 files changed, 58 insertions(+)
diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index 9f3c7e8..a57b25a 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -17,6 +17,8 @@
#include <linux/node.h>
#include <linux/compiler.h>
#include <linux/cpumask.h>
+#include <linux/percpu.h>
+#include <linux/smp.h>
struct device;
@@ -175,6 +177,8 @@ extern struct bus_type cpu_subsys;
extern void get_online_cpus(void);
extern void put_online_cpus(void);
+extern unsigned int get_online_cpus_atomic(void);
+extern void put_online_cpus_atomic(void);
extern void cpu_hotplug_disable(void);
extern void cpu_hotplug_enable(void);
#define hotcpu_notifier(fn, pri) cpu_notifier(fn, pri)
@@ -202,6 +206,22 @@ static inline void cpu_hotplug_driver_unlock(void)
#define put_online_cpus() do { } while (0)
#define cpu_hotplug_disable() do { } while (0)
#define cpu_hotplug_enable() do { } while (0)
+
+static inline unsigned int get_online_cpus_atomic(void)
+{
+ /*
+ * Disable preemption to avoid getting complaints from the
+ * debug_smp_processor_id() code.
+ */
+ preempt_disable();
+ return smp_processor_id();
+}
+
+static inline void put_online_cpus_atomic(void)
+{
+ preempt_enable();
+}
+
#define hotcpu_notifier(fn, pri) do { (void)(fn); } while (0)
/* These aren't inline functions due to a GCC bug. */
#define register_hotcpu_notifier(nb) ({ (void)(nb); 0; })
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 198a388..2d03398 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -154,6 +154,44 @@ void cpu_hotplug_enable(void)
cpu_maps_update_done();
}
+/*
+ * get_online_cpus_atomic - Prevent any CPU from going offline
+ *
+ * Atomic hotplug readers (tasks which wish to prevent CPUs from going
+ * offline during their critical section, but can't afford to sleep)
+ * can invoke this function to synchronize with CPU offline. This function
+ * can be called recursively, provided it is matched with an equal number
+ * of calls to put_online_cpus_atomic().
+ *
+ * Note: This does NOT prevent CPUs from coming online! It only prevents
+ * CPUs from going offline.
+ *
+ * Lock ordering rule: Strictly speaking, there is no lock ordering
+ * requirement here, but it is advisable to keep the locking consistent.
+ * As a simple rule-of-thumb, use these functions in the outer-most blocks
+ * of your critical sections, outside of other locks.
+ *
+ * Returns the current CPU number, with preemption disabled.
+ */
+unsigned int get_online_cpus_atomic(void)
+{
+ /*
+ * The current CPU hotplug implementation uses stop_machine() in
+ * the CPU offline path. And disabling preemption prevents
+ * stop_machine() from taking effect. Thus, this prevents any CPU
+ * from going offline.
+ */
+ preempt_disable();
+ return smp_processor_id();
+}
+EXPORT_SYMBOL_GPL(get_online_cpus_atomic);
+
+void put_online_cpus_atomic(void)
+{
+ preempt_enable();
+}
+EXPORT_SYMBOL_GPL(put_online_cpus_atomic);
+
#else /* #if CONFIG_HOTPLUG_CPU */
static void cpu_hotplug_begin(void) {}
static void cpu_hotplug_done(void) {}
^ permalink raw reply related
* [PATCH v3 02/45] CPU hotplug: Clarify the usage of different synchronization APIs
From: Srivatsa S. Bhat @ 2013-06-27 19:52 UTC (permalink / raw)
To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
walken, vincent.guittot, laijs, David.Laight
Cc: linux-arch, nikunj, zhong, linux-pm, fweisbec, Rafael J. Wysocki,
linux-kernel, rostedt, xiaoguangrong, sbw, Yasuaki Ishimatsu,
wangyun, Srivatsa S. Bhat, netdev, Thomas Gleixner, linuxppc-dev,
Andrew Morton
In-Reply-To: <20130627195136.29830.10445.stgit@srivatsabhat.in.ibm.com>
We have quite a few APIs now which help synchronize with CPU hotplug.
Among them, get/put_online_cpus() is the oldest and the most well-known,
so no problems there. By extension, its easy to comprehend the new
set : get/put_online_cpus_atomic().
But there is yet another set, which might appear tempting to use:
cpu_hotplug_disable()/cpu_hotplug_enable(). Add comments to clarify
that this latter set is NOT for general use and must be used only in
specific cases where the requirement is really to _disable_ hotplug
and not just to synchronize with it.
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---
kernel/cpu.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 2d03398..860f51a 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -139,6 +139,13 @@ static void cpu_hotplug_done(void)
* the 'cpu_hotplug_disabled' flag. The same lock is also acquired by the
* hotplug path before performing hotplug operations. So acquiring that lock
* guarantees mutual exclusion from any currently running hotplug operations.
+ *
+ * Note: In most cases, this is *NOT* the function you need. If you simply
+ * want to avoid racing with CPU hotplug operations, use get/put_online_cpus()
+ * or get/put_online_cpus_atomic(), depending on the situation.
+ *
+ * This set of functions is reserved for cases where you really wish to
+ * _disable_ CPU hotplug and not just synchronize with it.
*/
void cpu_hotplug_disable(void)
{
^ permalink raw reply related
* [PATCH v3 03/45] Documentation, CPU hotplug: Recommend usage of get/put_online_cpus_atomic()
From: Srivatsa S. Bhat @ 2013-06-27 19:52 UTC (permalink / raw)
To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
walken, vincent.guittot, laijs, David.Laight
Cc: linux-arch, nikunj, zhong, linux-pm, fweisbec, linux-doc,
linux-kernel, rostedt, xiaoguangrong, sbw, wangyun,
Srivatsa S. Bhat, netdev, Rob Landley, linuxppc-dev
In-Reply-To: <20130627195136.29830.10445.stgit@srivatsabhat.in.ibm.com>
Once stop_machine() is gone from the CPU offline path, we won't be able
to depend on disabling preemption to prevent CPUs from going offline
from under us.
So add documentation to recommend using the new get/put_online_cpus_atomic()
APIs to prevent CPUs from going offline, while invoking from atomic context.
Cc: Rob Landley <rob@landley.net>
Cc: linux-doc@vger.kernel.org
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---
Documentation/cpu-hotplug.txt | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/Documentation/cpu-hotplug.txt b/Documentation/cpu-hotplug.txt
index 9f40135..7b3ca60 100644
--- a/Documentation/cpu-hotplug.txt
+++ b/Documentation/cpu-hotplug.txt
@@ -113,13 +113,18 @@ Never use anything other than cpumask_t to represent bitmap of CPUs.
#include <linux/cpu.h>
get_online_cpus() and put_online_cpus():
-The above calls are used to inhibit cpu hotplug operations. While the
+The above calls are used to inhibit cpu hotplug operations, when invoked from
+non-atomic contexts (because the above functions can sleep). While the
cpu_hotplug.refcount is non zero, the cpu_online_mask will not change.
-If you merely need to avoid cpus going away, you could also use
-preempt_disable() and preempt_enable() for those sections.
-Just remember the critical section cannot call any
-function that can sleep or schedule this process away. The preempt_disable()
-will work as long as stop_machine_run() is used to take a cpu down.
+
+However, if you are executing in atomic context (ie., you can't afford to
+sleep), and you merely need to avoid cpus going offline, you can use
+get_online_cpus_atomic() and put_online_cpus_atomic() for those sections.
+Just remember the critical section cannot call any function that can sleep or
+schedule this process away. Using preempt_disable() will also work, as long
+as stop_machine() is used to take a CPU down. But we are going to get rid of
+stop_machine() in the CPU offline path soon, so it is strongly recommended
+to use the APIs mentioned above.
CPU Hotplug - Frequently Asked Questions.
@@ -360,6 +365,9 @@ A: There are two ways. If your code can be run in interrupt context, use
return err;
}
+ If my_func_on_cpu() itself cannot block, use get/put_online_cpus_atomic()
+ instead of get/put_online_cpus(), to prevent CPUs from going offline.
+
Q: How do we determine how many CPUs are available for hotplug.
A: There is no clear spec defined way from ACPI that can give us that
information today. Based on some input from Natalie of Unisys,
^ permalink raw reply related
* [PATCH v3 04/45] CPU hotplug: Add infrastructure to check lacking hotplug synchronization
From: Srivatsa S. Bhat @ 2013-06-27 19:53 UTC (permalink / raw)
To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
walken, vincent.guittot, laijs, David.Laight
Cc: linux-arch, Alex Shi, nikunj, zhong, linux-pm, fweisbec,
Rusty Russell, linux-kernel, rostedt, xiaoguangrong,
Rafael J. Wysocki, sbw, Yasuaki Ishimatsu, wangyun,
Srivatsa S. Bhat, netdev, Tejun Heo, Thomas Gleixner,
KOSAKI Motohiro, linuxppc-dev, Andrew Morton
In-Reply-To: <20130627195136.29830.10445.stgit@srivatsabhat.in.ibm.com>
Add a debugging infrastructure to warn if an atomic hotplug reader has not
invoked get_online_cpus_atomic() before traversing/accessing the
cpu_online_mask. Encapsulate these checks under a new debug config option
DEBUG_HOTPLUG_CPU.
This debugging infrastructure proves useful in the tree-wide conversion
of atomic hotplug readers from preempt_disable() to the new APIs, and
help us catch the places we missed, much before we actually get rid of
stop_machine(). We can perhaps remove the debugging checks later on.
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Alex Shi <alex.shi@intel.com>
Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---
include/linux/cpumask.h | 12 ++++++
kernel/cpu.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 101 insertions(+)
diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
index d08e4d2..9197ca4 100644
--- a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -101,6 +101,18 @@ extern const struct cpumask *const cpu_active_mask;
#define cpu_active(cpu) ((cpu) == 0)
#endif
+#ifdef CONFIG_DEBUG_HOTPLUG_CPU
+extern void check_hotplug_safe_cpumask(const struct cpumask *mask);
+extern void check_hotplug_safe_cpu(unsigned int cpu,
+ const struct cpumask *mask);
+#else
+static inline void check_hotplug_safe_cpumask(const struct cpumask *mask) { }
+static inline void check_hotplug_safe_cpu(unsigned int cpu,
+ const struct cpumask *mask)
+{
+}
+#endif
+
/* verify cpu argument to cpumask_* operators */
static inline unsigned int cpumask_check(unsigned int cpu)
{
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 860f51a..5297ec1 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -63,6 +63,92 @@ static struct {
.refcount = 0,
};
+#ifdef CONFIG_DEBUG_HOTPLUG_CPU
+
+static DEFINE_PER_CPU(unsigned long, atomic_reader_refcnt);
+
+static int current_is_hotplug_safe(const struct cpumask *mask)
+{
+
+ /* If we are not dealing with cpu_online_mask, don't complain. */
+ if (mask != cpu_online_mask)
+ return 1;
+
+ /* If this is the task doing hotplug, don't complain. */
+ if (unlikely(current == cpu_hotplug.active_writer))
+ return 1;
+
+ /* If we are in early boot, don't complain. */
+ if (system_state != SYSTEM_RUNNING)
+ return 1;
+
+ /*
+ * Check if the current task is in atomic context and it has
+ * invoked get_online_cpus_atomic() to synchronize with
+ * CPU Hotplug.
+ */
+ if (preempt_count() || irqs_disabled())
+ return this_cpu_read(atomic_reader_refcnt);
+ else
+ return 1; /* No checks for non-atomic contexts for now */
+}
+
+static inline void warn_hotplug_unsafe(void)
+{
+ WARN_ONCE(1, "Must use get/put_online_cpus_atomic() to synchronize"
+ " with CPU hotplug\n");
+}
+
+/*
+ * Check if the task (executing in atomic context) has the required protection
+ * against CPU hotplug, while accessing the specified cpumask.
+ */
+void check_hotplug_safe_cpumask(const struct cpumask *mask)
+{
+ if (!current_is_hotplug_safe(mask))
+ warn_hotplug_unsafe();
+}
+EXPORT_SYMBOL_GPL(check_hotplug_safe_cpumask);
+
+/*
+ * Similar to check_hotplug_safe_cpumask(), except that we don't complain
+ * if the task (executing in atomic context) is testing whether the CPU it
+ * is executing on is online or not.
+ *
+ * (A task executing with preemption disabled on a CPU, automatically prevents
+ * offlining that CPU, irrespective of the actual implementation of CPU
+ * offline. So we don't enforce holding of get_online_cpus_atomic() for that
+ * case).
+ */
+void check_hotplug_safe_cpu(unsigned int cpu, const struct cpumask *mask)
+{
+ if(!current_is_hotplug_safe(mask) && cpu != smp_processor_id())
+ warn_hotplug_unsafe();
+}
+EXPORT_SYMBOL_GPL(check_hotplug_safe_cpu);
+
+static inline void atomic_reader_refcnt_inc(void)
+{
+ this_cpu_inc(atomic_reader_refcnt);
+}
+
+static inline void atomic_reader_refcnt_dec(void)
+{
+ this_cpu_dec(atomic_reader_refcnt);
+}
+
+#else
+
+static inline void atomic_reader_refcnt_inc(void)
+{
+}
+
+static inline void atomic_reader_refcnt_dec(void)
+{
+}
+
+#endif
+
void get_online_cpus(void)
{
might_sleep();
@@ -189,12 +275,15 @@ unsigned int get_online_cpus_atomic(void)
* from going offline.
*/
preempt_disable();
+ atomic_reader_refcnt_inc();
+
return smp_processor_id();
}
EXPORT_SYMBOL_GPL(get_online_cpus_atomic);
void put_online_cpus_atomic(void)
{
+ atomic_reader_refcnt_dec();
preempt_enable();
}
EXPORT_SYMBOL_GPL(put_online_cpus_atomic);
^ permalink raw reply related
* [PATCH v3 05/45] CPU hotplug: Protect set_cpu_online() to avoid false-positives
From: Srivatsa S. Bhat @ 2013-06-27 19:53 UTC (permalink / raw)
To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
walken, vincent.guittot, laijs, David.Laight
Cc: linux-arch, nikunj, zhong, linux-pm, fweisbec, Rafael J. Wysocki,
linux-kernel, rostedt, xiaoguangrong, sbw, Yasuaki Ishimatsu,
wangyun, Srivatsa S. Bhat, netdev, Thomas Gleixner, linuxppc-dev,
Andrew Morton
In-Reply-To: <20130627195136.29830.10445.stgit@srivatsabhat.in.ibm.com>
When bringing a secondary CPU online, the task running on the CPU coming up
sets itself in the cpu_online_mask. This is safe even though this task is not
the hotplug writer task.
But it is kinda hard to teach this to the CPU hotplug debug infrastructure,
and if we get it wrong, we risk making the debug code too lenient, risking
false-negatives.
Luckily, all architectures use set_cpu_online() to manipulate the
cpu_online_mask. So, to avoid false-positive warnings from the CPU hotplug
debug code, encapsulate the body of set_cpu_online() within
get/put_online_cpus_atomic().
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---
kernel/cpu.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 5297ec1..35e7115 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -832,10 +832,14 @@ void set_cpu_present(unsigned int cpu, bool present)
void set_cpu_online(unsigned int cpu, bool online)
{
+ get_online_cpus_atomic();
+
if (online)
cpumask_set_cpu(cpu, to_cpumask(cpu_online_bits));
else
cpumask_clear_cpu(cpu, to_cpumask(cpu_online_bits));
+
+ put_online_cpus_atomic();
}
void set_cpu_active(unsigned int cpu, bool active)
^ permalink raw reply related
* [PATCH v3 06/45] CPU hotplug: Sprinkle debugging checks to catch locking bugs
From: Srivatsa S. Bhat @ 2013-06-27 19:53 UTC (permalink / raw)
To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
walken, vincent.guittot, laijs, David.Laight
Cc: linux-arch, Alex Shi, nikunj, zhong, linux-pm, fweisbec,
Rusty Russell, linux-kernel, rostedt, xiaoguangrong, sbw,
Joonsoo Kim, wangyun, Srivatsa S. Bhat, netdev, Tejun Heo,
Andrew Morton, KOSAKI Motohiro, linuxppc-dev
In-Reply-To: <20130627195136.29830.10445.stgit@srivatsabhat.in.ibm.com>
Now that we have a debug infrastructure in place to detect cases where
get/put_online_cpus_atomic() had to be used, add these checks at the
right spots to help catch places where we missed converting to the new
APIs.
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Alex Shi <alex.shi@intel.com>
Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Joonsoo Kim <js1304@gmail.com>
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---
include/linux/cpumask.h | 47 +++++++++++++++++++++++++++++++++++++++++++++--
lib/cpumask.c | 8 ++++++++
2 files changed, 53 insertions(+), 2 deletions(-)
diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
index 9197ca4..06d2c36 100644
--- a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -169,6 +169,7 @@ static inline unsigned int cpumask_any_but(const struct cpumask *mask,
*/
static inline unsigned int cpumask_first(const struct cpumask *srcp)
{
+ check_hotplug_safe_cpumask(srcp);
return find_first_bit(cpumask_bits(srcp), nr_cpumask_bits);
}
@@ -184,6 +185,8 @@ static inline unsigned int cpumask_next(int n, const struct cpumask *srcp)
/* -1 is a legal arg here. */
if (n != -1)
cpumask_check(n);
+
+ check_hotplug_safe_cpumask(srcp);
return find_next_bit(cpumask_bits(srcp), nr_cpumask_bits, n+1);
}
@@ -199,6 +202,8 @@ static inline unsigned int cpumask_next_zero(int n, const struct cpumask *srcp)
/* -1 is a legal arg here. */
if (n != -1)
cpumask_check(n);
+
+ check_hotplug_safe_cpumask(srcp);
return find_next_zero_bit(cpumask_bits(srcp), nr_cpumask_bits, n+1);
}
@@ -288,8 +293,15 @@ static inline void cpumask_clear_cpu(int cpu, struct cpumask *dstp)
*
* No static inline type checking - see Subtlety (1) above.
*/
-#define cpumask_test_cpu(cpu, cpumask) \
- test_bit(cpumask_check(cpu), cpumask_bits((cpumask)))
+#define cpumask_test_cpu(cpu, cpumask) \
+({ \
+ int __ret; \
+ \
+ check_hotplug_safe_cpu(cpu, cpumask); \
+ __ret = test_bit(cpumask_check(cpu), \
+ cpumask_bits((cpumask))); \
+ __ret; \
+})
/**
* cpumask_test_and_set_cpu - atomically test and set a cpu in a cpumask
@@ -349,6 +361,9 @@ static inline int cpumask_and(struct cpumask *dstp,
const struct cpumask *src1p,
const struct cpumask *src2p)
{
+ check_hotplug_safe_cpumask(src1p);
+ check_hotplug_safe_cpumask(src2p);
+
return bitmap_and(cpumask_bits(dstp), cpumask_bits(src1p),
cpumask_bits(src2p), nr_cpumask_bits);
}
@@ -362,6 +377,9 @@ static inline int cpumask_and(struct cpumask *dstp,
static inline void cpumask_or(struct cpumask *dstp, const struct cpumask *src1p,
const struct cpumask *src2p)
{
+ check_hotplug_safe_cpumask(src1p);
+ check_hotplug_safe_cpumask(src2p);
+
bitmap_or(cpumask_bits(dstp), cpumask_bits(src1p),
cpumask_bits(src2p), nr_cpumask_bits);
}
@@ -376,6 +394,9 @@ static inline void cpumask_xor(struct cpumask *dstp,
const struct cpumask *src1p,
const struct cpumask *src2p)
{
+ check_hotplug_safe_cpumask(src1p);
+ check_hotplug_safe_cpumask(src2p);
+
bitmap_xor(cpumask_bits(dstp), cpumask_bits(src1p),
cpumask_bits(src2p), nr_cpumask_bits);
}
@@ -392,6 +413,9 @@ static inline int cpumask_andnot(struct cpumask *dstp,
const struct cpumask *src1p,
const struct cpumask *src2p)
{
+ check_hotplug_safe_cpumask(src1p);
+ check_hotplug_safe_cpumask(src2p);
+
return bitmap_andnot(cpumask_bits(dstp), cpumask_bits(src1p),
cpumask_bits(src2p), nr_cpumask_bits);
}
@@ -404,6 +428,8 @@ static inline int cpumask_andnot(struct cpumask *dstp,
static inline void cpumask_complement(struct cpumask *dstp,
const struct cpumask *srcp)
{
+ check_hotplug_safe_cpumask(srcp);
+
bitmap_complement(cpumask_bits(dstp), cpumask_bits(srcp),
nr_cpumask_bits);
}
@@ -416,6 +442,9 @@ static inline void cpumask_complement(struct cpumask *dstp,
static inline bool cpumask_equal(const struct cpumask *src1p,
const struct cpumask *src2p)
{
+ check_hotplug_safe_cpumask(src1p);
+ check_hotplug_safe_cpumask(src2p);
+
return bitmap_equal(cpumask_bits(src1p), cpumask_bits(src2p),
nr_cpumask_bits);
}
@@ -428,6 +457,10 @@ static inline bool cpumask_equal(const struct cpumask *src1p,
static inline bool cpumask_intersects(const struct cpumask *src1p,
const struct cpumask *src2p)
{
+
+ check_hotplug_safe_cpumask(src1p);
+ check_hotplug_safe_cpumask(src2p);
+
return bitmap_intersects(cpumask_bits(src1p), cpumask_bits(src2p),
nr_cpumask_bits);
}
@@ -442,6 +475,9 @@ static inline bool cpumask_intersects(const struct cpumask *src1p,
static inline int cpumask_subset(const struct cpumask *src1p,
const struct cpumask *src2p)
{
+ check_hotplug_safe_cpumask(src1p);
+ check_hotplug_safe_cpumask(src2p);
+
return bitmap_subset(cpumask_bits(src1p), cpumask_bits(src2p),
nr_cpumask_bits);
}
@@ -470,6 +506,12 @@ static inline bool cpumask_full(const struct cpumask *srcp)
*/
static inline unsigned int cpumask_weight(const struct cpumask *srcp)
{
+ /*
+ * Often, we just want to have a rough estimate of the number of
+ * online CPUs, without going to the trouble of synchronizing with
+ * CPU hotplug. So don't invoke check_hotplug_safe_cpumask() here.
+ */
+
return bitmap_weight(cpumask_bits(srcp), nr_cpumask_bits);
}
@@ -507,6 +549,7 @@ static inline void cpumask_shift_left(struct cpumask *dstp,
static inline void cpumask_copy(struct cpumask *dstp,
const struct cpumask *srcp)
{
+ check_hotplug_safe_cpumask(srcp);
bitmap_copy(cpumask_bits(dstp), cpumask_bits(srcp), nr_cpumask_bits);
}
diff --git a/lib/cpumask.c b/lib/cpumask.c
index d327b87..481df57 100644
--- a/lib/cpumask.c
+++ b/lib/cpumask.c
@@ -7,12 +7,14 @@
int __first_cpu(const cpumask_t *srcp)
{
+ check_hotplug_safe_cpumask(srcp);
return min_t(int, NR_CPUS, find_first_bit(srcp->bits, NR_CPUS));
}
EXPORT_SYMBOL(__first_cpu);
int __next_cpu(int n, const cpumask_t *srcp)
{
+ check_hotplug_safe_cpumask(srcp);
return min_t(int, NR_CPUS, find_next_bit(srcp->bits, NR_CPUS, n+1));
}
EXPORT_SYMBOL(__next_cpu);
@@ -20,6 +22,7 @@ EXPORT_SYMBOL(__next_cpu);
#if NR_CPUS > 64
int __next_cpu_nr(int n, const cpumask_t *srcp)
{
+ check_hotplug_safe_cpumask(srcp);
return min_t(int, nr_cpu_ids,
find_next_bit(srcp->bits, nr_cpu_ids, n+1));
}
@@ -37,6 +40,9 @@ EXPORT_SYMBOL(__next_cpu_nr);
int cpumask_next_and(int n, const struct cpumask *src1p,
const struct cpumask *src2p)
{
+ check_hotplug_safe_cpumask(src1p);
+ check_hotplug_safe_cpumask(src2p);
+
while ((n = cpumask_next(n, src1p)) < nr_cpu_ids)
if (cpumask_test_cpu(n, src2p))
break;
@@ -57,6 +63,8 @@ int cpumask_any_but(const struct cpumask *mask, unsigned int cpu)
unsigned int i;
cpumask_check(cpu);
+ check_hotplug_safe_cpumask(mask);
+
for_each_cpu(i, mask)
if (i != cpu)
break;
^ permalink raw reply related
* [PATCH v3 07/45] CPU hotplug: Add _nocheck() variants of accessor functions
From: Srivatsa S. Bhat @ 2013-06-27 19:53 UTC (permalink / raw)
To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
walken, vincent.guittot, laijs, David.Laight
Cc: linux-arch, Alex Shi, nikunj, zhong, linux-pm, fweisbec,
Rusty Russell, linux-kernel, rostedt, xiaoguangrong, sbw,
Joonsoo Kim, wangyun, Srivatsa S. Bhat, netdev, Tejun Heo,
Andrew Morton, KOSAKI Motohiro, linuxppc-dev
In-Reply-To: <20130627195136.29830.10445.stgit@srivatsabhat.in.ibm.com>
Sometimes, we have situations where the synchronization design of a
particular subsystem handles CPU hotplug properly, but the details are
non-trivial, making it hard to teach this to the rudimentary hotplug
locking validator. In such cases, it would be useful to have a set of
_nocheck() variants of the cpu accessor functions, to avoid false-positive
warnings from the hotplug locking validator.
However, we won't go overboard with that; we'll add them only on a
case-by-case basis and mandate that the call-sites which use them add
a comment explaining why it is hotplug safe and hence justify the use
of the _nocheck() variants.
At the moment, the RCU and the percpu-counter code have legitimate
reasons to use the _nocheck() variants, so let's add them for
cpu_is_offline() and for_each_online_cpu(), for use in those subsystems
respectively.
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Alex Shi <alex.shi@intel.com>
Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Joonsoo Kim <js1304@gmail.com>
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---
include/linux/cpumask.h | 59 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 59 insertions(+)
diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
index 06d2c36..f577a7d 100644
--- a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -87,6 +87,7 @@ extern const struct cpumask *const cpu_active_mask;
#define num_present_cpus() cpumask_weight(cpu_present_mask)
#define num_active_cpus() cpumask_weight(cpu_active_mask)
#define cpu_online(cpu) cpumask_test_cpu((cpu), cpu_online_mask)
+#define cpu_online_nocheck(cpu) cpumask_test_cpu_nocheck((cpu), cpu_online_mask)
#define cpu_possible(cpu) cpumask_test_cpu((cpu), cpu_possible_mask)
#define cpu_present(cpu) cpumask_test_cpu((cpu), cpu_present_mask)
#define cpu_active(cpu) cpumask_test_cpu((cpu), cpu_active_mask)
@@ -96,6 +97,7 @@ extern const struct cpumask *const cpu_active_mask;
#define num_present_cpus() 1U
#define num_active_cpus() 1U
#define cpu_online(cpu) ((cpu) == 0)
+#define cpu_online_nocheck(cpu) cpu_online((cpu))
#define cpu_possible(cpu) ((cpu) == 0)
#define cpu_present(cpu) ((cpu) == 0)
#define cpu_active(cpu) ((cpu) == 0)
@@ -156,6 +158,8 @@ static inline unsigned int cpumask_any_but(const struct cpumask *mask,
#define for_each_cpu(cpu, mask) \
for ((cpu) = 0; (cpu) < 1; (cpu)++, (void)mask)
+#define for_each_cpu_nocheck(cpu, mask) \
+ for_each_cpu((cpu), (mask))
#define for_each_cpu_not(cpu, mask) \
for ((cpu) = 0; (cpu) < 1; (cpu)++, (void)mask)
#define for_each_cpu_and(cpu, mask, and) \
@@ -191,6 +195,24 @@ static inline unsigned int cpumask_next(int n, const struct cpumask *srcp)
}
/**
+ * cpumask_next_nocheck - get the next cpu in a cpumask, without checking
+ * for hotplug safety
+ * @n: the cpu prior to the place to search (ie. return will be > @n)
+ * @srcp: the cpumask pointer
+ *
+ * Returns >= nr_cpu_ids if no further cpus set.
+ */
+static inline unsigned int cpumask_next_nocheck(int n,
+ const struct cpumask *srcp)
+{
+ /* -1 is a legal arg here. */
+ if (n != -1)
+ cpumask_check(n);
+
+ return find_next_bit(cpumask_bits(srcp), nr_cpumask_bits, n+1);
+}
+
+/**
* cpumask_next_zero - get the next unset cpu in a cpumask
* @n: the cpu prior to the place to search (ie. return will be > @n)
* @srcp: the cpumask pointer
@@ -222,6 +244,21 @@ int cpumask_any_but(const struct cpumask *mask, unsigned int cpu);
(cpu) = cpumask_next((cpu), (mask)), \
(cpu) < nr_cpu_ids;)
+
+/**
+ * for_each_cpu_nocheck - iterate over every cpu in a mask,
+ * without checking for hotplug safety
+ * @cpu: the (optionally unsigned) integer iterator
+ * @mask: the cpumask pointer
+ *
+ * After the loop, cpu is >= nr_cpu_ids.
+ */
+#define for_each_cpu_nocheck(cpu, mask) \
+ for ((cpu) = -1; \
+ (cpu) = cpumask_next_nocheck((cpu), (mask)), \
+ (cpu) < nr_cpu_ids;)
+
+
/**
* for_each_cpu_not - iterate over every cpu in a complemented mask
* @cpu: the (optionally unsigned) integer iterator
@@ -304,6 +341,25 @@ static inline void cpumask_clear_cpu(int cpu, struct cpumask *dstp)
})
/**
+ * cpumask_test_cpu_nocheck - test for a cpu in a cpumask, without
+ * checking for hotplug safety
+ * @cpu: cpu number (< nr_cpu_ids)
+ * @cpumask: the cpumask pointer
+ *
+ * Returns 1 if @cpu is set in @cpumask, else returns 0
+ *
+ * No static inline type checking - see Subtlety (1) above.
+ */
+#define cpumask_test_cpu_nocheck(cpu, cpumask) \
+({ \
+ int __ret; \
+ \
+ __ret = test_bit(cpumask_check(cpu), \
+ cpumask_bits((cpumask))); \
+ __ret; \
+})
+
+/**
* cpumask_test_and_set_cpu - atomically test and set a cpu in a cpumask
* @cpu: cpu number (< nr_cpu_ids)
* @cpumask: the cpumask pointer
@@ -775,6 +831,8 @@ extern const DECLARE_BITMAP(cpu_all_bits, NR_CPUS);
#define for_each_possible_cpu(cpu) for_each_cpu((cpu), cpu_possible_mask)
#define for_each_online_cpu(cpu) for_each_cpu((cpu), cpu_online_mask)
+#define for_each_online_cpu_nocheck(cpu) \
+ for_each_cpu_nocheck((cpu), cpu_online_mask)
#define for_each_present_cpu(cpu) for_each_cpu((cpu), cpu_present_mask)
/* Wrappers for arch boot code to manipulate normally-constant masks */
@@ -823,6 +881,7 @@ static inline const struct cpumask *get_cpu_mask(unsigned int cpu)
}
#define cpu_is_offline(cpu) unlikely(!cpu_online(cpu))
+#define cpu_is_offline_nocheck(cpu) unlikely(!cpu_online_nocheck(cpu))
#if NR_CPUS <= BITS_PER_LONG
#define CPU_BITS_ALL \
^ permalink raw reply related
* [PATCH v3 08/45] CPU hotplug: Expose the new debug config option
From: Srivatsa S. Bhat @ 2013-06-27 19:53 UTC (permalink / raw)
To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
walken, vincent.guittot, laijs, David.Laight
Cc: linux-arch, Michel Lespinasse, Catalin Marinas, nikunj, zhong,
linux-pm, fweisbec, linux-kernel, rostedt, xiaoguangrong, sbw,
Akinobu Mita, wangyun, Srivatsa S. Bhat, netdev, Andrew Morton,
Paul E. McKenney, linuxppc-dev, Sergei Shtylyov
In-Reply-To: <20130627195136.29830.10445.stgit@srivatsabhat.in.ibm.com>
Now that we have all the pieces of the CPU hotplug debug infrastructure
in place, expose the feature by growing a new Kconfig option,
CONFIG_DEBUG_HOTPLUG_CPU.
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: "Paul E. McKenney" <paul.mckenney@linaro.org>
Cc: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Michel Lespinasse <walken@google.com>
Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---
lib/Kconfig.debug | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 7154f79..42e17f0 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -512,6 +512,14 @@ config DEBUG_PREEMPT
if kernel code uses it in a preemption-unsafe way. Also, the kernel
will detect preemption count underflows.
+config DEBUG_HOTPLUG_CPU
+ bool "Debug CPU hotplug"
+ depends on HOTPLUG_CPU
+ help
+ If you say Y here, the kernel will check all the accesses of
+ cpu_online_mask from atomic contexts, and will print warnings if
+ the task lacks appropriate synchronization with CPU hotplug.
+
config DEBUG_RT_MUTEXES
bool "RT Mutex debugging, deadlock detection"
depends on DEBUG_KERNEL && RT_MUTEXES
^ permalink raw reply related
* [PATCH v3 09/45] CPU hotplug: Convert preprocessor macros to static inline functions
From: Srivatsa S. Bhat @ 2013-06-27 19:54 UTC (permalink / raw)
To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
walken, vincent.guittot, laijs, David.Laight
Cc: linux-arch, nikunj, zhong, linux-pm, fweisbec, linux-kernel,
rostedt, xiaoguangrong, Rafael J. Wysocki, sbw, wangyun,
Srivatsa S. Bhat, netdev, Tejun Heo, Thomas Gleixner,
linuxppc-dev, Andrew Morton
In-Reply-To: <20130627195136.29830.10445.stgit@srivatsabhat.in.ibm.com>
Convert the macros in the CPU hotplug code to static inline C functions.
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---
include/linux/cpu.h | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index a57b25a..85431a0 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -202,10 +202,8 @@ static inline void cpu_hotplug_driver_unlock(void)
#else /* CONFIG_HOTPLUG_CPU */
-#define get_online_cpus() do { } while (0)
-#define put_online_cpus() do { } while (0)
-#define cpu_hotplug_disable() do { } while (0)
-#define cpu_hotplug_enable() do { } while (0)
+static inline void get_online_cpus(void) {}
+static inline void put_online_cpus(void) {}
static inline unsigned int get_online_cpus_atomic(void)
{
@@ -222,6 +220,9 @@ static inline void put_online_cpus_atomic(void)
preempt_enable();
}
+static inline void cpu_hotplug_disable(void) {}
+static inline void cpu_hotplug_enable(void) {}
+
#define hotcpu_notifier(fn, pri) do { (void)(fn); } while (0)
/* These aren't inline functions due to a GCC bug. */
#define register_hotcpu_notifier(nb) ({ (void)(nb); 0; })
^ permalink raw reply related
* [PATCH v3 10/45] smp: Use get/put_online_cpus_atomic() to prevent CPU offline
From: Srivatsa S. Bhat @ 2013-06-27 19:54 UTC (permalink / raw)
To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
walken, vincent.guittot, laijs, David.Laight
Cc: linux-arch, nikunj, zhong, linux-pm, fweisbec, Shaohua Li,
linux-kernel, rostedt, xiaoguangrong, sbw, Jan Beulich, wangyun,
Srivatsa S. Bhat, netdev, Andrew Morton, linuxppc-dev, liguang,
Wang YanQing
In-Reply-To: <20130627195136.29830.10445.stgit@srivatsabhat.in.ibm.com>
Once stop_machine() is gone from the CPU offline path, we won't be able
to depend on disabling preemption to prevent CPUs from going offline
from under us.
Use the get/put_online_cpus_atomic() APIs to prevent CPUs from going
offline, while invoking from atomic context.
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Wang YanQing <udknight@gmail.com>
Cc: Shaohua Li <shli@fusionio.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: liguang <lig.fnst@cn.fujitsu.com>
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---
kernel/smp.c | 52 ++++++++++++++++++++++++++++++----------------------
1 file changed, 30 insertions(+), 22 deletions(-)
diff --git a/kernel/smp.c b/kernel/smp.c
index 4dba0f7..1f36d6d 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -232,7 +232,7 @@ int smp_call_function_single(int cpu, smp_call_func_t func, void *info,
* prevent preemption and reschedule on another processor,
* as well as CPU removal
*/
- this_cpu = get_cpu();
+ this_cpu = get_online_cpus_atomic();
/*
* Can deadlock when called with interrupts disabled.
@@ -264,7 +264,7 @@ int smp_call_function_single(int cpu, smp_call_func_t func, void *info,
}
}
- put_cpu();
+ put_online_cpus_atomic();
return err;
}
@@ -294,7 +294,7 @@ int smp_call_function_any(const struct cpumask *mask,
int ret;
/* Try for same CPU (cheapest) */
- cpu = get_cpu();
+ cpu = get_online_cpus_atomic();
if (cpumask_test_cpu(cpu, mask))
goto call;
@@ -310,7 +310,7 @@ int smp_call_function_any(const struct cpumask *mask,
cpu = cpumask_any_and(mask, cpu_online_mask);
call:
ret = smp_call_function_single(cpu, func, info, wait);
- put_cpu();
+ put_online_cpus_atomic();
return ret;
}
EXPORT_SYMBOL_GPL(smp_call_function_any);
@@ -331,7 +331,8 @@ void __smp_call_function_single(int cpu, struct call_single_data *csd,
unsigned int this_cpu;
unsigned long flags;
- this_cpu = get_cpu();
+ this_cpu = get_online_cpus_atomic();
+
/*
* Can deadlock when called with interrupts disabled.
* We allow cpu's that are not yet online though, as no one else can
@@ -349,7 +350,8 @@ void __smp_call_function_single(int cpu, struct call_single_data *csd,
csd_lock(csd);
generic_exec_single(cpu, csd, wait);
}
- put_cpu();
+
+ put_online_cpus_atomic();
}
/**
@@ -370,7 +372,9 @@ void smp_call_function_many(const struct cpumask *mask,
smp_call_func_t func, void *info, bool wait)
{
struct call_function_data *cfd;
- int cpu, next_cpu, this_cpu = smp_processor_id();
+ int cpu, next_cpu, this_cpu;
+
+ this_cpu = get_online_cpus_atomic();
/*
* Can deadlock when called with interrupts disabled.
@@ -388,7 +392,7 @@ void smp_call_function_many(const struct cpumask *mask,
/* No online cpus? We're done. */
if (cpu >= nr_cpu_ids)
- return;
+ goto out;
/* Do we have another CPU which isn't us? */
next_cpu = cpumask_next_and(cpu, mask, cpu_online_mask);
@@ -398,7 +402,7 @@ void smp_call_function_many(const struct cpumask *mask,
/* Fastpath: do that cpu by itself. */
if (next_cpu >= nr_cpu_ids) {
smp_call_function_single(cpu, func, info, wait);
- return;
+ goto out;
}
cfd = &__get_cpu_var(cfd_data);
@@ -408,7 +412,7 @@ void smp_call_function_many(const struct cpumask *mask,
/* Some callers race with other cpus changing the passed mask */
if (unlikely(!cpumask_weight(cfd->cpumask)))
- return;
+ goto out;
/*
* After we put an entry into the list, cfd->cpumask may be cleared
@@ -443,6 +447,9 @@ void smp_call_function_many(const struct cpumask *mask,
csd_lock_wait(csd);
}
}
+
+out:
+ put_online_cpus_atomic();
}
EXPORT_SYMBOL(smp_call_function_many);
@@ -463,9 +470,9 @@ EXPORT_SYMBOL(smp_call_function_many);
*/
int smp_call_function(smp_call_func_t func, void *info, int wait)
{
- preempt_disable();
+ get_online_cpus_atomic();
smp_call_function_many(cpu_online_mask, func, info, wait);
- preempt_enable();
+ put_online_cpus_atomic();
return 0;
}
@@ -565,12 +572,12 @@ int on_each_cpu(void (*func) (void *info), void *info, int wait)
unsigned long flags;
int ret = 0;
- preempt_disable();
+ get_online_cpus_atomic();
ret = smp_call_function(func, info, wait);
local_irq_save(flags);
func(info);
local_irq_restore(flags);
- preempt_enable();
+ put_online_cpus_atomic();
return ret;
}
EXPORT_SYMBOL(on_each_cpu);
@@ -592,7 +599,7 @@ EXPORT_SYMBOL(on_each_cpu);
void on_each_cpu_mask(const struct cpumask *mask, smp_call_func_t func,
void *info, bool wait)
{
- int cpu = get_cpu();
+ unsigned int cpu = get_online_cpus_atomic();
smp_call_function_many(mask, func, info, wait);
if (cpumask_test_cpu(cpu, mask)) {
@@ -600,7 +607,7 @@ void on_each_cpu_mask(const struct cpumask *mask, smp_call_func_t func,
func(info);
local_irq_enable();
}
- put_cpu();
+ put_online_cpus_atomic();
}
EXPORT_SYMBOL(on_each_cpu_mask);
@@ -625,8 +632,9 @@ EXPORT_SYMBOL(on_each_cpu_mask);
* The function might sleep if the GFP flags indicates a non
* atomic allocation is allowed.
*
- * Preemption is disabled to protect against CPUs going offline but not online.
- * CPUs going online during the call will not be seen or sent an IPI.
+ * We use get/put_online_cpus_atomic() to protect against CPUs going
+ * offline but not online. CPUs going online during the call will
+ * not be seen or sent an IPI.
*
* You must not call this function with disabled interrupts or
* from a hardware interrupt handler or from a bottom half handler.
@@ -641,26 +649,26 @@ void on_each_cpu_cond(bool (*cond_func)(int cpu, void *info),
might_sleep_if(gfp_flags & __GFP_WAIT);
if (likely(zalloc_cpumask_var(&cpus, (gfp_flags|__GFP_NOWARN)))) {
- preempt_disable();
+ get_online_cpus_atomic();
for_each_online_cpu(cpu)
if (cond_func(cpu, info))
cpumask_set_cpu(cpu, cpus);
on_each_cpu_mask(cpus, func, info, wait);
- preempt_enable();
+ put_online_cpus_atomic();
free_cpumask_var(cpus);
} else {
/*
* No free cpumask, bother. No matter, we'll
* just have to IPI them one by one.
*/
- preempt_disable();
+ get_online_cpus_atomic();
for_each_online_cpu(cpu)
if (cond_func(cpu, info)) {
ret = smp_call_function_single(cpu, func,
info, wait);
WARN_ON_ONCE(!ret);
}
- preempt_enable();
+ put_online_cpus_atomic();
}
}
EXPORT_SYMBOL(on_each_cpu_cond);
^ permalink raw reply related
* [PATCH v3 11/45] sched/core: Use get/put_online_cpus_atomic() to prevent CPU offline
From: Srivatsa S. Bhat @ 2013-06-27 19:54 UTC (permalink / raw)
To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
walken, vincent.guittot, laijs, David.Laight
Cc: linux-arch, nikunj, zhong, linux-pm, Peter Zijlstra, fweisbec,
linux-kernel, rostedt, xiaoguangrong, sbw, Ingo Molnar, wangyun,
Srivatsa S. Bhat, netdev, linuxppc-dev
In-Reply-To: <20130627195136.29830.10445.stgit@srivatsabhat.in.ibm.com>
Once stop_machine() is gone from the CPU offline path, we won't be able
to depend on disabling preemption to prevent CPUs from going offline
from under us.
Use the get/put_online_cpus_atomic() APIs to prevent CPUs from going
offline, while invoking from atomic context.
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---
kernel/sched/core.c | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 9b1f2e5..9d870bf 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1160,11 +1160,11 @@ void kick_process(struct task_struct *p)
{
int cpu;
- preempt_disable();
+ get_online_cpus_atomic();
cpu = task_cpu(p);
if ((cpu != smp_processor_id()) && task_curr(p))
smp_send_reschedule(cpu);
- preempt_enable();
+ put_online_cpus_atomic();
}
EXPORT_SYMBOL_GPL(kick_process);
#endif /* CONFIG_SMP */
@@ -1172,6 +1172,9 @@ EXPORT_SYMBOL_GPL(kick_process);
#ifdef CONFIG_SMP
/*
* ->cpus_allowed is protected by both rq->lock and p->pi_lock
+ *
+ * Must be called within get/put_online_cpus_atomic(), to prevent
+ * CPUs from going offline from under us.
*/
static int select_fallback_rq(int cpu, struct task_struct *p)
{
@@ -1245,6 +1248,9 @@ out:
/*
* The caller (fork, wakeup) owns p->pi_lock, ->cpus_allowed is stable.
+ *
+ * Must be called within get/put_online_cpus_atomic(), to prevent
+ * CPUs from going offline from under us.
*/
static inline
int select_task_rq(struct task_struct *p, int sd_flags, int wake_flags)
@@ -1489,6 +1495,8 @@ try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)
unsigned long flags;
int cpu, success = 0;
+ get_online_cpus_atomic();
+
smp_wmb();
raw_spin_lock_irqsave(&p->pi_lock, flags);
if (!(p->state & state))
@@ -1531,6 +1539,7 @@ stat:
out:
raw_spin_unlock_irqrestore(&p->pi_lock, flags);
+ put_online_cpus_atomic();
return success;
}
@@ -1744,6 +1753,8 @@ void wake_up_new_task(struct task_struct *p)
unsigned long flags;
struct rq *rq;
+ get_online_cpus_atomic();
+
raw_spin_lock_irqsave(&p->pi_lock, flags);
#ifdef CONFIG_SMP
/*
@@ -1766,6 +1777,8 @@ void wake_up_new_task(struct task_struct *p)
p->sched_class->task_woken(rq, p);
#endif
task_rq_unlock(rq, p, &flags);
+
+ put_online_cpus_atomic();
}
#ifdef CONFIG_PREEMPT_NOTIFIERS
@@ -3879,6 +3892,8 @@ bool __sched yield_to(struct task_struct *p, bool preempt)
unsigned long flags;
int yielded = 0;
+ get_online_cpus_atomic();
+
local_irq_save(flags);
rq = this_rq();
@@ -3924,6 +3939,8 @@ out_unlock:
out_irq:
local_irq_restore(flags);
+ put_online_cpus_atomic();
+
if (yielded > 0)
schedule();
@@ -4324,9 +4341,11 @@ static int migration_cpu_stop(void *data)
* The original target cpu might have gone down and we might
* be on another cpu but it doesn't matter.
*/
+ get_online_cpus_atomic();
local_irq_disable();
__migrate_task(arg->task, raw_smp_processor_id(), arg->dest_cpu);
local_irq_enable();
+ put_online_cpus_atomic();
return 0;
}
^ permalink raw reply related
* [PATCH v3 12/45] migration: Use raw_spin_lock/unlock since interrupts are already disabled
From: Srivatsa S. Bhat @ 2013-06-27 19:54 UTC (permalink / raw)
To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
walken, vincent.guittot, laijs, David.Laight
Cc: linux-arch, nikunj, zhong, linux-pm, Peter Zijlstra, fweisbec,
linux-kernel, rostedt, xiaoguangrong, sbw, Ingo Molnar, wangyun,
Srivatsa S. Bhat, netdev, linuxppc-dev
In-Reply-To: <20130627195136.29830.10445.stgit@srivatsabhat.in.ibm.com>
We need not use the raw_spin_lock_irqsave/restore primitives because
all CPU_DYING notifiers run with interrupts disabled. So just use
raw_spin_lock/unlock.
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---
kernel/sched/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 9d870bf..ad8a554 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -4675,14 +4675,14 @@ migration_call(struct notifier_block *nfb, unsigned long action, void *hcpu)
case CPU_DYING:
sched_ttwu_pending();
/* Update our root-domain */
- raw_spin_lock_irqsave(&rq->lock, flags);
+ raw_spin_lock(&rq->lock); /* IRQs already disabled */
if (rq->rd) {
BUG_ON(!cpumask_test_cpu(cpu, rq->rd->span));
set_rq_offline(rq);
}
migrate_tasks(cpu);
BUG_ON(rq->nr_running != 1); /* the migration thread */
- raw_spin_unlock_irqrestore(&rq->lock, flags);
+ raw_spin_unlock(&rq->lock);
break;
case CPU_DEAD:
^ permalink raw reply related
* [PATCH v3 13/45] sched/fair: Use get/put_online_cpus_atomic() to prevent CPU offline
From: Srivatsa S. Bhat @ 2013-06-27 19:54 UTC (permalink / raw)
To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
walken, vincent.guittot, laijs, David.Laight
Cc: linux-arch, nikunj, zhong, linux-pm, Peter Zijlstra, fweisbec,
linux-kernel, rostedt, xiaoguangrong, sbw, Ingo Molnar, wangyun,
Srivatsa S. Bhat, netdev, linuxppc-dev
In-Reply-To: <20130627195136.29830.10445.stgit@srivatsabhat.in.ibm.com>
Once stop_machine() is gone from the CPU offline path, we won't be able
to depend on disabling preemption to prevent CPUs from going offline
from under us.
Use the get/put_online_cpus_atomic() APIs to prevent CPUs from going
offline, while invoking from atomic context.
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---
kernel/sched/fair.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index f77f9c5..62d98dd 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -3369,7 +3369,8 @@ done:
*
* Returns the target CPU number, or the same CPU if no balancing is needed.
*
- * preempt must be disabled.
+ * Must be called within get/put_online_cpus_atomic(), to prevent CPUs
+ * from going offline from under us.
*/
static int
select_task_rq_fair(struct task_struct *p, int sd_flag, int wake_flags)
@@ -5289,6 +5290,8 @@ void idle_balance(int this_cpu, struct rq *this_rq)
raw_spin_unlock(&this_rq->lock);
update_blocked_averages(this_cpu);
+
+ get_online_cpus_atomic();
rcu_read_lock();
for_each_domain(this_cpu, sd) {
unsigned long interval;
@@ -5312,6 +5315,7 @@ void idle_balance(int this_cpu, struct rq *this_rq)
}
}
rcu_read_unlock();
+ put_online_cpus_atomic();
raw_spin_lock(&this_rq->lock);
@@ -5338,6 +5342,7 @@ static int active_load_balance_cpu_stop(void *data)
struct rq *target_rq = cpu_rq(target_cpu);
struct sched_domain *sd;
+ get_online_cpus_atomic();
raw_spin_lock_irq(&busiest_rq->lock);
/* make sure the requested cpu hasn't gone down in the meantime */
@@ -5389,6 +5394,7 @@ static int active_load_balance_cpu_stop(void *data)
out_unlock:
busiest_rq->active_balance = 0;
raw_spin_unlock_irq(&busiest_rq->lock);
+ put_online_cpus_atomic();
return 0;
}
@@ -5549,6 +5555,7 @@ static void rebalance_domains(int cpu, enum cpu_idle_type idle)
update_blocked_averages(cpu);
+ get_online_cpus_atomic();
rcu_read_lock();
for_each_domain(cpu, sd) {
if (!(sd->flags & SD_LOAD_BALANCE))
@@ -5597,6 +5604,7 @@ out:
break;
}
rcu_read_unlock();
+ put_online_cpus_atomic();
/*
* next_balance will be updated only when there is a need.
@@ -5728,6 +5736,7 @@ static void run_rebalance_domains(struct softirq_action *h)
enum cpu_idle_type idle = this_rq->idle_balance ?
CPU_IDLE : CPU_NOT_IDLE;
+ get_online_cpus_atomic();
rebalance_domains(this_cpu, idle);
/*
@@ -5736,6 +5745,7 @@ static void run_rebalance_domains(struct softirq_action *h)
* stopped.
*/
nohz_idle_balance(this_cpu, idle);
+ put_online_cpus_atomic();
}
static inline int on_null_domain(int cpu)
@@ -5753,8 +5763,10 @@ void trigger_load_balance(struct rq *rq, int cpu)
likely(!on_null_domain(cpu)))
raise_softirq(SCHED_SOFTIRQ);
#ifdef CONFIG_NO_HZ_COMMON
+ get_online_cpus_atomic();
if (nohz_kick_needed(rq, cpu) && likely(!on_null_domain(cpu)))
nohz_balancer_kick(cpu);
+ put_online_cpus_atomic();
#endif
}
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox