LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v2] powerpc/pci: Improve device hotplug initialization
From: Guenter Roeck @ 2013-06-21 16:54 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Yuanquan Chen, linuxppc-dev, linux-kernel, Hiroo Matsumoto
In-Reply-To: <1370884688-7753-1-git-send-email-linux@roeck-us.net>

On Mon, Jun 10, 2013 at 10:18:08AM -0700, Guenter Roeck wrote:
> Commit 37f02195b (powerpc/pci: fix PCI-e devices rescan issue on powerpc
> platform) fixes a problem with interrupt and DMA initialization on hot
> plugged devices. With this commit, interrupt and DMA initialization for
> hot plugged devices is handled in the pci device enable function.
> 
> This approach has a couple of drawbacks. First, it creates two code paths
> for device initialization, one for hot plugged devices and another for devices
> known during the initial PCI scan. Second, the initialization code for hot
> plugged devices is only called when the device is enabled, ie typically
> in the probe function. Also, the platform specific setup code is called each
> time pci_enable_device() is called, not only once during device discovery,
> meaning it is actually called multiple times, once for devices discovered
> during the initial scan and again each time a driver is re-loaded.
> 
> The visible result is that interrupt pins are only assigned to hot plugged
> devices when the device driver is loaded. Effectively this changes the PCI
> probe API, since pci_dev->irq and the device's dma configuration will now
> only be valid after pci_enable() was called at least once. A more subtle
> change is that platform specific PCI device setup is moved from device
> discovery into the driver's probe function, more specifically into the
> pci_enable_device() call.
> 
> To fix the inconsistencies, add new function pcibios_add_device.
> Call pcibios_setup_device from pcibios_setup_bus_devices if device setup
> is not complete, and from pcibios_add_device if bus setup is complete.
> 
> With this change, device setup code is moved back into device initialization,
> and called exactly once for both static and hot plugged devices.
> 
> Cc: Yuanquan Chen <Yuanquan.Chen@freescale.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Hiroo Matsumoto <matsumoto.hiroo@jp.fujitsu.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> v2: Ensure that PCI bus fixup code has been executed before calling
>     device setup code.
> 
Hi Ben,

any comments/feedback on this approach ?

It is much less invasive than before and should address your concerns.

Thanks,
Guenter

>  arch/powerpc/kernel/pci-common.c |   17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> index 7f2273c..6909b13 100644
> --- a/arch/powerpc/kernel/pci-common.c
> +++ b/arch/powerpc/kernel/pci-common.c
> @@ -992,7 +992,7 @@ void pcibios_setup_bus_self(struct pci_bus *bus)
>  		ppc_md.pci_dma_bus_setup(bus);
>  }
>  
> -void pcibios_setup_device(struct pci_dev *dev)
> +static void pcibios_setup_device(struct pci_dev *dev)
>  {
>  	/* Fixup NUMA node as it may not be setup yet by the generic
>  	 * code and is needed by the DMA init
> @@ -1013,6 +1013,17 @@ void pcibios_setup_device(struct pci_dev *dev)
>  		ppc_md.pci_irq_fixup(dev);
>  }
>  
> +int pcibios_add_device(struct pci_dev *dev)
> +{
> +	/*
> +	 * We can only call pcibios_setup_device() after bus setup is complete,
> +	 * since some of the platform specific DMA setup code depends on it.
> +	 */
> +	if (dev->bus->is_added)
> +		pcibios_setup_device(dev);
> +	return 0;
> +}
> +
>  void pcibios_setup_bus_devices(struct pci_bus *bus)
>  {
>  	struct pci_dev *dev;
> @@ -1467,10 +1478,6 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
>  		if (ppc_md.pcibios_enable_device_hook(dev))
>  			return -EINVAL;
>  
> -	/* avoid pcie irq fix up impact on cardbus */
> -	if (dev->hdr_type != PCI_HEADER_TYPE_CARDBUS)
> -		pcibios_setup_device(dev);
> -
>  	return pci_enable_resources(dev, mask);
>  }
>  
> -- 
> 1.7.9.7
> 
> 

^ permalink raw reply

* Inbound PCI and Memory Corruption
From: Peter LaDow @ 2013-06-21 16:56 UTC (permalink / raw)
  To: linuxppc-dev

I'm posting this to the ppc-dev since I think the problem may be
specific to the PPC kernel.

We are running into a case where we get memory corruption when an
external PCI master writes to the processor.  We are using an MPC8349
with an external Intel 82540EP (an E1000) part.  I've spent several
weeks on the e1000 list trying to track down this problem, and after
some digging I'm thinking the problem is somewhere other than the
e1000 driver or the e1000 part.

We are running 3.0.57-rt82, though I can reliably re-create this
problem with 3.0.80 (vanilla, no preempt).  Basically it involves
bringing down the e1000 interface while it is on an extremely busy
network (i.e. a ton of traffic incoming traffic into the e1000).  From
the kernel panics I've seen, it appears that the incoming traffic is
present in the corrupted memory regions.

I'm suspecting some issue related to the DMA'ing of packet data from
the e1000 into main memory.  I've traced the PCI traffic with a bus
analyzer, and the part stops DMA'ing when disabled, and the driver
disables the part before unmapping the ring buffers (i.e. calls to
dma_unmap_*).

Prior to using the e1000 part, we were using an e100 part (an 82551).
We never experienced this problem, but then again, the driver is
different.  Some initial digging shows that the DMA is setup
different.  The e100 driver uses pci_map_single, but the e1000 driver
uses dma_alloc_coherent.  I don't know much about the guts of the
kernel related to PCI and DMA, but based upon the PCI traces and other
debugging, it seems to point there.

Below is the kernel panic output when the failure occurs.

Does anyone have an idea of where I can look to try and debug this?

Unable to handle kernel paging request for data at address 0x20454a46
 Faulting instruction address: 0xc0069924
 Oops: Kernel access of bad area, sig: 11 [#1]
 PREEMPT PPC Platform
 Modules linked in:
 NIP: c0069924 LR: c021cce0 CTR: c000cecc
 REGS: ed4f1c60 TRAP: 0300   Not tainted  (3.0.80-rt108)
 MSR: 00009032 <EE,ME,IR,DR>  CR: 24008248  XER: 00000000
 DAR: 20454a46, DSISR: 20000000
 TASK = eda46780[3106] 'ifconfig' THREAD: ed4f0000
 GPR00: 00000000 ed4f1d10 eda46780 20454a46 2d6fcc2a 000005f2 00000002 00000000
 GPR08: eda46780 ed6fd228 ed4f1cd0 000090b1 00000000 10084718 bfcceaec 10062044
 GPR16: 10062120 bfcceadc 00000000 bfcceac4 00000228 00000000 00008914 c01ac398
 GPR24: c01ac8c8 ed066520 00000061 ed0663a0 ef0448f0 00000000 00000001 ed575580
 NIP [c0069924] put_page+0x0/0x34
 LR [c021cce0] skb_release_data+0x78/0xc8
 Call Trace:
 [ed4f1d20] [c021c914] __kfree_skb+0x18/0xbc
 [ed4f1d30] [c01a7620] e1000_clean_rx_ring+0x10c/0x1a4
 [ed4f1d60] [c01a76e0] e1000_clean_all_rx_rings+0x28/0x54
 [ed4f1d70] [c01aac50] e1000_close+0x30/0xb4
 [ed4f1d90] [c0226e2c] __dev_close_many+0xa0/0xe0
 [ed4f1da0] [c0228c64] __dev_close+0x2c/0x4c
 [ed4f1dc0] [c0225224] __dev_change_flags+0xb8/0x140
 [ed4f1de0] [c0226d48] dev_change_flags+0x1c/0x60
 [ed4f1e00] [c027e7f8] devinet_ioctl+0x2a4/0x700
 [ed4f1e60] [c027f450] inet_ioctl+0xc8/0xfc
 [ed4f1e70] [c02147b0] sock_ioctl+0x260/0x2a0
 [ed4f1e90] [c009b468] vfs_ioctl+0x2c/0x58
 [ed4f1ea0] [c009bc44] do_vfs_ioctl+0x64c/0x6d4
 [ed4f1f10] [c009bd24] sys_ioctl+0x58/0x88
 [ed4f1f40] [c000e954] ret_from_syscall+0x0/0x38
 --- Exception: c01 at 0xff35a3c
     LR = 0xff359a0
 Instruction dump:
 7c0802a6 3c80c007 3884a500 90010024 38a10008 38000000 90010008 4bffff0d
 80010024 38210020 7c0803a6 4e800020
 <80030000> 7c691b78 700bc000 41a20008
 Kernel panic - not syncing: Fatal exception
 Call Trace:
 [ed4f1b90] [c0007ccc] show_stack+0x58/0x154 (unreliable)
 [ed4f1bd0] [c001d744] panic+0xb0/0x1d8
 [ed4f1c20] [c000b4b8] die+0x1ac/0x1d0
 [ed4f1c40] [c0011e38] bad_page_fault+0xe8/0xfc
 [ed4f1c50] [c000edf4] handle_page_fault+0x7c/0x80
 --- Exception: 300 at put_page+0x0/0x34
     LR = skb_release_data+0x78/0xc8
 [ed4f1d10] [00000000]   (null) (unreliable)
 [ed4f1d20] [c021c914] __kfree_skb+0x18/0xbc
 [ed4f1d30] [c01a7620] e1000_clean_rx_ring+0x10c/0x1a4
 [ed4f1d60] [c01a76e0] e1000_clean_all_rx_rings+0x28/0x54
 [ed4f1d70] [c01aac50] e1000_close+0x30/0xb4
 [ed4f1d90] [c0226e2c] __dev_close_many+0xa0/0xe0
 [ed4f1da0] [c0228c64] __dev_close+0x2c/0x4c
 [ed4f1dc0] [c0225224] __dev_change_flags+0xb8/0x140
 [ed4f1de0] [c0226d48] dev_change_flags+0x1c/0x60
 [ed4f1e00] [c027e7f8] devinet_ioctl+0x2a4/0x700
 [ed4f1e60] [c027f450] inet_ioctl+0xc8/0xfc
 [ed4f1e70] [c02147b0] sock_ioctl+0x260/0x2a0
 [ed4f1e90] [c009b468] vfs_ioctl+0x2c/0x58
 [ed4f1ea0] [c009bc44] do_vfs_ioctl+0x64c/0x6d4
 [ed4f1f10] [c009bd24] sys_ioctl+0x58/0x88
 [ed4f1f40] [c000e954] ret_from_syscall+0x0/0x38
 --- Exception: c01 at 0xff35a3c
     LR = 0xff359a0

When turning on SLAB checks, I see:

Slab corruption: size-16384 start=ed4ec000, len=16384
 690: 6b 6b ff ff ff ff ff ff b8 ac 6f 99 bf 8b 08 00
 6a0: 45 00 00 24 3f 34 00 00 80 11 ca cf 0a ca 0d 33
 6b0: 0a ca 0d ff 06 cc 06 cf 00 10 bc 1d c5 0b 40 01
 6c0: 00 10 00 33 00 00 00 00 00 00 00 00 00 00 3f dd
 6d0: ed f8 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
 ea0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b ff ff ff ff ff ff
 Slab corruption: size-2048 start=ed4e6570, len=2048
 Redzone: 0x9f911029d74e35b/0x9f911029d74e35b.
 Last user: [<  (null)>](0x0)
 0c0: 6b 6b ff ff ff ff ff ff 5c 26 0a 41 81 27 08 00
 0d0: 45 00 00 4e 7d 44 00 00 80 11 8c 79 0a ca 0d 4f
 0e0: 0a ca 0d ff 00 89 00 89 00 3a b5 a7 be 71 01 10
 0f0: 00 01 00 00 00 00 00 00 20 45 4c 45 43 45 50 46
 100: 49 43 41 43 41 43 41 43 41 43 41 43 41 43 41 43
 110: 41 43 41 43 41 43 41 41 41 00 00 20 00 01 02 5a
 Next obj: start=ed4e6d88, len=2048
 Redzone: 0xd84156c5635688c0/0xd84156c5635688c0.
 Last user: [<c021e294>](__netdev_alloc_skb+0x28/0x60)
 000: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a
 010: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a
 Slab corruption: size-2048 start=ed54eb48, len=2048
 Redzone: 0x9f911029d74e35b/0x9f911029d74e35b.
 Last user: [<c021cd1c>](skb_release_data+0xb4/0xc8)
 020: 6b 6b ff ff ff ff ff ff 18 03 73 e4 64 18 08 00
 030: 45 00 00 44 61 b8 00 00 80 11 a7 c9 0a ca 0d 95
 040: 0a ca 0d ff f1 ee 07 9b 00 30 53 78 30 53 73 66
 050: 54 77 78 32 41 41 42 4b 52 55 5a 47 54 55 4e 51
 060: 51 7a 49 41 52 47 39 73 62 33 4a 54 61 58 52 42
 070: 62 57 55 41 bc fd 94 9f 6b 6b 6b 6b 6b 6b 6b 6b
 Prev obj: start=ed54e330, len=2048
 Redzone: 0xd84156c5635688c0/0xd84156c5635688c0.
 Last user: [<c021e294>](__netdev_alloc_skb+0x28/0x60)
 000: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a
 010: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a
 Next obj: start=ed54f360, len=2048
 Redzone: 0xd84156c5635688c0/0xd84156c5635688c0.
 Last user: [<c021e294>](__netdev_alloc_skb+0x28/0x60)
 000: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a
 010: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a
 Slab corruption: size-2048 start=ed4ae6f0, len=2048
 Redzone: 0x9f911029d74e35b/0x9f911029d74e35b.
 Last user: [<c021cd1c>](skb_release_data+0xb4/0xc8)
 020: 6b 6b ff ff ff ff ff ff 00 1a e2 bd 06 44 81 00
 030: 00 f2 08 06 00 01 08 00 06 04 00 01 00 1a e2 bd
 040: 06 44 0a f1 0a 4b 00 00 00 00 00 00 0a f1 0a 4b
 050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 060: 00 00 37 ea e4 d5 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
 Next obj: start=ed4aef08, len=2048
 Redzone: 0xd84156c5635688c0/0xd84156c5635688c0.
 Last user: [<c01f2898>](rx_submit+0xa0/0x174)
 000: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a
 010: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a
 Slab corruption: size-2048 start=ed792928, len=2048
 Redzone: 0x9f911029d74e35b/0x9f911029d74e35b.
 Last user: [<c021cd1c>](skb_release_data+0xb4/0xc8)
 020: 6b 6b ff ff ff ff ff ff d4 be d9 a0 1d 8a 08 00
 030: 45 00 00 44 1c a9 00 00 80 11 ed 4b 0a ca 0d 22
 040: 0a ca 0d ff e6 ed 07 9b 00 30 63 13 48 74 77 37
 050: 55 46 51 4c 41 41 42 51 53 45 6c 4d 51 55 46 54
 060: 52 56 42 44 4d 67 42 73 62 33 4a 54 61 58 52 42
 070: 62 57 55 41 d0 91 83 aa 6b 6b 6b 6b 6b 6b 6b 6b
 Prev obj: start=ed792110, len=2048
 Redzone: 0xd84156c5635688c0/0xd84156c5635688c0.
 Last user: [<c021e294>](__netdev_alloc_skb+0x28/0x60)
 000: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a
 010: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a
 Next obj: start=ed793140, len=2048
 Redzone: 0xd84156c5635688c0/0xd84156c5635688c0.
 Last user: [<c021e294>](__netdev_alloc_skb+0x28/0x60)
 000: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a
 010: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a

After some digging through the list archives and online searchs, I
 thougth perhaps there were DMA issues (such as the controller DMA'ing
 into memory after the rings are disabled).  I turned on the DMA-API
 checks and see:

e1000 0000:00:13.0: DMA-API: device driver tries to free DMA memory it
 has not allocated [device address=0x000000004eae0000] [size=65535
 bytes]
 ------------[ cut here ]------------
 WARNING: at lib/dma-debug.c:811
 Modules linked in:
 NIP: c01450d4 LR: c01450d4 CTR: c0169388
 REGS: ed8fb7e0 TRAP: 0700   Not tainted  (3.0.57-rt82)
 MSR: 00029032 <EE,ME,CE,IR,DR>  CR: 24020482  XER: 20000000
 TASK = ed8f6b60[3394] 'ip' THREAD: ed8fa000
 GPR00: c01450d4 ed8fb890 ed8f6b60 00000093 00004718 ffffffff c01668a8 00000000
 GPR08: ed8f6b60 c03ca4c0 00004718 ed8f6c00 24020482 100456d0 c0302a48 c03029d4
 GPR16: c03ee31c ed8fbac8 ed732f10 ed8fbab0 ed8fbad8 00000000 c02fda70 00000000
 GPR24: ed732f00 ed8fb8d8 00000001 0000ffff 4eae0000 ed8fb8d8 c050f6b8 00000000
 NIP [c01450d4] check_unmap+0x1e0/0x7b0
 LR [c01450d4] check_unmap+0x1e0/0x7b0
 Call Trace:
 [ed8fb890] [c01450d4] check_unmap+0x1e0/0x7b0 (unreliable)
 [ed8fb8d0] [c01457a4] debug_dma_unmap_page+0x7c/0x90
 [ed8fb940] [c01ad344] e1000_unmap_and_free_tx_resource+0xf4/0x130
 [ed8fb960] [c01ad3a8] e1000_clean_tx_ring+0x28/0xac
 [ed8fb980] [c01aecb8] e1000_down+0x1e4/0x210
 [ed8fb9b0] [c01af488] e1000_close+0x30/0xb4
 [ed8fb9d0] [c022c2f0] __dev_close_many+0xa0/0xe0
 [ed8fb9e0] [c022e128] __dev_close+0x2c/0x4c
 [ed8fba00] [c022a748] __dev_change_flags+0xb8/0x140
 [ed8fba20] [c022c20c] dev_change_flags+0x1c/0x60
 [ed8fba40] [c023bc40] do_setlink+0x278/0x748
 [ed8fbaa0] [c023d044] rtnl_newlink+0x298/0x4b0
 [ed8fbbd0] [c023c5f4] rtnetlink_rcv_msg+0x210/0x23c
 [ed8fbbf0] [c02454bc] netlink_rcv_skb+0x5c/0xd4
 [ed8fbc10] [c023c3d0] rtnetlink_rcv+0x28/0x3c
 [ed8fbc30] [c0245200] netlink_unicast+0x244/0x2e4
 [ed8fbc70] [c0246020] netlink_sendmsg+0x260/0x2dc
 [ed8fbcc0] [c021a29c] sock_sendmsg+0xa0/0xc4
 [ed8fbda0] [c021b018] __sys_sendmsg+0x1d8/0x29c
 [ed8fbeb0] [c021b220] sys_sendmsg+0x40/0x70
 [ed8fbf00] [c021bfcc] sys_socketcall+0x1b0/0x230
 [ed8fbf40] [c000e9b4] ret_from_syscall+0x0/0x38
 --- Exception: c01 at 0xff0a9d4
     LR = 0x10020900
 Instruction dump:
 48000014 80a9002c 2f850000 40be0008 80a90008 813d0020 3c60c035 815d0024
 3863f06c 80fd0018 811d001c 4bed9c01 <0fe00000> 3d20c040 8009c360 2f800000
 ---[ end trace 0000000000000002 ]---

^ permalink raw reply

* Re: [PATCH] of: Specify initrd location using 64-bit
From: Santosh Shilimkar @ 2013-06-21 17:12 UTC (permalink / raw)
  To: James Hogan, Vineet Gupta
  Cc: Nicolas Pitre, linux-mips, Aurelien Jacquiot, Catalin Marinas,
	bigeasy, Will Deacon, Max Filippov, Paul Mackerras, Jonas Bonn,
	Russell King, linux-c6x-dev, x86, arm, robherring2, Grant Likely,
	linux-xtensa, devicetree-discuss, Mark Salter, Rob Herring,
	linux-arm-kernel, Chris Zankel, Michal Simek, linux-kernel,
	Ralf Baechle, linuxppc-dev
In-Reply-To: <51C40D78.4040603@imgtec.com>

Vineet, James,

On Friday 21 June 2013 04:23 AM, James Hogan wrote:
> On 21/06/13 05:39, Vineet Gupta wrote:
>> Hi Santosh,
>>
>> On 06/21/2013 06:22 AM, Santosh Shilimkar wrote:
>>> Cc: Vineet Gupta <vgupta@synopsys.com>
>>> Cc: Russell King <linux@arm.linux.org.uk>
>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>> Cc: Will Deacon <will.deacon@arm.com>
>>> Cc: Mark Salter <msalter@redhat.com>
>>> Cc: Aurelien Jacquiot <a-jacquiot@ti.com>
>>> Cc: James Hogan <james.hogan@imgtec.com>
>>> Cc: Michal Simek <monstr@monstr.eu>
>>> Cc: Ralf Baechle <ralf@linux-mips.org>
>>> Cc: Jonas Bonn <jonas@southpole.se>
>>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>>> Cc: Paul Mackerras <paulus@samba.org>
>>> Cc: x86@kernel.org
>>> Cc: arm@kernel.org
>>> Cc: Chris Zankel <chris@zankel.net>
>>> Cc: Max Filippov <jcmvbkbc@gmail.com>
>>> Cc: Grant Likely <grant.likely@linaro.org>
>>> Cc: Rob Herring <rob.herring@calxeda.com>
>>> Cc: bigeasy@linutronix.de
>>> Cc: robherring2@gmail.com
>>> Cc: Nicolas Pitre <nicolas.pitre@linaro.org>
>>>
>>> Cc: linux-arm-kernel@lists.infradead.org
>>> Cc: linux-c6x-dev@linux-c6x.org
>>> Cc: linux-mips@linux-mips.org
>>> Cc: linuxppc-dev@lists.ozlabs.org
>>> Cc: linux-xtensa@linux-xtensa.org
>>> Cc: devicetree-discuss@lists.ozlabs.org
>>>
>>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
>>> ---
>>>  arch/arc/mm/init.c            |    3 +--
>>>  arch/arm/mm/init.c            |    2 +-
>>>  arch/arm64/mm/init.c          |    3 +--
>>>  arch/c6x/kernel/devicetree.c  |    3 +--
>>>  arch/metag/mm/init.c          |    3 +--
>>>  arch/microblaze/kernel/prom.c |    3 +--
>>>  arch/mips/kernel/prom.c       |    3 +--
>>>  arch/openrisc/kernel/prom.c   |    3 +--
>>>  arch/powerpc/kernel/prom.c    |    3 +--
>>>  arch/x86/kernel/devicetree.c  |    3 +--
>>>  arch/xtensa/kernel/setup.c    |    3 +--
>>>  drivers/of/fdt.c              |   10 ++++++----
>>>  include/linux/of_fdt.h        |    3 +--
>>>  13 files changed, 18 insertions(+), 27 deletions(-)
>>>
>>> diff --git a/arch/arc/mm/init.c b/arch/arc/mm/init.c
>>> index 4a17736..3640c74 100644
>>> --- a/arch/arc/mm/init.c
>>> +++ b/arch/arc/mm/init.c
>>> @@ -157,8 +157,7 @@ void __init free_initrd_mem(unsigned long start, unsigned long end)
>>>  #endif
>>>  
>>>  #ifdef CONFIG_OF_FLATTREE
>>> -void __init early_init_dt_setup_initrd_arch(unsigned long start,
>>> -					    unsigned long end)
>>> +void __init early_init_dt_setup_initrd_arch(u64 start, u64 end)
>>>  {
>>>  	pr_err("%s(%lx, %lx)\n", __func__, start, end);
>>>  }
>>
>> To avoid gcc warnings, you need to fix the print format specifiers too.
> 
> The same thing goes for arch/metag too.
> 
Will update that in next version based on what we will end
after the dicussion. i.e u64 or phys_addr_t

Thanks for comments.

Regards,
Santosh

^ permalink raw reply

* Re: Inbound PCI and Memory Corruption
From: Peter LaDow @ 2013-06-21 17:14 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <CAN8Q1EdnN9+kKGy+voF3=EyPfbHv04Emfz=uA6XKMJEAkvLpAA@mail.gmail.com>

On Fri, Jun 21, 2013 at 9:56 AM, Peter LaDow <petela@gocougs.wsu.edu> wrote:
> We are running into a case where we get memory corruption when an
> external PCI master writes to the processor.  We are using an MPC8349
> with an external Intel 82540EP (an E1000) part.  I've spent several
> weeks on the e1000 list trying to track down this problem, and after
> some digging I'm thinking the problem is somewhere other than the
> e1000 driver or the e1000 part.

After a (finally!) successful search of the list archive, I did find this:

http://web.archiveorange.com/archive/v/9IQA26gPvdf4foaTcmCV

Which seems very related to my problem.  However, the patch that is
ultimately referenced is in place in 3.0.80 (see
https://lists.ozlabs.org/pipermail/linuxppc-dev/2006-February/021267.html).

Hmmm...  perhaps our FDT is bad?

Thanks,
Pete

^ permalink raw reply

* Re: [PATCH] of: Specify initrd location using 64-bit
From: Santosh Shilimkar @ 2013-06-21 17:20 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior, robherring2
  Cc: Nicolas Pitre, linux-mips, Catalin Marinas, Aurelien Jacquiot,
	Will Deacon, Max Filippov, Paul Mackerras, Jonas Bonn,
	Russell King, linux-c6x-dev, x86, arm, Mark Salter, Grant Likely,
	linux-xtensa, James Hogan, devicetree-discuss, Rob Herring,
	linux-arm-kernel, Chris Zankel, Michal Simek, Vineet Gupta,
	linux-kernel, Ralf Baechle, linuxppc-dev
In-Reply-To: <51C4171C.9050908@linutronix.de>

On Friday 21 June 2013 05:04 AM, Sebastian Andrzej Siewior wrote:
> On 06/21/2013 02:52 AM, Santosh Shilimkar wrote:
>> diff --git a/arch/microblaze/kernel/prom.c b/arch/microblaze/kernel/prom.c
>> index 0a2c68f..62e2e8f 100644
>> --- a/arch/microblaze/kernel/prom.c
>> +++ b/arch/microblaze/kernel/prom.c
>> @@ -136,8 +136,7 @@ void __init early_init_devtree(void *params)
>>  }
>>  
>>  #ifdef CONFIG_BLK_DEV_INITRD
>> -void __init early_init_dt_setup_initrd_arch(unsigned long start,
>> -		unsigned long end)
>> +void __init early_init_dt_setup_initrd_arch(u64 start, u64 end)
>>  {
>>  	initrd_start = (unsigned long)__va(start);
>>  	initrd_end = (unsigned long)__va(end);
> 
> I think it would better to go here for phys_addr_t instead of u64. This
> would force you in of_flat_dt_match() to check if the value passed from
> DT specifies a memory address outside of 32bit address space and the
> kernel can't deal with this because its phys_addr_t is 32bit only due
> to a Kconfig switch.
> 
> For x86, the initrd has to remain in the 32bit address space so passing
> the initrd in the upper range would violate the ABI. Not sure if this
> is true for other archs as well (ARM obviously not).
> 
That pretty much means phys_addr_t. It will work for me as well but
in last thread from consistency with memory and reserved node, Rob
insisted to keep it as u64. So before I re-spin another version,
would like to here what Rob has to say considering the x86 requirement.

Rob,
Are you ok with phys_addr_t since your concern was about rest
of the memory specific bits of the device-tree code use u64 ?

Regards,
Santosh

^ permalink raw reply

* Re: [PATCH v2] PowerPC: kernel: need return the related error code when failure occurs
From: Chen Gang @ 2013-06-22  4:08 UTC (permalink / raw)
  To: paulus@samba.org, Michael Ellerman, Benjamin Herrenschmidt
  Cc: Linux-Arch, Arnd Bergmann, Jiri Kosina,
	linux-kernel@vger.kernel.org, zhangyanfei,
	linuxppc-dev@lists.ozlabs.org


Hello Maintainers:

Please help check this patch whether is OK, when you have time.

Thanks.

On 05/21/2013 05:20 PM, Chen Gang wrote:
>
> When error occurs, need return the related error code to let upper
> caller know about it.
>
> ppc_md.nvram_size() can return the error code (e.g. core99_nvram_size()
> in 'arch/powerpc/platforms/powermac/nvram.c').
>
> Also set ret value when only need it, so can save structions for normal
> cases.
>
> The original related patch: "f9ce299 [PATCH] powerpc: fix large nvram
> access".
>
>
> Signed-off-by: Chen Gang <gang.chen@asianux.com>
> ---
>  arch/powerpc/kernel/nvram_64.c |   20 ++++++++++++++------
>  1 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
> index 48fbc2b..8213ee1 100644
> --- a/arch/powerpc/kernel/nvram_64.c
> +++ b/arch/powerpc/kernel/nvram_64.c
> @@ -84,22 +84,30 @@ static ssize_t dev_nvram_read(struct file *file, char __user *buf,
>  	char *tmp = NULL;
>  	ssize_t size;
>  
> -	ret = -ENODEV;
> -	if (!ppc_md.nvram_size)
> +	if (!ppc_md.nvram_size) {
> +		ret = -ENODEV;
>  		goto out;
> +	}
>  
> -	ret = 0;
>  	size = ppc_md.nvram_size();
> -	if (*ppos >= size || size < 0)
> +	if (size < 0) {
> +		ret = size;
> +		goto out;
> +	}
> +
> +	if (*ppos >= size) {
> +		ret = 0;
>  		goto out;
> +	}
>  
>  	count = min_t(size_t, count, size - *ppos);
>  	count = min(count, PAGE_SIZE);
>  
> -	ret = -ENOMEM;
>  	tmp = kmalloc(count, GFP_KERNEL);
> -	if (!tmp)
> +	if (!tmp) {
> +		ret = -ENOMEM;
>  		goto out;
> +	}
>  
>  	ret = ppc_md.nvram_read(tmp, count, ppos);
>  	if (ret <= 0)
>


Thanks
-- 
Chen Gang

Asianux Corporation

^ permalink raw reply

* Re: [PATCH 3/4] KVM: PPC: Add support for IOMMU in-kernel handling
From: Alexey Kardashevskiy @ 2013-06-22  8:25 UTC (permalink / raw)
  To: Alex Williamson
  Cc: kvm@vger.kernel.org mailing list, Joerg Roedel, Rusty Russell,
	Alexander Graf, kvm-ppc, open list, Paul Mackerras, linuxppc-dev,
	David Gibson
In-Reply-To: <1371740113.32709.22.camel@ul30vt.home>

On 06/21/2013 12:55 AM, 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.
> 
>>  and this is exactly what vfio_group_add_external_user() and
>> vfio_group_del_external_user() do. The difference is only in absolute value
>> - 2 vs. 3.
>>
>> No change in behaviour whether I use new vfio API or simply hold file* till
>> KVM closes fd created when IOMMU was connected to LIOBN.
> 
> By that notion you could open(/dev/vfio/$GROUP) and you're safe, right?
> But what about SET_CONTAINER & SET_IOMMU?  All that you guarantee
> holding the file pointer is that the vfio_group exists.
> 
>> And while this counter is not zero, QEMU cannot take ownership over the group.
>>
>> I am definitely still missing the bigger picture...
> 
> The bigger picture is that the group needs to exist AND it needs to be
> setup and maintained to have IOMMU protection.  Actually, my first stab
> at add_external_user doesn't look sufficient, it needs to look more like
> vfio_group_get_device_fd, checking group->container->iommu and
> group_viable().


This makes sense. If you did this, that would be great. Without it, I
really cannot see how the proposed inc/dec of container_users is better
than simple holding file*. Thanks.


> As written it would allow an external user after
> SET_CONTAINER without SET_IOMMU.  It should also be part of the API that
> the external user must hold the file reference between add_external_use
> and del_external_user and do cleanup on any exit paths.  Thanks,


-- 
Alexey

^ permalink raw reply

* [PATCH] ADB: Replace __WAITQUEUE_INITIALIZER with more standard DECLARE_WAITQUEUE.
From: Robert P. J. Day @ 2013-06-22  8:29 UTC (permalink / raw)
  To: linuxppc-dev


Signed-off-by: Robert P. J. Day <rpjday@crashcourse.ca>

---

  just FYI, that invocation of __WAITQUEUE_INITIALIER() is the only
one in the entire current source tree so it seems a bit out of place.
not tested, but there *should* be no functional change.

diff --git a/drivers/macintosh/adb.c b/drivers/macintosh/adb.c
index b026896..04a5049 100644
--- a/drivers/macintosh/adb.c
+++ b/drivers/macintosh/adb.c
@@ -697,7 +697,7 @@ static ssize_t adb_read(struct file *file, char __user *buf,
 	int ret = 0;
 	struct adbdev_state *state = file->private_data;
 	struct adb_request *req;
-	wait_queue_t wait = __WAITQUEUE_INITIALIZER(wait,current);
+	DECLARE_WAITQUEUE(wait,current);
 	unsigned long flags;

 	if (count < 2)

rday

-- 

========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                        http://crashcourse.ca

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================

^ permalink raw reply related

* Re: [PATCH 3/4] KVM: PPC: Add support for IOMMU in-kernel handling
From: David Gibson @ 2013-06-22 12:03 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: <1371740113.32709.22.camel@ul30vt.home>

[-- Attachment #1: Type: text/plain, Size: 1950 bytes --]

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.

-- 
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 3/4] KVM: PPC: Add support for IOMMU in-kernel handling
From: Alex Williamson @ 2013-06-22 14:28 UTC (permalink / raw)
  To: David Gibson
  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: <20130622120304.GB25265@voom.fritz.box>

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,

Alex

^ permalink raw reply

* Re: [PATCH 3/4] KVM: PPC: Add support for IOMMU in-kernel handling
From: Benjamin Herrenschmidt @ 2013-06-22 23:28 UTC (permalink / raw)
  To: David Gibson
  Cc: kvm@vger.kernel.org mailing list, Alexey Kardashevskiy,
	Joerg Roedel, Rusty Russell, Alexander Graf, kvm-ppc, open list,
	Alex Williamson, Paul Mackerras, linuxppc-dev
In-Reply-To: <20130622120304.GB25265@voom.fritz.box>

On Sat, 2013-06-22 at 22:03 +1000, David Gibson wrote:
> 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.

Interestingly, how are we going to extend that when/if we implement
DDW ?

DDW means an API by which the guest can request the creation of
additional iommus for a given device (typically, in addition to the
default smallish 32-bit one using 4k pages, the guest can request
a larger window in 64-bit space using a larger page size).

Cheers,
Ben.

^ permalink raw reply

* Re: Inbound PCI and Memory Corruption
From: Benjamin Herrenschmidt @ 2013-06-23  0:00 UTC (permalink / raw)
  To: Peter LaDow; +Cc: linuxppc-dev
In-Reply-To: <CAN8Q1Ed9Lpg1Sog5tQODe+jqCRXBBH3d_UeR5usdc23CD7vwjg@mail.gmail.com>

On Fri, 2013-06-21 at 10:14 -0700, Peter LaDow wrote:
> After a (finally!) successful search of the list archive, I did find
> this:
> 
> http://web.archiveorange.com/archive/v/9IQA26gPvdf4foaTcmCV
> 
> Which seems very related to my problem.  However, the patch that is
> ultimately referenced is in place in 3.0.80 (see
> https://lists.ozlabs.org/pipermail/linuxppc-dev/2006-February/021267.html).
> 
> Hmmm...  perhaps our FDT is bad?

Afaik e300 is slightly out of order, maybe it's missing a memory barrier
somewhere.... One thing to try is to add some to the dma_map/unmap ops.

Also audit the driver to ensure that it properly uses barriers when
populating descriptors (and maybe compare to a more recent version of
the driver upstream).

Ben.

^ permalink raw reply

* [PATCH 00/45] CPU hotplug: stop_machine()-free CPU hotplug, part 1
From: Srivatsa S. Bhat @ 2013-06-23 13:37 UTC (permalink / raw)
  To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
	walken, vincent.guittot, laijs
  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-v1


Thank you very much!


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: https://lkml.org/lkml/2012/12/7/287
   v2: https://lkml.org/lkml/2012/12/5/322
   v1: https://lkml.org/lkml/2012/12/4/88

--
 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: 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 get/put_online_cpus_atomic() to prevent CPU offline
      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 get/put_online_cpus_atomic() to prevent CPU offline
      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
      kvm/vmx: 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/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/kvm/vmx.c                            |   13 +--
 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                           |   27 +++++
 include/linux/cpumask.h                       |   59 +++++++++++-
 kernel/cpu.c                                  |  124 +++++++++++++++++++++++++
 kernel/irq/manage.c                           |    7 +
 kernel/irq/proc.c                             |    3 +
 kernel/rcutree.c                              |    4 +
 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                             |    9 ++
 lib/cpumask.c                                 |    8 ++
 lib/percpu_counter.c                          |    2 
 net/core/dev.c                                |    9 +-
 virt/kvm/kvm_main.c                           |    8 +-
 58 files changed, 591 insertions(+), 129 deletions(-)


Regards,
Srivatsa S. Bhat
IBM Linux Technology Center

^ permalink raw reply

* [PATCH 01/45] CPU hotplug: Provide APIs to prevent CPU offline from atomic context
From: Srivatsa S. Bhat @ 2013-06-23 13:38 UTC (permalink / raw)
  To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
	walken, vincent.guittot, laijs
  Cc: linux-arch, nikunj, zhong, linux-pm, fweisbec, linux-kernel,
	rostedt, xiaoguangrong, Rafael J. Wysocki, sbw, Yasuaki Ishimatsu,
	wangyun, srivatsa.bhat, netdev, Tejun Heo, Thomas Gleixner,
	linuxppc-dev, Andrew Morton
In-Reply-To: <20130623133642.19094.16038.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.

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 |   18 ++++++++++++++++++
 kernel/cpu.c        |   38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+)

diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index 9f3c7e8..e06c3ad 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -175,6 +175,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 +204,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 02/45] CPU hotplug: Clarify the usage of different synchronization APIs
From: Srivatsa S. Bhat @ 2013-06-23 13:38 UTC (permalink / raw)
  To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
	walken, vincent.guittot, laijs
  Cc: linux-arch, nikunj, zhong, linux-pm, fweisbec, Rafael J. Wysocki,
	linux-kernel, rostedt, xiaoguangrong, sbw, Yasuaki Ishimatsu,
	wangyun, srivatsa.bhat, netdev, Thomas Gleixner, linuxppc-dev,
	Andrew Morton
In-Reply-To: <20130623133642.19094.16038.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 03/45] Documentation, CPU hotplug: Recommend usage of get/put_online_cpus_atomic()
From: Srivatsa S. Bhat @ 2013-06-23 13:38 UTC (permalink / raw)
  To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
	walken, vincent.guittot, laijs
  Cc: linux-arch, nikunj, zhong, linux-pm, fweisbec, linux-doc,
	linux-kernel, rostedt, xiaoguangrong, sbw, wangyun, srivatsa.bhat,
	netdev, Rob Landley, linuxppc-dev
In-Reply-To: <20130623133642.19094.16038.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 04/45] CPU hotplug: Add infrastructure to check lacking hotplug synchronization
From: Srivatsa S. Bhat @ 2013-06-23 13:38 UTC (permalink / raw)
  To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
	walken, vincent.guittot, laijs
  Cc: linux-arch, Alex Shi, nikunj, zhong, linux-pm, fweisbec,
	Rusty Russell, linux-kernel, rostedt, xiaoguangrong,
	Rafael J. Wysocki, sbw, Yasuaki Ishimatsu, wangyun, srivatsa.bhat,
	netdev, Tejun Heo, Thomas Gleixner, KOSAKI Motohiro, linuxppc-dev,
	Andrew Morton
In-Reply-To: <20130623133642.19094.16038.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            |   75 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 87 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..e90d9d7 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -63,6 +63,72 @@ 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);
+
+#endif
+
 void get_online_cpus(void)
 {
 	might_sleep();
@@ -189,13 +255,22 @@ unsigned int get_online_cpus_atomic(void)
 	 * from going offline.
 	 */
 	preempt_disable();
+
+#ifdef CONFIG_DEBUG_HOTPLUG_CPU
+	this_cpu_inc(atomic_reader_refcnt);
+#endif
 	return smp_processor_id();
 }
 EXPORT_SYMBOL_GPL(get_online_cpus_atomic);
 
 void put_online_cpus_atomic(void)
 {
+
+#ifdef CONFIG_DEBUG_HOTPLUG_CPU
+	this_cpu_dec(atomic_reader_refcnt);
+#endif
 	preempt_enable();
+
 }
 EXPORT_SYMBOL_GPL(put_online_cpus_atomic);
 

^ permalink raw reply related

* [PATCH 05/45] CPU hotplug: Protect set_cpu_online() to avoid false-positives
From: Srivatsa S. Bhat @ 2013-06-23 13:39 UTC (permalink / raw)
  To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
	walken, vincent.guittot, laijs
  Cc: linux-arch, nikunj, zhong, linux-pm, fweisbec, Rafael J. Wysocki,
	linux-kernel, rostedt, xiaoguangrong, sbw, Yasuaki Ishimatsu,
	wangyun, srivatsa.bhat, netdev, Thomas Gleixner, linuxppc-dev,
	Andrew Morton
In-Reply-To: <20130623133642.19094.16038.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 e90d9d7..23df9ba 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -818,10 +818,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 06/45] CPU hotplug: Sprinkle debugging checks to catch locking bugs
From: Srivatsa S. Bhat @ 2013-06-23 13:39 UTC (permalink / raw)
  To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
	walken, vincent.guittot, laijs
  Cc: linux-arch, Alex Shi, nikunj, zhong, linux-pm, fweisbec,
	Rusty Russell, linux-kernel, rostedt, xiaoguangrong, sbw,
	Joonsoo Kim, wangyun, srivatsa.bhat, netdev, Tejun Heo,
	Andrew Morton, KOSAKI Motohiro, linuxppc-dev
In-Reply-To: <20130623133642.19094.16038.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 07/45] CPU hotplug: Expose the new debug config option
From: Srivatsa S. Bhat @ 2013-06-23 13:39 UTC (permalink / raw)
  To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
	walken, vincent.guittot, laijs
  Cc: linux-arch, Michel Lespinasse, Catalin Marinas, nikunj, zhong,
	linux-pm, fweisbec, linux-kernel, rostedt, xiaoguangrong, sbw,
	Akinobu Mita, wangyun, srivatsa.bhat, netdev, Andrew Morton,
	Paul E. McKenney, linuxppc-dev
In-Reply-To: <20130623133642.19094.16038.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>
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---

 lib/Kconfig.debug |    9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 566cf2b..6be1e72 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -512,6 +512,15 @@ 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
+	default n
+	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 08/45] CPU hotplug: Convert preprocessor macros to static inline functions
From: Srivatsa S. Bhat @ 2013-06-23 13:39 UTC (permalink / raw)
  To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
	walken, vincent.guittot, laijs
  Cc: linux-arch, nikunj, zhong, linux-pm, fweisbec, linux-kernel,
	rostedt, xiaoguangrong, Rafael J. Wysocki, sbw, wangyun,
	srivatsa.bhat, netdev, Tejun Heo, Thomas Gleixner, linuxppc-dev,
	Andrew Morton
In-Reply-To: <20130623133642.19094.16038.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 e06c3ad..d91919b 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -200,10 +200,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)
 {
@@ -220,6 +218,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 09/45] smp: Use get/put_online_cpus_atomic() to prevent CPU offline
From: Srivatsa S. Bhat @ 2013-06-23 13:40 UTC (permalink / raw)
  To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
	walken, vincent.guittot, laijs
  Cc: linux-arch, nikunj, zhong, linux-pm, fweisbec, Shaohua Li,
	linux-kernel, rostedt, xiaoguangrong, sbw, Jan Beulich, wangyun,
	srivatsa.bhat, netdev, Andrew Morton, linuxppc-dev, liguang,
	Wang YanQing
In-Reply-To: <20130623133642.19094.16038.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 10/45] sched/core: Use get/put_online_cpus_atomic() to prevent CPU offline
From: Srivatsa S. Bhat @ 2013-06-23 13:40 UTC (permalink / raw)
  To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
	walken, vincent.guittot, laijs
  Cc: linux-arch, nikunj, zhong, linux-pm, Peter Zijlstra, fweisbec,
	linux-kernel, rostedt, xiaoguangrong, sbw, Ingo Molnar, wangyun,
	srivatsa.bhat, netdev, linuxppc-dev
In-Reply-To: <20130623133642.19094.16038.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 195658b..accd550 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;
 }
 
@@ -1753,6 +1762,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
 	/*
@@ -1773,6 +1784,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
@@ -3886,6 +3899,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();
 
@@ -3931,6 +3946,8 @@ out_unlock:
 out_irq:
 	local_irq_restore(flags);
 
+	put_online_cpus_atomic();
+
 	if (yielded > 0)
 		schedule();
 
@@ -4331,9 +4348,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 11/45] migration: Use raw_spin_lock/unlock since interrupts are already disabled
From: Srivatsa S. Bhat @ 2013-06-23 13:40 UTC (permalink / raw)
  To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
	walken, vincent.guittot, laijs
  Cc: linux-arch, nikunj, zhong, linux-pm, Peter Zijlstra, fweisbec,
	linux-kernel, rostedt, xiaoguangrong, sbw, Ingo Molnar, wangyun,
	srivatsa.bhat, netdev, linuxppc-dev
In-Reply-To: <20130623133642.19094.16038.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 accd550..ff26f54 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -4682,14 +4682,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 12/45] sched/fair: Use get/put_online_cpus_atomic() to prevent CPU offline
From: Srivatsa S. Bhat @ 2013-06-23 13:40 UTC (permalink / raw)
  To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
	walken, vincent.guittot, laijs
  Cc: linux-arch, nikunj, zhong, linux-pm, Peter Zijlstra, fweisbec,
	linux-kernel, rostedt, xiaoguangrong, sbw, Ingo Molnar, wangyun,
	srivatsa.bhat, netdev, linuxppc-dev
In-Reply-To: <20130623133642.19094.16038.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 c0ac2c3..88f056e 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -3338,7 +3338,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)
@@ -5267,6 +5268,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;
@@ -5290,6 +5293,7 @@ void idle_balance(int this_cpu, struct rq *this_rq)
 		}
 	}
 	rcu_read_unlock();
+	put_online_cpus_atomic();
 
 	raw_spin_lock(&this_rq->lock);
 
@@ -5316,6 +5320,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 */
@@ -5367,6 +5372,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;
 }
 
@@ -5527,6 +5533,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))
@@ -5575,6 +5582,7 @@ out:
 			break;
 	}
 	rcu_read_unlock();
+	put_online_cpus_atomic();
 
 	/*
 	 * next_balance will be updated only when there is a need.
@@ -5706,6 +5714,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);
 
 	/*
@@ -5714,6 +5723,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)
@@ -5731,8 +5741,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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox