LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 1/5] powerpc/spufs: simplify spufs core dumping
From: Jeremy Kerr @ 2020-04-28  2:51 UTC (permalink / raw)
  To: Al Viro, Christoph Hellwig
  Cc: Arnd Bergmann, Linus Torvalds, linux-kernel, Eric W . Biederman,
	linux-fsdevel, Andrew Morton, linuxppc-dev
In-Reply-To: <20200427204953.GY23230@ZenIV.linux.org.uk>

Hi Al & Christoph,

> Again, this really needs fixing.  Preferably - as a separate commit
> preceding this series, so that it could be
> backported.  simple_read_from_buffer() is a blocking operation.

I'll put together a patch that fixes this.

Christoph: I'll do it in a way that matches your changes to the _read
functions, so hopefully those hunks would just drop from your change,
leaving only the _dump additions. Would that work?

Cheers,


Jeremy


^ permalink raw reply

* [Bug 199471] [Bisected][Regression] windfarm_pm* no longer gets automatically loaded when CONFIG_I2C_POWERMAC=y is set
From: bugzilla-daemon @ 2020-04-28  3:14 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <bug-199471-206035@https.bugzilla.kernel.org/>

https://bugzilla.kernel.org/show_bug.cgi?id=199471

Michael Ellerman (michael@ellerman.id.au) changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |VERIFIED

--- Comment #26 from Michael Ellerman (michael@ellerman.id.au) ---
OK thanks all.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

^ permalink raw reply

* [Bug 199471] [Bisected][Regression] windfarm_pm* no longer gets automatically loaded when CONFIG_I2C_POWERMAC=y is set
From: bugzilla-daemon @ 2020-04-28  3:14 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <bug-199471-206035@https.bugzilla.kernel.org/>

https://bugzilla.kernel.org/show_bug.cgi?id=199471

Michael Ellerman (michael@ellerman.id.au) changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|VERIFIED                    |CLOSED

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

^ permalink raw reply

* [Bug 104871] bcl+8 in arch/powerpc/kernel/vdso64/datapage.S causes branch prediction issues
From: bugzilla-daemon @ 2020-04-28  3:17 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <bug-104871-206035@https.bugzilla.kernel.org/>

https://bugzilla.kernel.org/show_bug.cgi?id=104871

Michael Ellerman (michael@ellerman.id.au) changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |michael@ellerman.id.au
         Resolution|---                         |CODE_FIX

--- Comment #2 from Michael Ellerman (michael@ellerman.id.au) ---
Fixed in:

c974809a26a1 ("powerpc/vdso: Avoid link stack corruption in __get_datapage()")

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c974809a26a13e40254dbe3cf46f49aa32acca11

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

^ permalink raw reply

* [Bug 104871] bcl+8 in arch/powerpc/kernel/vdso64/datapage.S causes branch prediction issues
From: bugzilla-daemon @ 2020-04-28  3:17 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <bug-104871-206035@https.bugzilla.kernel.org/>

https://bugzilla.kernel.org/show_bug.cgi?id=104871

Michael Ellerman (michael@ellerman.id.au) changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

^ permalink raw reply

* Re: [PATCH v3 1/3] powerpc/rtas: Export rtas_error_rc
From: Michael Ellerman @ 2020-04-28  3:31 UTC (permalink / raw)
  To: Sam Bobroff, linuxppc-dev; +Cc: Nathan Lynch, Oliver O'Halloran
In-Reply-To: <e9c245df4a0b1cd1f68171c81e0d9e64a13ab0e9.1587704308.git.sbobroff@linux.ibm.com>

Sam Bobroff <sbobroff@linux.ibm.com> writes:
> Export rtas_error_rc() so that it can be used by other users of
> rtas_call() (which is already exported).
>
> Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
> ---
> v3 * New in this version.
>
>  arch/powerpc/include/asm/rtas.h | 1 +
>  arch/powerpc/kernel/rtas.c      | 3 ++-
>  2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
> index 3c1887351c71..7c9e4d3635cf 100644
> --- a/arch/powerpc/include/asm/rtas.h
> +++ b/arch/powerpc/include/asm/rtas.h
> @@ -379,6 +379,7 @@ extern int rtas_set_rtc_time(struct rtc_time *rtc_time);
>  
>  extern unsigned int rtas_busy_delay_time(int status);
>  extern unsigned int rtas_busy_delay(int status);
> +extern int rtas_error_rc(int rtas_rc);
>  
>  extern int early_init_dt_scan_rtas(unsigned long node,
>  		const char *uname, int depth, void *data);
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index c5fa251b8950..238bf112d29a 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -518,7 +518,7 @@ unsigned int rtas_busy_delay(int status)
>  }
>  EXPORT_SYMBOL(rtas_busy_delay);
>  
> -static int rtas_error_rc(int rtas_rc)
> +int rtas_error_rc(int rtas_rc)
>  {
>  	int rc;
>  
> @@ -546,6 +546,7 @@ static int rtas_error_rc(int rtas_rc)
>  	}
>  	return rc;
>  }
> +EXPORT_SYMBOL(rtas_error_rc);

Will it be used in a module somewhere?

AFAICS the only caller you add is built-in.

cheers

^ permalink raw reply

* [PATCH v4 0/2] powerpc/eeh: Release EEH device state synchronously
From: Sam Bobroff @ 2020-04-28  3:45 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nathan Lynch, Oliver O'Halloran

Hi everyone,

Here are some fixes and cleanups that have come from other work but that I
think stand on their own.

Only one patch ("Release EEH device state synchronously", suggested by Oliver
O'Halloran) is a significant change: it moves the cleanup of some EEH device
data out of the (possibly asynchronous) device release handler and into the
(synchronously called) bus notifier. This is useful for future work as it makes
it easier to reason about the lifetimes of EEH structures.

Note that I've left a few WARN_ON_ONCEs in the code because I'm paranoid, but I
have not been able to hit them during testing.

Cheers,
Sam.

Notes for v4:
Stopped using rtas_error_rc() as it is too specific, intead just translate the
one code that is valid for this RTAS call. Therefore, the new patch to export
rtas_error_rc() is dropped.

Notes for v3:
I've tweaked the fix for pseries_eeh_configure_bridge() to return the correct
error code (even though it's not used) by calling an already present RTAS
function, rtas_error_rc(). However, I had to make another change to export that
function and while it does seem like the right thing to do, but I'm concerned
it's a bit out of scope for such a small fix.

Notes for v2:

I've dropped both cleanup patches (3/4, 4/4) because that type of cleanup
(replacing a call to eeh_rmv_from_parent_pe() with one to eeh_remove_device())
is incorrect: if called during recovery, it will cause edev->pe to remain set
when it would have been cleared previously. This would lead to stale
information in the edev. I think there should be a way to simplify the code
around EEH_PE_KEEP but I'll look at that separately.

Patch set changelog follows:

Patch set v4: 
Patch 1/2 (was 2/3): powerpc/eeh: fix pseries_eeh_configure_bridge()
- Just handle the error translation locally, as it's specific to the RTAS call,
  but log the unaltered code in case it's useful for debugging.
Patch 2/2 (was 3/3): powerpc/eeh: Release EEH device state synchronously
Dropped (was 1/3) powerpc/rtas: Export rtas_error_rc

Patch set v3: 
Patch 1/3 (new in this version): powerpc/rtas: Export rtas_error_rc
Patch 2/3 (was 1/2): powerpc/eeh: fix pseries_eeh_configure_bridge()
Patch 3/3 (was 2/2): powerpc/eeh: Release EEH device state synchronously

Patch set v2: 
Patch 1/2: powerpc/eeh: fix pseries_eeh_configure_bridge()
Patch 2/2: powerpc/eeh: Release EEH device state synchronously
- Added comment explaining why the add case can't be handled similarly to the remove case.
Dropped (was 4/4) powerpc/eeh: Clean up edev cleanup for VFs
Dropped (was 3/4) powerpc/eeh: Remove workaround from eeh_add_device_late()

Patch set v1:
Patch 1/4: powerpc/eeh: fix pseries_eeh_configure_bridge()
Patch 2/4: powerpc/eeh: Release EEH device state synchronously
Patch 3/4: powerpc/eeh: Remove workaround from eeh_add_device_late()
Patch 4/4: powerpc/eeh: Clean up edev cleanup for VFs

Sam Bobroff (2):
  powerpc/eeh: fix pseries_eeh_configure_bridge()
  powerpc/eeh: Release EEH device state synchronously

 arch/powerpc/kernel/eeh.c                    | 31 ++++++++++++++++++++
 arch/powerpc/kernel/pci-hotplug.c            |  2 --
 arch/powerpc/platforms/pseries/eeh_pseries.c |  8 ++++-
 3 files changed, 38 insertions(+), 3 deletions(-)

-- 
2.22.0.216.g00a2a96fc9


^ permalink raw reply

* [PATCH v4 2/2] powerpc/eeh: Release EEH device state synchronously
From: Sam Bobroff @ 2020-04-28  3:45 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nathan Lynch, Oliver O'Halloran
In-Reply-To: <cover.1588045502.git.sbobroff@linux.ibm.com>

EEH device state is currently removed (by eeh_remove_device()) during
the device release handler, which is invoked as the device's reference
count drops to zero. This may take some time, or forever, as other
threads may hold references.

However, the PCI device state is released synchronously by
pci_stop_and_remove_bus_device(). This mismatch causes problems, for
example the device may be re-discovered as a new device before the
release handler has been called, leaving the PCI and EEH state
mismatched.

So instead, call eeh_remove_device() from the bus device removal
handlers, which are called synchronously in the removal path.

Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
 arch/powerpc/kernel/eeh.c         | 31 +++++++++++++++++++++++++++++++
 arch/powerpc/kernel/pci-hotplug.c |  2 --
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 17cb3e9b5697..64361311bc8e 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -1106,6 +1106,37 @@ static int eeh_init(void)
 
 core_initcall_sync(eeh_init);
 
+static int eeh_device_notifier(struct notifier_block *nb,
+			       unsigned long action, void *data)
+{
+	struct device *dev = data;
+
+	switch (action) {
+	/*
+	 * Note: It's not possible to perform EEH device addition (i.e.
+	 * {pseries,pnv}_pcibios_bus_add_device()) here because it depends on
+	 * the device's resources, which have not yet been set up.
+	 */
+	case BUS_NOTIFY_DEL_DEVICE:
+		eeh_remove_device(to_pci_dev(dev));
+		break;
+	default:
+		break;
+	}
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block eeh_device_nb = {
+	.notifier_call = eeh_device_notifier,
+};
+
+static __init int eeh_set_bus_notifier(void)
+{
+	bus_register_notifier(&pci_bus_type, &eeh_device_nb);
+	return 0;
+}
+arch_initcall(eeh_set_bus_notifier);
+
 /**
  * eeh_add_device_early - Enable EEH for the indicated device node
  * @pdn: PCI device node for which to set up EEH
diff --git a/arch/powerpc/kernel/pci-hotplug.c b/arch/powerpc/kernel/pci-hotplug.c
index d6a67f814983..28e9aa274f64 100644
--- a/arch/powerpc/kernel/pci-hotplug.c
+++ b/arch/powerpc/kernel/pci-hotplug.c
@@ -57,8 +57,6 @@ void pcibios_release_device(struct pci_dev *dev)
 	struct pci_controller *phb = pci_bus_to_host(dev->bus);
 	struct pci_dn *pdn = pci_get_pdn(dev);
 
-	eeh_remove_device(dev);
-
 	if (phb->controller_ops.release_device)
 		phb->controller_ops.release_device(dev);
 
-- 
2.22.0.216.g00a2a96fc9


^ permalink raw reply related

* [PATCH v4 1/2] powerpc/eeh: fix pseries_eeh_configure_bridge()
From: Sam Bobroff @ 2020-04-28  3:45 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nathan Lynch, Oliver O'Halloran
In-Reply-To: <cover.1588045502.git.sbobroff@linux.ibm.com>

If a device is hot unplgged during EEH recovery, it's possible for the
RTAS call to ibm,configure-pe in pseries_eeh_configure() to return
parameter error (-3), however negative return values are not checked
for and this leads to an infinite loop.

Fix this by correctly bailing out on negative values.

Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
v4 - Just handle the error translation locally, as it's specific to the RTAS call,
     but log the unaltered code in case it's useful for debugging.

 arch/powerpc/platforms/pseries/eeh_pseries.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
index 893ba3f562c4..04c1ed79bc6e 100644
--- a/arch/powerpc/platforms/pseries/eeh_pseries.c
+++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
@@ -607,6 +607,8 @@ static int pseries_eeh_configure_bridge(struct eeh_pe *pe)
 
 		if (!ret)
 			return ret;
+		if (ret < 0)
+			break;
 
 		/*
 		 * If RTAS returns a delay value that's above 100ms, cut it
@@ -627,7 +629,11 @@ static int pseries_eeh_configure_bridge(struct eeh_pe *pe)
 
 	pr_warn("%s: Unable to configure bridge PHB#%x-PE#%x (%d)\n",
 		__func__, pe->phb->global_number, pe->addr, ret);
-	return ret;
+	/* PAPR defines -3 as "Parameter Error" for this function: */
+	if (ret == -3)
+		return -EINVAL;
+	else
+		return -EIO;
 }
 
 /**
-- 
2.22.0.216.g00a2a96fc9


^ permalink raw reply related

* Re: [PATCH v3 2/4] hugetlbfs: move hugepagesz= parsing to arch independent code
From: Sandipan Das @ 2020-04-28  4:17 UTC (permalink / raw)
  To: Mike Kravetz
  Cc: linux-doc, Catalin Marinas, Dave Hansen, Heiko Carstens, Peter Xu,
	linux-mm, Paul Mackerras, sparclinux, linux-riscv, Will Deacon,
	Mina Almasry, linux-s390, Jonathan Corbet, Christian Borntraeger,
	Ingo Molnar, Longpeng, Albert Ou, Vasily Gorbik, Paul Walmsley,
	Thomas Gleixner, linux-arm-kernel, Nitesh Narayan Lal,
	Randy Dunlap, linux-kernel, Palmer Dabbelt, Andrew Morton,
	linuxppc-dev, David S.Miller
In-Reply-To: <b1f04f9f-fa46-c2a0-7693-4a0679d2a1ee@oracle.com>

Hi Mike,

On 28/04/20 12:39 am, Mike Kravetz wrote:
> On 4/27/20 10:25 AM, Mike Kravetz wrote:
>> On 4/26/20 10:04 PM, Sandipan Das wrote:
>>> On 18/04/20 12:20 am, Mike Kravetz wrote:
>>>> Now that architectures provide arch_hugetlb_valid_size(), parsing
>>>> of "hugepagesz=" can be done in architecture independent code.
>>>
>>> This isn't working as expected on powerpc64.
>>>
>>>   [    0.000000] Kernel command line: root=UUID=dc7b49cf-95a2-4996-8e7d-7c64ddc7a6ff hugepagesz=16G hugepages=2 
>>>   [    0.000000] HugeTLB: huge pages not supported, ignoring hugepagesz = 16G
>>>   [    0.000000] HugeTLB: huge pages not supported, ignoring hugepages = 2
>>>   [    0.284177] HugeTLB registered 16.0 MiB page size, pre-allocated 0 pages
>>>   [    0.284182] HugeTLB registered 16.0 GiB page size, pre-allocated 0 pages
>>>   [    2.585062]     hugepagesz=16G
>>>   [    2.585063]     hugepages=2
>>>
>>
>> In the new arch independent version of hugepages_setup, I added the following
>> code in patch 4 off this series:
>>
>>> +	if (!hugepages_supported()) {
>>> +		pr_warn("HugeTLB: huge pages not supported, ignoring hugepages = %s\n", s);
>>> +		return 0;
>>> +	}
>>> +
>>
>> The easy solution is to remove all the hugepages_supported() checks from
>> command line parsing routines and rely on the later check in hugetlb_init().
> 
> Here is a patch to address the issue.  Sorry, as my series breaks all hugetlb
> command line processing on powerpc.
> 
> Sandipan, can you test the following patch?
> 

The following patch does fix the issue. Thanks.

Tested-by: Sandipan Das <sandipan@linux.ibm.com>


> From 480fe2847361e2a85aeec1fb39fe643bb7100a07 Mon Sep 17 00:00:00 2001
> From: Mike Kravetz <mike.kravetz@oracle.com>
> Date: Mon, 27 Apr 2020 11:37:30 -0700
> Subject: [PATCH] hugetlbfs: fix changes to command line processing
> 
> Previously, a check for hugepages_supported was added before processing
> hugetlb command line parameters.  On some architectures such as powerpc,
> hugepages_supported() is not set to true until after command line
> processing.  Therefore, no hugetlb command line parameters would be
> accepted.
> 
> Remove the additional checks for hugepages_supported.  In hugetlb_init,
> print a warning if !hugepages_supported and command line parameters were
> specified.
> 
> Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
> ---
>  mm/hugetlb.c | 20 ++++----------------
>  1 file changed, 4 insertions(+), 16 deletions(-)
> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 1075abdb5717..5548e8851b93 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -3212,8 +3212,11 @@ static int __init hugetlb_init(void)
>  {
>  	int i;
>  
> -	if (!hugepages_supported())
> +	if (!hugepages_supported()) {
> +		if (hugetlb_max_hstate || default_hstate_max_huge_pages)
> +			pr_warn("HugeTLB: huge pages not supported, ignoring associated command-line parameters\n");
>  		return 0;
> +	}
>  
>  	/*
>  	 * Make sure HPAGE_SIZE (HUGETLB_PAGE_ORDER) hstate exists.  Some
> @@ -3315,11 +3318,6 @@ static int __init hugepages_setup(char *s)
>  	unsigned long *mhp;
>  	static unsigned long *last_mhp;
>  
> -	if (!hugepages_supported()) {
> -		pr_warn("HugeTLB: huge pages not supported, ignoring hugepages = %s\n", s);
> -		return 0;
> -	}
> -
>  	if (!parsed_valid_hugepagesz) {
>  		pr_warn("HugeTLB: hugepages=%s does not follow a valid hugepagesz, ignoring\n", s);
>  		parsed_valid_hugepagesz = true;
> @@ -3372,11 +3370,6 @@ static int __init hugepagesz_setup(char *s)
>  	struct hstate *h;
>  
>  	parsed_valid_hugepagesz = false;
> -	if (!hugepages_supported()) {
> -		pr_warn("HugeTLB: huge pages not supported, ignoring hugepagesz = %s\n", s);
> -		return 0;
> -	}
> -
>  	size = (unsigned long)memparse(s, NULL);
>  
>  	if (!arch_hugetlb_valid_size(size)) {
> @@ -3424,11 +3417,6 @@ static int __init default_hugepagesz_setup(char *s)
>  	unsigned long size;
>  
>  	parsed_valid_hugepagesz = false;
> -	if (!hugepages_supported()) {
> -		pr_warn("HugeTLB: huge pages not supported, ignoring default_hugepagesz = %s\n", s);
> -		return 0;
> -	}
> -
>  	if (parsed_default_hugepagesz) {
>  		pr_err("HugeTLB: default_hugepagesz previously specified, ignoring %s\n", s);
>  		return 0;
> 

^ permalink raw reply

* Re: [PATCH v6 03/28] powerpc/xmon: Move breakpoints to text section
From: Christophe Leroy @ 2020-04-28  5:19 UTC (permalink / raw)
  To: Jordan Niethe, linuxppc-dev; +Cc: alistair, npiggin, bala24, dja
In-Reply-To: <20200428015814.15380-4-jniethe5@gmail.com>



Le 28/04/2020 à 03:57, Jordan Niethe a écrit :
> The instructions for xmon's breakpoint are stored bpt_table[] which is in
> the data section. This is problematic as the data section may be marked
> as no execute. Move bpt_table[] to the text section.
> 
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> v6: - New to series. Was part of the previous patch.
>      - Make BPT_SIZE available in assembly
> ---
>   arch/powerpc/kernel/asm-offsets.c |  8 ++++++++
>   arch/powerpc/xmon/Makefile        |  2 +-
>   arch/powerpc/xmon/xmon.c          |  6 +-----
>   arch/powerpc/xmon/xmon_bpts.S     |  9 +++++++++
>   arch/powerpc/xmon/xmon_bpts.h     | 14 ++++++++++++++
>   5 files changed, 33 insertions(+), 6 deletions(-)
>   create mode 100644 arch/powerpc/xmon/xmon_bpts.S
>   create mode 100644 arch/powerpc/xmon/xmon_bpts.h
> 
> diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
> index c25e562f1cd9..2401f415f423 100644
> --- a/arch/powerpc/kernel/asm-offsets.c
> +++ b/arch/powerpc/kernel/asm-offsets.c
> @@ -70,6 +70,10 @@
>   #include <asm/fixmap.h>
>   #endif
>   
> +#ifdef CONFIG_XMON
> +#include "../xmon/xmon_bpts.h"
> +#endif
> +
>   #define STACK_PT_REGS_OFFSET(sym, val)	\
>   	DEFINE(sym, STACK_FRAME_OVERHEAD + offsetof(struct pt_regs, val))
>   
> @@ -783,5 +787,9 @@ int main(void)
>   	DEFINE(VIRT_IMMR_BASE, (u64)__fix_to_virt(FIX_IMMR_BASE));
>   #endif
>   
> +#ifdef CONFIG_XMON
> +	DEFINE(BPT_SIZE, BPT_SIZE);
> +#endif
> +
>   	return 0;
>   }
> diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
> index c3842dbeb1b7..515a13ea6f28 100644
> --- a/arch/powerpc/xmon/Makefile
> +++ b/arch/powerpc/xmon/Makefile
> @@ -21,7 +21,7 @@ endif
>   
>   ccflags-$(CONFIG_PPC64) := $(NO_MINIMAL_TOC)
>   
> -obj-y			+= xmon.o nonstdio.o spr_access.o
> +obj-y			+= xmon.o nonstdio.o spr_access.o xmon_bpts.o
>   
>   ifdef CONFIG_XMON_DISASSEMBLY
>   obj-y			+= ppc-dis.o ppc-opc.o
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index a064392df1b8..f7ce3ea8694c 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -62,6 +62,7 @@
>   
>   #include "nonstdio.h"
>   #include "dis-asm.h"
> +#include "xmon_bpts.h"
>   
>   #ifdef CONFIG_SMP
>   static cpumask_t cpus_in_xmon = CPU_MASK_NONE;
> @@ -108,7 +109,6 @@ struct bpt {
>   #define BP_TRAP		2
>   #define BP_DABR		4
>   
> -#define NBPTS	256
>   static struct bpt bpts[NBPTS];
>   static struct bpt dabr;
>   static struct bpt *iabr;
> @@ -116,10 +116,6 @@ static unsigned bpinstr = 0x7fe00008;	/* trap */
>   
>   #define BP_NUM(bp)	((bp) - bpts + 1)
>   
> -#define BPT_SIZE       (sizeof(unsigned int) * 2)
> -#define BPT_WORDS      (BPT_SIZE / sizeof(unsigned int))
> -static unsigned int bpt_table[NBPTS * BPT_WORDS];
> -
>   /* Prototypes */
>   static int cmds(struct pt_regs *);
>   static int mread(unsigned long, void *, int);
> diff --git a/arch/powerpc/xmon/xmon_bpts.S b/arch/powerpc/xmon/xmon_bpts.S
> new file mode 100644
> index 000000000000..f3ad0ab50854
> --- /dev/null
> +++ b/arch/powerpc/xmon/xmon_bpts.S
> @@ -0,0 +1,9 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#include <asm/ppc_asm.h>
> +#include <asm/asm-compat.h>
> +#include <asm/asm-offsets.h>
> +#include "xmon_bpts.h"
> +
> +.global bpt_table
> +bpt_table:
> +	.space NBPTS * BPT_SIZE

No alignment required ? Standard alignment (probably 4 bytes ?) is 
acceptable ?


> diff --git a/arch/powerpc/xmon/xmon_bpts.h b/arch/powerpc/xmon/xmon_bpts.h
> new file mode 100644
> index 000000000000..b7e94375db86
> --- /dev/null
> +++ b/arch/powerpc/xmon/xmon_bpts.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef XMON_BPTS_H
> +#define XMON_BPTS_H
> +
> +#define NBPTS	256
> +#ifndef __ASSEMBLY__
> +#define BPT_SIZE	(sizeof(unsigned int) * 2)
> +#define BPT_WORDS	(BPT_SIZE / sizeof(unsigned int))
> +
> +extern unsigned int bpt_table[NBPTS * BPT_WORDS];
> +
> +#endif /* __ASSEMBLY__ */
> +
> +#endif /* XMON_BPTS_H */
> 

Christophe

^ permalink raw reply

* Re: [PATCH v6 03/28] powerpc/xmon: Move breakpoints to text section
From: Jordan Niethe @ 2020-04-28  5:30 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Alistair Popple, Nicholas Piggin, Balamuruhan S, linuxppc-dev,
	Daniel Axtens
In-Reply-To: <6761670b-545c-0120-48e3-1cadf047a95f@c-s.fr>

On Tue, Apr 28, 2020 at 3:20 PM Christophe Leroy
<christophe.leroy@c-s.fr> wrote:
>
>
>
> Le 28/04/2020 à 03:57, Jordan Niethe a écrit :
> > The instructions for xmon's breakpoint are stored bpt_table[] which is in
> > the data section. This is problematic as the data section may be marked
> > as no execute. Move bpt_table[] to the text section.
> >
> > Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> > ---
> > v6: - New to series. Was part of the previous patch.
> >      - Make BPT_SIZE available in assembly
> > ---
> >   arch/powerpc/kernel/asm-offsets.c |  8 ++++++++
> >   arch/powerpc/xmon/Makefile        |  2 +-
> >   arch/powerpc/xmon/xmon.c          |  6 +-----
> >   arch/powerpc/xmon/xmon_bpts.S     |  9 +++++++++
> >   arch/powerpc/xmon/xmon_bpts.h     | 14 ++++++++++++++
> >   5 files changed, 33 insertions(+), 6 deletions(-)
> >   create mode 100644 arch/powerpc/xmon/xmon_bpts.S
> >   create mode 100644 arch/powerpc/xmon/xmon_bpts.h
> >
> > diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
> > index c25e562f1cd9..2401f415f423 100644
> > --- a/arch/powerpc/kernel/asm-offsets.c
> > +++ b/arch/powerpc/kernel/asm-offsets.c
> > @@ -70,6 +70,10 @@
> >   #include <asm/fixmap.h>
> >   #endif
> >
> > +#ifdef CONFIG_XMON
> > +#include "../xmon/xmon_bpts.h"
> > +#endif
> > +
> >   #define STACK_PT_REGS_OFFSET(sym, val)      \
> >       DEFINE(sym, STACK_FRAME_OVERHEAD + offsetof(struct pt_regs, val))
> >
> > @@ -783,5 +787,9 @@ int main(void)
> >       DEFINE(VIRT_IMMR_BASE, (u64)__fix_to_virt(FIX_IMMR_BASE));
> >   #endif
> >
> > +#ifdef CONFIG_XMON
> > +     DEFINE(BPT_SIZE, BPT_SIZE);
> > +#endif
> > +
> >       return 0;
> >   }
> > diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
> > index c3842dbeb1b7..515a13ea6f28 100644
> > --- a/arch/powerpc/xmon/Makefile
> > +++ b/arch/powerpc/xmon/Makefile
> > @@ -21,7 +21,7 @@ endif
> >
> >   ccflags-$(CONFIG_PPC64) := $(NO_MINIMAL_TOC)
> >
> > -obj-y                        += xmon.o nonstdio.o spr_access.o
> > +obj-y                        += xmon.o nonstdio.o spr_access.o xmon_bpts.o
> >
> >   ifdef CONFIG_XMON_DISASSEMBLY
> >   obj-y                       += ppc-dis.o ppc-opc.o
> > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> > index a064392df1b8..f7ce3ea8694c 100644
> > --- a/arch/powerpc/xmon/xmon.c
> > +++ b/arch/powerpc/xmon/xmon.c
> > @@ -62,6 +62,7 @@
> >
> >   #include "nonstdio.h"
> >   #include "dis-asm.h"
> > +#include "xmon_bpts.h"
> >
> >   #ifdef CONFIG_SMP
> >   static cpumask_t cpus_in_xmon = CPU_MASK_NONE;
> > @@ -108,7 +109,6 @@ struct bpt {
> >   #define BP_TRAP             2
> >   #define BP_DABR             4
> >
> > -#define NBPTS        256
> >   static struct bpt bpts[NBPTS];
> >   static struct bpt dabr;
> >   static struct bpt *iabr;
> > @@ -116,10 +116,6 @@ static unsigned bpinstr = 0x7fe00008;    /* trap */
> >
> >   #define BP_NUM(bp)  ((bp) - bpts + 1)
> >
> > -#define BPT_SIZE       (sizeof(unsigned int) * 2)
> > -#define BPT_WORDS      (BPT_SIZE / sizeof(unsigned int))
> > -static unsigned int bpt_table[NBPTS * BPT_WORDS];
> > -
> >   /* Prototypes */
> >   static int cmds(struct pt_regs *);
> >   static int mread(unsigned long, void *, int);
> > diff --git a/arch/powerpc/xmon/xmon_bpts.S b/arch/powerpc/xmon/xmon_bpts.S
> > new file mode 100644
> > index 000000000000..f3ad0ab50854
> > --- /dev/null
> > +++ b/arch/powerpc/xmon/xmon_bpts.S
> > @@ -0,0 +1,9 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#include <asm/ppc_asm.h>
> > +#include <asm/asm-compat.h>
> > +#include <asm/asm-offsets.h>
> > +#include "xmon_bpts.h"
> > +
> > +.global bpt_table
> > +bpt_table:
> > +     .space NBPTS * BPT_SIZE
>
> No alignment required ? Standard alignment (probably 4 bytes ?) is
> acceptable ?
No, I'll add a .balign    4 to be sure.
>
>
> > diff --git a/arch/powerpc/xmon/xmon_bpts.h b/arch/powerpc/xmon/xmon_bpts.h
> > new file mode 100644
> > index 000000000000..b7e94375db86
> > --- /dev/null
> > +++ b/arch/powerpc/xmon/xmon_bpts.h
> > @@ -0,0 +1,14 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef XMON_BPTS_H
> > +#define XMON_BPTS_H
> > +
> > +#define NBPTS        256
> > +#ifndef __ASSEMBLY__
> > +#define BPT_SIZE     (sizeof(unsigned int) * 2)
> > +#define BPT_WORDS    (BPT_SIZE / sizeof(unsigned int))
> > +
> > +extern unsigned int bpt_table[NBPTS * BPT_WORDS];
> > +
> > +#endif /* __ASSEMBLY__ */
> > +
> > +#endif /* XMON_BPTS_H */
> >
>
> Christophe

^ permalink raw reply

* Re: [PATCH v4] pci: Make return value of pcie_capability_read*() consistent
From: Yicong Yang @ 2020-04-28  2:19 UTC (permalink / raw)
  To: Bjorn Helgaas, Saheed Bolarinwa
  Cc: Thomas Bogendoerfer, linux-pci, skhan, linux-mips, linux-kernel,
	bjorn, linuxppc-dev
In-Reply-To: <20200427181304.GA214573@google.com>

On 2020/4/28 2:13, Bjorn Helgaas wrote:
>
> I'm starting to think we're approaching this backwards.  I searched
> for PCIBIOS_FUNC_NOT_SUPPORTED, PCIBIOS_BAD_VENDOR_ID, and the other
> error values.  Almost every use is a *return* in a config accessor.
> There are very, very few *tests* for these values.

If we have certain reasons to reserve PCI_BIOS* error to identify PCI errors
in PCI drivers, maybe redefine the PCI_BIOS* to generic error codes can solve
the issues, and no need to call pcibios_err_to_errno() to do the conversion.
Few changes may be made to current codes. One possible patch may
look like below. Otherwise, maybe convert all PCI_BIOS* errors to generic error
codes is a better idea.

Not sure it's the best way or not. Just FYI.


diff --git a/include/linux/pci.h b/include/linux/pci.h
index 83ce1cd..843987c 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -675,14 +675,18 @@ static inline bool pci_dev_msi_enabled(struct pci_dev *pci_dev) { return false;
 
 /* Error values that may be returned by PCI functions */
 #define PCIBIOS_SUCCESSFUL		0x00
-#define PCIBIOS_FUNC_NOT_SUPPORTED	0x81
-#define PCIBIOS_BAD_VENDOR_ID		0x83
-#define PCIBIOS_DEVICE_NOT_FOUND	0x86
-#define PCIBIOS_BAD_REGISTER_NUMBER	0x87
-#define PCIBIOS_SET_FAILED		0x88
-#define PCIBIOS_BUFFER_TOO_SMALL	0x89
-
-/* Translate above to generic errno for passing back through non-PCI code */
+#define PCIBIOS_FUNC_NOT_SUPPORTED	-ENOENT
+#define PCIBIOS_BAD_VENDOR_ID		-ENOTTY
+#define PCIBIOS_DEVICE_NOT_FOUND	-ENODEV
+#define PCIBIOS_BAD_REGISTER_NUMBER	-EFAULT
+#define PCIBIOS_SET_FAILED		-EIO
+#define PCIBIOS_BUFFER_TOO_SMALL	-ENOSPC
+
+/**
+ * Translate above to generic errno for passing back through non-PCI code
+ *
+ * Deprecated. Use the PCIBIOS_* directly without a translation.
+ */
 static inline int pcibios_err_to_errno(int err)
 {
 	if (err <= PCIBIOS_SUCCESSFUL)
@@ -690,17 +694,12 @@ static inline int pcibios_err_to_errno(int err)
 
 	switch (err) {
 	case PCIBIOS_FUNC_NOT_SUPPORTED:
-		return -ENOENT;
 	case PCIBIOS_BAD_VENDOR_ID:
-		return -ENOTTY;
 	case PCIBIOS_DEVICE_NOT_FOUND:
-		return -ENODEV;
 	case PCIBIOS_BAD_REGISTER_NUMBER:
-		return -EFAULT;
 	case PCIBIOS_SET_FAILED:
-		return -EIO;
 	case PCIBIOS_BUFFER_TOO_SMALL:
-		return -ENOSPC;
+		return err;
 	}
 
 	return -ERANGE;


>
> For example, the only tests for PCIBIOS_FUNC_NOT_SUPPORTED are in
> xen_pcibios_err_to_errno() and pcibios_err_to_errno(), i.e., we're
> just converting that value to -ENOENT or the Xen-specific thing.
>
> So I think the best approach might be to remove the PCIBIOS_* error
> values completely and replace them with the corresponding values from
> pcibios_err_to_errno().  For example, a part of the patch would look
> like this:
>
> diff --git a/arch/mips/pci/ops-emma2rh.c b/arch/mips/pci/ops-emma2rh.c
> index 65f47344536c..d4d9c902c147 100644
> --- a/arch/mips/pci/ops-emma2rh.c
> +++ b/arch/mips/pci/ops-emma2rh.c
> @@ -100,7 +100,7 @@ static int pci_config_read(struct pci_bus *bus, unsigned int devfn, int where,
>  		break;
>  	default:
>  		emma2rh_out32(EMMA2RH_PCI_IWIN0_CTR, backup_win0);
> -		return PCIBIOS_FUNC_NOT_SUPPORTED;
> +		return -ENOENT;
>  	}
>  
>  	emma2rh_out32(EMMA2RH_PCI_IWIN0_CTR, backup_win0);
> @@ -149,7 +149,7 @@ static int pci_config_write(struct pci_bus *bus, unsigned int devfn, int where,
>  		break;
>  	default:
>  		emma2rh_out32(EMMA2RH_PCI_IWIN0_CTR, backup_win0);
> -		return PCIBIOS_FUNC_NOT_SUPPORTED;
> +		return -ENOENT;
>  	}
>  	*(volatile u32 *)(base + (PCI_FUNC(devfn) << 8) +
>  			  (where & 0xfffffffc)) = data;
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 83ce1cdf5676..f95637a8d391 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -675,7 +675,6 @@ static inline bool pci_dev_msi_enabled(struct pci_dev *pci_dev) { return false;
>  
>  /* Error values that may be returned by PCI functions */
>  #define PCIBIOS_SUCCESSFUL		0x00
> -#define PCIBIOS_FUNC_NOT_SUPPORTED	0x81
>  #define PCIBIOS_BAD_VENDOR_ID		0x83
>  #define PCIBIOS_DEVICE_NOT_FOUND	0x86
>  #define PCIBIOS_BAD_REGISTER_NUMBER	0x87
> @@ -689,8 +688,6 @@ static inline int pcibios_err_to_errno(int err)
>  		return err; /* Assume already errno */
>  
>  	switch (err) {
> -	case PCIBIOS_FUNC_NOT_SUPPORTED:
> -		return -ENOENT;
>  	case PCIBIOS_BAD_VENDOR_ID:
>  		return -ENOTTY;
>  	case PCIBIOS_DEVICE_NOT_FOUND:
> .
>


^ permalink raw reply related

* [PATCH] tty: hvc: Fix data abort due to race in hvc_open
From: Raghavendra Rao Ananta @ 2020-04-28  3:26 UTC (permalink / raw)
  To: gregkh, jslaby, andrew; +Cc: rananta, linuxppc-dev, linux-kernel

Potentially, hvc_open() can be called in parallel when two tasks calls
open() on /dev/hvcX. In such a scenario, if the hp->ops->notifier_add()
callback in the function fails, where it sets the tty->driver_data to
NULL, the parallel hvc_open() can see this NULL and cause a memory abort.
Hence, serialize hvc_open and check if tty->private_data is NULL before
proceeding ahead.

The issue can be easily reproduced by launching two tasks simultaneously
that does nothing but open() and close() on /dev/hvcX.
For example:
$ ./simple_open_close /dev/hvc0 & ./simple_open_close /dev/hvc0 &

Signed-off-by: Raghavendra Rao Ananta <rananta@codeaurora.org>
---
 drivers/tty/hvc/hvc_console.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
index 436cc51c92c3..ebe26fe5ac09 100644
--- a/drivers/tty/hvc/hvc_console.c
+++ b/drivers/tty/hvc/hvc_console.c
@@ -75,6 +75,8 @@ static LIST_HEAD(hvc_structs);
  */
 static DEFINE_MUTEX(hvc_structs_mutex);
 
+/* Mutex to serialize hvc_open */
+static DEFINE_MUTEX(hvc_open_mutex);
 /*
  * This value is used to assign a tty->index value to a hvc_struct based
  * upon order of exposure via hvc_probe(), when we can not match it to
@@ -346,16 +348,24 @@ static int hvc_install(struct tty_driver *driver, struct tty_struct *tty)
  */
 static int hvc_open(struct tty_struct *tty, struct file * filp)
 {
-	struct hvc_struct *hp = tty->driver_data;
+	struct hvc_struct *hp;
 	unsigned long flags;
 	int rc = 0;
 
+	mutex_lock(&hvc_open_mutex);
+
+	hp = tty->driver_data;
+	if (!hp) {
+		rc = -EIO;
+		goto out;
+	}
+
 	spin_lock_irqsave(&hp->port.lock, flags);
 	/* Check and then increment for fast path open. */
 	if (hp->port.count++ > 0) {
 		spin_unlock_irqrestore(&hp->port.lock, flags);
 		hvc_kick();
-		return 0;
+		goto out;
 	} /* else count == 0 */
 	spin_unlock_irqrestore(&hp->port.lock, flags);
 
@@ -384,6 +394,8 @@ static int hvc_open(struct tty_struct *tty, struct file * filp)
 	/* Force wakeup of the polling thread */
 	hvc_kick();
 
+out:
+	mutex_unlock(&hvc_open_mutex);
 	return rc;
 }
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related

* Re: [PATCH 1/2] powerpc: Discard .rela* sections if CONFIG_RELOCATABLE is undefined
From: Christophe Leroy @ 2020-04-28  5:31 UTC (permalink / raw)
  To: H.J. Lu, linux-kernel
  Cc: linux-arch, Yu-cheng Yu, Kees Cook, Arnd Bergmann, Paul Mackerras,
	Naveen N . Rao, Borislav Petkov, linuxppc-dev
In-Reply-To: <20200428014900.407098-1-hjl.tools@gmail.com>

Hi,

Le 28/04/2020 à 03:48, H.J. Lu a écrit :
> arch/powerpc/kernel/vmlinux.lds.S has
> 
>          DISCARDS
>          /DISCARD/ : {
>                  *(*.EMB.apuinfo)
>                  *(.glink .iplt .plt .rela* .comment)
>                  *(.gnu.version*)
>                  *(.gnu.attributes)
>                  *(.eh_frame)
>          }
> 
> Since .rela* sections are needed when CONFIG_RELOCATABLE is defined,
> change to discard .rela* sections if CONFIG_RELOCATABLE is undefined.

Your explanation and especially the subject are unclear.

 From the subject you understand that you are adding a discard of the 
.rela* sections if CONFIG_RELOCATABLE is undefined.

But when reading the patch, you see that it is the contrary: you are 
removing a discard of the .rela* sections if CONFIG_RELOCATABLE is defined.


So I think the subject could instead be:

	Only discard .rela* sections when CONFIG_RELOCATABLE is undefined

Or maybe better:

	Keep .rela* sections when CONFIG_RELOCATABLE is defined

And the explanation could be:

	Since .rela* sections are needed when CONFIG_RELOCATABLE
	is defined, don't discard .rela* sections if
	CONFIG_RELOCATABLE is defined.

> 
> Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
> Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
> ---
>   arch/powerpc/kernel/vmlinux.lds.S | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S
> index 31a0f201fb6f..4ba07734a210 100644
> --- a/arch/powerpc/kernel/vmlinux.lds.S
> +++ b/arch/powerpc/kernel/vmlinux.lds.S
> @@ -366,9 +366,12 @@ SECTIONS
>   	DISCARDS
>   	/DISCARD/ : {
>   		*(*.EMB.apuinfo)
> -		*(.glink .iplt .plt .rela* .comment)
> +		*(.glink .iplt .plt .comment)
>   		*(.gnu.version*)
>   		*(.gnu.attributes)
>   		*(.eh_frame)
> +#ifndef CONFIG_RELOCATABLE
> +		*(.rela*)
> +#endif
>   	}
>   }
> 

Christophe

^ permalink raw reply

* Re: [PATCH v6 03/28] powerpc/xmon: Move breakpoints to text section
From: Christophe Leroy @ 2020-04-28  5:36 UTC (permalink / raw)
  To: Jordan Niethe
  Cc: Alistair Popple, Nicholas Piggin, Balamuruhan S, linuxppc-dev,
	Daniel Axtens
In-Reply-To: <CACzsE9oF4eLH_5JUrSC1XfyWG_QzJ45HK=Rj+XwfxhuVxe5k0g@mail.gmail.com>



Le 28/04/2020 à 07:30, Jordan Niethe a écrit :
> On Tue, Apr 28, 2020 at 3:20 PM Christophe Leroy
> <christophe.leroy@c-s.fr> wrote:
>>
>>
>>
>> Le 28/04/2020 à 03:57, Jordan Niethe a écrit :
>>> The instructions for xmon's breakpoint are stored bpt_table[] which is in
>>> the data section. This is problematic as the data section may be marked
>>> as no execute. Move bpt_table[] to the text section.
>>>
>>> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
>>> ---
>>> v6: - New to series. Was part of the previous patch.
>>>       - Make BPT_SIZE available in assembly
>>> ---
>>>    arch/powerpc/kernel/asm-offsets.c |  8 ++++++++
>>>    arch/powerpc/xmon/Makefile        |  2 +-
>>>    arch/powerpc/xmon/xmon.c          |  6 +-----
>>>    arch/powerpc/xmon/xmon_bpts.S     |  9 +++++++++
>>>    arch/powerpc/xmon/xmon_bpts.h     | 14 ++++++++++++++
>>>    5 files changed, 33 insertions(+), 6 deletions(-)
>>>    create mode 100644 arch/powerpc/xmon/xmon_bpts.S
>>>    create mode 100644 arch/powerpc/xmon/xmon_bpts.h
>>>
>>> diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
>>> index c25e562f1cd9..2401f415f423 100644
>>> --- a/arch/powerpc/kernel/asm-offsets.c
>>> +++ b/arch/powerpc/kernel/asm-offsets.c
>>> @@ -70,6 +70,10 @@
>>>    #include <asm/fixmap.h>
>>>    #endif
>>>
>>> +#ifdef CONFIG_XMON
>>> +#include "../xmon/xmon_bpts.h"
>>> +#endif
>>> +
>>>    #define STACK_PT_REGS_OFFSET(sym, val)      \
>>>        DEFINE(sym, STACK_FRAME_OVERHEAD + offsetof(struct pt_regs, val))
>>>
>>> @@ -783,5 +787,9 @@ int main(void)
>>>        DEFINE(VIRT_IMMR_BASE, (u64)__fix_to_virt(FIX_IMMR_BASE));
>>>    #endif
>>>
>>> +#ifdef CONFIG_XMON
>>> +     DEFINE(BPT_SIZE, BPT_SIZE);
>>> +#endif
>>> +
>>>        return 0;
>>>    }
>>> diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
>>> index c3842dbeb1b7..515a13ea6f28 100644
>>> --- a/arch/powerpc/xmon/Makefile
>>> +++ b/arch/powerpc/xmon/Makefile
>>> @@ -21,7 +21,7 @@ endif
>>>
>>>    ccflags-$(CONFIG_PPC64) := $(NO_MINIMAL_TOC)
>>>
>>> -obj-y                        += xmon.o nonstdio.o spr_access.o
>>> +obj-y                        += xmon.o nonstdio.o spr_access.o xmon_bpts.o
>>>
>>>    ifdef CONFIG_XMON_DISASSEMBLY
>>>    obj-y                       += ppc-dis.o ppc-opc.o
>>> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
>>> index a064392df1b8..f7ce3ea8694c 100644
>>> --- a/arch/powerpc/xmon/xmon.c
>>> +++ b/arch/powerpc/xmon/xmon.c
>>> @@ -62,6 +62,7 @@
>>>
>>>    #include "nonstdio.h"
>>>    #include "dis-asm.h"
>>> +#include "xmon_bpts.h"
>>>
>>>    #ifdef CONFIG_SMP
>>>    static cpumask_t cpus_in_xmon = CPU_MASK_NONE;
>>> @@ -108,7 +109,6 @@ struct bpt {
>>>    #define BP_TRAP             2
>>>    #define BP_DABR             4
>>>
>>> -#define NBPTS        256
>>>    static struct bpt bpts[NBPTS];
>>>    static struct bpt dabr;
>>>    static struct bpt *iabr;
>>> @@ -116,10 +116,6 @@ static unsigned bpinstr = 0x7fe00008;    /* trap */
>>>
>>>    #define BP_NUM(bp)  ((bp) - bpts + 1)
>>>
>>> -#define BPT_SIZE       (sizeof(unsigned int) * 2)
>>> -#define BPT_WORDS      (BPT_SIZE / sizeof(unsigned int))
>>> -static unsigned int bpt_table[NBPTS * BPT_WORDS];
>>> -
>>>    /* Prototypes */
>>>    static int cmds(struct pt_regs *);
>>>    static int mread(unsigned long, void *, int);
>>> diff --git a/arch/powerpc/xmon/xmon_bpts.S b/arch/powerpc/xmon/xmon_bpts.S
>>> new file mode 100644
>>> index 000000000000..f3ad0ab50854
>>> --- /dev/null
>>> +++ b/arch/powerpc/xmon/xmon_bpts.S
>>> @@ -0,0 +1,9 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +#include <asm/ppc_asm.h>
>>> +#include <asm/asm-compat.h>
>>> +#include <asm/asm-offsets.h>
>>> +#include "xmon_bpts.h"
>>> +
>>> +.global bpt_table
>>> +bpt_table:
>>> +     .space NBPTS * BPT_SIZE
>>
>> No alignment required ? Standard alignment (probably 4 bytes ?) is
>> acceptable ?
> No, I'll add a .balign    4 to be sure.

I think it is aligned to 4 by default. My question was to know whether 4 
is enough.
I see BPT_SIZE is 8, should the alignment be at least 8 ?

>>
>>
>>> diff --git a/arch/powerpc/xmon/xmon_bpts.h b/arch/powerpc/xmon/xmon_bpts.h
>>> new file mode 100644
>>> index 000000000000..b7e94375db86
>>> --- /dev/null
>>> +++ b/arch/powerpc/xmon/xmon_bpts.h
>>> @@ -0,0 +1,14 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +#ifndef XMON_BPTS_H
>>> +#define XMON_BPTS_H
>>> +
>>> +#define NBPTS        256
>>> +#ifndef __ASSEMBLY__
>>> +#define BPT_SIZE     (sizeof(unsigned int) * 2)
>>> +#define BPT_WORDS    (BPT_SIZE / sizeof(unsigned int))
>>> +
>>> +extern unsigned int bpt_table[NBPTS * BPT_WORDS];
>>> +
>>> +#endif /* __ASSEMBLY__ */
>>> +
>>> +#endif /* XMON_BPTS_H */
>>>
>>
>> Christophe

^ permalink raw reply

* Re: [PATCH v8 0/7] powerpc/perf: Add json file metric support for the hv_24x7 socket/chip level events
From: kajoljain @ 2020-04-28  6:32 UTC (permalink / raw)
  To: acme, linuxppc-dev, mpe, sukadev
  Cc: mark.rutland, ravi.bangoria, maddy, tglx, jmario, mpetlan, peterz,
	gregkh, linux-kernel, alexander.shishkin, linux-perf-users, ak,
	yao.jin, anju, mamatha4, jolsa, namhyung, mingo, kan.liang
In-Reply-To: <20200401203340.31402-1-kjain@linux.ibm.com>

Hi Arnaldo,
	Please let me know if there any changes required in this patchset,
as some of its patches are still not part of your perf/core tree.

Thanks,
Kajol Jain

On 4/2/20 2:03 AM, Kajol Jain wrote:
> Patchset adds json file metric support for the hv_24x7 socket/chip level
> events. "hv_24x7" pmu interface events needs system dependent parameter
> like socket/chip/core. For example, hv_24x7 chip level events needs
> specific chip-id to which the data is requested should be added as part
> of pmu events.
> 
> So to enable JSON file support to "hv_24x7" interface, patchset reads
> total number of sockets details in sysfs under 
> "/sys/devices/hv_24x7/interface/".
> 
> Second patch of the patchset adds expr_scanner_ctx object to hold user
> data for the expr scanner, which can be used to hold runtime parameter.
> 
> Patch 4 & 6 of the patchset handles perf tool plumbing needed to replace
> the "?" character in the metric expression to proper value and hv_24x7
> json metric file for different Socket/chip resources.
> 
> Patch set also enable Hz/hz prinitg for --metric-only option to print
> metric data for bus frequency.
> 
> Applied and tested all these patches cleanly on top of jiri's flex changes
> with the changes done by Kan Liang for "Support metric group constraint"
> patchset and made required changes.
> 
> Also apply this patch on top of the fix patch send earlier
> for printing metric name incase overlapping events.
> https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/commit/?h=perf/core&id=37cd7f65bf71a48f25eeb6d9be5dacb20d008ea6
> 
> Changelog:
> v7 -> v8
> - Add test case for testing parsing of "?" in metric expression
> - Reaname variables name to runtime
> 
> v6 -> v7
> - Spit patchset into two patch series one for kernel changes and other
>   for tool side changes.
> - Made changes Suggested by Jiri, including rather then reading runtime
>   parameter from metric name, actually add it in structure egroup and
>   metric_expr.
> - As we don't need to read runtime parameter from metric name,
>   now I am not appending it and rather just printing it in
>   generic_metric function.
> 
> Kernel Side changes patch series: https://lkml.org/lkml/2020/3/27/58
> 
> v5 -> v6
> - resolve compilation issue due to rearranging patch series.
> - Rather then adding new function to take careof case for runtime param
>   in metricgroup__add_metric, using metricgroup__add_metric_param itself
>   for that work.
> - Address some optimization suggested like using directly file path
>   rather then adding new macro in header.c
> - Change commit message on patch where we are adding "?" support
>   by adding simple example.
> 
> v4 -> v5
> - Using sysfs__read_int instead of sysfs__read_ull while reading
>   parameter value in powerpc/util/header.c file.
> 
> - Using asprintf rather then malloc and sprintf 
>   Suggested by Arnaldo Carvalho de Melo
> 
> - Break patch 6 from previous version to two patch,
>   - One to add refactor current "metricgroup__add_metric" function
>     and another where actually "?" handling infra added.
> 
> - Add expr__runtimeparam as part of 'expr_scanner_ctx' struct
>   rather then making it global variable. Thanks Jiri for
>   adding this structure to hold user data for the expr scanner.
> 
> - Add runtime param as agrugement to function 'expr__find_other'
>   and 'expr__parse' and made changes on references accordingly.
> 
> v3 -> v4
> - Apply these patch on top of Kan liang changes.
>   As suggested by Jiri.
> 
> v2 -> v3
> - Remove setting  event_count to 0 part in function 'h_24x7_event_read'
>   with comment rather then adding 0 to event_count value.
>   Suggested by: Sukadev Bhattiprolu
> 
> - Apply tool side changes require to replace "?" on Jiri's flex patch
>   series and made all require changes to make it compatible with added
>   flex change.
> 
> v1 -> v2
> - Rename hv-24x7 metric json file as nest_metrics.json
> 
> Jiri Olsa (2):
>   perf expr: Add expr_ prefix for parse_ctx and parse_id
>   perf expr: Add expr_scanner_ctx object
> 
> Kajol Jain (5):
>   perf/tools: Refactoring metricgroup__add_metric function
>   perf/tools: Enhance JSON/metric infrastructure to handle "?"
>   perf/tests/expr: Added test for runtime param in metric expression
>   tools/perf: Enable Hz/hz prinitg for --metric-only option
>   perf/tools/pmu-events/powerpc: Add hv_24x7 socket/chip level metric
>     events
> 
>  tools/perf/arch/powerpc/util/header.c         |  8 ++
>  .../arch/powerpc/power9/nest_metrics.json     | 19 +++++
>  tools/perf/tests/expr.c                       | 20 +++--
>  tools/perf/util/expr.c                        | 25 +++---
>  tools/perf/util/expr.h                        | 19 +++--
>  tools/perf/util/expr.l                        | 37 ++++++---
>  tools/perf/util/expr.y                        |  6 +-
>  tools/perf/util/metricgroup.c                 | 78 +++++++++++++------
>  tools/perf/util/metricgroup.h                 |  2 +
>  tools/perf/util/stat-display.c                |  2 -
>  tools/perf/util/stat-shadow.c                 | 19 +++--
>  11 files changed, 164 insertions(+), 71 deletions(-)
>  create mode 100644 tools/perf/pmu-events/arch/powerpc/power9/nest_metrics.json
> 

^ permalink raw reply

* Re: [PATCH 1/5] powerpc/spufs: simplify spufs core dumping
From: Christoph Hellwig @ 2020-04-28  6:57 UTC (permalink / raw)
  To: Jeremy Kerr
  Cc: Arnd Bergmann, linuxppc-dev, linux-kernel, Eric W . Biederman,
	linux-fsdevel, Andrew Morton, Linus Torvalds, Christoph Hellwig,
	Al Viro
In-Reply-To: <fc3b45c91e5cd50baa1fec7710f1e64cbe616f77.camel@ozlabs.org>

On Tue, Apr 28, 2020 at 10:51:56AM +0800, Jeremy Kerr wrote:
> Hi Al & Christoph,
> 
> > Again, this really needs fixing.  Preferably - as a separate commit
> > preceding this series, so that it could be
> > backported.  simple_read_from_buffer() is a blocking operation.
> 
> I'll put together a patch that fixes this.
> 
> Christoph: I'll do it in a way that matches your changes to the _read
> functions, so hopefully those hunks would just drop from your change,
> leaving only the _dump additions. Would that work?

Sure.

^ permalink raw reply

* Re: [PATCH 2/7] signal: factor copy_siginfo_to_external32 from copy_siginfo_to_user32
From: Christoph Hellwig @ 2020-04-28  7:09 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Arnd Bergmann, Linus Torvalds, linux-kernel, Alexander Viro,
	Jeremy Kerr, linux-fsdevel, linuxppc-dev, Christoph Hellwig,
	Eric W . Biederman
In-Reply-To: <20200427154050.e431ad7fb228610cc6b95973@linux-foundation.org>

On Mon, Apr 27, 2020 at 03:40:50PM -0700, Andrew Morton wrote:
> > https://www.spinics.net/lists/kernel/msg3473847.html
> > https://www.spinics.net/lists/kernel/msg3473840.html
> > https://www.spinics.net/lists/kernel/msg3473843.html
> 
> OK, but that doesn't necessitate the above monstrosity?  How about
> 
> static int __copy_siginfo_to_user32(struct compat_siginfo __user *to,
> 			     const struct kernel_siginfo *from, bool x32_ABI)
> {
> 	struct compat_siginfo new;
> 	copy_siginfo_to_external32(&new, from);
> 	...
> }
> 
> int copy_siginfo_to_user32(struct compat_siginfo __user *to,
> 			   const struct kernel_siginfo *from)
> {
> #if defined(CONFIG_X86_X32_ABI) || defined(CONFIG_IA32_EMULATION)
> 	return __copy_siginfo_to_user32(to, from, in_x32_syscall());
> #else
> 	return __copy_siginfo_to_user32(to, from, 0);
> #endif
> }
> 
> Or something like that - I didn't try very hard.  We know how to do
> this stuff, and surely this thing isn't how!

I guess that might be a worthwhile middle ground.  Still not a fan of
all these ifdefs..

^ permalink raw reply

* Re: [PATCH 2/7] signal: factor copy_siginfo_to_external32 from copy_siginfo_to_user32
From: Christophe Leroy @ 2020-04-28  7:45 UTC (permalink / raw)
  To: Christoph Hellwig, Andrew Morton
  Cc: Eric W . Biederman, Arnd Bergmann, linuxppc-dev, linux-kernel,
	Jeremy Kerr, linux-fsdevel, Linus Torvalds, Alexander Viro
In-Reply-To: <20200428070935.GE18754@lst.de>



Le 28/04/2020 à 09:09, Christoph Hellwig a écrit :
> On Mon, Apr 27, 2020 at 03:40:50PM -0700, Andrew Morton wrote:
>>> https://www.spinics.net/lists/kernel/msg3473847.html
>>> https://www.spinics.net/lists/kernel/msg3473840.html
>>> https://www.spinics.net/lists/kernel/msg3473843.html
>>
>> OK, but that doesn't necessitate the above monstrosity?  How about
>>
>> static int __copy_siginfo_to_user32(struct compat_siginfo __user *to,
>> 			     const struct kernel_siginfo *from, bool x32_ABI)
>> {
>> 	struct compat_siginfo new;
>> 	copy_siginfo_to_external32(&new, from);
>> 	...
>> }
>>
>> int copy_siginfo_to_user32(struct compat_siginfo __user *to,
>> 			   const struct kernel_siginfo *from)
>> {
>> #if defined(CONFIG_X86_X32_ABI) || defined(CONFIG_IA32_EMULATION)
>> 	return __copy_siginfo_to_user32(to, from, in_x32_syscall());
>> #else
>> 	return __copy_siginfo_to_user32(to, from, 0);
>> #endif
>> }
>>
>> Or something like that - I didn't try very hard.  We know how to do
>> this stuff, and surely this thing isn't how!
> 
> I guess that might be a worthwhile middle ground.  Still not a fan of
> all these ifdefs..
> 

Can't we move the small X32 specific part out of 
__copy_siginfo_to_user32(), in an arch specific helper that voids for 
other architectures ?

Something like:

		if (!arch_special_something(&new, from)) {
			new.si_utime = from->si_utime;
			new.si_stime = from->si_stime;
		}

Then the arch_special_something() does what it wants in x86 and returns 
1, and for architectures not implementating it, a generic version return 
0 all the time.

Christophe

^ permalink raw reply

* Re: [PATCH 2/7] signal: factor copy_siginfo_to_external32 from copy_siginfo_to_user32
From: Christoph Hellwig @ 2020-04-28  7:48 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Eric W . Biederman, Arnd Bergmann, linuxppc-dev, linux-kernel,
	Alexander Viro, linux-fsdevel, Andrew Morton, Linus Torvalds,
	Christoph Hellwig, Jeremy Kerr
In-Reply-To: <ddbaba35-9cc5-dfb9-3cae-51b026de5b65@c-s.fr>

On Tue, Apr 28, 2020 at 09:45:46AM +0200, Christophe Leroy wrote:
>> I guess that might be a worthwhile middle ground.  Still not a fan of
>> all these ifdefs..
>>
>
> Can't we move the small X32 specific part out of 
> __copy_siginfo_to_user32(), in an arch specific helper that voids for other 
> architectures ?
>
> Something like:
>
> 		if (!arch_special_something(&new, from)) {
> 			new.si_utime = from->si_utime;
> 			new.si_stime = from->si_stime;
> 		}
>
> Then the arch_special_something() does what it wants in x86 and returns 1, 
> and for architectures not implementating it, a generic version return 0 all 
> the time.

The main issue is that we need an explicit paramter to select x32,
as it can't just be discovered from the calling context otherwise.
The rest is just sugarcoating.


^ permalink raw reply

* [PATCH v2 1/3] powerpc/numa: Set numa_node for all possible cpus
From: Srikar Dronamraju @ 2020-04-28  9:38 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Michal Hocko, Srikar Dronamraju, Linus Torvalds, linux-kernel,
	linux-mm, Mel Gorman, Kirill A. Shutemov, Christopher Lameter,
	linuxppc-dev, Vlastimil Babka
In-Reply-To: <20200428093836.27190-1-srikar@linux.vnet.ibm.com>

A Powerpc system with multiple possible nodes and with CONFIG_NUMA
enabled always used to have a node 0, even if node 0 does not any cpus
or memory attached to it. As per PAPR, node affinity of a cpu is only
available once its present / online. For all cpus that are possible but
not present, cpu_to_node() would point to node 0.

To ensure a cpuless, memoryless dummy node is not online, powerpc need
to make sure all possible but not present cpu_to_node are set to a
proper node.

Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Cc: Michal Hocko <mhocko@suse.com>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: "Kirill A. Shutemov" <kirill@shutemov.name>
Cc: Christopher Lameter <cl@linux.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
Changelog v1:->v2:
- Rebased to v5.7-rc3

 arch/powerpc/mm/numa.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index 9fcf2d195830..b3615b7fdbdf 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -931,8 +931,20 @@ void __init mem_topology_setup(void)
 
 	reset_numa_cpu_lookup_table();
 
-	for_each_present_cpu(cpu)
-		numa_setup_cpu(cpu);
+	for_each_possible_cpu(cpu) {
+		/*
+		 * Powerpc with CONFIG_NUMA always used to have a node 0,
+		 * even if it was memoryless or cpuless. For all cpus that
+		 * are possible but not present, cpu_to_node() would point
+		 * to node 0. To remove a cpuless, memoryless dummy node,
+		 * powerpc need to make sure all possible but not present
+		 * cpu_to_node are set to a proper node.
+		 */
+		if (cpu_present(cpu))
+			numa_setup_cpu(cpu);
+		else
+			set_cpu_numa_node(cpu, first_online_node);
+	}
 }
 
 void __init initmem_init(void)
-- 
2.20.1


^ permalink raw reply related

* [PATCH v2 2/3] powerpc/numa: Prefer node id queried from vphn
From: Srikar Dronamraju @ 2020-04-28  9:38 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Michal Hocko, Srikar Dronamraju, Linus Torvalds, linux-kernel,
	linux-mm, Mel Gorman, Kirill A. Shutemov, Christopher Lameter,
	linuxppc-dev, Vlastimil Babka
In-Reply-To: <20200428093836.27190-1-srikar@linux.vnet.ibm.com>

Node id queried from the static device tree may not
be correct. For example: it may always show 0 on a shared processor.
Hence prefer the node id queried from vphn and fallback on the device tree
based node id if vphn query fails.

Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Cc: Michal Hocko <mhocko@suse.com>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: "Kirill A. Shutemov" <kirill@shutemov.name>
Cc: Christopher Lameter <cl@linux.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
Changelog v1:->v2:
- Rebased to v5.7-rc3

 arch/powerpc/mm/numa.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index b3615b7fdbdf..281531340230 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -719,20 +719,20 @@ static int __init parse_numa_properties(void)
 	 */
 	for_each_present_cpu(i) {
 		struct device_node *cpu;
-		int nid;
-
-		cpu = of_get_cpu_node(i, NULL);
-		BUG_ON(!cpu);
-		nid = of_node_to_nid_single(cpu);
-		of_node_put(cpu);
+		int nid = vphn_get_nid(i);
 
 		/*
 		 * Don't fall back to default_nid yet -- we will plug
 		 * cpus into nodes once the memory scan has discovered
 		 * the topology.
 		 */
-		if (nid < 0)
-			continue;
+		if (nid == NUMA_NO_NODE) {
+			cpu = of_get_cpu_node(i, NULL);
+			if (cpu) {
+				nid = of_node_to_nid_single(cpu);
+				of_node_put(cpu);
+			}
+		}
 		node_set_online(nid);
 	}
 
-- 
2.20.1


^ permalink raw reply related

* [PATCH v2 0/3] Offline memoryless cpuless node 0
From: Srikar Dronamraju @ 2020-04-28  9:38 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Michal Hocko, Srikar Dronamraju, Linus Torvalds, linux-kernel,
	linux-mm, Mel Gorman, Kirill A. Shutemov, Christopher Lameter,
	linuxppc-dev, Vlastimil Babka

Changelog v1:->v2:
- Rebased to v5.7-rc3
- Updated the changelog.

Linux kernel configured with CONFIG_NUMA on a system with multiple
possible nodes, marks node 0 as online at boot. However in practice,
there are systems which have node 0 as memoryless and cpuless.

This can cause
1. numa_balancing to be enabled on systems with only one online node.
2. Existence of dummy (cpuless and memoryless) node which can confuse
users/scripts looking at output of lscpu / numactl.

This patchset wants to correct this anomaly.

This should only affect systems that have CONFIG_MEMORYLESS_NODES.
Currently there are only 2 architectures ia64 and powerpc that have this
config.

Note: Patch 3 in this patch series depends on patches 1 and 2.
Without patches 1 and 2, patch 3 might crash powerpc.

v5.7-rc3
 available: 2 nodes (0,2)
 node 0 cpus:
 node 0 size: 0 MB
 node 0 free: 0 MB
 node 2 cpus: 0 1 2 3 4 5 6 7
 node 2 size: 32625 MB
 node 2 free: 31490 MB
 node distances:
 node   0   2
   0:  10  20
   2:  20  10

proc and sys files
------------------
 /sys/devices/system/node/online:            0,2
 /proc/sys/kernel/numa_balancing:            1
 /sys/devices/system/node/has_cpu:           2
 /sys/devices/system/node/has_memory:        2
 /sys/devices/system/node/has_normal_memory: 2
 /sys/devices/system/node/possible:          0-31

v5.7-rc3 + patches
------------------
 available: 1 nodes (2)
 node 2 cpus: 0 1 2 3 4 5 6 7
 node 2 size: 32625 MB
 node 2 free: 31487 MB
 node distances:
 node   2
   2:  10

proc and sys files
------------------
/sys/devices/system/node/online:            2
/proc/sys/kernel/numa_balancing:            0
/sys/devices/system/node/has_cpu:           2
/sys/devices/system/node/has_memory:        2
/sys/devices/system/node/has_normal_memory: 2
/sys/devices/system/node/possible:          0-31

Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Cc: Michal Hocko <mhocko@suse.com>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: "Kirill A. Shutemov" <kirill@shutemov.name>
Cc: Christopher Lameter <cl@linux.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>

Srikar Dronamraju (3):
  powerpc/numa: Set numa_node for all possible cpus
  powerpc/numa: Prefer node id queried from vphn
  mm/page_alloc: Keep memoryless cpuless node 0 offline

 arch/powerpc/mm/numa.c | 32 ++++++++++++++++++++++----------
 mm/page_alloc.c        |  4 +++-
 2 files changed, 25 insertions(+), 11 deletions(-)

-- 
2.20.1


^ permalink raw reply

* [PATCH v2 3/3] mm/page_alloc: Keep memoryless cpuless node 0 offline
From: Srikar Dronamraju @ 2020-04-28  9:38 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Michal Hocko, Srikar Dronamraju, Linus Torvalds, linux-kernel,
	linux-mm, Mel Gorman, Kirill A. Shutemov, Christopher Lameter,
	linuxppc-dev, Vlastimil Babka
In-Reply-To: <20200428093836.27190-1-srikar@linux.vnet.ibm.com>

Currently Linux kernel with CONFIG_NUMA on a system with multiple
possible nodes, marks node 0 as online at boot.  However in practice,
there are systems which have node 0 as memoryless and cpuless.

This can cause numa_balancing to be enabled on systems with only one node
with memory and CPUs. The existence of this dummy node which is cpuless and
memoryless node can confuse users/scripts looking at output of lscpu /
numactl.

By marking, N_ONLINE as NODE_MASK_NONE, lets stop assuming that Node 0 is
always online.

v5.7-rc3
 available: 2 nodes (0,2)
 node 0 cpus:
 node 0 size: 0 MB
 node 0 free: 0 MB
 node 2 cpus: 0 1 2 3 4 5 6 7
 node 2 size: 32625 MB
 node 2 free: 31490 MB
 node distances:
 node   0   2
   0:  10  20
   2:  20  10

proc and sys files
------------------
 /sys/devices/system/node/online:            0,2
 /proc/sys/kernel/numa_balancing:            1
 /sys/devices/system/node/has_cpu:           2
 /sys/devices/system/node/has_memory:        2
 /sys/devices/system/node/has_normal_memory: 2
 /sys/devices/system/node/possible:          0-31

v5.7-rc3 + patch
------------------
 available: 1 nodes (2)
 node 2 cpus: 0 1 2 3 4 5 6 7
 node 2 size: 32625 MB
 node 2 free: 31487 MB
 node distances:
 node   2
   2:  10

proc and sys files
------------------
/sys/devices/system/node/online:            2
/proc/sys/kernel/numa_balancing:            0
/sys/devices/system/node/has_cpu:           2
/sys/devices/system/node/has_memory:        2
/sys/devices/system/node/has_normal_memory: 2
/sys/devices/system/node/possible:          0-31

Note: On Powerpc, cpu_to_node of possible but not present cpus would
previously return 0. Hence this commit depends on commit ("powerpc/numa: Set
numa_node for all possible cpus") and commit ("powerpc/numa: Prefer node id
queried from vphn"). Without the 2 commits, Powerpc system might crash.

Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Cc: Michal Hocko <mhocko@suse.com>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: "Kirill A. Shutemov" <kirill@shutemov.name>
Cc: Christopher Lameter <cl@linux.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
Changelog v1:->v2:
- Rebased to v5.7-rc3
- Updated the changelog.

 mm/page_alloc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 69827d4fa052..03b89592af04 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -116,8 +116,10 @@ EXPORT_SYMBOL(latent_entropy);
  */
 nodemask_t node_states[NR_NODE_STATES] __read_mostly = {
 	[N_POSSIBLE] = NODE_MASK_ALL,
+#ifdef CONFIG_NUMA
+	[N_ONLINE] = NODE_MASK_NONE,
+#else
 	[N_ONLINE] = { { [0] = 1UL } },
-#ifndef CONFIG_NUMA
 	[N_NORMAL_MEMORY] = { { [0] = 1UL } },
 #ifdef CONFIG_HIGHMEM
 	[N_HIGH_MEMORY] = { { [0] = 1UL } },
-- 
2.20.1


^ 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