LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH V2 1/2] mm: move FAULT_AROUND_ORDER to arch/
From: Kirill A. Shutemov @ 2014-04-04 13:17 UTC (permalink / raw)
  To: Madhavan Srinivasan
  Cc: linux-arch, riel, rusty, peterz, x86, linux-kernel, linux-mm, ak,
	paulus, mgorman, akpm, linuxppc-dev, mingo, kirill.shutemov
In-Reply-To: <1396592835-24767-2-git-send-email-maddy@linux.vnet.ibm.com>

On Fri, Apr 04, 2014 at 11:57:14AM +0530, Madhavan Srinivasan wrote:
> Kirill A. Shutemov with faultaround patchset introduced
> vm_ops->map_pages() for mapping easy accessible pages around
> fault address in hope to reduce number of minor page faults.
> 
> This patch creates infrastructure to move the FAULT_AROUND_ORDER
> to arch/ using Kconfig. This will enable architecture maintainers
> to decide on suitable FAULT_AROUND_ORDER value based on
> performance data for that architecture. Patch also adds
> FAULT_AROUND_ORDER Kconfig element in arch/X86.
> 
> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
> ---
>  arch/x86/Kconfig   |    4 ++++
>  include/linux/mm.h |    9 +++++++++
>  mm/memory.c        |   12 +++++-------
>  3 files changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 9c0a657..5833f22 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1177,6 +1177,10 @@ config DIRECT_GBPAGES
>  	  support it. This can improve the kernel's performance a tiny bit by
>  	  reducing TLB pressure. If in doubt, say "Y".
>  
> +config FAULT_AROUND_ORDER
> +	int
> +	default "4"
> +
>  # Common NUMA Features
>  config NUMA
>  	bool "Numa Memory Allocation and Scheduler Support"
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 0bd4359..b93c1c3 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -26,6 +26,15 @@ struct file_ra_state;
>  struct user_struct;
>  struct writeback_control;
>  
> +/*
> + * Fault around order is a control knob to decide the fault around pages.
> + * Default value is set to 0UL (disabled), but the arch can override it as
> + * desired.
> + */
> +#ifndef CONFIG_FAULT_AROUND_ORDER
> +#define CONFIG_FAULT_AROUND_ORDER 0
> +#endif
> +

I don't think it should be in header file: nobody except mm/memory.c cares.
Just put it instead '#define FAULT_AROUND_ORDER'.

>  #ifndef CONFIG_NEED_MULTIPLE_NODES	/* Don't use mapnrs, do it properly */
>  extern unsigned long max_mapnr;
>  
> diff --git a/mm/memory.c b/mm/memory.c
> index b02c584..22a4a89 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -3358,10 +3358,8 @@ void do_set_pte(struct vm_area_struct *vma, unsigned long address,
>  	update_mmu_cache(vma, address, pte);
>  }
>  
> -#define FAULT_AROUND_ORDER 4
> -
>  #ifdef CONFIG_DEBUG_FS
> -static unsigned int fault_around_order = FAULT_AROUND_ORDER;
> +static unsigned int fault_around_order = CONFIG_FAULT_AROUND_ORDER;
>  
>  static int fault_around_order_get(void *data, u64 *val)
>  {
> @@ -3371,7 +3369,7 @@ static int fault_around_order_get(void *data, u64 *val)
>  
>  static int fault_around_order_set(void *data, u64 val)
>  {
> -	BUILD_BUG_ON((1UL << FAULT_AROUND_ORDER) > PTRS_PER_PTE);
> +	BUILD_BUG_ON((1UL << CONFIG_FAULT_AROUND_ORDER) > PTRS_PER_PTE);
>  	if (1UL << val > PTRS_PER_PTE)
>  		return -EINVAL;
>  	fault_around_order = val;
> @@ -3406,14 +3404,14 @@ static inline unsigned long fault_around_pages(void)
>  {
>  	unsigned long nr_pages;
>  
> -	nr_pages = 1UL << FAULT_AROUND_ORDER;
> +	nr_pages = 1UL << CONFIG_FAULT_AROUND_ORDER;
>  	BUILD_BUG_ON(nr_pages > PTRS_PER_PTE);
>  	return nr_pages;
>  }
>  
>  static inline unsigned long fault_around_mask(void)
>  {
> -	return ~((1UL << (PAGE_SHIFT + FAULT_AROUND_ORDER)) - 1);
> +	return ~((1UL << (PAGE_SHIFT + CONFIG_FAULT_AROUND_ORDER)) - 1);
>  }
>  #endif
>  
> @@ -3471,7 +3469,7 @@ static int do_read_fault(struct mm_struct *mm, struct vm_area_struct *vma,
>  	 * if page by the offset is not ready to be mapped (cold cache or
>  	 * something).
>  	 */
> -	if (vma->vm_ops->map_pages) {
> +	if ((vma->vm_ops->map_pages) && (fault_around_pages() > 1)) {

	if (vma->vm_ops->map_pages && fault_around_pages()) {

>  		pte = pte_offset_map_lock(mm, pmd, address, &ptl);
>  		do_fault_around(vma, address, pte, pgoff, flags);
>  		if (!pte_same(*pte, orig_pte))
> -- 
> 1.7.10.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
 Kirill A. Shutemov

^ permalink raw reply

* Re: [PATCH 09/20] of/fdt: create common debugfs
From: Michal Simek @ 2014-04-04 13:22 UTC (permalink / raw)
  To: Rob Herring
  Cc: Grant Likely, linuxppc-dev, Paul Mackerras,
	linux-kernel@vger.kernel.org
In-Reply-To: <CAL_JsqKFK8YDoODYKg5b8SmDzqdNSS6UvYr3_atzhcL2uR-=rA@mail.gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 3686 bytes --]

On 04/04/2014 03:00 PM, Rob Herring wrote:
> On Fri, Apr 4, 2014 at 7:16 AM, Michal Simek <monstr@monstr.eu> wrote:
>> On 04/04/2014 12:16 AM, Rob Herring wrote:
>>> From: Rob Herring <robh@kernel.org>
>>>
>>> Both powerpc and microblaze have the same FDT blob in debugfs feature.
>>> Move this to common location and remove the powerpc and microblaze
>>> implementations. This feature could become more useful when FDT
>>> overlay support is added.
>>>
>>> This changes the path of the blob from "$arch/flat-device-tree" to
>>> "device-tree/flat-device-tree".
> 
> [snip]
> 
>>> -#if defined(CONFIG_DEBUG_FS) && defined(DEBUG)
>>> -static struct debugfs_blob_wrapper flat_dt_blob;
>>> -
>>> -static int __init export_flat_device_tree(void)
>>> -{
>>> -     struct dentry *d;
>>> -
>>> -     flat_dt_blob.data = initial_boot_params;
>>> -     flat_dt_blob.size = initial_boot_params->totalsize;
>>
>> As I see even microblaze version was buggy.
> 
> How so?

if you compare it with powerpc version here is missing
be to cpu conversion.

> 
>> ...
>>
>>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>>> index fa16a91..2085d47 100644
>>> --- a/drivers/of/fdt.c
>>> +++ b/drivers/of/fdt.c
>>> @@ -20,6 +20,7 @@
>>>  #include <linux/string.h>
>>>  #include <linux/errno.h>
>>>  #include <linux/slab.h>
>>> +#include <linux/debugfs.h>
>>>
>>>  #include <asm/setup.h>  /* for COMMAND_LINE_SIZE */
>>>  #ifdef CONFIG_PPC
>>> @@ -1084,4 +1085,27 @@ void __init unflatten_and_copy_device_tree(void)
>>>       unflatten_device_tree();
>>>  }
>>>
>>> +#if defined(CONFIG_DEBUG_FS) && defined(DEBUG)
>>> +static struct debugfs_blob_wrapper flat_dt_blob;
>>> +
>>> +static int __init of_flat_dt_debugfs_export_fdt(void)
>>> +{
>>> +     struct dentry *d = debugfs_create_dir("device-tree", NULL);
>>> +
>>> +     if (!d)
>>> +             return -ENOENT;
>>> +
>>> +     flat_dt_blob.data = initial_boot_params;
>>> +     flat_dt_blob.size = fdt_totalsize(initial_boot_params);
>>
>> Have you tried to compile this?
>>
>> From my tests fdt_totalsize is not available for target just for host
>> from libfdt.h
>>
>> drivers/of/fdt.c: In function 'of_flat_dt_debugfs_export_fdt':
>> drivers/of/fdt.c:957:2: error: implicit declaration of function 'fdt_totalsize' [-Werror=implicit-function-declaration]
> 
> Ah, it needs to be re-ordered after the libfdt conversion when
> libfdt.h gets added.

I just pick some of them not all of them and send email. :-(

Anyway I am testing it for microblaze and getting problem
caused by this patch:
commit 3d2ee8571ac0580d49c3f41fa28336289934900a
Author: Rob Herring <robh@kernel.org>
Date:   Wed Apr 2 15:10:14 2014 -0500

    of/fdt: Convert FDT functions to use libfdt

And reason is that in unflatten_dt_node()

pathp = fdt_get_name(blob, *poffset, &l);

is returning NULL
and here
	/* version 0x10 has a more compact unit name here instead of the full
	 * path. we accumulate the full path size using "fpsize", we'll rebuild
	 * it later. We detect this because the first character of the name is
	 * not '/'.
	 */
	if ((*pathp) != '/') {

code is trying to read it which is causing this kernel bug:
Oops: kernel access of bad area, sig: 11

It means fdt_next_node(is doing something wrong)

Any easy way how to debug it?

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


[-- Attachment #1.2: system.dts --]
[-- Type: text/plain, Size: 13423 bytes --]

/*
 * Device Tree Generator version: 1.1
 *
 * (C) Copyright 2007-2013 Xilinx, Inc.
 * (C) Copyright 2007-2013 Michal Simek
 * (C) Copyright 2007-2012 PetaLogix Qld Pty Ltd
 *
 * Michal SIMEK <monstr@monstr.eu>
 *
 * CAUTION: This file is automatically generated by libgen.
 * Version: Xilinx EDK 2013.3 EDK_P.20131013
 * Today is: Thursday, the 21 of November, 2013; 19:47:51
 *
 * XPS project directory: .
 */

/dts-v1/;
/ {
	#address-cells = <1>;
	#size-cells = <1>;
	compatible = "xlnx,microblaze";
	hard-reset-gpios = <&reset_gpio 0 1>;
	model = ".";
	aliases {
		ethernet0 = &axi_ethernet_eth_buf;
		serial0 = &rs232_uart;
	} ;
	chosen {
		bootargs = "console=ttyS0,115200 earlyprintk";
		linux,stdout-path = "/axi@0/serial@40400000";
	} ;
	cpus {
		#address-cells = <1>;
		#cpus = <0x1>;
		#size-cells = <0>;
		microblaze_0: cpu@0 {
			bus-handle = <&microblaze_0_axi_periph_xbar>;
			clock-frequency = <100000000>;
			compatible = "xlnx,microblaze-9.2";
			d-cache-baseaddr = <0x80000000>;
			d-cache-highaddr = <0xbfffffff>;
			d-cache-line-size = <0x20>;
			d-cache-size = <0x4000>;
			device_type = "cpu";
			i-cache-baseaddr = <0x80000000>;
			i-cache-highaddr = <0xbfffffff>;
			i-cache-line-size = <0x10>;
			i-cache-size = <0x4000>;
			interrupt-handle = <&microblaze_0_axi_intc>;
			model = "microblaze,9.2";
			reg = <0>;
			timebase-frequency = <100000000>;
			xlnx,addr-tag-bits = <0x10>;
			xlnx,allow-dcache-wr = <0x1>;
			xlnx,allow-icache-wr = <0x1>;
			xlnx,area-optimized = <0x0>;
			xlnx,async-interrupt = <0x1>;
			xlnx,avoid-primitives = <0x0>;
			xlnx,base-vectors = <0x0>;
			xlnx,branch-target-cache-size = <0x0>;
			xlnx,cache-byte-size = <0x4000>;
			xlnx,d-axi = <0x1>;
			xlnx,d-lmb = <0x1>;
			xlnx,data-size = <0x20>;
			xlnx,dcache-addr-tag = <0x10>;
			xlnx,dcache-always-used = <0x1>;
			xlnx,dcache-byte-size = <0x4000>;
			xlnx,dcache-data-width = <0x0>;
			xlnx,dcache-force-tag-lutram = <0x0>;
			xlnx,dcache-line-len = <0x8>;
			xlnx,dcache-use-writeback = <0x1>;
			xlnx,dcache-victims = <0x0>;
			xlnx,debug-enabled = <0x1>;
			xlnx,div-zero-exception = <0x1>;
			xlnx,dynamic-bus-sizing = <0x0>;
			xlnx,ecc-use-ce-exception = <0x0>;
			xlnx,edge-is-positive = <0x0>;
			xlnx,enable-discrete-ports = <0x0>;
			xlnx,endianness = <0x1>;
			xlnx,fault-tolerant = <0x0>;
			xlnx,fpu-exception = <0x0>;
			xlnx,freq = <0x5f5e100>;
			xlnx,fsl-exception = <0x0>;
			xlnx,fsl-links = <0x0>;
			xlnx,i-axi = <0x0>;
			xlnx,i-lmb = <0x1>;
			xlnx,icache-always-used = <0x1>;
			xlnx,icache-data-width = <0x0>;
			xlnx,icache-force-tag-lutram = <0x0>;
			xlnx,icache-line-len = <0x4>;
			xlnx,icache-streams = <0x1>;
			xlnx,icache-victims = <0x8>;
			xlnx,ill-opcode-exception = <0x1>;
			xlnx,instance = "kc705_microblaze_0_0";
			xlnx,interconnect = <0x2>;
			xlnx,interrupt-is-edge = <0x0>;
			xlnx,lockstep-select = <0x0>;
			xlnx,lockstep-slave = <0x0>;
			xlnx,mmu-dtlb-size = <0x4>;
			xlnx,mmu-itlb-size = <0x2>;
			xlnx,mmu-privileged-instr = <0x0>;
			xlnx,mmu-tlb-access = <0x3>;
			xlnx,mmu-zones = <0x2>;
			xlnx,number-of-pc-brk = <0x1>;
			xlnx,number-of-rd-addr-brk = <0x0>;
			xlnx,number-of-wr-addr-brk = <0x0>;
			xlnx,opcode-0x0-illegal = <0x1>;
			xlnx,optimization = <0x0>;
			xlnx,pc-width = <0x20>;
			xlnx,pvr = <0x2>;
			xlnx,pvr-user1 = <0x0>;
			xlnx,pvr-user2 = <0x0>;
			xlnx,reset-msr = <0x0>;
			xlnx,sco = <0x0>;
			xlnx,trace = <0x0>;
			xlnx,unaligned-exceptions = <0x1>;
			xlnx,use-barrel = <0x1>;
			xlnx,use-branch-target-cache = <0x1>;
			xlnx,use-config-reset = <0x0>;
			xlnx,use-dcache = <0x1>;
			xlnx,use-div = <0x1>;
			xlnx,use-ext-brk = <0x0>;
			xlnx,use-ext-nm-brk = <0x0>;
			xlnx,use-extended-fsl-instr = <0x0>;
			xlnx,use-fpu = <0x0>;
			xlnx,use-hw-mul = <0x2>;
			xlnx,use-icache = <0x1>;
			xlnx,use-interrupt = <0x2>;
			xlnx,use-mmu = <0x3>;
			xlnx,use-msr-instr = <0x1>;
			xlnx,use-pcmp-instr = <0x1>;
			xlnx,use-reorder-instr = <0x1>;
			xlnx,use-stack-protection = <0x0>;
		} ;
	} ;
	ddr3_sdram: memory@80000000 {
		device_type = "memory";
		reg = <0x80000000 0x40000000>;
	} ;
	microblaze_0_axi_periph_xbar: axi@0 {
		#address-cells = <1>;
		#size-cells = <1>;
		compatible = "xlnx,axi-crossbar-2.1", "simple-bus";
		ranges ;
		axi_ethernet_dma: axi-dma@41e00000 {
			axistream-connected = <&axi_ethernet_eth_buf>;
			axistream-control-connected = <&axi_ethernet_eth_buf>;
			compatible = "xlnx,axi-dma-7.1", "xlnx,axi-dma-1.00.a";
			interrupt-parent = <&microblaze_0_axi_intc>;
			interrupts = <2 2>, <3 2>;
			reg = <0x41e00000 0x10000>;
			xlnx,dlytmr-resolution = <0x7d>;
			xlnx,enable-multi-channel = <0x0>;
			xlnx,include-mm2s = <0x1>;
			xlnx,include-mm2s-dre = <0x1>;
			xlnx,include-mm2s-sf = <0x1>;
			xlnx,include-s2mm = <0x1>;
			xlnx,include-s2mm-dre = <0x1>;
			xlnx,include-s2mm-sf = <0x1>;
			xlnx,include-sg = <0x1>;
			xlnx,micro-dma = <0x0>;
			xlnx,mm2s-burst-size = <0x10>;
			xlnx,num-mm2s-channels = <0x1>;
			xlnx,num-s2mm-channels = <0x1>;
			xlnx,prmry-is-aclk-async = <0x0>;
			xlnx,s2mm-burst-size = <0x10>;
			xlnx,sg-include-stscntrl-strm = <0x1>;
			xlnx,sg-length-width = <0xe>;
			xlnx,sg-use-stsapp-length = <0x1>;
		} ;
		axi_ethernet_eth_buf: axi-ethernet@44a00000 {
			axistream-connected = <&axi_ethernet_dma>;
			axistream-control-connected = <&axi_ethernet_dma>;
			clock-frequency = <100000000>;
			compatible = "xlnx,axi-ethernet-buffer-2.0", "xlnx,axi-ethernet-1.00.a";
			device_type = "network";
			interrupt-parent = <&microblaze_0_axi_intc>;
			interrupts = <1 2>;
			local-mac-address = [ 00 0a 35 00 42 4a ];
			phy-handle = <&phy0>;
			reg = <0x44a00000 0x40000>;
			xlnx,avb = <0x0>;
			xlnx,halfdup = "1";
			xlnx,include-io = "1";
			xlnx,mcast-extend = <0x0>;
			xlnx,phy-type = <0x1>;
			xlnx,phyaddr = <0x1>;
			xlnx,rxcsum = <0x0>;
			xlnx,rxmem = <0x1000>;
			xlnx,rxvlan-strp = <0x0>;
			xlnx,rxvlan-tag = <0x0>;
			xlnx,rxvlan-tran = <0x0>;
			xlnx,stats = <0x0>;
			xlnx,txcsum = <0x0>;
			xlnx,txmem = <0x1000>;
			xlnx,txvlan-strp = <0x0>;
			xlnx,txvlan-tag = <0x0>;
			xlnx,txvlan-tran = <0x0>;
			xlnx,type = <0x1>;
			mdio {
				#address-cells = <1>;
				#size-cells = <0>;
				phy0: phy@7 {
					compatible = "marvell,88e1111";
					device_type = "ethernet-phy";
					reg = <7>;
				} ;
			} ;
		} ;
		axi_timer_0: system-timer@41c00000 {
			clock-frequency = <100000000>;
			compatible = "xlnx,axi-timer-2.0", "xlnx,xps-timer-1.00.a";
			interrupt-parent = <&microblaze_0_axi_intc>;
			interrupts = <0 2>;
			reg = <0x41c00000 0x10000>;
			xlnx,count-width = <0x20>;
			xlnx,gen0-assert = <0x1>;
			xlnx,gen1-assert = <0x1>;
			xlnx,one-timer-only = <0x0>;
			xlnx,trig0-assert = <0x1>;
			xlnx,trig1-assert = <0x1>;
		} ;
		dip_switches_4bits: gpio@40010000 {
			#gpio-cells = <2>;
			compatible = "xlnx,axi-gpio-2.0", "xlnx,xps-gpio-1.00.a";
			gpio-controller ;
			reg = <0x40010000 0x10000>;
			xlnx,all-inputs = <0x1>;
			xlnx,all-inputs-2 = <0x0>;
			xlnx,all-outputs = <0x0>;
			xlnx,all-outputs-2 = <0x0>;
			xlnx,dout-default = <0x0>;
			xlnx,dout-default-2 = <0x0>;
			xlnx,gpio-width = <0x4>;
			xlnx,gpio2-width = <0x20>;
			xlnx,interrupt-present = <0x0>;
			xlnx,is-dual = <0x0>;
			xlnx,tri-default = <0xffffffff>;
			xlnx,tri-default-2 = <0xffffffff>;
		} ;
		led_8bits: gpio@40020000 {
			#gpio-cells = <2>;
			compatible = "xlnx,axi-gpio-2.0", "xlnx,xps-gpio-1.00.a";
			gpio-controller ;
			reg = <0x40020000 0x10000>;
			xlnx,all-inputs = <0x0>;
			xlnx,all-inputs-2 = <0x0>;
			xlnx,all-outputs = <0x1>;
			xlnx,all-outputs-2 = <0x0>;
			xlnx,dout-default = <0x0>;
			xlnx,dout-default-2 = <0x0>;
			xlnx,gpio-width = <0x8>;
			xlnx,gpio2-width = <0x20>;
			xlnx,interrupt-present = <0x0>;
			xlnx,is-dual = <0x0>;
			xlnx,tri-default = <0xffffffff>;
			xlnx,tri-default-2 = <0xffffffff>;
		} ;
		microblaze_0_axi_intc: interrupt-controller@41200000 {
			#interrupt-cells = <0x2>;
			compatible = "xlnx,axi-intc-4.0", "xlnx,xps-intc-1.00.a";
			interrupt-controller ;
			reg = <0x41200000 0x10000>;
			xlnx,kind-of-intr = <0x0>;
			xlnx,num-intr-inputs = <0x5>;
		} ;
		primary_flash: flash@60000000 {
			#address-cells = <1>;
			#size-cells = <1>;
			bank-width = <2>;
			compatible = "xlnx,axi-emc-2.0", "cfi-flash";
			reg = <0x60000000 0x8000000>;
			xlnx,axi-clk-period-ps = <0x2710>;
			xlnx,include-datawidth-matching-0 = <0x1>;
			xlnx,include-datawidth-matching-1 = <0x1>;
			xlnx,include-datawidth-matching-2 = <0x1>;
			xlnx,include-datawidth-matching-3 = <0x1>;
			xlnx,include-negedge-ioregs = <0x0>;
			xlnx,instance = "axi_emc_inst";
			xlnx,lflash-period-ps = <0x4e20>;
			xlnx,linear-flash-sync-burst = <0x0>;
			xlnx,max-mem-width = <0x10>;
			xlnx,mem0-type = <0x2>;
			xlnx,mem0-width = <0x10>;
			xlnx,mem1-type = <0x2>;
			xlnx,mem1-width = <0x10>;
			xlnx,mem2-type = <0x2>;
			xlnx,mem2-width = <0x10>;
			xlnx,mem3-type = <0x2>;
			xlnx,mem3-width = <0x10>;
			xlnx,num-banks-mem = <0x1>;
			xlnx,parity-type-mem-0 = <0x0>;
			xlnx,parity-type-mem-1 = <0x0>;
			xlnx,parity-type-mem-2 = <0x0>;
			xlnx,parity-type-mem-3 = <0x0>;
			xlnx,s-axi-en-reg = <0x0>;
			xlnx,s-axi-mem-addr-width = <0x20>;
			xlnx,s-axi-mem-data-width = <0x20>;
			xlnx,s-axi-mem-id-width = <0x1>;
			xlnx,s-axi-reg-addr-width = <0x5>;
			xlnx,s-axi-reg-data-width = <0x20>;
			xlnx,synch-pipedelay-0 = <0x1>;
			xlnx,synch-pipedelay-1 = <0x1>;
			xlnx,synch-pipedelay-2 = <0x1>;
			xlnx,synch-pipedelay-3 = <0x1>;
			xlnx,tavdv-ps-mem-0 = <0x1fbd0>;
			xlnx,tavdv-ps-mem-1 = <0x3a98>;
			xlnx,tavdv-ps-mem-2 = <0x3a98>;
			xlnx,tavdv-ps-mem-3 = <0x3a98>;
			xlnx,tcedv-ps-mem-0 = <0x1fbd0>;
			xlnx,tcedv-ps-mem-1 = <0x3a98>;
			xlnx,tcedv-ps-mem-2 = <0x3a98>;
			xlnx,tcedv-ps-mem-3 = <0x3a98>;
			xlnx,thzce-ps-mem-0 = <0x1b58>;
			xlnx,thzce-ps-mem-1 = <0x1b58>;
			xlnx,thzce-ps-mem-2 = <0x1b58>;
			xlnx,thzce-ps-mem-3 = <0x1b58>;
			xlnx,thzoe-ps-mem-0 = <0x1b58>;
			xlnx,thzoe-ps-mem-1 = <0x1b58>;
			xlnx,thzoe-ps-mem-2 = <0x1b58>;
			xlnx,thzoe-ps-mem-3 = <0x1b58>;
			xlnx,tlzwe-ps-mem-0 = <0xc350>;
			xlnx,tlzwe-ps-mem-1 = <0x0>;
			xlnx,tlzwe-ps-mem-2 = <0x0>;
			xlnx,tlzwe-ps-mem-3 = <0x0>;
			xlnx,tpacc-ps-flash-0 = <0x61a8>;
			xlnx,tpacc-ps-flash-1 = <0x61a8>;
			xlnx,tpacc-ps-flash-2 = <0x61a8>;
			xlnx,tpacc-ps-flash-3 = <0x61a8>;
			xlnx,twc-ps-mem-0 = <0x3a98>;
			xlnx,twc-ps-mem-1 = <0x3a98>;
			xlnx,twc-ps-mem-2 = <0x3a98>;
			xlnx,twc-ps-mem-3 = <0x3a98>;
			xlnx,twp-ps-mem-0 = <0x13880>;
			xlnx,twp-ps-mem-1 = <0x2ee0>;
			xlnx,twp-ps-mem-2 = <0x2ee0>;
			xlnx,twp-ps-mem-3 = <0x2ee0>;
			xlnx,twph-ps-mem-0 = <0x13880>;
			xlnx,twph-ps-mem-1 = <0x2ee0>;
			xlnx,twph-ps-mem-2 = <0x2ee0>;
			xlnx,twph-ps-mem-3 = <0x2ee0>;
			xlnx,wr-rec-time-mem-0 = <0x186a0>;
			xlnx,wr-rec-time-mem-1 = <0x6978>;
			xlnx,wr-rec-time-mem-2 = <0x6978>;
			xlnx,wr-rec-time-mem-3 = <0x6978>;
			partition@0x00000000 {
				label = "fpga";
				reg = <0x00000000 0x00b00000>;
			};
			partition@0x00b00000 {
				label = "boot";
				reg = <0x00b00000 0x00040000>;
			};
			partition@0x00b40000 {
				label = "bootenv";
				reg = <0x00b40000 0x00020000>;
			};
			partition@0x00b60000 {
				label = "image";
				reg = <0x00b60000 0x00c00000>;
			};
			partition@0x01760000 {
				label = "spare";
				reg = <0x01760000 0x00000000>;
			};
		} ;
		push_buttons_5bits: gpio@40030000 {
			#gpio-cells = <2>;
			compatible = "xlnx,axi-gpio-2.0", "xlnx,xps-gpio-1.00.a";
			gpio-controller ;
			reg = <0x40030000 0x10000>;
			xlnx,all-inputs = <0x1>;
			xlnx,all-inputs-2 = <0x0>;
			xlnx,all-outputs = <0x0>;
			xlnx,all-outputs-2 = <0x0>;
			xlnx,dout-default = <0x0>;
			xlnx,dout-default-2 = <0x0>;
			xlnx,gpio-width = <0x5>;
			xlnx,gpio2-width = <0x20>;
			xlnx,interrupt-present = <0x0>;
			xlnx,is-dual = <0x0>;
			xlnx,tri-default = <0xffffffff>;
			xlnx,tri-default-2 = <0xffffffff>;
		} ;
		reset_gpio: gpio@40000000 {
			#gpio-cells = <2>;
			compatible = "xlnx,axi-gpio-2.0", "xlnx,xps-gpio-1.00.a";
			gpio-controller ;
			reg = <0x40000000 0x10000>;
			xlnx,all-inputs = <0x0>;
			xlnx,all-inputs-2 = <0x0>;
			xlnx,all-outputs = <0x1>;
			xlnx,all-outputs-2 = <0x0>;
			xlnx,dout-default = <0x0>;
			xlnx,dout-default-2 = <0x0>;
			xlnx,gpio-width = <0x1>;
			xlnx,gpio2-width = <0x20>;
			xlnx,interrupt-present = <0x0>;
			xlnx,is-dual = <0x0>;
			xlnx,tri-default = <0xffffffff>;
			xlnx,tri-default-2 = <0xffffffff>;
		} ;
		rs232_uart: serial@40400000 {
			clock-frequency = <100000000>;
			compatible = "xlnx,axi-uart16550-2.0", "xlnx,xps-uart16550-2.00.a", "ns16550a";
			current-speed = <115200>;
			device_type = "serial";
			interrupt-parent = <&microblaze_0_axi_intc>;
			interrupts = <4 2>;
			reg = <0x40400000 0x2000>;
			reg-offset = <0x1000>;
			reg-shift = <2>;
			xlnx,external-xin-clk-hz = <0x17d7840>;
			xlnx,external-xin-clk-hz-d = <0x19>;
			xlnx,has-external-rclk = <0x0>;
			xlnx,has-external-xin = <0x0>;
			xlnx,is-a-16550 = <0x1>;
			xlnx,s-axi-aclk-freq-hz-d = <0x64>;
			xlnx,use-modem-ports = <0x1>;
			xlnx,use-user-ports = <0x1>;
		} ;
	} ;
} ;

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

^ permalink raw reply

* Re: [PATCH 09/20] of/fdt: create common debugfs
From: Rob Herring @ 2014-04-04 13:32 UTC (permalink / raw)
  To: Michal Simek
  Cc: Grant Likely, linuxppc-dev, Paul Mackerras,
	linux-kernel@vger.kernel.org
In-Reply-To: <533EB207.1020203@monstr.eu>

On Fri, Apr 4, 2014 at 8:22 AM, Michal Simek <monstr@monstr.eu> wrote:
> On 04/04/2014 03:00 PM, Rob Herring wrote:
>> On Fri, Apr 4, 2014 at 7:16 AM, Michal Simek <monstr@monstr.eu> wrote:
>>> On 04/04/2014 12:16 AM, Rob Herring wrote:
>>>> From: Rob Herring <robh@kernel.org>
>>>>
>>>> Both powerpc and microblaze have the same FDT blob in debugfs feature.
>>>> Move this to common location and remove the powerpc and microblaze
>>>> implementations. This feature could become more useful when FDT
>>>> overlay support is added.
>>
>> [snip]

> Anyway I am testing it for microblaze and getting problem
> caused by this patch:
> commit 3d2ee8571ac0580d49c3f41fa28336289934900a
> Author: Rob Herring <robh@kernel.org>
> Date:   Wed Apr 2 15:10:14 2014 -0500
>
>     of/fdt: Convert FDT functions to use libfdt
>
> And reason is that in unflatten_dt_node()
>
> pathp = fdt_get_name(blob, *poffset, &l);
>
> is returning NULL
> and here
>         /* version 0x10 has a more compact unit name here instead of the full
>          * path. we accumulate the full path size using "fpsize", we'll rebuild
>          * it later. We detect this because the first character of the name is
>          * not '/'.
>          */
>         if ((*pathp) != '/') {
>
> code is trying to read it which is causing this kernel bug:
> Oops: kernel access of bad area, sig: 11
>
> It means fdt_next_node(is doing something wrong)
>
> Any easy way how to debug it?

I didn't think fdt_get_path should fail. Can you add a print of
*poffset and pathp values.

Rob

^ permalink raw reply

* Re: [PATCH 09/20] of/fdt: create common debugfs
From: Michal Simek @ 2014-04-04 14:11 UTC (permalink / raw)
  To: Rob Herring
  Cc: Grant Likely, linuxppc-dev, Paul Mackerras,
	linux-kernel@vger.kernel.org
In-Reply-To: <CAL_Jsq+tf4wgjB7uTNipWEj5d6tPkd6QwO_xiW87P51Y_y48vw@mail.gmail.com>

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

On 04/04/2014 03:32 PM, Rob Herring wrote:
> On Fri, Apr 4, 2014 at 8:22 AM, Michal Simek <monstr@monstr.eu> wrote:
>> On 04/04/2014 03:00 PM, Rob Herring wrote:
>>> On Fri, Apr 4, 2014 at 7:16 AM, Michal Simek <monstr@monstr.eu> wrote:
>>>> On 04/04/2014 12:16 AM, Rob Herring wrote:
>>>>> From: Rob Herring <robh@kernel.org>
>>>>>
>>>>> Both powerpc and microblaze have the same FDT blob in debugfs feature.
>>>>> Move this to common location and remove the powerpc and microblaze
>>>>> implementations. This feature could become more useful when FDT
>>>>> overlay support is added.
>>>
>>> [snip]
> 
>> Anyway I am testing it for microblaze and getting problem
>> caused by this patch:
>> commit 3d2ee8571ac0580d49c3f41fa28336289934900a
>> Author: Rob Herring <robh@kernel.org>
>> Date:   Wed Apr 2 15:10:14 2014 -0500
>>
>>     of/fdt: Convert FDT functions to use libfdt
>>
>> And reason is that in unflatten_dt_node()
>>
>> pathp = fdt_get_name(blob, *poffset, &l);
>>
>> is returning NULL
>> and here
>>         /* version 0x10 has a more compact unit name here instead of the full
>>          * path. we accumulate the full path size using "fpsize", we'll rebuild
>>          * it later. We detect this because the first character of the name is
>>          * not '/'.
>>          */
>>         if ((*pathp) != '/') {
>>
>> code is trying to read it which is causing this kernel bug:
>> Oops: kernel access of bad area, sig: 11
>>
>> It means fdt_next_node(is doing something wrong)
>>
>> Any easy way how to debug it?
> 
> I didn't think fdt_get_path should fail. Can you add a print of
> *poffset and pathp values.

Early console on uart16650 at 0x40401000
bootconsole [earlyser0] enabled
Ramdisk addr 0x00000000,
FDT at 0x806b96d4
Linux version 3.14.0-rc8-next-20140331-12541-g43b5fdb-dirty (monstr@monstr-desktop) (gcc version 4.6.4 20120924 (prerelease) (crosstool-NG 1.18.0) ) #51 Fri Apr 4 16:09:17 CEST 2014
 -> unflatten_device_tree()
Unflattening device tree:
magic: d00dfeed
size: 00014e20
version: 00000011
unflatten_dt_node blob c0298b00, c0331f84 0
unflatten_dt_node 1a c0298b3c/ -- len 0
unflatten_dt_node 8 old offset 0
unflatten_dt_node 8 new offset 6c
unflatten_dt_node blob c0298b00, c0331f84 6c
unflatten_dt_node 1a c0298ba8/aliases -- len 7
unflatten_dt_node 8 old offset 6c
unflatten_dt_node 8 new offset cc
unflatten_dt_node blob c0298b00, c0331f84 cc
unflatten_dt_node 1a c0298c08/chosen -- len 6
unflatten_dt_node 8 old offset cc
unflatten_dt_node 8 new offset 130
unflatten_dt_node blob c0298b00, c0331f84 130
unflatten_dt_node 1a c0298c6c/cpus -- len 4
unflatten_dt_node 8 old offset 130
unflatten_dt_node 8 new offset 16c
unflatten_dt_node blob c0298b00, c0331f84 16c
unflatten_dt_node 1a c0298ca8/cpu@0 -- len 5
unflatten_dt_node 8 old offset 16c
unflatten_dt_node 8 new offset 7bc
unflatten_dt_node blob c0298b00, c0331f84 7bc
unflatten_dt_node 1a   (null)/NULL -- len fffffffc

Below is the possition of debug messages.

Thanks,
Michal

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index ee8853c..5422e4e 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -8,6 +8,7 @@
  * modify it under the terms of the GNU General Public License
  * version 2 as published by the Free Software Foundation.
  */
+#define DEBUG

 #include <linux/kernel.h>
 #include <linux/initrd.h>
@@ -116,9 +117,15 @@ static void * unflatten_dt_node(struct boot_param_header *blob,
        int has_name = 0;
        int new_format = 0;

+printk("%s blob %x, %x %x\n", __func__, blob, poffset,  *poffset);
+
        pathp = fdt_get_name(blob, *poffset, &l);
+printk("%s 1a %p/%s -- len %x\n", __func__, pathp, pathp ? pathp : "NULL", l);
+
        allocl = l++;

+       if (!pathp)
+               while (1);
        /* version 0x10 has a more compact unit name here instead of the full
         * path. we accumulate the full path size using "fpsize", we'll rebuild
         * it later. We detect this because the first character of the name is
@@ -267,7 +274,9 @@ static void * unflatten_dt_node(struct boot_param_header *blob,
        }

        old_depth = depth;
+printk("%s 8 old offset %x\n", __func__, *poffset);
        *poffset = fdt_next_node(blob, *poffset, &depth);
+printk("%s 8 new offset %x\n", __func__, *poffset);
        while (*poffset > 0 && depth > old_depth) {
                mem = unflatten_dt_node(blob, mem, poffset, np, allnextpp,
                                                fpsize);






-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

^ permalink raw reply related

* Re: [PATCH V2 1/2] mm: move FAULT_AROUND_ORDER to arch/
From: Dave Hansen @ 2014-04-04 16:18 UTC (permalink / raw)
  To: Madhavan Srinivasan, linux-kernel, linuxppc-dev, linux-mm,
	linux-arch, x86
  Cc: riel, ak, peterz, rusty, paulus, mgorman, akpm, mingo,
	kirill.shutemov
In-Reply-To: <1396592835-24767-2-git-send-email-maddy@linux.vnet.ibm.com>

On 04/03/2014 11:27 PM, Madhavan Srinivasan wrote:
> This patch creates infrastructure to move the FAULT_AROUND_ORDER
> to arch/ using Kconfig. This will enable architecture maintainers
> to decide on suitable FAULT_AROUND_ORDER value based on
> performance data for that architecture. Patch also adds
> FAULT_AROUND_ORDER Kconfig element in arch/X86.

Please don't do it this way.

In mm/Kconfig, put

	config FAULT_AROUND_ORDER
		int
		default 1234 if POWERPC
		default 4

The way you have it now, every single architecture that needs to enable
this has to go put that in their Kconfig.  That's madness.  This way,
you only put it in one place, and folks only have to care if they want
to change the default to be something other than 4.

^ permalink raw reply

* Re: [PATCH V2 1/2] mm: move FAULT_AROUND_ORDER to arch/
From: David Miller @ 2014-04-04 17:50 UTC (permalink / raw)
  To: dave.hansen
  Cc: linux-arch, rusty, maddy, riel, x86, linux-kernel, linux-mm,
	peterz, ak, paulus, mgorman, akpm, linuxppc-dev, mingo,
	kirill.shutemov
In-Reply-To: <533EDB63.8090909@intel.com>

From: Dave Hansen <dave.hansen@intel.com>
Date: Fri, 04 Apr 2014 09:18:43 -0700

> On 04/03/2014 11:27 PM, Madhavan Srinivasan wrote:
>> This patch creates infrastructure to move the FAULT_AROUND_ORDER
>> to arch/ using Kconfig. This will enable architecture maintainers
>> to decide on suitable FAULT_AROUND_ORDER value based on
>> performance data for that architecture. Patch also adds
>> FAULT_AROUND_ORDER Kconfig element in arch/X86.
> 
> Please don't do it this way.
> 
> In mm/Kconfig, put
> 
> 	config FAULT_AROUND_ORDER
> 		int
> 		default 1234 if POWERPC
> 		default 4
> 
> The way you have it now, every single architecture that needs to enable
> this has to go put that in their Kconfig.  That's madness.  This way,
> you only put it in one place, and folks only have to care if they want
> to change the default to be something other than 4.

It looks more like it's necessary only to change the default, not
to enable it.  Unless I read his patch wrong...

^ permalink raw reply

* Re: [PATCH] cpuidle: add freescale e500 family porcessors idle support
From: Scott Wood @ 2014-04-04 23:00 UTC (permalink / raw)
  To: Dongsheng Wang
  Cc: chenhui.zhao, linux-pm, daniel.lezcano, rjw, jason.jin,
	linuxppc-dev
In-Reply-To: <1396341234-40515-1-git-send-email-dongsheng.wang@freescale.com>

On Tue, 2014-04-01 at 16:33 +0800, Dongsheng Wang wrote:
> From: Wang Dongsheng <dongsheng.wang@freescale.com>
> 
> Add cpuidle support for e500 family, using cpuidle framework to
> manage various low power modes. The new implementation will remain
> compatible with original idle method.
> 
> I have done test about power consumption and latency. Cpuidle framework
> will make CPU response time faster than original method, but power
> consumption is higher than original method.
> 
> Power consumption:
> The original method, power consumption is 10.51202 (W).
> The cpuidle framework, power consumption is 10.5311 (W).
> 
> Latency:
> The original method, avg latency is 6782 (us).
> The cpuidle framework, avg latency is 6482 (us).
> 
> Initially, this supports PW10, PW20 and subsequent patches will support
> DOZE/NAP and PH10, PH20.

Have you tried tuning the timebase bit for the original method, to match
what you're doing in cpuidle and/or for general optimization?

Do you get similar results for a variety of workloads?

> +static struct cpuidle_state pw_idle_states[] = {
> +	{
> +		.name = "pw10",
> +		.desc = "pw10",
> +		.flags = CPUIDLE_FLAG_TIME_VALID,
> +		.exit_latency = 0,
> +		.target_residency = 0,
> +		.enter = &pw10_enter
> +	},
> +
> +	{
> +		.name = "pw20",
> +		.desc = "pw20-core-idle",
> +		.flags = CPUIDLE_FLAG_TIME_VALID,
> +		.exit_latency = 1,
> +		.target_residency = 50,
> +		.enter = &pw20_enter
> +	},
> +};

Where did exit_latency and target_residency come from?  It looks rather
different from the ~1ms delay before entering PW20 with the original
method.

Though, I'd have expected a shorter PW20 delay to have longer wake
latency, due to entering the lower power state more often...

> +static void replace_orig_idle(void *dummy)
> +{
> +	return;
> +}

Why?  If you're using this as some sort of barrier to ensure that all
CPUs have done something before continuing, explain that, along with why
it matters.

-Scott

^ permalink raw reply

* Re: [1/2, v9] powerpc/mpc85xx:Add initial device tree support of T104x
From: Prabhakar Kushwaha @ 2014-04-06 13:18 UTC (permalink / raw)
  To: Scott Wood; +Cc: Varun Sethi, Poonam Aggrwal, linuxppc-dev, Priyanka Jain
In-Reply-To: <20140319223334.GA28875@home.buserror.net>


On 3/20/2014 4:03 AM, Scott Wood wrote:
> On Sat, Jan 25, 2014 at 05:10:59PM +0530, Prabhakar Kushwaha wrote:
>> +	corenet-cf@18000 {
>> +		compatible = "fsl,corenet-cf";
>> +		reg = <0x18000 0x1000>;
>> +		interrupts = <16 2 1 31>;
>> +		fsl,ccf-num-csdids = <32>;
>> +		fsl,ccf-num-snoopids = <32>;
>> +	};
> I know this isn't a new problem, but this needs a binding -- and a
> different compatible from p4080-era CCF.  AFAICT it's a completely
> different programming model, and even the block version registers weren't
> present in the original version.

No binding present for corenet-cf looks like new binding needs to be 
sent with possible compatabile.


>> +/include/ "qoriq-mpic.dtsi"
>> +
>> +	guts: global-utilities@e0000 {
>> +		compatible = "fsl,t1040-device-config", "fsl,qoriq-device-config-2.0";
>> +		reg = <0xe0000 0xe00>;
>> +		fsl,has-rstcr;
>> +		fsl,liodn-bits = <12>;
>> +	};
>> +
>> +	clockgen: global-utilities@e1000 {
>> +		compatible = "fsl,t1040-clockgen", "fsl,qoriq-clockgen-2.0",
>> +				   "fixed-clock";
>> +		ranges = <0x0 0xe1000 0x1000>;
>> +		clock-frequency = <100000000>;
> Why is clock-frequency hardcoded here rather than supplied by U-Boot?
> Especially since this is an SoC file, not a board file.

Your are correct.
Means, clock-frequency should be added to clockgen in board device tree ??


>> +		reg = <0xe1000 0x1000>;
>> +		clock-output-names = "sysclk";
>> +		#address-cells = <1>;
>> +		#size-cells = <1>;
> clock-output-names and fixed-clock doesn't belong on this node.

Yes, clock-output-names and fixed-clock should be present in sysclk node.


>
>> +
>> +		sysclk: sysclk {
>> +			#clock-cells = <0>;
>> +			compatible = "fsl,qoriq-sysclk-2.0";
>> +			clock-output-names = "sysclk";
>> +		};
>> +
>> +
>> +		pll0: pll0@800 {
>> +			#clock-cells = <1>;
>> +			reg = <0x800 4>;
>> +			compatible = "fsl,qoriq-core-pll-2.0";
>> +			clocks = <&clockgen>;
>> +			clock-output-names = "pll0", "pll0-div2", "pll0-div4";
>> +		};
>> +
>> +		pll1: pll1@820 {
>> +			#clock-cells = <1>;
>> +			reg = <0x820 4>;
>> +			compatible = "fsl,qoriq-core-pll-2.0";
>> +			clocks = <&clockgen>;
>> +			clock-output-names = "pll1", "pll1-div2", "pll1-div4";
>> +		};
> clocks should point to sysclk.

got it!!

>
>> +	display@180000 {
>> +		compatible = "fsl,t1040-diu", "fsl,diu";
>> +		reg = <0x180000 1000>;
>> +		interrupts = <74 2 0 0>;
>> +	};
>> +
>> +/include/ "qoriq-sata2-0.dtsi"
>> +sata@220000 {
>> +			fsl,iommu-parent = <&pamu0>;
>> +			fsl,liodn-reg = <&guts 0x550>; /* SATA1LIODNR */
>> +};
>> +/include/ "qoriq-sata2-1.dtsi"
>> +sata@221000 {
>> +			fsl,iommu-parent = <&pamu0>;
>> +			fsl,liodn-reg = <&guts 0x554>; /* SATA2LIODNR */
>> +};
>> +/include/ "qoriq-sec5.0-0.dtsi"
>> +};
> Whitespace
>
>
i did not find this whitespace :(


Regards,
Prabhakar

^ permalink raw reply

* Re: [PATCH 1/2] fixup: "powerpc/perf: Add support for the hv 24x7 interface"
From: Benjamin Herrenschmidt @ 2014-04-06 23:48 UTC (permalink / raw)
  To: Cody P Schafer
  Cc: cody+local, LKML, Michael Ellerman, David.Laight, Anton Blanchard,
	Linux PPC
In-Reply-To: <1396476654-20623-2-git-send-email-cody@linux.vnet.ibm.com>

On Wed, 2014-04-02 at 15:10 -0700, Cody P Schafer wrote:
> Make the "not enabled" message less awful.
> 
> Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
> ---
>  arch/powerpc/perf/hv-24x7.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
> index 297c9105..3246ea2 100644
> --- a/arch/powerpc/perf/hv-24x7.c
> +++ b/arch/powerpc/perf/hv-24x7.c
> @@ -491,7 +491,7 @@ static int hv_24x7_init(void)
>  
>  	hret = hv_perf_caps_get(&caps);
>  	if (hret) {
> -		pr_info("could not obtain capabilities, error 0x%80lx, not enabling\n",
> +		pr_info("could not obtain capabilities, not enabling (%ld)\n",
>  				hret);

That's not much less awful... The message gives 0 informations about
where it comes from .. what is it that couldn't obtain capabilities ?

Also, sentences start with a Capital.

Cheers,
Ben.
 

^ permalink raw reply

* Re: [PATCH] powerpc/mm: numa pte should be handled via slow path in get_user_pages_fast
From: Benjamin Herrenschmidt @ 2014-04-06 23:49 UTC (permalink / raw)
  To: Aneesh Kumar K.V; +Cc: linuxppc-dev, paulus
In-Reply-To: <1396454859-5274-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com>

On Wed, 2014-04-02 at 21:37 +0530, Aneesh Kumar K.V wrote:
> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
> 
> We need to handle numa pte via the slow path

Is this -stable material ? If yes how far back ?

Cheers,
Ben.

> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
>  arch/powerpc/mm/gup.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/arch/powerpc/mm/gup.c b/arch/powerpc/mm/gup.c
> index c5f734e20b0f..d8746684f606 100644
> --- a/arch/powerpc/mm/gup.c
> +++ b/arch/powerpc/mm/gup.c
> @@ -36,6 +36,11 @@ static noinline int gup_pte_range(pmd_t pmd, unsigned long addr,
>  	do {
>  		pte_t pte = ACCESS_ONCE(*ptep);
>  		struct page *page;
> +		/*
> +		 * Similar to the PMD case, NUMA hinting must take slow path
> +		 */
> +		if (pte_numa(pte))
> +			return 0;
>  
>  		if ((pte_val(pte) & mask) != result)
>  			return 0;
> @@ -75,6 +80,14 @@ static int gup_pmd_range(pud_t pud, unsigned long addr, unsigned long end,
>  		if (pmd_none(pmd) || pmd_trans_splitting(pmd))
>  			return 0;
>  		if (pmd_huge(pmd) || pmd_large(pmd)) {
> +			/*
> +			 * NUMA hinting faults need to be handled in the GUP
> +			 * slowpath for accounting purposes and so that they
> +			 * can be serialised against THP migration.
> +			 */
> +			if (pmd_numa(pmd))
> +				return 0;
> +
>  			if (!gup_hugepte((pte_t *)pmdp, PMD_SIZE, addr, next,
>  					 write, pages, nr))
>  				return 0;

^ permalink raw reply

* Re: [PATCH] powerpc/le: Avoid creatng R_PPC64_TOCSAVE relocations for modules.
From: Anton Blanchard @ 2014-04-06 23:55 UTC (permalink / raw)
  To: Tony Breeds; +Cc: Rusty Russell, LinuxPPC-dev, Alan Modra
In-Reply-To: <20140312041201.GA13555@thor.bakeyournoodle.com>


Hi Tony,

> When building modules with a native le toolchain the linker will
> generate R_PPC64_TOCSAVE relocations when it's safe to omit saving r2
> on a plt call.  This isn't helpful in the conext of a kernel module
> and the kernel will fail to load those modules with an error like:
> 	nf_conntrack: Unknown ADD relocation: 109
> 
> This patch tells the linker to avoid createing R_PPC64_TOCSAVE
> relocations allowing modules to load.

Looks good.

Signed-off-by: Anton Blanchard <anton@samba.org>

> +ifeq ($(call cc-option-yn,-mno-save-toc-indirect),y)
> +       KBUILD_CFLAGS_MODULE += -mno-save-toc-indirect
> +endif

Low priority bikeshedding, but could we save ourselves a few lines with:

KBUILD_CFLAGS_MODULE += $(call cc-option,-mno-save-toc-indirect)

Anton

^ permalink raw reply

* Re: [PATCH 09/20] of/fdt: create common debugfs
From: Rob Herring @ 2014-04-07  0:42 UTC (permalink / raw)
  To: Michal Simek
  Cc: Grant Likely, linuxppc-dev, Paul Mackerras,
	linux-kernel@vger.kernel.org
In-Reply-To: <533EBD7D.6090403@monstr.eu>

On Fri, Apr 4, 2014 at 9:11 AM, Michal Simek <monstr@monstr.eu> wrote:
> On 04/04/2014 03:32 PM, Rob Herring wrote:
>> On Fri, Apr 4, 2014 at 8:22 AM, Michal Simek <monstr@monstr.eu> wrote:
>>> On 04/04/2014 03:00 PM, Rob Herring wrote:
>>>> On Fri, Apr 4, 2014 at 7:16 AM, Michal Simek <monstr@monstr.eu> wrote:
>>>>> On 04/04/2014 12:16 AM, Rob Herring wrote:
>>>>>> From: Rob Herring <robh@kernel.org>
>>>>>>
>>>>>> Both powerpc and microblaze have the same FDT blob in debugfs feature.
>>>>>> Move this to common location and remove the powerpc and microblaze
>>>>>> implementations. This feature could become more useful when FDT
>>>>>> overlay support is added.
>>>>
>>>> [snip]
>>
>>> Anyway I am testing it for microblaze and getting problem
>>> caused by this patch:
>>> commit 3d2ee8571ac0580d49c3f41fa28336289934900a
>>> Author: Rob Herring <robh@kernel.org>
>>> Date:   Wed Apr 2 15:10:14 2014 -0500
>>>
>>>     of/fdt: Convert FDT functions to use libfdt
>>>
>>> And reason is that in unflatten_dt_node()
>>>
>>> pathp = fdt_get_name(blob, *poffset, &l);
>>>
>>> is returning NULL
>>> and here
>>>         /* version 0x10 has a more compact unit name here instead of the full
>>>          * path. we accumulate the full path size using "fpsize", we'll rebuild
>>>          * it later. We detect this because the first character of the name is
>>>          * not '/'.
>>>          */
>>>         if ((*pathp) != '/') {
>>>
>>> code is trying to read it which is causing this kernel bug:
>>> Oops: kernel access of bad area, sig: 11
>>>
>>> It means fdt_next_node(is doing something wrong)
>>>
>>> Any easy way how to debug it?
>>
>> I didn't think fdt_get_path should fail. Can you add a print of
>> *poffset and pathp values.

I think I've fixed this now and updated the branch. Please test again
when you have a chance.

Thanks,
Rob

^ permalink raw reply

* Re: [PATCH 1/2] fixup: "powerpc/perf: Add support for the hv 24x7 interface"
From: Anton Blanchard @ 2014-04-07  1:22 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Cody P Schafer, cody+local, LKML, Michael Ellerman, David.Laight,
	Linux PPC
In-Reply-To: <1396828110.3671.21.camel@pasglop>

Hi

> That's not much less awful... The message gives 0 informations about
> where it comes from .. what is it that couldn't obtain capabilities ?

Cody is at least using pr_fmt to give some information on what went
wrong:

[    1.635040] hv-24x7: not a virtualized system, not enabling
[    1.635079] hv-gpci: not a virtualized system, not enabling

I think this is way too chatty though. If every driver printed a couple
of lines we'd end up with thousands of useless lines in the dmesg.

I still would prefer to see these as pr_debug lines.

Anton

^ permalink raw reply

* ISA kernel modules that build and oops on ppc64
From: Anton Blanchard @ 2014-04-07  1:54 UTC (permalink / raw)
  To: linuxppc-dev


Hi,

I built an allmodconfig kernel last Friday and tried inserting every module.
There were quite a few that oops because they probe ISA space.

Anton
--

Unable to handle kernel paging request for data at address 0xd0000800000001f2
NIP [d000000003f3ff98] probe_chip_type+0x244/0x11d4 [pata_legacy]

Unable to handle kernel paging request for data at address 0xd000080072656669
NIP [d000000004a23334] spk_serial_init+0x1c4/0xe98 [speakup]

Unable to handle kernel paging request for data at address 0xd000080000000201
NIP [d000000003f172a4] ns558_isa_probe+0x124/0x1034 [ns558]

Unable to handle kernel paging request for data at address 0xd000080000000266
NIP [d00000000495cbe0] mk712_init+0x14c/0x738 [mk712]

Unable to handle kernel paging request for data at address 0xd000080000000145
NIP [d000000008413a94] fdomain_is_valid_port+0xd8/0x910 [fdomain]

Unable to handle kernel paging request for data at address 0xd00008000000002e
NIP [d000000008e7ff9c] sch311x_detect+0x150/0xc70 [gpio_sch311x] 

Unable to handle kernel paging request for data at address 0xd00008000000002e
NIP [d00000000d8c4b14] detect_and_report_it87+0x148/0x1090 [parport_pc]

Unable to handle kernel paging request for data at address 0xd000080000000201
NIP [d000000005eb0f08] l4_add_card+0xec/0xa34 [lightning]

Unable to handle kernel paging request for data at address 0xd000080000000200
NIP [d00000000b69b410] com90xx_probe+0x490/0x2800 [com90xx]

Unable to handle kernel paging request for data at address 0xd00008000000002e
NIP [d0000000106b0298] wbsd_scan+0x184/0x7f8 [wbsd]

Unable to handle kernel paging request for data at address 0xd000080000000370
NIP [d000000010ff62d4] w83977af_probe+0x154/0x1fc0 [w83977af_ir]

Unable to handle kernel paging request for data at address 0xd000080000000150
NIP [d0000000112351a4] nsc_ircc_init+0x1d4/0x6d4 [nsc_ircc]

Unable to handle kernel paging request for data at address 0xd0000800000003f0
NIP [d000000011d0df54] smsc_ircc_probe+0x120/0x940 [smsc_ircc2]

Unable to handle kernel paging request for data at address 0xd0000800000003f0
NIP [d000000011b2112c] ali_ircc_init+0x264/0xe44 [ali_ircc]

^ permalink raw reply

* Re: [PATCH V2 1/2] mm: move FAULT_AROUND_ORDER to arch/
From: Benjamin Herrenschmidt @ 2014-04-07  5:45 UTC (permalink / raw)
  To: Dave Hansen
  Cc: linux-arch, rusty, Madhavan Srinivasan, riel, x86, linux-kernel,
	linux-mm, peterz, ak, paulus, mgorman, akpm, linuxppc-dev, mingo,
	kirill.shutemov
In-Reply-To: <533EDB63.8090909@intel.com>

On Fri, 2014-04-04 at 09:18 -0700, Dave Hansen wrote:
> On 04/03/2014 11:27 PM, Madhavan Srinivasan wrote:
> > This patch creates infrastructure to move the FAULT_AROUND_ORDER
> > to arch/ using Kconfig. This will enable architecture maintainers
> > to decide on suitable FAULT_AROUND_ORDER value based on
> > performance data for that architecture. Patch also adds
> > FAULT_AROUND_ORDER Kconfig element in arch/X86.
> 
> Please don't do it this way.
> 
> In mm/Kconfig, put
> 
> 	config FAULT_AROUND_ORDER
> 		int
> 		default 1234 if POWERPC
> 		default 4
> 
> The way you have it now, every single architecture that needs to enable
> this has to go put that in their Kconfig.  That's madness.  This way,
> you only put it in one place, and folks only have to care if they want
> to change the default to be something other than 4.

Also does it have to be a constant ? Maddy here tested on our POWER
servers. The "Sweet spot" value might be VERY different on an embedded
chip or even on a future generation of server chip.

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH 09/20] of/fdt: create common debugfs
From: Michal Simek @ 2014-04-07  7:04 UTC (permalink / raw)
  To: Rob Herring
  Cc: Grant Likely, linuxppc-dev, Paul Mackerras,
	linux-kernel@vger.kernel.org
In-Reply-To: <CAL_JsqJ28ajOD7UNG6nH07ACjOji_=WrL-K8rAqsRpYL-X1K6Q@mail.gmail.com>

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

On 04/07/2014 02:42 AM, Rob Herring wrote:
> On Fri, Apr 4, 2014 at 9:11 AM, Michal Simek <monstr@monstr.eu> wrote:
>> On 04/04/2014 03:32 PM, Rob Herring wrote:
>>> On Fri, Apr 4, 2014 at 8:22 AM, Michal Simek <monstr@monstr.eu> wrote:
>>>> On 04/04/2014 03:00 PM, Rob Herring wrote:
>>>>> On Fri, Apr 4, 2014 at 7:16 AM, Michal Simek <monstr@monstr.eu> wrote:
>>>>>> On 04/04/2014 12:16 AM, Rob Herring wrote:
>>>>>>> From: Rob Herring <robh@kernel.org>
>>>>>>>
>>>>>>> Both powerpc and microblaze have the same FDT blob in debugfs feature.
>>>>>>> Move this to common location and remove the powerpc and microblaze
>>>>>>> implementations. This feature could become more useful when FDT
>>>>>>> overlay support is added.
>>>>>
>>>>> [snip]
>>>
>>>> Anyway I am testing it for microblaze and getting problem
>>>> caused by this patch:
>>>> commit 3d2ee8571ac0580d49c3f41fa28336289934900a
>>>> Author: Rob Herring <robh@kernel.org>
>>>> Date:   Wed Apr 2 15:10:14 2014 -0500
>>>>
>>>>     of/fdt: Convert FDT functions to use libfdt
>>>>
>>>> And reason is that in unflatten_dt_node()
>>>>
>>>> pathp = fdt_get_name(blob, *poffset, &l);
>>>>
>>>> is returning NULL
>>>> and here
>>>>         /* version 0x10 has a more compact unit name here instead of the full
>>>>          * path. we accumulate the full path size using "fpsize", we'll rebuild
>>>>          * it later. We detect this because the first character of the name is
>>>>          * not '/'.
>>>>          */
>>>>         if ((*pathp) != '/') {
>>>>
>>>> code is trying to read it which is causing this kernel bug:
>>>> Oops: kernel access of bad area, sig: 11
>>>>
>>>> It means fdt_next_node(is doing something wrong)
>>>>
>>>> Any easy way how to debug it?
>>>
>>> I didn't think fdt_get_path should fail. Can you add a print of
>>> *poffset and pathp values.
> 
> I think I've fixed this now and updated the branch. Please test again
> when you have a chance.

yep. The updated branch works for Microblaze.

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

^ permalink raw reply

* [PATCH v2] powerpc 32: Provides VIRT_CPU_ACCOUNTING
From: Christophe Leroy @ 2014-04-07  7:31 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, scottwood
  Cc: linuxppc-dev, linux-kernel

This patch provides VIRT_CPU_ACCOUTING to PPC32 architecture.
Unlike PPC64, PPC32 doesn't use the PACA convention. Therefore the
implementation is taken from the IA64 architecture.
It is based on additional information added to the Task Info structure.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>

Index: b/arch/powerpc/Kconfig
===================================================================
--- a/arch/powerpc/Kconfig	(revision 5607)
+++ b/arch/powerpc/Kconfig	(revision 5611)
@@ -138,6 +138,7 @@
 	select OLD_SIGSUSPEND
 	select OLD_SIGACTION if PPC32
 	select HAVE_DEBUG_STACKOVERFLOW
+	select HAVE_VIRT_CPU_ACCOUNTING
 
 config EARLY_PRINTK
 	bool
Index: a/arch/powerpc/kernel/time.c
===================================================================
--- a/arch/powerpc/kernel/time.c	(revision 5607)
+++ b/arch/powerpc/kernel/time.c	(revision 5611)
@@ -162,7 +162,9 @@
 
 cputime_t cputime_one_jiffy;
 
+#ifdef CONFIG_PPC_SPLPAR
 void (*dtl_consumer)(struct dtl_entry *, u64);
+#endif
 
 static void calc_cputime_factors(void)
 {
@@ -178,6 +180,7 @@
 	__cputime_clockt_factor = res.result_low;
 }
 
+#ifdef CONFIG_PPC64
 /*
  * Read the SPURR on systems that have it, otherwise the PURR,
  * or if that doesn't exist return the timebase value passed in.
@@ -190,6 +193,7 @@
 		return mfspr(SPRN_PURR);
 	return tb;
 }
+#endif
 
 #ifdef CONFIG_PPC_SPLPAR
 
@@ -291,6 +295,7 @@
  * Account time for a transition between system, hard irq
  * or soft irq state.
  */
+#ifdef CONFIG_PPC64
 static u64 vtime_delta(struct task_struct *tsk,
 			u64 *sys_scaled, u64 *stolen)
 {
@@ -377,7 +382,70 @@
 	get_paca()->utime_sspurr = 0;
 	account_user_time(tsk, utime, utimescaled);
 }
+#else
 
+void vtime_account_user(struct task_struct *tsk)
+{
+	cputime_t delta_utime;
+	struct thread_info *ti = task_thread_info(tsk);
+
+	if (ti->ac_utime) {
+		delta_utime = ti->ac_utime;
+		account_user_time(tsk, delta_utime, delta_utime);
+		ti->ac_utime = 0;
+	}
+}
+
+/*
+ * Called from the context switch with interrupts disabled, to charge all
+ * accumulated times to the current process, and to prepare accounting on
+ * the next process.
+ */
+void arch_vtime_task_switch(struct task_struct *prev)
+{
+	struct thread_info *pi = task_thread_info(prev);
+	struct thread_info *ni = task_thread_info(current);
+
+	ni->ac_stamp = pi->ac_stamp;
+	ni->ac_stime = ni->ac_utime = 0;
+}
+
+/*
+ * Account time for a transition between system, hard irq or soft irq state.
+ * Note that this function is called with interrupts enabled.
+ */
+static cputime_t vtime_delta(struct task_struct *tsk)
+{
+	struct thread_info *ti = task_thread_info(tsk);
+	__u32 delta_stime;
+	__u32 now;
+
+	WARN_ON_ONCE(!irqs_disabled());
+
+	now = mftbl();
+
+	delta_stime = ti->ac_stime + (now - ti->ac_stamp);
+	ti->ac_stime = 0;
+	ti->ac_stamp = now;
+
+	return (cputime_t)delta_stime;
+}
+
+void vtime_account_system(struct task_struct *tsk)
+{
+	cputime_t delta = vtime_delta(tsk);
+
+	account_system_time(tsk, 0, delta, delta);
+}
+EXPORT_SYMBOL_GPL(vtime_account_system);
+
+void vtime_account_idle(struct task_struct *tsk)
+{
+	account_idle_time(vtime_delta(tsk));
+}
+
+#endif
+
 #else /* ! CONFIG_VIRT_CPU_ACCOUNTING_NATIVE */
 #define calc_cputime_factors()
 #endif
@@ -871,6 +939,8 @@
 		       ppc_proc_freq / 1000000, ppc_proc_freq % 1000000);
 	}
 
+	mttbl(0);
+	mttbu(0);
 	tb_ticks_per_jiffy = ppc_tb_freq / HZ;
 	tb_ticks_per_sec = ppc_tb_freq;
 	tb_ticks_per_usec = ppc_tb_freq / 1000000;
Index: b/arch/powerpc/kernel/entry_32.S
===================================================================
--- a/arch/powerpc/kernel/entry_32.S	(revision 5607)
+++ b/arch/powerpc/kernel/entry_32.S	(revision 5611)
@@ -177,6 +177,12 @@
 	addi	r12,r12,-1
 	stw	r12,4(r11)
 #endif
+#ifdef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE
+	CURRENT_THREAD_INFO(r9, r1)
+	tophys(r9, r9)
+	ACCOUNT_CPU_USER_ENTRY(r9, r11, r12)
+#endif
+
 	b	3f
 
 2:	/* if from kernel, check interrupted DOZE/NAP mode and
@@ -406,6 +412,13 @@
 	lwarx	r7,0,r1
 END_FTR_SECTION_IFSET(CPU_FTR_NEED_PAIRED_STWCX)
 	stwcx.	r0,0,r1			/* to clear the reservation */
+#ifdef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE
+	andi.	r4,r8,MSR_PR
+	beq	3f
+	CURRENT_THREAD_INFO(r4, r1)
+	ACCOUNT_CPU_USER_EXIT(r4, r5, r7)
+3:
+#endif
 	lwz	r4,_LINK(r1)
 	lwz	r5,_CCR(r1)
 	mtlr	r4
@@ -841,6 +854,10 @@
 	andis.	r10,r0,DBCR0_IDM@h
 	bnel-	load_dbcr0
 #endif
+#ifdef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE
+	CURRENT_THREAD_INFO(r9, r1)
+	ACCOUNT_CPU_USER_EXIT(r9, r10, r11)
+#endif
 
 	b	restore
 
Index: b/arch/powerpc/kernel/asm-offsets.c
===================================================================
--- a/arch/powerpc/kernel/asm-offsets.c	(revision 5607)
+++ b/arch/powerpc/kernel/asm-offsets.c	(revision 5611)
@@ -167,6 +167,12 @@
 	DEFINE(TI_PREEMPT, offsetof(struct thread_info, preempt_count));
 	DEFINE(TI_TASK, offsetof(struct thread_info, task));
 	DEFINE(TI_CPU, offsetof(struct thread_info, cpu));
+#if defined(CONFIG_VIRT_CPU_ACCOUNTING_NATIVE) && defined(CONFIG_PPC32)
+	DEFINE(TI_AC_STAMP, offsetof(struct thread_info, ac_stamp));
+	DEFINE(TI_AC_LEAVE, offsetof(struct thread_info, ac_leave));
+	DEFINE(TI_AC_STIME, offsetof(struct thread_info, ac_stime));
+	DEFINE(TI_AC_UTIME, offsetof(struct thread_info, ac_utime));
+#endif
 
 #ifdef CONFIG_PPC64
 	DEFINE(DCACHEL1LINESIZE, offsetof(struct ppc64_caches, dline_size));
Index: b/arch/powerpc/include/asm/thread_info.h
===================================================================
--- a/arch/powerpc/include/asm/thread_info.h	(revision 5607)
+++ b/arch/powerpc/include/asm/thread_info.h	(revision 5611)
@@ -43,6 +43,12 @@
 	int		cpu;			/* cpu we're on */
 	int		preempt_count;		/* 0 => preemptable,
 						   <0 => BUG */
+#if defined(CONFIG_VIRT_CPU_ACCOUNTING_NATIVE) && defined(CONFIG_PPC32)
+	u32 ac_stamp;
+	u32 ac_leave;
+	u32 ac_stime;
+	u32 ac_utime;
+#endif
 	struct restart_block restart_block;
 	unsigned long	local_flags;		/* private flags for thread */
 
Index: b/arch/powerpc/include/asm/cputime.h
===================================================================
--- a/arch/powerpc/include/asm/cputime.h	(revision 5607)
+++ b/arch/powerpc/include/asm/cputime.h	(revision 5611)
@@ -228,7 +228,11 @@
 
 #define cputime64_to_clock_t(ct)	cputime_to_clock_t((cputime_t)(ct))
 
+#ifdef CONFIG_PPC64
 static inline void arch_vtime_task_switch(struct task_struct *tsk) { }
+#else
+extern void arch_vtime_task_switch(struct task_struct *tsk);
+#endif
 
 #endif /* __KERNEL__ */
 #endif /* CONFIG_VIRT_CPU_ACCOUNTING_NATIVE */
Index: b/arch/powerpc/include/asm/ppc_asm.h
===================================================================
--- a/arch/powerpc/include/asm/ppc_asm.h	(revision 5607)
+++ b/arch/powerpc/include/asm/ppc_asm.h	(revision 5611)
@@ -25,10 +25,16 @@
  */
 
 #ifndef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE
+#ifdef CONFIG_PPC64
 #define ACCOUNT_CPU_USER_ENTRY(ra, rb)
 #define ACCOUNT_CPU_USER_EXIT(ra, rb)
+#else /* CONFIG_PPC64 */
+#define ACCOUNT_CPU_USER_ENTRY(ti, ra, rb)
+#define ACCOUNT_CPU_USER_EXIT(ti, ra, rb)
+#endif /* CONFIG_PPC64 */
 #define ACCOUNT_STOLEN_TIME
-#else
+#else /* CONFIG_VIRT_CPU_ACCOUNTING_NATIVE */
+#ifdef CONFIG_PPC64
 #define ACCOUNT_CPU_USER_ENTRY(ra, rb)					\
 	MFTB(ra);			/* get timebase */		\
 	ld	rb,PACA_STARTTIME_USER(r13);				\
@@ -68,7 +74,27 @@
 #define ACCOUNT_STOLEN_TIME
 
 #endif /* CONFIG_PPC_SPLPAR */
+#else /* CONFIG_PPC64 */
+#define ACCOUNT_CPU_USER_ENTRY(ti, ra, rb)				\
+	MFTB(ra);							\
+	lwz rb, TI_AC_LEAVE(ti);					\
+	stw ra, TI_AC_STAMP(ti);	/* AC_STAMP = NOW */		\
+	subf rb, rb, ra;		/* R = NOW - AC_LEAVE */	\
+	lwz ra, TI_AC_UTIME(ti);					\
+	add ra, rb, ra;			/* AC_UTIME += R */		\
+	stw ra, TI_AC_UTIME(ti);					\
 
+#define ACCOUNT_CPU_USER_EXIT(ti, ra, rb)				\
+	MFTB(ra);							\
+	lwz rb, TI_AC_STAMP(ti);					\
+	stw ra, TI_AC_LEAVE(ti);					\
+	subf rb, rb, ra;		/* R = NOW - AC_STAMP */	\
+	lwz ra, TI_AC_STIME(ti);					\
+	add ra, rb, ra;			/* AC_STIME += R */		\
+	stw ra, TI_AC_STIME(ti);					\
+
+#endif /* CONFIG_PPC64 */
+
 #endif /* CONFIG_VIRT_CPU_ACCOUNTING_NATIVE */
 
 /*
Index: b/arch/powerpc/platforms/Kconfig.cputype
===================================================================
--- a/arch/powerpc/platforms/Kconfig.cputype	(revision 5607)
+++ b/arch/powerpc/platforms/Kconfig.cputype	(revision 5611)
@@ -1,7 +1,6 @@
 config PPC64
 	bool "64-bit kernel"
 	default n
-	select HAVE_VIRT_CPU_ACCOUNTING
 	help
 	  This option selects whether a 32-bit or a 64-bit kernel
 	  will be built.

^ permalink raw reply

* Re: [PATCH] powerpc/mm: numa pte should be handled via slow path in get_user_pages_fast
From: Aneesh Kumar K.V @ 2014-04-07  7:38 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, paulus
In-Reply-To: <1396828173.3671.22.camel@pasglop>

Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:

> On Wed, 2014-04-02 at 21:37 +0530, Aneesh Kumar K.V wrote:
>> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>> 
>> We need to handle numa pte via the slow path
>
> Is this -stable material ? If yes how far back ?

I am not sure we really need to backport this. We got numa faulting bits
in 3.14. Currently there are two out-standing patches. One is this and
the other. http://mid.gmane.org/1390292129-15871-1-git-send-email-pingfank@linux.vnet.ibm.com

powernv: kvm: make _PAGE_NUMA take effect

I would not consider them critical enough to impact the general user of
3.14 kernel.

-aneesh

>
> Cheers,
> Ben.
>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>> ---
>>  arch/powerpc/mm/gup.c | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>> 
>> diff --git a/arch/powerpc/mm/gup.c b/arch/powerpc/mm/gup.c
>> index c5f734e20b0f..d8746684f606 100644
>> --- a/arch/powerpc/mm/gup.c
>> +++ b/arch/powerpc/mm/gup.c
>> @@ -36,6 +36,11 @@ static noinline int gup_pte_range(pmd_t pmd, unsigned long addr,
>>  	do {
>>  		pte_t pte = ACCESS_ONCE(*ptep);
>>  		struct page *page;
>> +		/*
>> +		 * Similar to the PMD case, NUMA hinting must take slow path
>> +		 */
>> +		if (pte_numa(pte))
>> +			return 0;
>>  
>>  		if ((pte_val(pte) & mask) != result)
>>  			return 0;
>> @@ -75,6 +80,14 @@ static int gup_pmd_range(pud_t pud, unsigned long addr, unsigned long end,
>>  		if (pmd_none(pmd) || pmd_trans_splitting(pmd))
>>  			return 0;
>>  		if (pmd_huge(pmd) || pmd_large(pmd)) {
>> +			/*
>> +			 * NUMA hinting faults need to be handled in the GUP
>> +			 * slowpath for accounting purposes and so that they
>> +			 * can be serialised against THP migration.
>> +			 */
>> +			if (pmd_numa(pmd))
>> +				return 0;
>> +
>>  			if (!gup_hugepte((pte_t *)pmdp, PMD_SIZE, addr, next,
>>  					 write, pages, nr))
>>  				return 0;

^ permalink raw reply

* Re: [PATCH v2] powernv: kvm: make _PAGE_NUMA take effect
From: Aneesh Kumar K.V @ 2014-04-07  7:42 UTC (permalink / raw)
  To: Alexander Graf, Liu ping fan, Alexander Graf
  Cc: Paul Mackerras, linuxppc-dev, kvm-devel, kvm-ppc
In-Reply-To: <533D47CD.906@suse.com>

Alexander Graf <agraf@suse.com> writes:

> On 03.04.14 04:36, Liu ping fan wrote:
>> Hi Alex, could you help to pick up this patch? since  v3.14 kernel can
>> enable numa fault for powerpc.
>
> What bad happens without this patch? We map a page even though it was 
> declared to get NUMA migrated? What happens next?

Nothing much, we won't be properly accounting the numa access in the
host.  What we want to achieve is to convert a guest access of the page to
a host fault so that we can do proper numa access accounting in the
host. This would enable us to migrate the page to the correct numa
node. 

>
> I'm trying to figure out whether I need to mark this with a stable tag 
> for 3.14.
>
>

-aneesh

^ permalink raw reply

* Re: [PATCH v2] powernv: kvm: make _PAGE_NUMA take effect
From: Alexander Graf @ 2014-04-07  8:36 UTC (permalink / raw)
  To: Aneesh Kumar K.V, Alexander Graf, Liu ping fan
  Cc: Paul Mackerras, linuxppc-dev, kvm-devel, kvm-ppc
In-Reply-To: <87bnwdshzu.fsf@linux.vnet.ibm.com>


On 07.04.14 09:42, Aneesh Kumar K.V wrote:
> Alexander Graf <agraf@suse.com> writes:
>
>> On 03.04.14 04:36, Liu ping fan wrote:
>>> Hi Alex, could you help to pick up this patch? since  v3.14 kernel can
>>> enable numa fault for powerpc.
>> What bad happens without this patch? We map a page even though it was
>> declared to get NUMA migrated? What happens next?
> Nothing much, we won't be properly accounting the numa access in the
> host.  What we want to achieve is to convert a guest access of the page to
> a host fault so that we can do proper numa access accounting in the
> host. This would enable us to migrate the page to the correct numa
> node.

Ok, so no breakages, just less performance. I wouldn't consider it 
stable material then :).


Alex

^ permalink raw reply

* Re: [PATCH] power, sched: stop updating inside arch_update_cpu_topology() when nothing to be update
From: Srivatsa S. Bhat @ 2014-04-07  9:51 UTC (permalink / raw)
  To: Michael wang
  Cc: sfr, Srikar Dronamraju, LKML, jlarrew, paulus, alistair, nfont,
	Andrew Morton, rcj, linuxppc-dev
In-Reply-To: <533E2BA2.6000107@linux.vnet.ibm.com>

Hi Michael,

On 04/04/2014 09:18 AM, Michael wang wrote:
> Hi, Srivatsa
> 
> Thanks for your reply :)
> 
> On 04/03/2014 04:50 PM, Srivatsa S. Bhat wrote:
> [snip]
>>
>> Now, the interesting thing to note here is that, if CPU0's node was already
>> set as node0, *nothing* should go wrong, since its just a redundant update.
>> However, if CPU0's original node mapping was something different, or if
>> node0 doesn't even exist in the machine, then the system can crash.
> 
> By printk I confirmed all cpus was belong to node 1 at very beginning,
> and things become magically after the wrong updating...
>

Ok, thanks!
 
>>
>> Have you verified that CPU0's node mapping is different from node 0?
>> That is, boot the kernel with "numa=debug" in the kernel command line and
>> it will print out the cpu-to-node associativity during boot. That way you
>> can figure out what was the original associativity that was set. This will
>> confirm the theory that the hypervisor sent a redundant update, but because
>> of the weird pre-allocation using kzalloc that we do inside
>> arch_update_cpu_topology(), we wrongly updated CPU0's mapping as CPU0 <-> Node0.
> 
> Associativity should changes, otherwise we won't continue the updating,
> and empty updates[] was confirmed to show up inside
> arch_update_cpu_topology().
> 

Ah, ok, that makes it very clear. So, I agree that your patch is correct,
but I think the comment in your patch can be enhanced a bit. I'll suggest
something if I manage to come up with a better wording.

> What I can't make sure is whether this is legal, notify changes but no
> changes happen sounds weird...however, even if it's legal, a check in
> here still make sense IMHO.
> 

That looks like a bug in the hypervisor/firmware. But the Linux kernel should
be able to handle such NULL updates without crashing. So yes, your patch makes
sense to me.

Thank you!

Regards,
Srivatsa S. Bhat

>>
>>> Thus we should stop the updating in such cases, this patch will achieve
>>> this and fix the issue.
>>>
>>> CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>>> CC: Paul Mackerras <paulus@samba.org>
>>> CC: Nathan Fontenot <nfont@linux.vnet.ibm.com>
>>> CC: Stephen Rothwell <sfr@canb.auug.org.au>
>>> CC: Andrew Morton <akpm@linux-foundation.org>
>>> CC: Robert Jennings <rcj@linux.vnet.ibm.com>
>>> CC: Jesse Larrew <jlarrew@linux.vnet.ibm.com>
>>> CC: "Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com>
>>> CC: Alistair Popple <alistair@popple.id.au>
>>> Signed-off-by: Michael Wang <wangyun@linux.vnet.ibm.com>
>>> ---
>>>  arch/powerpc/mm/numa.c |    9 +++++++++
>>>  1 file changed, 9 insertions(+)
>>>
>>> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
>>> index 30a42e2..6757690 100644
>>> --- a/arch/powerpc/mm/numa.c
>>> +++ b/arch/powerpc/mm/numa.c
>>> @@ -1591,6 +1591,14 @@ int arch_update_cpu_topology(void)
>>>  		cpu = cpu_last_thread_sibling(cpu);
>>>  	}
>>>
>>> +	/*
>>> +	 * The 'cpu_associativity_changes_mask' could be cleared if
>>> +	 * all the cpus it indicates won't change their node, in
>>> +	 * which case the 'updated_cpus' will be empty.
>>> +	 */
>>> +	if (!cpumask_weight(&updated_cpus))
>>> +		goto out;
>>> +
>>>  	stop_machine(update_cpu_topology, &updates[0], &updated_cpus);
>>>
>>>  	/*
>>> @@ -1612,6 +1620,7 @@ int arch_update_cpu_topology(void)
>>>  		changed = 1;
>>>  	}
>>>
>>> +out:
>>>  	kfree(updates);
>>>  	return changed;
>>>  }
>>>
>>

^ permalink raw reply

* Re: [PATCH] power, sched: stop updating inside arch_update_cpu_topology() when nothing to be update
From: Srivatsa S. Bhat @ 2014-04-07 10:15 UTC (permalink / raw)
  To: Michael wang
  Cc: sfr, LKML, paulus, alistair, nfont, Andrew Morton, rcj,
	linuxppc-dev
In-Reply-To: <533B8431.8090507@linux.vnet.ibm.com>

Hi Michael,

On 04/02/2014 08:59 AM, Michael wang wrote:
> During the testing, we encounter below WARN followed by Oops:
> 
> 	WARNING: at kernel/sched/core.c:6218
> 	...
> 	NIP [c000000000101660] .build_sched_domains+0x11d0/0x1200
> 	LR [c000000000101358] .build_sched_domains+0xec8/0x1200
> 	PACATMSCRATCH [800000000000f032]
> 	Call Trace:
> 	[c00000001b103850] [c000000000101358] .build_sched_domains+0xec8/0x1200
> 	[c00000001b1039a0] [c00000000010aad4] .partition_sched_domains+0x484/0x510
> 	[c00000001b103aa0] [c00000000016d0a8] .rebuild_sched_domains+0x68/0xa0
> 	[c00000001b103b30] [c00000000005cbf0] .topology_work_fn+0x10/0x30
> 	...
> 	Oops: Kernel access of bad area, sig: 11 [#1]
> 	...
> 	NIP [c00000000045c000] .__bitmap_weight+0x60/0xf0
> 	LR [c00000000010132c] .build_sched_domains+0xe9c/0x1200
> 	PACATMSCRATCH [8000000000029032]
> 	Call Trace:
> 	[c00000001b1037a0] [c000000000288ff4] .kmem_cache_alloc_node_trace+0x184/0x3a0
> 	[c00000001b103850] [c00000000010132c] .build_sched_domains+0xe9c/0x1200
> 	[c00000001b1039a0] [c00000000010aad4] .partition_sched_domains+0x484/0x510
> 	[c00000001b103aa0] [c00000000016d0a8] .rebuild_sched_domains+0x68/0xa0
> 	[c00000001b103b30] [c00000000005cbf0] .topology_work_fn+0x10/0x30
> 	...
> 
> This was caused by that 'sd->groups == NULL' after building groups, which
> was caused by the empty 'sd->span'.
> 
> The cpu's domain contain nothing because the cpu was assigned to wrong
> node inside arch_update_cpu_topology() by calling update_lookup_table()
> with the uninitialized param, in the case when there is nothing to be
> update.
> 

Can you reword the above paragraph to something like this:

The cpu's domain contained nothing because the cpu was assigned to a wrong
node, due to the following unfortunate sequence of events:

1. The hypervisor sent a topology update to the guest OS, to notify changes
   to the cpu-node mapping. However, the update was actually redundant - i.e.,
   the "new" mapping was exactly the same as the old one.

2. Due to this, the 'updated_cpus' mask turned out to be empty after exiting
   the 'for-loop' in arch_update_cpu_topology().

3. So we ended up calling stop-machine() with an empty cpumask list, which made
   stop-machine internally elect cpumask_first(cpu_online_mask), i.e., CPU0 as
   the cpu to run the payload (the update_cpu_topology() function).

4. This causes update_cpu_topology() to be run by CPU0. And since 'updates'
   is kzalloc()'ed inside arch_update_cpu_topology(), update_cpu_topology()
   finds update->cpu as well as update->new_nid to be 0. In other words, we
   end up assigning CPU0 (and eventually its siblings) to node 0, incorrectly.

This causes the sched-domain rebuild code to break and crash the system.


> Thus we should stop the updating in such cases, this patch will achieve
> this and fix the issue.
> 

We can reword this part as:

Fix this by skipping the topology update in cases where we find that
the topology has not actually changed in reality (ie., spurious updates).

> CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> CC: Paul Mackerras <paulus@samba.org>
> CC: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> CC: Stephen Rothwell <sfr@canb.auug.org.au>
> CC: Andrew Morton <akpm@linux-foundation.org>
> CC: Robert Jennings <rcj@linux.vnet.ibm.com>
> CC: Jesse Larrew <jlarrew@linux.vnet.ibm.com>
> CC: "Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com>
> CC: Alistair Popple <alistair@popple.id.au>
> Signed-off-by: Michael Wang <wangyun@linux.vnet.ibm.com>
> ---
>  arch/powerpc/mm/numa.c |    9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> index 30a42e2..6757690 100644
> --- a/arch/powerpc/mm/numa.c
> +++ b/arch/powerpc/mm/numa.c
> @@ -1591,6 +1591,14 @@ int arch_update_cpu_topology(void)
>  		cpu = cpu_last_thread_sibling(cpu);
>  	}
>  
> +	/*
> +	 * The 'cpu_associativity_changes_mask' could be cleared if
> +	 * all the cpus it indicates won't change their node, in
> +	 * which case the 'updated_cpus' will be empty.
> +	 */

How about rewording the comment like this:

In cases where we have nothing to update (because the updates list
is too short or because the new topology is same as the old one),
skip invoking update_cpu_topology() via stop-machine(). This is
necessary (and not just a fast-path optimization) because stop-machine
can end up electing a random CPU to run update_cpu_topology(), and
thus trick us into setting up incorrect cpu-node mappings (since
'updates' is kzalloc()'ed).

Regards,
Srivatsa S. Bhat

> +	if (!cpumask_weight(&updated_cpus))
> +		goto out;
> +
>  	stop_machine(update_cpu_topology, &updates[0], &updated_cpus);
>  
>  	/*
> @@ -1612,6 +1620,7 @@ int arch_update_cpu_topology(void)
>  		changed = 1;
>  	}
>  
> +out:
>  	kfree(updates);
>  	return changed;
>  }
> 

^ permalink raw reply

* Re: cscope: issue with symlinks in tools/testing/selftests/powerpc/copyloops/
From: Neil Horman @ 2014-04-07 10:42 UTC (permalink / raw)
  To: Yann Droneaud
  Cc: Hans-Bernhard Bröker, linux-kernel, Neil Horman,
	cscope-devel, Anton Blanchard, linuxppc-dev,
	Hans-Bernhard Broeker
In-Reply-To: <1396530975.4361.28.camel@localhost.localdomain>

On Thu, Apr 03, 2014 at 03:16:15PM +0200, Yann Droneaud wrote:
> Hi,
> 
> I'm using cscope to browse kernel sources, but I'm facing warnings from
> the tool since following commit:
> 
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=22d651dcef536c75f75537290bf3da5038e68b6b
> 
>     commit 22d651dcef536c75f75537290bf3da5038e68b6b
>     Author: Michael Ellerman <mpe@ellerman.id.au>
>     Date:   Tue Jan 21 15:22:17 2014 +1100
> 
>     selftests/powerpc: Import Anton's memcpy / copy_tofrom_user tests
>     
>     Turn Anton's memcpy / copy_tofrom_user test into something that can
>     live in tools/testing/selftests.
>     
>     It requires one turd in arch/powerpc/lib/memcpy_64.S, but it's 
>     pretty harmless IMHO.
>     
>     We are sailing very close to the wind with the feature macros. We 
>     define them to nothing, which currently means we get a few extra 
>     nops and include the unaligned calls.
>     
>     Signed-off-by: Anton Blanchard <anton@samba.org>
>     Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>     Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> 
> cscope reports error when generating the cross-reference database:
> 
>     $ make ALLSOURCE_ARCHS=all O=./obj-cscope/ cscope
>       GEN     cscope
>     cscope: cannot find
> file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/copyuser_power7.S
>     cscope: cannot find
> file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/memcpy_64.S
>     cscope: cannot find
> file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/memcpy_power7.S
>     cscope: cannot find
> file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/copyuser_64.S
> 
> And when calling cscope from ./obj-cscope/ directory, it reports errors
> too.
> 
> Hopefully it doesn't stop it from working, so I'm still able to use
> cscope to browse kernel sources.
> 
No, it won't stop it from working, it just won't search those files.  I don't
recall exactly the reason, but IIRC there was a big discussion long ago about
symlinks and our ability to support them (around version 1.94 I think).  We
decided to not handle symlinks, as they would either point outside our search
tree, which we didn't want to include, or would point to another file in the
search tree, which made loading them pointless (as we would cover the search in
the pointed file).

Neil

> It's a rather uncommon side effect of having (for the first time ?)
> sources files as symlinks: looking for symlinks in the kernel sources
> returns only:
> 
>     $ find . -type l
>     ./arch/mips/boot/dts/include/dt-bindings
>     ./arch/microblaze/boot/dts/system.dts
>     ./arch/powerpc/boot/dts/include/dt-bindings
>     ./arch/metag/boot/dts/include/dt-bindings
>     ./arch/arm/boot/dts/include/dt-bindings
>     ./tools/testing/selftests/powerpc/copyloops/copyuser_power7.S
>     ./tools/testing/selftests/powerpc/copyloops/memcpy_64.S
>     ./tools/testing/selftests/powerpc/copyloops/memcpy_power7.S
>     ./tools/testing/selftests/powerpc/copyloops/copyuser_64.S
>     ./obj-cscope/source
>     ./Documentation/DocBook/vidioc-g-sliced-vbi-cap.xml
>     ./Documentation/DocBook/vidioc-decoder-cmd.xml
> ...
>     ./Documentation/DocBook/media-func-ioctl.xml
>     ./Documentation/DocBook/vidioc-enumoutput.xml
> 
> 
> So one can wonder if having symlinked sources files is an expected
> supported feature for kbuild and all the various kernel
> tools/infrastructure ?
> 
> Regarding cscope specifically, it does not support symlink, and it's the
> expected behavior according to the bug reports I was able to find:
> 
> #214 cscope ignores symlinks to files 
> http://sourceforge.net/p/cscope/bugs/214/
> 
> #229 -I options doesn't handle symbolic link
> http://sourceforge.net/p/cscope/bugs/229/
> 
> #247 cscope: cannot find file 
> http://sourceforge.net/p/cscope/bugs/247/
> 
> #252 cscope: cannot find file *** 
> http://sourceforge.net/p/cscope/bugs/252/
> 
> #261 Regression - version 15.7a does not follow symbolic links 
> http://sourceforge.net/p/cscope/bugs/261/
> 
> 
> Regards.
> 
> -- 
> Yann Droneaud
> OPTEYA
> 
> 
> 

^ permalink raw reply

* Re: cscope: issue with symlinks in tools/testing/selftests/powerpc/copyloops/
From: Gerhard Sittig @ 2014-04-07 12:42 UTC (permalink / raw)
  To: Neil Horman
  Cc: Yann Droneaud, linux-kernel, Neil Horman, cscope-devel,
	Anton Blanchard, Hans-Bernhard Broeker, linuxppc-dev,
	Hans-Bernhard Bröker
In-Reply-To: <20140407104216.GB5287@hmsreliant.think-freely.org>

On Mon, 2014-04-07 at 06:42 -0400, Neil Horman wrote:
> 
> On Thu, Apr 03, 2014 at 03:16:15PM +0200, Yann Droneaud wrote:
> > 
> > [ ... ]
> > 
> > cscope reports error when generating the cross-reference database:
> > 
> >     $ make ALLSOURCE_ARCHS=all O=./obj-cscope/ cscope
> >       GEN     cscope
> >     cscope: cannot find
> > file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/copyuser_power7.S
> >     cscope: cannot find
> > file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/memcpy_64.S
> >     cscope: cannot find
> > file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/memcpy_power7.S
> >     cscope: cannot find
> > file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/copyuser_64.S
> > 
> > And when calling cscope from ./obj-cscope/ directory, it reports errors
> > too.
> > 
> > Hopefully it doesn't stop it from working, so I'm still able to use
> > cscope to browse kernel sources.
> > 
> No, it won't stop it from working, it just won't search those files.  I don't
> recall exactly the reason, but IIRC there was a big discussion long ago about
> symlinks and our ability to support them (around version 1.94 I think).  We
> decided to not handle symlinks, as they would either point outside our search
> tree, which we didn't want to include, or would point to another file in the
> search tree, which made loading them pointless (as we would cover the search in
> the pointed file).

So there are valid reasons to not process those filesystem
entries.  Would it be useful to not emit the warnings then?  Or
to silent those warnings when the user knows it's perfectly legal
to skip those filesytem entries?  Like what you can do with the
ctags(1) command and its --links option.


virtually yours
Gerhard Sittig
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office@denx.de

^ permalink raw reply

* Re: cscope: issue with symlinks in tools/testing/selftests/powerpc/copyloops/
From: Neil Horman @ 2014-04-07 15:36 UTC (permalink / raw)
  To: Gerhard Sittig
  Cc: Yann Droneaud, linux-kernel, Neil Horman, cscope-devel,
	Anton Blanchard, Hans-Bernhard Broeker, linuxppc-dev,
	Hans-Bernhard Bröker
In-Reply-To: <20140407124259.GZ11339@book.gsilab.sittig.org>

On Mon, Apr 07, 2014 at 02:42:59PM +0200, Gerhard Sittig wrote:
> On Mon, 2014-04-07 at 06:42 -0400, Neil Horman wrote:
> > 
> > On Thu, Apr 03, 2014 at 03:16:15PM +0200, Yann Droneaud wrote:
> > > 
> > > [ ... ]
> > > 
> > > cscope reports error when generating the cross-reference database:
> > > 
> > >     $ make ALLSOURCE_ARCHS=all O=./obj-cscope/ cscope
> > >       GEN     cscope
> > >     cscope: cannot find
> > > file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/copyuser_power7.S
> > >     cscope: cannot find
> > > file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/memcpy_64.S
> > >     cscope: cannot find
> > > file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/memcpy_power7.S
> > >     cscope: cannot find
> > > file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/copyuser_64.S
> > > 
> > > And when calling cscope from ./obj-cscope/ directory, it reports errors
> > > too.
> > > 
> > > Hopefully it doesn't stop it from working, so I'm still able to use
> > > cscope to browse kernel sources.
> > > 
> > No, it won't stop it from working, it just won't search those files.  I don't
> > recall exactly the reason, but IIRC there was a big discussion long ago about
> > symlinks and our ability to support them (around version 1.94 I think).  We
> > decided to not handle symlinks, as they would either point outside our search
> > tree, which we didn't want to include, or would point to another file in the
> > search tree, which made loading them pointless (as we would cover the search in
> > the pointed file).
> 
> So there are valid reasons to not process those filesystem
> entries.  Would it be useful to not emit the warnings then?  Or
> to silent those warnings when the user knows it's perfectly legal
> to skip those filesytem entries?  Like what you can do with the
> ctags(1) command and its --links option.
> 
I would see no problem with an option to do that.  I'd like to make it opt-in,
so that people who want to know about symlink issues will still see them, but
I'd be supportive of an option to quiet them
Neil

> 
> virtually yours
> Gerhard Sittig
> -- 
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office@denx.de
> 

^ permalink raw reply


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