* Re: [PATCH v2 5/6] powerpc/pseries/iommu: Make use of DDW even if it does not map the partition
From: Leonardo Bras @ 2020-07-01 19:57 UTC (permalink / raw)
To: Alexey Kardashevskiy, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Thiago Jung Bauermann, Ram Pai
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <18df09c0-ef83-a0d8-1143-1cb4d50bf6b7@ozlabs.ru>
On Wed, 2020-07-01 at 18:16 +1000, Alexey Kardashevskiy wrote:
>
> On 24/06/2020 16:24, Leonardo Bras wrote:
> > As of today, if a DDW is created and can't map the whole partition, it's
> > removed and the default DMA window "ibm,dma-window" is used instead.
> >
> > Usually this DDW is bigger than the default DMA window, so it would be
> > better to make use of it instead.
> >
> > Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
> > ---
> > arch/powerpc/platforms/pseries/iommu.c | 28 +++++++++++++++++---------
> > 1 file changed, 19 insertions(+), 9 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
> > index 4fcf00016fb1..2d217cda4075 100644
> > --- a/arch/powerpc/platforms/pseries/iommu.c
> > +++ b/arch/powerpc/platforms/pseries/iommu.c
> > @@ -685,7 +685,7 @@ static void pci_dma_bus_setup_pSeriesLP(struct pci_bus *bus)
> > struct iommu_table *tbl;
> > struct device_node *dn, *pdn;
> > struct pci_dn *ppci;
> > - const __be32 *dma_window = NULL;
> > + const __be32 *dma_window = NULL, *alt_dma_window = NULL;
> >
> > dn = pci_bus_to_OF_node(bus);
> >
> > @@ -699,8 +699,13 @@ static void pci_dma_bus_setup_pSeriesLP(struct pci_bus *bus)
> > break;
> > }
> >
> > + /* If there is a DDW available, use it instead */
> > + alt_dma_window = of_get_property(pdn, DIRECT64_PROPNAME, NULL);
>
> It is not necessarily "direct" anymore as the name suggests, you may
> want to change that. DMA64_PROPNAME, may be. Thanks,
>
Yeah, you are right.
I will change this for next version, also changing the string name to
reflect this.
-#define DIRECT64_PROPNAME "linux,direct64-ddr-window-info"
+#define DMA64_PROPNAME "linux,dma64-ddr-window-info"
Is that ok?
Thank you for helping!
^ permalink raw reply
* Re: [PATCH v2 4/6] powerpc/pseries/iommu: Remove default DMA window before creating DDW
From: Leonardo Bras @ 2020-07-01 19:48 UTC (permalink / raw)
To: Alexey Kardashevskiy, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Thiago Jung Bauermann, Ram Pai
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <e00340a3-1070-a787-5acc-0bfc37f73dff@ozlabs.ru>
On Wed, 2020-07-01 at 18:17 +1000, Alexey Kardashevskiy wrote:
>
> On 24/06/2020 16:24, Leonardo Bras wrote:
> > On LoPAR "DMA Window Manipulation Calls", it's recommended to remove the
> > default DMA window for the device, before attempting to configure a DDW,
> > in order to make the maximum resources available for the next DDW to be
> > created.
> >
> > This is a requirement for some devices to use DDW, given they only
> > allow one DMA window.
>
> Devices never know about these windows, it is purely PHB's side of
> things. A device can access any address on the bus, the bus can generate
> an exception if there is no window behind the address OR some other
> device's MMIO. We could actually create a second window in addition to
> the first one and allocate bus addresses from both, we just simplifying
> this by merging two separate non-adjacent windows into one.
That's interesting, I was not aware of this.
I will try to improve this commit message with this info.
Thanks for sharing!
> > > > If setting up a new DDW fails anywhere after the removal of this
> > default DMA window, it's needed to restore the default DMA window.
> > For this, an implementation of ibm,reset-pe-dma-windows rtas call is
> > needed:
> >
> > Platforms supporting the DDW option starting with LoPAR level 2.7 implement
> > ibm,ddw-extensions. The first extension available (index 2) carries the
> > token for ibm,reset-pe-dma-windows rtas call, which is used to restore
> > the default DMA window for a device, if it has been deleted.
> >
> > It does so by resetting the TCE table allocation for the PE to it's
> > boot time value, available in "ibm,dma-window" device tree node.
> >
> > Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
> > ---
> > arch/powerpc/platforms/pseries/iommu.c | 70 ++++++++++++++++++++++----
> > 1 file changed, 61 insertions(+), 9 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
> > index a8840d9e1c35..4fcf00016fb1 100644
> > --- a/arch/powerpc/platforms/pseries/iommu.c
> > +++ b/arch/powerpc/platforms/pseries/iommu.c
> > @@ -1029,6 +1029,39 @@ static phys_addr_t ddw_memory_hotplug_max(void)
> > return max_addr;
> > }
> >
> > +/*
> > + * Platforms supporting the DDW option starting with LoPAR level 2.7 implement
> > + * ibm,ddw-extensions, which carries the rtas token for
> > + * ibm,reset-pe-dma-windows.
> > + * That rtas-call can be used to restore the default DMA window for the device.
> > + */
> > +static void reset_dma_window(struct pci_dev *dev, struct device_node *par_dn)
> > +{
> > + int ret;
> > + u32 cfg_addr, ddw_ext[DDW_EXT_RESET_DMA_WIN + 1];
> > + u64 buid;
> > + struct device_node *dn;
> > + struct pci_dn *pdn;
> > +
> > + ret = of_property_read_u32_array(par_dn, "ibm,ddw-extensions",
> > + &ddw_ext[0], DDW_EXT_RESET_DMA_WIN + 1);
> > + if (ret)
> > + return;
> > +
> > + dn = pci_device_to_OF_node(dev);
> > + pdn = PCI_DN(dn);
> > + buid = pdn->phb->buid;
> > + cfg_addr = ((pdn->busno << 16) | (pdn->devfn << 8));
> > +
> > + ret = rtas_call(ddw_ext[DDW_EXT_RESET_DMA_WIN], 3, 1, NULL, cfg_addr,
> > + BUID_HI(buid), BUID_LO(buid));
> > + if (ret)
> > + dev_info(&dev->dev,
> > + "ibm,reset-pe-dma-windows(%x) %x %x %x returned %d ",
> > + ddw_ext[1], cfg_addr, BUID_HI(buid), BUID_LO(buid),
>
> s/ddw_ext[1]/ddw_ext[DDW_EXT_RESET_DMA_WIN]/
Good catch! I missed this one.
>
>
> > + ret);
> > +}
> > +
> > /*
> > * If the PE supports dynamic dma windows, and there is space for a table
> > * that can map all pages in a linear offset, then setup such a table,
> > @@ -1049,8 +1082,9 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
> > u64 dma_addr, max_addr;
> > struct device_node *dn;
> > u32 ddw_avail[DDW_APPLICABLE_SIZE];
> > +
>
> Unrelated new empty line.
Fixed!
>
>
> > struct direct_window *window;
> > - struct property *win64;
> > + struct property *win64, *default_win = NULL, *ddw_ext = NULL;
> > struct dynamic_dma_window_prop *ddwprop;
> > struct failed_ddw_pdn *fpdn;
> >
> > @@ -1085,7 +1119,7 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
> > if (ret)
> > goto out_failed;
> >
> > - /*
> > + /*
> > * Query if there is a second window of size to map the
> > * whole partition. Query returns number of windows, largest
> > * block assigned to PE (partition endpoint), and two bitmasks
> > @@ -1096,15 +1130,31 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
> > if (ret != 0)
> > goto out_failed;
> >
> > + /*
> > + * If there is no window available, remove the default DMA window,
> > + * if it's present. This will make all the resources available to the
> > + * new DDW window.
> > + * If anything fails after this, we need to restore it, so also check
> > + * for extensions presence.
> > + */
> > if (query.windows_available == 0) {
>
> Does phyp really always advertise 0 windows for these VFs? What is in
> the largest_available_block when windows_available==0?
For this VF, it always advertise 0 windows before removing the default
DMA window. The largest available block size is the same as after the
removal (256GB). The only value that changes after removal is the
number of available windows. Here some debug prints:
[ 3.473149] mlx5_core 4005:01:00.0: ibm,query-pe-dma-windows(53)
10000 8000000 29004005 returned 0
[ 3.473162] mlx5_core 4005:01:00.0: windows_available = 0,
largest_block = 400000, page_size = 3, migration_capable = 3
[ 3.473332] mlx5_core 4005:01:00.0: ibm,query-pe-dma-windows(53)
10000 8000000 29004005 returned 0
[ 3.473345] mlx5_core 4005:01:00.0: windows_available = 1,
largest_block = 400000, page_size = 3, migration_capable = 3
>
>
> > - /*
> > - * no additional windows are available for this device.
> > - * We might be able to reallocate the existing window,
> > - * trading in for a larger page size.
> > - */
> > - dev_dbg(&dev->dev, "no free dynamic windows");
> > - goto out_failed;
> > + default_win = of_find_property(pdn, "ibm,dma-window", NULL);
> > + ddw_ext = of_find_property(pdn, "ibm,ddw-extensions", NULL);
> > + if (default_win && ddw_ext)
> > + remove_dma_window(pdn, ddw_avail, default_win);
> > +
> > + /* Query again, to check if the window is available */
> > + ret = query_ddw(dev, ddw_avail, &query, pdn);
> > + if (ret != 0)
> > + goto out_failed;
> > +
> > + if (query.windows_available == 0) {
> > + /* no windows are available for this device. */
> > + dev_dbg(&dev->dev, "no free dynamic windows");
> > + goto out_failed;
> > + }
> > }
> > +
>
> Unrelated new empty line. Thanks,
Fixed!
Thank you!
>
> > if (query.page_size & 4) {
> > page_shift = 24; /* 16MB */
> > } else if (query.page_size & 2) {
> > @@ -1194,6 +1244,8 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
> > kfree(win64);
> >
> > out_failed:
> > + if (default_win && ddw_ext)
> > + reset_dma_window(dev, pdn);
> >
> > fpdn = kzalloc(sizeof(*fpdn), GFP_KERNEL);
> > if (!fpdn)
> >
^ permalink raw reply
* Memory: 880608K/983040K .... 36896K reserved ?
From: Joakim Tjernlund @ 2020-07-01 19:00 UTC (permalink / raw)
To: linuxppc-dev@ozlabs.org
I cannot figure out how the xxxK reserved item works in:
Memory: 880608K/983040K available (9532K kernel code, 1104K rwdata, 3348K rodata, 1088K init, 1201K bss, 36896K reserved ...
Is there a way to tune(lower it) this memory?
Jocke
^ permalink raw reply
* Re: [PATCH 01/11] kexec_file: allow archs to handle special regions while locating memory hole
From: Hari Bathini @ 2020-07-01 18:31 UTC (permalink / raw)
To: Dave Young
Cc: Thiago Jung Bauermann, Pingfan Liu, Petr Tesarik, Kexec-ml,
Mahesh J Salgaonkar, Mimi Zohar, lkml, linuxppc-dev, Sourabh Jain,
Andrew Morton, Vivek Goyal, Eric Biederman
In-Reply-To: <20200701074659.GA3878@dhcp-128-65.nay.redhat.com>
On 01/07/20 1:16 pm, Dave Young wrote:
> On 06/29/20 at 05:26pm, Hari Bathini wrote:
>> Hi Petr,
>>
>> On 29/06/20 5:09 pm, Petr Tesarik wrote:
>>> Hi Hari,
>>>
>>> is there any good reason to add two more functions with a very similar
>>> name to an existing function? AFAICS all you need is a way to call a
>>> PPC64-specific function from within kexec_add_buffer (PATCH 4/11), so
>>> you could add something like this:
>>>
>>> int __weak arch_kexec_locate_mem_hole(struct kexec_buf *kbuf)
>>> {
>>> return 0;
>>> }
>>>
>>> Call this function from kexec_add_buffer where appropriate and then
>>> override it for PPC64 (it roughly corresponds to your
>>> kexec_locate_mem_hole_ppc64() from PATCH 4/11).
>>>
>>> FWIW it would make it easier for me to follow the resulting code.
>>
>> Right, Petr.
>>
>> I was trying out a few things before I ended up with what I sent here.
>> Bu yeah.. I did realize arch_kexec_locate_mem_hole() would have been better
>> after sending out v1. Will take care of that in v2.
>
> Another way is use arch private function to locate mem hole, then set
> kbuf->mem, and then call kexec_add_buf, it will skip the common locate
> hole function.
Dave, I did think about it. But there are a couple of places this can get
tricky. One is ima_add_kexec_buffer() and the other is kexec_elf_load().
These call sites could be updated to set kbuf->mem before kexec_add_buffer().
But the current approach seemed like the better option for it creates a
single point of control in setting up segment buffers and also, makes adding
any new segments simpler, arch-specific segments or otherwise.
Thanks
Hari
^ permalink raw reply
* Re: [PATCH 10/20] dm: stop using ->queuedata
From: Mike Snitzer @ 2020-07-01 17:24 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, linux-xtensa, linux-s390, linux-m68k, linux-nvdimm,
dm-devel, linux-nvme, linux-kernel, linux-raid, linux-bcache,
linuxppc-dev, drbd-dev
In-Reply-To: <20200701085947.3354405-11-hch@lst.de>
On Wed, Jul 01 2020 at 4:59am -0400,
Christoph Hellwig <hch@lst.de> wrote:
> Instead of setting up the queuedata as well just use one private data
> field.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Acked-by: Mike Snitzer <snitzer@redhat.com>
^ permalink raw reply
* Re: [PATCH 04/11] ppc64/kexec_file: avoid stomping memory used by special regions
From: Hari Bathini @ 2020-07-01 18:18 UTC (permalink / raw)
To: Dave Young
Cc: Pingfan Liu, Kexec-ml, Petr Tesarik, lkml, Sourabh Jain,
Mahesh J Salgaonkar, linuxppc-dev, Vivek Goyal, Andrew Morton,
Mimi Zohar, Thiago Jung Bauermann, Eric Biederman
In-Reply-To: <20200701074012.GA4496@dhcp-128-65.nay.redhat.com>
On 01/07/20 1:10 pm, Dave Young wrote:
> Hi Hari,
> On 06/27/20 at 12:35am, Hari Bathini wrote:
>> crashkernel region could have an overlap with special memory regions
>> like opal, rtas, tce-table & such. These regions are referred to as
>> exclude memory ranges. Setup this ranges during image probe in order
>> to avoid them while finding the buffer for different kdump segments.
>> Implement kexec_locate_mem_hole_ppc64() that locates a memory hole
>> accounting for these ranges. Also, override arch_kexec_add_buffer()
>> to locate a memory hole & later call __kexec_add_buffer() function
>> with kbuf->mem set to skip the generic locate memory hole lookup.
>>
>> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
>> ---
>> arch/powerpc/include/asm/crashdump-ppc64.h | 10 +
>> arch/powerpc/include/asm/kexec.h | 7 -
>> arch/powerpc/kexec/elf_64.c | 7 +
>> arch/powerpc/kexec/file_load_64.c | 292 ++++++++++++++++++++++++++++
>> 4 files changed, 312 insertions(+), 4 deletions(-)
>> create mode 100644 arch/powerpc/include/asm/crashdump-ppc64.h
>>
> [snip]
>> /**
>> + * get_exclude_memory_ranges - Get exclude memory ranges. This list includes
>> + * regions like opal/rtas, tce-table, initrd,
>> + * kernel, htab which should be avoided while
>> + * setting up kexec load segments.
>> + * @mem_ranges: Range list to add the memory ranges to.
>> + *
>> + * Returns 0 on success, negative errno on error.
>> + */
>> +static int get_exclude_memory_ranges(struct crash_mem **mem_ranges)
>> +{
>> + int ret;
>> +
>> + ret = add_tce_mem_ranges(mem_ranges);
>> + if (ret)
>> + goto out;
>> +
>> + ret = add_initrd_mem_range(mem_ranges);
>> + if (ret)
>> + goto out;
>> +
>> + ret = add_htab_mem_range(mem_ranges);
>> + if (ret)
>> + goto out;
>> +
>> + ret = add_kernel_mem_range(mem_ranges);
>> + if (ret)
>> + goto out;
>> +
>> + ret = add_rtas_mem_range(mem_ranges, false);
>> + if (ret)
>> + goto out;
>> +
>> + ret = add_opal_mem_range(mem_ranges, false);
>> + if (ret)
>> + goto out;
>> +
>> + ret = add_reserved_ranges(mem_ranges);
>> + if (ret)
>> + goto out;
>> +
>> + /* exclude memory ranges should be sorted for easy lookup */
>> + sort_memory_ranges(*mem_ranges);
>> +out:
>> + if (ret)
>> + pr_err("Failed to setup exclude memory ranges\n");
>> + return ret;
>> +}
>
> I'm confused about the "overlap with crashkernel memory", does that mean
> those normal kernel used memory could be put in crashkernel reserved
There are regions that could overlap with crashkernel region but they are
not normal kernel used memory though. These are regions that kernel and/or
f/w chose to place at a particular address for real mode accessibility
and/or memory layout between kernel & f/w kind of thing.
> memory range? If so why can't just skip those areas while crashkernel
> doing the reservation?
crashkernel region has a dependency to be in the first memory block for it
to be accessible in real mode. Accommodating this requirement while addressing
other requirements would mean something like what we have now. A list of
possible special memory regions in crashkernel region to take care of.
I have plans to split crashkernel region into low & high to have exclusive
regions for crashkernel, even if that means to have two of them. But that
is for another day with its own set of complexities to deal with...
Thanks
Hari
^ permalink raw reply
* Re: [PATCH 16/20] block: move ->make_request_fn to struct block_device_operations
From: Dan Williams @ 2020-07-01 16:08 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, linux-xtensa, linux-nvdimm, linux-s390, linux-m68k,
linux-nvme, Linux Kernel Mailing List, linux-raid,
device-mapper development, linux-bcache, linuxppc-dev, drbd-dev
In-Reply-To: <20200701085947.3354405-17-hch@lst.de>
On Wed, Jul 1, 2020 at 2:01 AM Christoph Hellwig <hch@lst.de> wrote:
>
> The make_request_fn is a little weird in that it sits directly in
> struct request_queue instead of an operation vector. Replace it with
> a block_device_operations method called submit_bio (which describes much
> better what it does). Also remove the request_queue argument to it, as
> the queue can be derived pretty trivially from the bio.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
[..]
> drivers/nvdimm/blk.c | 5 +-
> drivers/nvdimm/btt.c | 5 +-
> drivers/nvdimm/pmem.c | 5 +-
For drivers/nvdimm
Acked-by: Dan Williams <dan.j.williams@intel.com>
^ permalink raw reply
* Re: [PATCH v2 2/6] powerpc/pseries/iommu: Update call to ibm,query-pe-dma-windows
From: Leonardo Bras @ 2020-07-01 14:04 UTC (permalink / raw)
To: Alexey Kardashevskiy, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Thiago Jung Bauermann, Ram Pai
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <5411e8a1-02a3-1287-40bf-ccc9db7a4f88@ozlabs.ru>
On Wed, 2020-07-01 at 18:17 +1000, Alexey Kardashevskiy wrote:
>
> > +#define DDW_EXT_SIZE 0
> > +#define DDW_EXT_RESET_DMA_WIN 1
> > +#define DDW_EXT_QUERY_OUT_SIZE 2
>
> #define DDW_EXT_LAST (DDW_EXT_QUERY_OUT_SIZE + 1)
> ...
>
>
> > +
> > static struct iommu_table_group *iommu_pseries_alloc_group(int node)
> > {
> > struct iommu_table_group *table_group;
> > @@ -339,7 +343,7 @@ struct direct_window {
> > /* Dynamic DMA Window support */
> > struct ddw_query_response {
> > u32 windows_available;
> > - u32 largest_available_block;
> > + u64 largest_available_block;
> > u32 page_size;
> > u32 migration_capable;
> > };
> > @@ -875,13 +879,29 @@ static int find_existing_ddw_windows(void)
> > machine_arch_initcall(pseries, find_existing_ddw_windows);
> >
> > static int query_ddw(struct pci_dev *dev, const u32 *ddw_avail,
> > - struct ddw_query_response *query)
> > + struct ddw_query_response *query,
> > + struct device_node *parent)
> > {
> > struct device_node *dn;
> > struct pci_dn *pdn;
> > - u32 cfg_addr;
> > + u32 cfg_addr, query_out[5], ddw_ext[DDW_EXT_QUERY_OUT_SIZE + 1];
>
> ... and use DDW_EXT_LAST here.
Because of the growing nature of ddw-extensions, I intentionally let
this be (DDW_EXT_QUERY_OUT_SIZE + 1). If we create a DDW_EXT_LAST, it
will be incremented in the future if more extensions come to exist.
I mean, I previously saw no reason for allocating space for extensions
after the desired one, as they won't be used here.
>
>
> > u64 buid;
> > - int ret;
> > + int ret, out_sz;
> > +
> > + /*
> > + * From LoPAR level 2.8, "ibm,ddw-extensions" index 3 can rule how many
> > + * output parameters ibm,query-pe-dma-windows will have, ranging from
> > + * 5 to 6.
> > + */
> > +
> > + ret = of_property_read_u32_array(parent, "ibm,ddw-extensions",
> > + &ddw_ext[0],
> > + DDW_EXT_QUERY_OUT_SIZE + 1);
In this case, I made sure not to cross (DDW_EXT_QUERY_OUT_SIZE + 1)
while reading the extensions from the property.
What do you think about it?
Best regards,
Leonardo
^ permalink raw reply
* Re: rename ->make_request_fn and move it to the block_device_operations v2
From: Jens Axboe @ 2020-07-01 13:43 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-bcache, linux-xtensa, linux-nvdimm, linux-s390, dm-devel,
linux-nvme, linux-kernel, linux-raid, linux-m68k, linuxppc-dev,
drbd-dev
In-Reply-To: <20200701085947.3354405-1-hch@lst.de>
On 7/1/20 2:59 AM, Christoph Hellwig wrote:
> Hi Jens,
>
> this series moves the make_request_fn method into block_device_operations
> with the much more descriptive ->submit_bio name. It then also gives
> generic_make_request a more descriptive name, and further optimize the
> path to issue to blk-mq, removing the need for the direct_make_request
> bypass.
Thanks, I'll try this again.
--
Jens Axboe
^ permalink raw reply
* [PATCH v2 2/2] powerpc/papr_scm: Add support for fetching nvdimm 'fuel-gauge' metric
From: Vaibhav Jain @ 2020-07-01 13:35 UTC (permalink / raw)
To: linuxppc-dev, linux-nvdimm
Cc: Santosh Sivaraj, Oliver O'Halloran, Aneesh Kumar K . V,
Vaibhav Jain, Dan Williams, Ira Weiny
In-Reply-To: <20200701133510.4613-1-vaibhav@linux.ibm.com>
We add support for reporting 'fuel-gauge' NVDIMM metric via
PAPR_PDSM_HEALTH pdsm payload. 'fuel-gauge' metric indicates the usage
life remaining of a papr-scm compatible NVDIMM. PHYP exposes this
metric via the H_SCM_PERFORMANCE_STATS.
The metric value is returned from the pdsm by extending the return
payload 'struct nd_papr_pdsm_health' without breaking the ABI. A new
field 'dimm_fuel_gauge' to hold the metric value is introduced at the
end of the payload struct and its presence is indicated by by
extension flag PDSM_DIMM_HEALTH_RUN_GAUGE_VALID.
The patch introduces a new function papr_pdsm_fuel_gauge() that is
called from papr_pdsm_health(). If fetching NVDIMM performance stats
is supported then 'papr_pdsm_fuel_gauge()' allocated an output buffer
large enough to hold the performance stat and passes it to
drc_pmem_query_stats() that issues the HCALL to PHYP. The return value
of the stat is then populated in the 'struct
nd_papr_pdsm_health.dimm_fuel_gauge' field with extension flag
'PDSM_DIMM_HEALTH_RUN_GAUGE_VALID' set in 'struct
nd_papr_pdsm_health.extension_flags'
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
Changelog:
v2:
* Restructure code in papr_pdsm_fuel_gauge() to handle error case
first [ Ira ]
* Ignore the return value of papr_pdsm_fuel_gauge() in
papr_psdm_health() [ Ira ]
---
arch/powerpc/include/uapi/asm/papr_pdsm.h | 9 +++++
arch/powerpc/platforms/pseries/papr_scm.c | 49 +++++++++++++++++++++++
2 files changed, 58 insertions(+)
diff --git a/arch/powerpc/include/uapi/asm/papr_pdsm.h b/arch/powerpc/include/uapi/asm/papr_pdsm.h
index 9ccecc1d6840..50ef95e2f5b1 100644
--- a/arch/powerpc/include/uapi/asm/papr_pdsm.h
+++ b/arch/powerpc/include/uapi/asm/papr_pdsm.h
@@ -72,6 +72,11 @@
#define PAPR_PDSM_DIMM_CRITICAL 2
#define PAPR_PDSM_DIMM_FATAL 3
+/* struct nd_papr_pdsm_health.extension_flags field flags */
+
+/* Indicate that the 'dimm_fuel_gauge' field is valid */
+#define PDSM_DIMM_HEALTH_RUN_GAUGE_VALID 1
+
/*
* Struct exchanged between kernel & ndctl in for PAPR_PDSM_HEALTH
* Various flags indicate the health status of the dimm.
@@ -84,6 +89,7 @@
* dimm_locked : Contents of the dimm cant be modified until CEC reboot
* dimm_encrypted : Contents of dimm are encrypted.
* dimm_health : Dimm health indicator. One of PAPR_PDSM_DIMM_XXXX
+ * dimm_fuel_gauge : Life remaining of DIMM as a percentage from 0-100
*/
struct nd_papr_pdsm_health {
union {
@@ -96,6 +102,9 @@ struct nd_papr_pdsm_health {
__u8 dimm_locked;
__u8 dimm_encrypted;
__u16 dimm_health;
+
+ /* Extension flag PDSM_DIMM_HEALTH_RUN_GAUGE_VALID */
+ __u16 dimm_fuel_gauge;
};
__u8 buf[ND_PDSM_PAYLOAD_MAX_SIZE];
};
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index bde2433822b2..b02bed8475f9 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -498,6 +498,51 @@ static int is_cmd_valid(struct nvdimm *nvdimm, unsigned int cmd, void *buf,
return 0;
}
+static int papr_pdsm_fuel_gauge(struct papr_scm_priv *p,
+ union nd_pdsm_payload *payload)
+{
+ int rc, size;
+ u64 statval;
+ struct papr_scm_perf_stat *stat;
+ struct papr_scm_perf_stats *stats;
+
+ /* Silently fail if fetching performance metrics isn't supported */
+ if (!p->stat_buffer_len)
+ return 0;
+
+ /* Allocate request buffer enough to hold single performance stat */
+ size = sizeof(struct papr_scm_perf_stats) +
+ sizeof(struct papr_scm_perf_stat);
+
+ stats = kzalloc(size, GFP_KERNEL);
+ if (!stats)
+ return -ENOMEM;
+
+ stat = &stats->scm_statistic[0];
+ memcpy(&stat->stat_id, "MemLife ", sizeof(stat->stat_id));
+ stat->stat_val = 0;
+
+ /* Fetch the fuel gauge and populate it in payload */
+ rc = drc_pmem_query_stats(p, stats, size, 1, NULL);
+ if (rc) {
+ dev_dbg(&p->pdev->dev, "Err(%d) fetching fuel gauge\n", rc);
+ goto free_stats;
+ }
+
+ statval = be64_to_cpu(stat->stat_val);
+ dev_dbg(&p->pdev->dev,
+ "Fetched fuel-gauge %llu", statval);
+ payload->health.extension_flags |=
+ PDSM_DIMM_HEALTH_RUN_GAUGE_VALID;
+ payload->health.dimm_fuel_gauge = statval;
+
+ rc = sizeof(struct nd_papr_pdsm_health);
+
+free_stats:
+ kfree(stats);
+ return rc;
+}
+
/* Fetch the DIMM health info and populate it in provided package. */
static int papr_pdsm_health(struct papr_scm_priv *p,
union nd_pdsm_payload *payload)
@@ -538,6 +583,10 @@ static int papr_pdsm_health(struct papr_scm_priv *p,
/* struct populated hence can release the mutex now */
mutex_unlock(&p->health_mutex);
+
+ /* Populate the fuel gauge meter in the payload */
+ papr_pdsm_fuel_gauge(p, payload);
+
rc = sizeof(struct nd_papr_pdsm_health);
out:
--
2.26.2
^ permalink raw reply related
* [PATCH v2 1/2] powerpc/papr_scm: Fetch nvdimm performance stats from PHYP
From: Vaibhav Jain @ 2020-07-01 13:35 UTC (permalink / raw)
To: linuxppc-dev, linux-nvdimm
Cc: Santosh Sivaraj, Oliver O'Halloran, Aneesh Kumar K . V,
Vaibhav Jain, Dan Williams, Ira Weiny
In-Reply-To: <20200701133510.4613-1-vaibhav@linux.ibm.com>
Update papr_scm.c to query dimm performance statistics from PHYP via
H_SCM_PERFORMANCE_STATS hcall and export them to user-space as PAPR
specific NVDIMM attribute 'perf_stats' in sysfs. The patch also
provide a sysfs ABI documentation for the stats being reported and
their meanings.
During NVDIMM probe time in papr_scm_nvdimm_init() a special variant
of H_SCM_PERFORMANCE_STATS hcall is issued to check if collection of
performance statistics is supported or not. If successful then a PHYP
returns a maximum possible buffer length needed to read all
performance stats. This returned value is stored in a per-nvdimm
attribute 'stat_buffer_len'.
The layout of request buffer for reading NVDIMM performance stats from
PHYP is defined in 'struct papr_scm_perf_stats' and 'struct
papr_scm_perf_stat'. These structs are used in newly introduced
drc_pmem_query_stats() that issues the H_SCM_PERFORMANCE_STATS hcall.
The sysfs access function perf_stats_show() uses value
'stat_buffer_len' to allocate a buffer large enough to hold all
possible NVDIMM performance stats and passes it to
drc_pmem_query_stats() to populate. Finally statistics reported in the
buffer are formatted into the sysfs access function output buffer.
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
Changelog:
v2:
* Fixed a bug in drc_pmem_query_stats() that was passing a NULL pointer
to virt_to_pfn() [ Ira ]
* Updated 'struct papr_scm_perf_stats' and 'struct papr_scm_perf_stat'
to use big-endian types. [ Aneesh ]
* s/len_stat_buffer/stat_buffer_len/ [ Aneesh ]
* s/statistics_id/stat_id/ , s/statistics_val/stat_val/ [ Aneesh ]
* Conversion from Big endian to cpu endian happens later rather than
just after its fetched from PHYP.
* Changed a log statement to unambiguously report dimm performance
stats are not available for the given nvdimm [ Ira ]
* Restructed some code to handle error case first [ Ira ]
---
Documentation/ABI/testing/sysfs-bus-papr-pmem | 27 ++++
arch/powerpc/platforms/pseries/papr_scm.c | 134 ++++++++++++++++++
2 files changed, 161 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-papr-pmem b/Documentation/ABI/testing/sysfs-bus-papr-pmem
index 5b10d036a8d4..c1a67275c43f 100644
--- a/Documentation/ABI/testing/sysfs-bus-papr-pmem
+++ b/Documentation/ABI/testing/sysfs-bus-papr-pmem
@@ -25,3 +25,30 @@ Description:
NVDIMM have been scrubbed.
* "locked" : Indicating that NVDIMM contents cant
be modified until next power cycle.
+
+What: /sys/bus/nd/devices/nmemX/papr/perf_stats
+Date: May, 2020
+KernelVersion: v5.9
+Contact: linuxppc-dev <linuxppc-dev@lists.ozlabs.org>, linux-nvdimm@lists.01.org,
+Description:
+ (RO) Report various performance stats related to papr-scm NVDIMM
+ device. Each stat is reported on a new line with each line
+ composed of a stat-identifier followed by it value. Below are
+ currently known dimm performance stats which are reported:
+
+ * "CtlResCt" : Controller Reset Count
+ * "CtlResTm" : Controller Reset Elapsed Time
+ * "PonSecs " : Power-on Seconds
+ * "MemLife " : Life Remaining
+ * "CritRscU" : Critical Resource Utilization
+ * "HostLCnt" : Host Load Count
+ * "HostSCnt" : Host Store Count
+ * "HostSDur" : Host Store Duration
+ * "HostLDur" : Host Load Duration
+ * "MedRCnt " : Media Read Count
+ * "MedWCnt " : Media Write Count
+ * "MedRDur " : Media Read Duration
+ * "MedWDur " : Media Write Duration
+ * "CchRHCnt" : Cache Read Hit Count
+ * "CchWHCnt" : Cache Write Hit Count
+ * "FastWCnt" : Fast Write Count
\ No newline at end of file
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index 9c569078a09f..bde2433822b2 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -62,6 +62,26 @@
PAPR_PMEM_HEALTH_FATAL | \
PAPR_PMEM_HEALTH_UNHEALTHY)
+#define PAPR_SCM_PERF_STATS_EYECATCHER __stringify(SCMSTATS)
+#define PAPR_SCM_PERF_STATS_VERSION 0x1
+
+/* Struct holding a single performance metric */
+struct papr_scm_perf_stat {
+ u8 stat_id[8];
+ __be64 stat_val;
+} __packed;
+
+/* Struct exchanged between kernel and PHYP for fetching drc perf stats */
+struct papr_scm_perf_stats {
+ u8 eye_catcher[8];
+ /* Should be PAPR_SCM_PERF_STATS_VERSION */
+ __be32 stats_version;
+ /* Number of stats following */
+ __be32 num_statistics;
+ /* zero or more performance matrics */
+ struct papr_scm_perf_stat scm_statistic[];
+} __packed;
+
/* private struct associated with each region */
struct papr_scm_priv {
struct platform_device *pdev;
@@ -89,6 +109,9 @@ struct papr_scm_priv {
/* Health information for the dimm */
u64 health_bitmap;
+
+ /* length of the stat buffer as expected by phyp */
+ size_t stat_buffer_len;
};
static int drc_pmem_bind(struct papr_scm_priv *p)
@@ -194,6 +217,65 @@ static int drc_pmem_query_n_bind(struct papr_scm_priv *p)
return drc_pmem_bind(p);
}
+/*
+ * Query the Dimm performance stats from PHYP and copy them (if returned) to
+ * provided struct papr_scm_perf_stats instance 'stats' of 'size' in bytes.
+ * The value of R4 is copied to 'out' if the pointer is provided.
+ */
+static int drc_pmem_query_stats(struct papr_scm_priv *p,
+ struct papr_scm_perf_stats *buff_stats,
+ size_t size, unsigned int num_stats,
+ uint64_t *out)
+{
+ unsigned long ret[PLPAR_HCALL_BUFSIZE];
+ s64 rc;
+
+ /* Setup the out buffer */
+ if (buff_stats) {
+ memcpy(buff_stats->eye_catcher,
+ PAPR_SCM_PERF_STATS_EYECATCHER, 8);
+ buff_stats->stats_version =
+ cpu_to_be32(PAPR_SCM_PERF_STATS_VERSION);
+ buff_stats->num_statistics =
+ cpu_to_be32(num_stats);
+ } else {
+ /* In case of no out buffer ignore the size */
+ size = 0;
+ }
+
+ /*
+ * Do the HCALL asking PHYP for info and if R4 was requested
+ * return its value in 'out' variable.
+ */
+ rc = plpar_hcall(H_SCM_PERFORMANCE_STATS, ret, p->drc_index,
+ buff_stats ? virt_to_phys(buff_stats) : 0,
+ size);
+ if (out)
+ *out = ret[0];
+
+ if (rc == H_PARTIAL) {
+ dev_err(&p->pdev->dev,
+ "Unknown performance stats, Err:0x%016lX\n", ret[0]);
+ return -ENOENT;
+ } else if (rc != H_SUCCESS) {
+ dev_err(&p->pdev->dev,
+ "Failed to query performance stats, Err:%lld\n", rc);
+ return -EIO;
+ }
+
+ /* Successfully fetched the requested stats from phyp */
+ if (size != 0)
+ dev_dbg(&p->pdev->dev,
+ "Performance stats returned %d stats\n",
+ be32_to_cpu(buff_stats->num_statistics));
+ else
+ /* Handle case where stat buffer size was requested */
+ dev_dbg(&p->pdev->dev,
+ "Performance stats size %ld\n", ret[0]);
+
+ return 0;
+}
+
/*
* Issue hcall to retrieve dimm health info and populate papr_scm_priv with the
* health information.
@@ -631,6 +713,48 @@ static int papr_scm_ndctl(struct nvdimm_bus_descriptor *nd_desc,
return 0;
}
+static ssize_t perf_stats_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int index, rc;
+ struct seq_buf s;
+ struct papr_scm_perf_stat *stat;
+ struct papr_scm_perf_stats *stats;
+ struct nvdimm *dimm = to_nvdimm(dev);
+ struct papr_scm_priv *p = nvdimm_provider_data(dimm);
+
+ if (!p->stat_buffer_len)
+ return -ENOENT;
+
+ /* Allocate the buffer for phyp where stats are written */
+ stats = kzalloc(p->stat_buffer_len, GFP_KERNEL);
+ if (!stats)
+ return -ENOMEM;
+
+ /* Ask phyp to return all dimm perf stats */
+ rc = drc_pmem_query_stats(p, stats, p->stat_buffer_len, 0, NULL);
+ if (rc)
+ goto free_stats;
+ /*
+ * Go through the returned output buffer and print stats and
+ * values. Since stat_id is essentially a char string of
+ * 8 bytes, simply use the string format specifier to print it.
+ */
+ seq_buf_init(&s, buf, PAGE_SIZE);
+ for (index = 0, stat = stats->scm_statistic;
+ index < be32_to_cpu(stats->num_statistics);
+ ++index, ++stat) {
+ seq_buf_printf(&s, "%.8s = 0x%016llX\n",
+ stat->stat_id,
+ be64_to_cpu(stat->stat_val));
+ }
+
+free_stats:
+ kfree(stats);
+ return rc ? rc : seq_buf_used(&s);
+}
+DEVICE_ATTR_RO(perf_stats);
+
static ssize_t flags_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -676,6 +800,7 @@ DEVICE_ATTR_RO(flags);
/* papr_scm specific dimm attributes */
static struct attribute *papr_nd_attributes[] = {
&dev_attr_flags.attr,
+ &dev_attr_perf_stats.attr,
NULL,
};
@@ -696,6 +821,7 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
struct nd_region_desc ndr_desc;
unsigned long dimm_flags;
int target_nid, online_nid;
+ u64 stat_size;
p->bus_desc.ndctl = papr_scm_ndctl;
p->bus_desc.module = THIS_MODULE;
@@ -759,6 +885,14 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
dev_info(dev, "Region registered with target node %d and online node %d",
target_nid, online_nid);
+ /* Try retriving the stat buffer and see if its supported */
+ if (!drc_pmem_query_stats(p, NULL, 0, 0, &stat_size)) {
+ p->stat_buffer_len = (size_t)stat_size;
+ dev_dbg(&p->pdev->dev, "Max perf-stat size %lu-bytes\n",
+ p->stat_buffer_len);
+ } else {
+ dev_info(&p->pdev->dev, "Dimm performance stats unavailable\n");
+ }
return 0;
err: nvdimm_bus_unregister(p->bus);
--
2.26.2
^ permalink raw reply related
* [PATCH v2 0/2] powerpc/papr_scm: add support for reporting NVDIMM 'life_used_percentage' metric
From: Vaibhav Jain @ 2020-07-01 13:35 UTC (permalink / raw)
To: linuxppc-dev, linux-nvdimm
Cc: Santosh Sivaraj, Oliver O'Halloran, Aneesh Kumar K . V,
Vaibhav Jain, Dan Williams, Ira Weiny
Changes since v1 [1]:
* Minor restructuring of code as suggested by Ira
* Renaming of few members of 'struct par_scm_perf_[stat|stats]'
* Fixed a bug where a NULL pointer was potentially passed to
virt_to_phys().
* Using Big endian type rather than cpu native type so receive data
from PHYP in 'struct par_scm_perf_[stat|stats]'
* Some minor log message improvements.
[1] https://lore.kernel.org/linux-nvdimm/20200622042451.22448-1-vaibhav@linux.ibm.com
---
This small patchset implements kernel side support for reporting
'life_used_percentage' metric in NDCTL with dimm health output for
papr-scm NVDIMMs. With corresponding NDCTL side changes [2] output for
should be like:
$ sudo ndctl list -DH
[
{
"dev":"nmem0",
"health":{
"health_state":"ok",
"life_used_percentage":0,
"shutdown_state":"clean"
}
}
]
PHYP supports H_SCM_PERFORMANCE_STATS hcall through which an LPAR can
fetch various performance stats including 'fuel_gauge' percentage for
an NVDIMM. 'fuel_gauge' metric indicates the usable life remaining of
an NVDIMM expressed as percentage and 'life_used_percentage' can be
calculated as 'life_used_percentage = 100 - fuel_gauge'.
Structure of the patchset
=========================
First patch implements necessary scaffolding needed to issue the
H_SCM_PERFORMANCE_STATS hcall and fetch performance stats
catalogue. The patch also implements support for 'perf_stats' sysfs
attribute to report the full catalogue of supported performance stats
by PHYP.
Second and final patch implements support for sending this value to
libndctl by extending the PAPR_PDSM_HEALTH pdsm payload to add a new
field named 'dimm_fuel_gauge' to it.
References
==========
[2]
https://github.com/vaibhav92/ndctl/tree/papr_scm_health_v13_run_guage
Vaibhav Jain (2):
powerpc/papr_scm: Fetch nvdimm performance stats from PHYP
powerpc/papr_scm: Add support for fetching nvdimm 'fuel-gauge' metric
Documentation/ABI/testing/sysfs-bus-papr-pmem | 27 +++
arch/powerpc/include/uapi/asm/papr_pdsm.h | 9 +
arch/powerpc/platforms/pseries/papr_scm.c | 183 ++++++++++++++++++
3 files changed, 219 insertions(+)
--
2.26.2
^ permalink raw reply
* Re: [PATCH v2 30/30] misc: cxl: flash: Remove unused pointer
From: Lee Jones @ 2020-07-01 13:31 UTC (permalink / raw)
To: Greg KH; +Cc: Frederic Barrat, linuxppc-dev, linux-kernel, arnd,
Andrew Donnellan
In-Reply-To: <20200701131357.GA2298198@kroah.com>
On Wed, 01 Jul 2020, Greg KH wrote:
> On Wed, Jul 01, 2020 at 09:31:18AM +0100, Lee Jones wrote:
> > The DRC index pointer us updated on an OPCODE_ADD, but never
> > actually read. Remove the used pointer and shift up OPCODE_ADD
> > to group with OPCODE_DELETE which also provides a noop.
> >
> > Fixes the following W=1 kernel build warning:
> >
> > drivers/misc/cxl/flash.c: In function ‘update_devicetree’:
> > drivers/misc/cxl/flash.c:178:16: warning: variable ‘drc_index’ set but not used [-Wunused-but-set-variable]
> > 178 | __be32 *data, drc_index, phandle;
> > | ^~~~~~~~~
> >
> > Cc: Frederic Barrat <fbarrat@linux.ibm.com>
> > Cc: Andrew Donnellan <ajd@linux.ibm.com>
> > Cc: linuxppc-dev@lists.ozlabs.org
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> > drivers/misc/cxl/flash.c | 7 ++-----
> > 1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/misc/cxl/flash.c b/drivers/misc/cxl/flash.c
> > index cb9cca35a2263..24e3dfcc91a74 100644
> > --- a/drivers/misc/cxl/flash.c
> > +++ b/drivers/misc/cxl/flash.c
> > @@ -175,7 +175,7 @@ static int update_devicetree(struct cxl *adapter, s32 scope)
> > struct update_nodes_workarea *unwa;
> > u32 action, node_count;
> > int token, rc, i;
> > - __be32 *data, drc_index, phandle;
> > + __be32 *data, phandle;
> > char *buf;
> >
> > token = rtas_token("ibm,update-nodes");
> > @@ -206,15 +206,12 @@ static int update_devicetree(struct cxl *adapter, s32 scope)
> >
> > switch (action) {
> > case OPCODE_DELETE:
> > + case OPCODE_ADD:
> > /* nothing to do */
> > break;
> > case OPCODE_UPDATE:
> > update_node(phandle, scope);
> > break;
> > - case OPCODE_ADD:
> > - /* nothing to do, just move pointer */
> > - drc_index = *data++;
> > - break;
>
> I think this is not correct, as *data++ changes the value there, and so
> this changes the logic of the code.
Great spot.
Looks like I overlooked that the pointer itself is being incremented.
> Dropping this one from my queue.
Sounds good.
I'll fix-up and send this separately at a later date.
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH v2 1/6] powerpc/pseries/iommu: Create defines for operations in ibm,ddw-applicable
From: Leonardo Bras @ 2020-07-01 13:28 UTC (permalink / raw)
To: Alexey Kardashevskiy, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Thiago Jung Bauermann, Ram Pai
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <b0caaaa0-14c9-51de-bb92-5be8ccaa418d@ozlabs.ru>
On Wed, 2020-07-01 at 18:16 +1000, Alexey Kardashevskiy wrote:
>
> On 24/06/2020 16:24, Leonardo Bras wrote:
> > Create defines to help handling ibm,ddw-applicable values, avoiding
> > confusion about the index of given operations.
> >
> > Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
> > ---
> > arch/powerpc/platforms/pseries/iommu.c | 40 +++++++++++++++-----------
> > 1 file changed, 23 insertions(+), 17 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
> > index 6d47b4a3ce39..68d2aa9c71a8 100644
> > --- a/arch/powerpc/platforms/pseries/iommu.c
> > +++ b/arch/powerpc/platforms/pseries/iommu.c
> > @@ -39,6 +39,11 @@
> >
> > #include "pseries.h"
> >
> > +#define DDW_QUERY_PE_DMA_WIN 0
> > +#define DDW_CREATE_PE_DMA_WIN 1
> > +#define DDW_REMOVE_PE_DMA_WIN 2
> > +#define DDW_APPLICABLE_SIZE 3
>
> #define DDW_APPLICABLE_SIZE (DDW_REMOVE_PE_DMA_WIN + 1)
>
> thanks,
Thanks for the feedback!
About this (and patch #2), would it be better to use enum ?
enum {
DDW_QUERY_PE_DMA_WIN,
DDW_CREATE_PE_DMA_WIN,
DDW_REMOVE_PE_DMA_WIN,
DDW_APPLICABLE_SIZE
}
IMO, it looks better than all the defines before.
What do you think?
Best regards,
^ permalink raw reply
* Re: [PATCH v2 30/30] misc: cxl: flash: Remove unused pointer
From: Greg KH @ 2020-07-01 13:13 UTC (permalink / raw)
To: Lee Jones
Cc: Frederic Barrat, linuxppc-dev, linux-kernel, arnd,
Andrew Donnellan
In-Reply-To: <20200701083118.45744-31-lee.jones@linaro.org>
On Wed, Jul 01, 2020 at 09:31:18AM +0100, Lee Jones wrote:
> The DRC index pointer us updated on an OPCODE_ADD, but never
> actually read. Remove the used pointer and shift up OPCODE_ADD
> to group with OPCODE_DELETE which also provides a noop.
>
> Fixes the following W=1 kernel build warning:
>
> drivers/misc/cxl/flash.c: In function ‘update_devicetree’:
> drivers/misc/cxl/flash.c:178:16: warning: variable ‘drc_index’ set but not used [-Wunused-but-set-variable]
> 178 | __be32 *data, drc_index, phandle;
> | ^~~~~~~~~
>
> Cc: Frederic Barrat <fbarrat@linux.ibm.com>
> Cc: Andrew Donnellan <ajd@linux.ibm.com>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
> drivers/misc/cxl/flash.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/misc/cxl/flash.c b/drivers/misc/cxl/flash.c
> index cb9cca35a2263..24e3dfcc91a74 100644
> --- a/drivers/misc/cxl/flash.c
> +++ b/drivers/misc/cxl/flash.c
> @@ -175,7 +175,7 @@ static int update_devicetree(struct cxl *adapter, s32 scope)
> struct update_nodes_workarea *unwa;
> u32 action, node_count;
> int token, rc, i;
> - __be32 *data, drc_index, phandle;
> + __be32 *data, phandle;
> char *buf;
>
> token = rtas_token("ibm,update-nodes");
> @@ -206,15 +206,12 @@ static int update_devicetree(struct cxl *adapter, s32 scope)
>
> switch (action) {
> case OPCODE_DELETE:
> + case OPCODE_ADD:
> /* nothing to do */
> break;
> case OPCODE_UPDATE:
> update_node(phandle, scope);
> break;
> - case OPCODE_ADD:
> - /* nothing to do, just move pointer */
> - drc_index = *data++;
> - break;
I think this is not correct, as *data++ changes the value there, and so
this changes the logic of the code.
Dropping this one from my queue.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 04/11] ppc64/kexec_file: avoid stomping memory used by special regions
From: piliu @ 2020-07-01 12:53 UTC (permalink / raw)
To: Dave Young, Hari Bathini
Cc: Kexec-ml, lkml, Petr Tesarik, Mahesh J Salgaonkar, linuxppc-dev,
Sourabh Jain, Vivek Goyal, Andrew Morton, Mimi Zohar,
Thiago Jung Bauermann, Eric Biederman
In-Reply-To: <20200701074012.GA4496@dhcp-128-65.nay.redhat.com>
On 07/01/2020 03:40 PM, Dave Young wrote:
> Hi Hari,
> On 06/27/20 at 12:35am, Hari Bathini wrote:
>> crashkernel region could have an overlap with special memory regions
>> like opal, rtas, tce-table & such. These regions are referred to as
>> exclude memory ranges. Setup this ranges during image probe in order
>> to avoid them while finding the buffer for different kdump segments.
>> Implement kexec_locate_mem_hole_ppc64() that locates a memory hole
>> accounting for these ranges. Also, override arch_kexec_add_buffer()
>> to locate a memory hole & later call __kexec_add_buffer() function
>> with kbuf->mem set to skip the generic locate memory hole lookup.
>>
>> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
>> ---
>> arch/powerpc/include/asm/crashdump-ppc64.h | 10 +
>> arch/powerpc/include/asm/kexec.h | 7 -
>> arch/powerpc/kexec/elf_64.c | 7 +
>> arch/powerpc/kexec/file_load_64.c | 292 ++++++++++++++++++++++++++++
>> 4 files changed, 312 insertions(+), 4 deletions(-)
>> create mode 100644 arch/powerpc/include/asm/crashdump-ppc64.h
>>
> [snip]
>> /**
>> + * get_exclude_memory_ranges - Get exclude memory ranges. This list includes
>> + * regions like opal/rtas, tce-table, initrd,
>> + * kernel, htab which should be avoided while
>> + * setting up kexec load segments.
>> + * @mem_ranges: Range list to add the memory ranges to.
>> + *
>> + * Returns 0 on success, negative errno on error.
>> + */
>> +static int get_exclude_memory_ranges(struct crash_mem **mem_ranges)
>> +{
>> + int ret;
>> +
>> + ret = add_tce_mem_ranges(mem_ranges);
>> + if (ret)
>> + goto out;
>> +
>> + ret = add_initrd_mem_range(mem_ranges);
>> + if (ret)
>> + goto out;
>> +
>> + ret = add_htab_mem_range(mem_ranges);
>> + if (ret)
>> + goto out;
>> +
>> + ret = add_kernel_mem_range(mem_ranges);
>> + if (ret)
>> + goto out;
>> +
>> + ret = add_rtas_mem_range(mem_ranges, false);
>> + if (ret)
>> + goto out;
>> +
>> + ret = add_opal_mem_range(mem_ranges, false);
>> + if (ret)
>> + goto out;
>> +
>> + ret = add_reserved_ranges(mem_ranges);
>> + if (ret)
>> + goto out;
>> +
>> + /* exclude memory ranges should be sorted for easy lookup */
>> + sort_memory_ranges(*mem_ranges);
>> +out:
>> + if (ret)
>> + pr_err("Failed to setup exclude memory ranges\n");
>> + return ret;
>> +}
>
> I'm confused about the "overlap with crashkernel memory", does that mean
> those normal kernel used memory could be put in crashkernel reserved
> memory range? If so why can't just skip those areas while crashkernel
> doing the reservation?
I raised the same question in another mail. As Hari's answer, "kexec -p"
skips these ranges in user space. And the same logic should be done in
"kexec -s -p"
Regards,
Pingfan
^ permalink raw reply
* Re: [PATCH v5 3/3] mm/page_alloc: Keep memoryless cpuless node 0 offline
From: Michal Hocko @ 2020-07-01 12:23 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Gautham R Shenoy, David Hildenbrand, Linus Torvalds, linux-kernel,
linux-mm, Satheesh Rajendran, Mel Gorman, Kirill A. Shutemov,
Christopher Lameter, linuxppc-dev, Andrew Morton, Vlastimil Babka
In-Reply-To: <20200630040125.GA31617@linux.vnet.ibm.com>
On Tue 30-06-20 09:31:25, Srikar Dronamraju wrote:
> * Christopher Lameter <cl@linux.com> [2020-06-29 14:58:40]:
>
> > On Wed, 24 Jun 2020, Srikar Dronamraju wrote:
> >
> > > 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.
> >
> > Maybe add something to explain why you are not simply mapping the
> > existing memory to NUMA node 0 which is after all just a numbering scheme
> > used by the kernel and can be used arbitrarily?
> >
>
> I thought Michal Hocko already gave a clear picture on why mapping is a bad
> idea. https://lore.kernel.org/lkml/20200316085425.GB11482@dhcp22.suse.cz/t/#u
> Are you suggesting that we add that as part of the changelog?
Well, I was not aware x86 already does renumber. So there is a certain
precendence. As I've said I do not really like that but this is what
already is happening. If renumbering is not an option then just handle
that in the ppc code explicitly. Generic solution would be preferable of
course but as I've said it is really hard to check for correctness and
potential subtle issues.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH v5 3/3] mm/page_alloc: Keep memoryless cpuless node 0 offline
From: Michal Hocko @ 2020-07-01 12:21 UTC (permalink / raw)
To: David Hildenbrand
Cc: Gautham R Shenoy, Srikar Dronamraju, Linus Torvalds, linux-kernel,
linux-mm, Satheesh Rajendran, Mel Gorman, Kirill A. Shutemov,
Andrew Morton, linuxppc-dev, Christopher Lameter, Vlastimil Babka
In-Reply-To: <12945273-d788-710d-e8d7-974966529c7d@redhat.com>
On Wed 01-07-20 13:30:57, David Hildenbrand wrote:
> On 01.07.20 13:06, David Hildenbrand wrote:
> > On 01.07.20 13:01, Srikar Dronamraju wrote:
> >> * David Hildenbrand <david@redhat.com> [2020-07-01 12:15:54]:
> >>
> >>> On 01.07.20 12:04, Srikar Dronamraju wrote:
> >>>> * Michal Hocko <mhocko@kernel.org> [2020-07-01 10:42:00]:
> >>>>
> >>>>>
> >>>>>>
> >>>>>> 2. Also existence of dummy node also leads to inconsistent information. The
> >>>>>> number of online nodes is inconsistent with the information in the
> >>>>>> device-tree and resource-dump
> >>>>>>
> >>>>>> 3. When the dummy node is present, single node non-Numa systems end up showing
> >>>>>> up as NUMA systems and numa_balancing gets enabled. This will mean we take
> >>>>>> the hit from the unnecessary numa hinting faults.
> >>>>>
> >>>>> I have to say that I dislike the node online/offline state and directly
> >>>>> exporting that to the userspace. Users should only care whether the node
> >>>>> has memory/cpus. Numa nodes can be online without any memory. Just
> >>>>> offline all the present memory blocks but do not physically hot remove
> >>>>> them and you are in the same situation. If users are confused by an
> >>>>> output of tools like numactl -H then those could be updated and hide
> >>>>> nodes without any memory&cpus.
> >>>>>
> >>>>> The autonuma problem sounds interesting but again this patch doesn't
> >>>>> really solve the underlying problem because I strongly suspect that the
> >>>>> problem is still there when a numa node gets all its memory offline as
> >>>>> mentioned above.
I would really appreciate a feedback to these two as well.
> >>>>> While I completely agree that making node 0 special is wrong, I have
> >>>>> still hard time to review this very simply looking patch because all the
> >>>>> numa initialization is so spread around that this might just blow up
> >>>>> at unexpected places. IIRC we have discussed testing in the previous
> >>>>> version and David has provided a way to emulate these configurations
> >>>>> on x86. Did you manage to use those instruction for additional testing
> >>>>> on other than ppc architectures?
> >>>>>
> >>>>
> >>>> I have tried all the steps that David mentioned and reported back at
> >>>> https://lore.kernel.org/lkml/20200511174731.GD1961@linux.vnet.ibm.com/t/#u
> >>>>
> >>>> As a summary, David's steps are still not creating a memoryless/cpuless on
> >>>> x86 VM.
> >>>
> >>> Now, that is wrong. You get a memoryless/cpuless node, which is *not
> >>> online*. Once you hotplug some memory, it will switch online. Once you
> >>> remove memory, it will switch back offline.
> >>>
> >>
> >> Let me clarify, we are looking for a node 0 which is cpuless/memoryless at
> >> boot. The code in question tries to handle a cpuless/memoryless node 0 at
> >> boot.
> >
> > I was just correcting your statement, because it was wrong.
> >
> > Could be that x86 code maps PXM 1 to node 0 because PXM 1 does neither
> > have CPUs nor memory. That would imply that we can, in fact, never have
> > node 0 offline during boot.
> >
>
> Yep, looks like it.
>
> [ 0.009726] SRAT: PXM 1 -> APIC 0x00 -> Node 0
> [ 0.009727] SRAT: PXM 1 -> APIC 0x01 -> Node 0
> [ 0.009727] SRAT: PXM 1 -> APIC 0x02 -> Node 0
> [ 0.009728] SRAT: PXM 1 -> APIC 0x03 -> Node 0
> [ 0.009731] ACPI: SRAT: Node 0 PXM 1 [mem 0x00000000-0x0009ffff]
> [ 0.009732] ACPI: SRAT: Node 0 PXM 1 [mem 0x00100000-0xbfffffff]
> [ 0.009733] ACPI: SRAT: Node 0 PXM 1 [mem 0x100000000-0x13fffffff]
This begs a question whether ppc can do the same thing?
I would swear that we've had x86 system with node 0 but I cannot really
find it and it is possible that it was not x86 after all...
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH v5 3/3] mm/page_alloc: Keep memoryless cpuless node 0 offline
From: David Hildenbrand @ 2020-07-01 11:30 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Gautham R Shenoy, Linus Torvalds, linux-kernel, Michal Hocko,
linux-mm, Satheesh Rajendran, Mel Gorman, Kirill A. Shutemov,
Andrew Morton, linuxppc-dev, Christopher Lameter, Vlastimil Babka
In-Reply-To: <0468f965-8762-76a3-93de-3987cf859927@redhat.com>
On 01.07.20 13:06, David Hildenbrand wrote:
> On 01.07.20 13:01, Srikar Dronamraju wrote:
>> * David Hildenbrand <david@redhat.com> [2020-07-01 12:15:54]:
>>
>>> On 01.07.20 12:04, Srikar Dronamraju wrote:
>>>> * Michal Hocko <mhocko@kernel.org> [2020-07-01 10:42:00]:
>>>>
>>>>>
>>>>>>
>>>>>> 2. Also existence of dummy node also leads to inconsistent information. The
>>>>>> number of online nodes is inconsistent with the information in the
>>>>>> device-tree and resource-dump
>>>>>>
>>>>>> 3. When the dummy node is present, single node non-Numa systems end up showing
>>>>>> up as NUMA systems and numa_balancing gets enabled. This will mean we take
>>>>>> the hit from the unnecessary numa hinting faults.
>>>>>
>>>>> I have to say that I dislike the node online/offline state and directly
>>>>> exporting that to the userspace. Users should only care whether the node
>>>>> has memory/cpus. Numa nodes can be online without any memory. Just
>>>>> offline all the present memory blocks but do not physically hot remove
>>>>> them and you are in the same situation. If users are confused by an
>>>>> output of tools like numactl -H then those could be updated and hide
>>>>> nodes without any memory&cpus.
>>>>>
>>>>> The autonuma problem sounds interesting but again this patch doesn't
>>>>> really solve the underlying problem because I strongly suspect that the
>>>>> problem is still there when a numa node gets all its memory offline as
>>>>> mentioned above.
>>>>>
>>>>> While I completely agree that making node 0 special is wrong, I have
>>>>> still hard time to review this very simply looking patch because all the
>>>>> numa initialization is so spread around that this might just blow up
>>>>> at unexpected places. IIRC we have discussed testing in the previous
>>>>> version and David has provided a way to emulate these configurations
>>>>> on x86. Did you manage to use those instruction for additional testing
>>>>> on other than ppc architectures?
>>>>>
>>>>
>>>> I have tried all the steps that David mentioned and reported back at
>>>> https://lore.kernel.org/lkml/20200511174731.GD1961@linux.vnet.ibm.com/t/#u
>>>>
>>>> As a summary, David's steps are still not creating a memoryless/cpuless on
>>>> x86 VM.
>>>
>>> Now, that is wrong. You get a memoryless/cpuless node, which is *not
>>> online*. Once you hotplug some memory, it will switch online. Once you
>>> remove memory, it will switch back offline.
>>>
>>
>> Let me clarify, we are looking for a node 0 which is cpuless/memoryless at
>> boot. The code in question tries to handle a cpuless/memoryless node 0 at
>> boot.
>
> I was just correcting your statement, because it was wrong.
>
> Could be that x86 code maps PXM 1 to node 0 because PXM 1 does neither
> have CPUs nor memory. That would imply that we can, in fact, never have
> node 0 offline during boot.
>
Yep, looks like it.
[ 0.009726] SRAT: PXM 1 -> APIC 0x00 -> Node 0
[ 0.009727] SRAT: PXM 1 -> APIC 0x01 -> Node 0
[ 0.009727] SRAT: PXM 1 -> APIC 0x02 -> Node 0
[ 0.009728] SRAT: PXM 1 -> APIC 0x03 -> Node 0
[ 0.009731] ACPI: SRAT: Node 0 PXM 1 [mem 0x00000000-0x0009ffff]
[ 0.009732] ACPI: SRAT: Node 0 PXM 1 [mem 0x00100000-0xbfffffff]
[ 0.009733] ACPI: SRAT: Node 0 PXM 1 [mem 0x100000000-0x13fffffff]
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: [PATCH v2 02/10] KVM: PPC: Book3S HV: Save/restore new PMU registers
From: Paul Mackerras @ 2020-07-01 11:11 UTC (permalink / raw)
To: Athira Rajeev; +Cc: mikey, maddy, linuxppc-dev
In-Reply-To: <1593595262-1433-3-git-send-email-atrajeev@linux.vnet.ibm.com>
On Wed, Jul 01, 2020 at 05:20:54AM -0400, Athira Rajeev wrote:
> PowerISA v3.1 has added new performance monitoring unit (PMU)
> special purpose registers (SPRs). They are
>
> Monitor Mode Control Register 3 (MMCR3)
> Sampled Instruction Event Register A (SIER2)
> Sampled Instruction Event Register B (SIER3)
>
> Patch addes support to save/restore these new
> SPRs while entering/exiting guest.
This mostly looks reasonable, at a quick glance at least, but I am
puzzled by two of the changes you are making. See below.
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 6bf66649..c265800 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -1698,7 +1698,8 @@ static int kvmppc_get_one_reg_hv(struct kvm_vcpu *vcpu, u64 id,
> *val = get_reg_val(id, vcpu->arch.sdar);
> break;
> case KVM_REG_PPC_SIER:
> - *val = get_reg_val(id, vcpu->arch.sier);
> + i = id - KVM_REG_PPC_SIER;
> + *val = get_reg_val(id, vcpu->arch.sier[i]);
This is inside a switch (id) statement, so here we know that id is
KVM_REG_PPC_SIER. In other words i will always be zero, so what is
the point of doing the subtraction?
> break;
> case KVM_REG_PPC_IAMR:
> *val = get_reg_val(id, vcpu->arch.iamr);
> @@ -1919,7 +1920,8 @@ static int kvmppc_set_one_reg_hv(struct kvm_vcpu *vcpu, u64 id,
> vcpu->arch.sdar = set_reg_val(id, *val);
> break;
> case KVM_REG_PPC_SIER:
> - vcpu->arch.sier = set_reg_val(id, *val);
> + i = id - KVM_REG_PPC_SIER;
> + vcpu->arch.sier[i] = set_reg_val(id, *val);
Same comment here.
I think that new defines for the new registers will need to be added
to arch/powerpc/include/uapi/asm/kvm.h and
Documentation/virt/kvm/api.rst, and then new cases will need to be
added to these switch statements.
By the way, please cc kvm-ppc@vger.kernel.org and kvm@vger.kernel.org
on KVM patches.
Paul.
^ permalink raw reply
* Re: [PATCH v5 3/3] mm/page_alloc: Keep memoryless cpuless node 0 offline
From: David Hildenbrand @ 2020-07-01 11:06 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Gautham R Shenoy, Linus Torvalds, linux-kernel, Michal Hocko,
linux-mm, Satheesh Rajendran, Mel Gorman, Kirill A. Shutemov,
Andrew Morton, linuxppc-dev, Christopher Lameter, Vlastimil Babka
In-Reply-To: <20200701110145.GC17918@linux.vnet.ibm.com>
On 01.07.20 13:01, Srikar Dronamraju wrote:
> * David Hildenbrand <david@redhat.com> [2020-07-01 12:15:54]:
>
>> On 01.07.20 12:04, Srikar Dronamraju wrote:
>>> * Michal Hocko <mhocko@kernel.org> [2020-07-01 10:42:00]:
>>>
>>>>
>>>>>
>>>>> 2. Also existence of dummy node also leads to inconsistent information. The
>>>>> number of online nodes is inconsistent with the information in the
>>>>> device-tree and resource-dump
>>>>>
>>>>> 3. When the dummy node is present, single node non-Numa systems end up showing
>>>>> up as NUMA systems and numa_balancing gets enabled. This will mean we take
>>>>> the hit from the unnecessary numa hinting faults.
>>>>
>>>> I have to say that I dislike the node online/offline state and directly
>>>> exporting that to the userspace. Users should only care whether the node
>>>> has memory/cpus. Numa nodes can be online without any memory. Just
>>>> offline all the present memory blocks but do not physically hot remove
>>>> them and you are in the same situation. If users are confused by an
>>>> output of tools like numactl -H then those could be updated and hide
>>>> nodes without any memory&cpus.
>>>>
>>>> The autonuma problem sounds interesting but again this patch doesn't
>>>> really solve the underlying problem because I strongly suspect that the
>>>> problem is still there when a numa node gets all its memory offline as
>>>> mentioned above.
>>>>
>>>> While I completely agree that making node 0 special is wrong, I have
>>>> still hard time to review this very simply looking patch because all the
>>>> numa initialization is so spread around that this might just blow up
>>>> at unexpected places. IIRC we have discussed testing in the previous
>>>> version and David has provided a way to emulate these configurations
>>>> on x86. Did you manage to use those instruction for additional testing
>>>> on other than ppc architectures?
>>>>
>>>
>>> I have tried all the steps that David mentioned and reported back at
>>> https://lore.kernel.org/lkml/20200511174731.GD1961@linux.vnet.ibm.com/t/#u
>>>
>>> As a summary, David's steps are still not creating a memoryless/cpuless on
>>> x86 VM.
>>
>> Now, that is wrong. You get a memoryless/cpuless node, which is *not
>> online*. Once you hotplug some memory, it will switch online. Once you
>> remove memory, it will switch back offline.
>>
>
> Let me clarify, we are looking for a node 0 which is cpuless/memoryless at
> boot. The code in question tries to handle a cpuless/memoryless node 0 at
> boot.
I was just correcting your statement, because it was wrong.
Could be that x86 code maps PXM 1 to node 0 because PXM 1 does neither
have CPUs nor memory. That would imply that we can, in fact, never have
node 0 offline during boot.
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: [PATCH v5 3/3] mm/page_alloc: Keep memoryless cpuless node 0 offline
From: Srikar Dronamraju @ 2020-07-01 11:01 UTC (permalink / raw)
To: David Hildenbrand
Cc: Gautham R Shenoy, Linus Torvalds, linux-kernel, Michal Hocko,
linux-mm, Satheesh Rajendran, Mel Gorman, Kirill A. Shutemov,
Andrew Morton, linuxppc-dev, Christopher Lameter, Vlastimil Babka
In-Reply-To: <184102af-ecf2-c834-db46-173ab2e66f51@redhat.com>
* David Hildenbrand <david@redhat.com> [2020-07-01 12:15:54]:
> On 01.07.20 12:04, Srikar Dronamraju wrote:
> > * Michal Hocko <mhocko@kernel.org> [2020-07-01 10:42:00]:
> >
> >>
> >>>
> >>> 2. Also existence of dummy node also leads to inconsistent information. The
> >>> number of online nodes is inconsistent with the information in the
> >>> device-tree and resource-dump
> >>>
> >>> 3. When the dummy node is present, single node non-Numa systems end up showing
> >>> up as NUMA systems and numa_balancing gets enabled. This will mean we take
> >>> the hit from the unnecessary numa hinting faults.
> >>
> >> I have to say that I dislike the node online/offline state and directly
> >> exporting that to the userspace. Users should only care whether the node
> >> has memory/cpus. Numa nodes can be online without any memory. Just
> >> offline all the present memory blocks but do not physically hot remove
> >> them and you are in the same situation. If users are confused by an
> >> output of tools like numactl -H then those could be updated and hide
> >> nodes without any memory&cpus.
> >>
> >> The autonuma problem sounds interesting but again this patch doesn't
> >> really solve the underlying problem because I strongly suspect that the
> >> problem is still there when a numa node gets all its memory offline as
> >> mentioned above.
> >>
> >> While I completely agree that making node 0 special is wrong, I have
> >> still hard time to review this very simply looking patch because all the
> >> numa initialization is so spread around that this might just blow up
> >> at unexpected places. IIRC we have discussed testing in the previous
> >> version and David has provided a way to emulate these configurations
> >> on x86. Did you manage to use those instruction for additional testing
> >> on other than ppc architectures?
> >>
> >
> > I have tried all the steps that David mentioned and reported back at
> > https://lore.kernel.org/lkml/20200511174731.GD1961@linux.vnet.ibm.com/t/#u
> >
> > As a summary, David's steps are still not creating a memoryless/cpuless on
> > x86 VM.
>
> Now, that is wrong. You get a memoryless/cpuless node, which is *not
> online*. Once you hotplug some memory, it will switch online. Once you
> remove memory, it will switch back offline.
>
Let me clarify, we are looking for a node 0 which is cpuless/memoryless at
boot. The code in question tries to handle a cpuless/memoryless node 0 at
boot.
With the steps that you gave the node 0 was always populated, node 1 or
some other node would be memoryless/cpuless and offline. But that should
have no impact by patch.
I don't see how adding/hotplugging/removing memory to a node after boot is
going to affect the changes that I have made. Please do correct me if I have
misunderstood.
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply
* Re: [PATCH v5 3/3] mm/page_alloc: Keep memoryless cpuless node 0 offline
From: David Hildenbrand @ 2020-07-01 10:15 UTC (permalink / raw)
To: Srikar Dronamraju, Michal Hocko
Cc: Gautham R Shenoy, Linus Torvalds, linux-kernel, linux-mm,
Satheesh Rajendran, Mel Gorman, Kirill A. Shutemov, Andrew Morton,
linuxppc-dev, Christopher Lameter, Vlastimil Babka
In-Reply-To: <20200701100442.GB17918@linux.vnet.ibm.com>
On 01.07.20 12:04, Srikar Dronamraju wrote:
> * Michal Hocko <mhocko@kernel.org> [2020-07-01 10:42:00]:
>
>>
>>>
>>> 2. Also existence of dummy node also leads to inconsistent information. The
>>> number of online nodes is inconsistent with the information in the
>>> device-tree and resource-dump
>>>
>>> 3. When the dummy node is present, single node non-Numa systems end up showing
>>> up as NUMA systems and numa_balancing gets enabled. This will mean we take
>>> the hit from the unnecessary numa hinting faults.
>>
>> I have to say that I dislike the node online/offline state and directly
>> exporting that to the userspace. Users should only care whether the node
>> has memory/cpus. Numa nodes can be online without any memory. Just
>> offline all the present memory blocks but do not physically hot remove
>> them and you are in the same situation. If users are confused by an
>> output of tools like numactl -H then those could be updated and hide
>> nodes without any memory&cpus.
>>
>> The autonuma problem sounds interesting but again this patch doesn't
>> really solve the underlying problem because I strongly suspect that the
>> problem is still there when a numa node gets all its memory offline as
>> mentioned above.
>>
>> While I completely agree that making node 0 special is wrong, I have
>> still hard time to review this very simply looking patch because all the
>> numa initialization is so spread around that this might just blow up
>> at unexpected places. IIRC we have discussed testing in the previous
>> version and David has provided a way to emulate these configurations
>> on x86. Did you manage to use those instruction for additional testing
>> on other than ppc architectures?
>>
>
> I have tried all the steps that David mentioned and reported back at
> https://lore.kernel.org/lkml/20200511174731.GD1961@linux.vnet.ibm.com/t/#u
>
> As a summary, David's steps are still not creating a memoryless/cpuless on
> x86 VM.
Now, that is wrong. You get a memoryless/cpuless node, which is *not
online*. Once you hotplug some memory, it will switch online. Once you
remove memory, it will switch back offline.
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: [PATCH v5 3/3] mm/page_alloc: Keep memoryless cpuless node 0 offline
From: Srikar Dronamraju @ 2020-07-01 10:04 UTC (permalink / raw)
To: Michal Hocko
Cc: Gautham R Shenoy, David Hildenbrand, Linus Torvalds, linux-kernel,
linux-mm, Satheesh Rajendran, Mel Gorman, Kirill A. Shutemov,
Andrew Morton, linuxppc-dev, Christopher Lameter, Vlastimil Babka
In-Reply-To: <20200701084200.GN2369@dhcp22.suse.cz>
* Michal Hocko <mhocko@kernel.org> [2020-07-01 10:42:00]:
>
> >
> > 2. Also existence of dummy node also leads to inconsistent information. The
> > number of online nodes is inconsistent with the information in the
> > device-tree and resource-dump
> >
> > 3. When the dummy node is present, single node non-Numa systems end up showing
> > up as NUMA systems and numa_balancing gets enabled. This will mean we take
> > the hit from the unnecessary numa hinting faults.
>
> I have to say that I dislike the node online/offline state and directly
> exporting that to the userspace. Users should only care whether the node
> has memory/cpus. Numa nodes can be online without any memory. Just
> offline all the present memory blocks but do not physically hot remove
> them and you are in the same situation. If users are confused by an
> output of tools like numactl -H then those could be updated and hide
> nodes without any memory&cpus.
>
> The autonuma problem sounds interesting but again this patch doesn't
> really solve the underlying problem because I strongly suspect that the
> problem is still there when a numa node gets all its memory offline as
> mentioned above.
>
> While I completely agree that making node 0 special is wrong, I have
> still hard time to review this very simply looking patch because all the
> numa initialization is so spread around that this might just blow up
> at unexpected places. IIRC we have discussed testing in the previous
> version and David has provided a way to emulate these configurations
> on x86. Did you manage to use those instruction for additional testing
> on other than ppc architectures?
>
I have tried all the steps that David mentioned and reported back at
https://lore.kernel.org/lkml/20200511174731.GD1961@linux.vnet.ibm.com/t/#u
As a summary, David's steps are still not creating a memoryless/cpuless on
x86 VM. I have tried booting with Numa/non-numa on all the x86 machines that
I could get to.
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply
* [powerpc:merge] BUILD SUCCESS 3c0356e8994ed88e5234897c0ffee4188f8b9287
From: kernel test robot @ 2020-07-01 9:40 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git merge
branch HEAD: 3c0356e8994ed88e5234897c0ffee4188f8b9287 Automatic merge of 'master', 'next' and 'fixes' (2020-06-30 22:07)
elapsed time: 1282m
configs tested: 98
configs skipped: 1
The following configs have been built successfully.
More configs may be tested in the coming days.
arm defconfig
arm allyesconfig
arm allmodconfig
arm allnoconfig
arm64 allyesconfig
arm64 defconfig
arm64 allmodconfig
arm64 allnoconfig
i386 allyesconfig
i386 defconfig
i386 debian-10.3
i386 allnoconfig
ia64 allmodconfig
ia64 defconfig
ia64 allnoconfig
ia64 allyesconfig
m68k allmodconfig
m68k allnoconfig
m68k sun3_defconfig
m68k defconfig
m68k allyesconfig
nios2 defconfig
nios2 allyesconfig
openrisc defconfig
c6x allyesconfig
c6x allnoconfig
openrisc allyesconfig
nds32 defconfig
nds32 allnoconfig
csky allyesconfig
csky defconfig
alpha defconfig
alpha allyesconfig
xtensa allyesconfig
h8300 allyesconfig
h8300 allmodconfig
xtensa defconfig
arc defconfig
arc allyesconfig
sh allmodconfig
sh allnoconfig
microblaze allnoconfig
mips allyesconfig
mips allnoconfig
mips allmodconfig
parisc allnoconfig
parisc defconfig
parisc allyesconfig
parisc allmodconfig
powerpc defconfig
powerpc allyesconfig
powerpc rhel-kconfig
powerpc allmodconfig
powerpc allnoconfig
i386 randconfig-a001-20200630
i386 randconfig-a003-20200630
i386 randconfig-a002-20200630
i386 randconfig-a004-20200630
i386 randconfig-a005-20200630
i386 randconfig-a006-20200630
x86_64 randconfig-a011-20200630
x86_64 randconfig-a014-20200630
x86_64 randconfig-a013-20200630
x86_64 randconfig-a015-20200630
x86_64 randconfig-a016-20200630
x86_64 randconfig-a012-20200630
i386 randconfig-a011-20200630
i386 randconfig-a016-20200630
i386 randconfig-a015-20200630
i386 randconfig-a012-20200630
i386 randconfig-a014-20200630
i386 randconfig-a013-20200630
riscv allyesconfig
riscv allnoconfig
riscv defconfig
riscv allmodconfig
s390 allyesconfig
s390 allnoconfig
s390 allmodconfig
s390 defconfig
sparc allyesconfig
sparc defconfig
sparc64 defconfig
sparc64 allnoconfig
sparc64 allyesconfig
sparc64 allmodconfig
um allmodconfig
um allnoconfig
um allyesconfig
um defconfig
x86_64 rhel
x86_64 rhel-7.2-clear
x86_64 lkp
x86_64 fedora-25
x86_64 rhel-7.6
x86_64 rhel-7.6-kselftests
x86_64 rhel-8.3
x86_64 kexec
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox