* Re: [PATCH 00/15] powerpc/watchpoint: Preparation for more than one watchpoint
From: Ravi Bangoria @ 2020-03-18 12:52 UTC (permalink / raw)
To: Christophe Leroy
Cc: apopple, mikey, Ravi Bangoria, peterz, oleg, npiggin,
linux-kernel, paulus, jolsa, fweisbec, naveen.n.rao, linuxppc-dev,
mingo
In-Reply-To: <b7148b91-e3db-d48a-7294-5c18fc801933@c-s.fr>
On 3/16/20 8:35 PM, Christophe Leroy wrote:
>
>
> Le 09/03/2020 à 09:57, Ravi Bangoria a écrit :
>> So far, powerpc Book3S code has been written with an assumption of only
>> one watchpoint. But future power architecture is introducing second
>> watchpoint register (DAWR). Even though this patchset does not enable
>> 2nd DAWR, it make the infrastructure ready so that enabling 2nd DAWR
>> should just be a matter of changing count.
>
> Some book3s (e300 family for instance, I think G2 as well) already have a DABR2 in addition to DABR.
> Will this series allow to use it as well ?
I wasn't aware of that. I'll take a look at their specs and check if they
can piggyback on this series for 2nd DABR.
Thanks,
Ravi
^ permalink raw reply
* Re: [PATCH v2 0/8] mm/memory_hotplug: allow to specify a default online_type
From: Baoquan He @ 2020-03-18 13:05 UTC (permalink / raw)
To: David Hildenbrand
Cc: Yumei Huang, linux-hyperv, Michal Hocko, Rafael J. Wysocki,
Michal Hocko, linux-mm, Paul Mackerras, K. Y. Srinivasan, Wei Liu,
Stephen Hemminger, Eduardo Habkost, Haiyang Zhang, Wei Yang,
Andrew Morton, Oscar Salvador, Greg Kroah-Hartman, linux-kernel,
Milan Zamazal, Igor Mammedov, Vitaly Kuznetsov, linuxppc-dev
In-Reply-To: <20200317104942.11178-1-david@redhat.com>
On 03/17/20 at 11:49am, David Hildenbrand wrote:
> Distributions nowadays use udev rules ([1] [2]) to specify if and
> how to online hotplugged memory. The rules seem to get more complex with
> many special cases. Due to the various special cases,
> CONFIG_MEMORY_HOTPLUG_DEFAULT_ONLINE cannot be used. All memory hotplug
> is handled via udev rules.
>
> Everytime we hotplug memory, the udev rule will come to the same
> conclusion. Especially Hyper-V (but also soon virtio-mem) add a lot of
> memory in separate memory blocks and wait for memory to get onlined by user
> space before continuing to add more memory blocks (to not add memory faster
> than it is getting onlined). This of course slows down the whole memory
> hotplug process.
>
> To make the job of distributions easier and to avoid udev rules that get
> more and more complicated, let's extend the mechanism provided by
> - /sys/devices/system/memory/auto_online_blocks
> - "memhp_default_state=" on the kernel cmdline
> to be able to specify also "online_movable" as well as "online_kernel"
This patch series looks good, thanks. Since Andrew has merged it to -mm again,
I won't add my Reviewed-by to bother.
Hi David, Vitaly
There are several things unclear to me.
So, these improved interfaces are used to alleviate the burden of the
existing udev rules, or try to replace it? As you know, we have been
using udev rules to interact between kernel and user space on bare metal,
and guests who want to hot add/remove.
And also the OOM issue in hyperV when onlining pages after adding memory
block. I am not a virt devel expert, could this happen on bare metal
system?
Thanks
Baoquan
^ permalink raw reply
* Re: [PATCH 4/5] selftests/powerpc: Add NX-GZIP engine decompress testcase
From: Raphael M Zinsly @ 2020-03-18 13:08 UTC (permalink / raw)
To: Daniel Axtens, linuxppc-dev, linux-crypto; +Cc: abali, haren, herbert
In-Reply-To: <877dzinq30.fsf@dja-thinkpad.axtens.net>
Thanks for the reviews Daniel, I'll use your testcases and address the
issues you found, I still have some questions bellow:
On 18/03/2020 03:18, Daniel Axtens wrote:
> Raphael Moreira Zinsly <rzinsly@linux.ibm.com> writes:
>
>> Include a decompression testcase for the powerpc NX-GZIP
>> engine.
>
> I compiled gzip with the AFL++ fuzzer and generated a corpus of tests to
> run against this decompressor. I also fuzzed the decompressor
> directly. I found a few issues. I _think_ they're just in the userspace
> but I'm a bit too early in the process to know.
>
> I realise this is self-test code but:
> a) it stops me testing more deeply, and
I don't understand what do you mean by that, what did you couldn't test?
> b) it looks like some of this code is shared with https://github.com/libnxz/power-gzip/
Is that an issue?
This selftest were develop by the same team that develop the userspace
library, the first version of this tests were pushed there in order for
the team to review. It uses some of the headers of the library to access
the accelerator and part of the code in the samples.
Regards,
Raphael
>
> The issues I've found are:
>
> 1) In the ERR_NX_DATA_LENGTH case, the decompressor doesn't check that
> you're making forward progress, so you can provoke it into an
> infinite loop.
>
> Here's an _extremely_ ugly fix:
>
> diff --git a/tools/testing/selftests/powerpc/nx-gzip/gunz_test.c b/tools/testing/selftests/powerpc/nx-gzip/gunz_test.c
> index 653de92698cc..236a1f567656 100644
> --- a/tools/testing/selftests/powerpc/nx-gzip/gunz_test.c
> +++ b/tools/testing/selftests/powerpc/nx-gzip/gunz_test.c
> @@ -343,6 +343,8 @@ int decompress_file(int argc, char **argv, void *devhandle)
> nx_dde_t dde_out[6] __attribute__((aligned (128)));
> int pgfault_retries;
>
> + int last_first_used = 0;
> +
> /* when using mmap'ed files */
> off_t input_file_offset;
>
> @@ -642,6 +644,11 @@ int decompress_file(int argc, char **argv, void *devhandle)
> first_used = fifo_used_first_bytes(cur_in, used_in, fifo_in_len);
> last_used = fifo_used_last_bytes(cur_in, used_in, fifo_in_len);
>
> + if (first_used > 0 && last_first_used > 0) {
> + assert(first_used != last_first_used);
> + }
> + last_first_used = first_used;
> +
> if (first_used > 0)
> nx_append_dde(ddl_in, fifo_in + cur_in, first_used);
>
>
> 2) It looks like you can provoke an out-of-bounds write. I've seen both
> infinte loops printing something that seems to come from the file
> content like:
>
> 57201: Got signal 11 si_code 3, si_addr 0xcacacacacacacac8
>
> or a less bizzare address like
>
> 19285: Got signal 11 si_code 1, si_addr 0x7fffcf1b0000
>
> Depending on the build I've also seen the stack smasher protection fire.
>
> I don't understand the code well enough to figure out how this comes to
> be just yet.
>
> I've included a few test cases as attachments. I've preconverted them
> with xxd to avoid anything that might flag suspicious gzip files!
> Decompress them then use `xxd -r attachment testcase.gz` to convert them
> back.
>
> Regards,
> Daniel
>
>
>
>
>>
>> Signed-off-by: Bulent Abali <abali@us.ibm.com>
>> Signed-off-by: Raphael Moreira Zinsly <rzinsly@linux.ibm.com>
>> ---
>> .../selftests/powerpc/nx-gzip/Makefile | 7 +-
>> .../selftests/powerpc/nx-gzip/gunz_test.c | 1058 +++++++++++++++++
>> 2 files changed, 1062 insertions(+), 3 deletions(-)
>> create mode 100644 tools/testing/selftests/powerpc/nx-gzip/gunz_test.c
>>
>> diff --git a/tools/testing/selftests/powerpc/nx-gzip/Makefile b/tools/testing/selftests/powerpc/nx-gzip/Makefile
>> index ab903f63bbbd..82abc19a49a0 100644
>> --- a/tools/testing/selftests/powerpc/nx-gzip/Makefile
>> +++ b/tools/testing/selftests/powerpc/nx-gzip/Makefile
>> @@ -1,9 +1,9 @@
>> CC = gcc
>> CFLAGS = -O3
>> INC = ./inc
>> -SRC = gzfht_test.c
>> +SRC = gzfht_test.c gunz_test.c
>> OBJ = $(SRC:.c=.o)
>> -TESTS = gzfht_test
>> +TESTS = gzfht_test gunz_test
>> EXTRA_SOURCES = gzip_vas.c
>>
>> all: $(TESTS)
>> @@ -16,6 +16,7 @@ $(TESTS): $(OBJ)
>>
>> run_tests: $(TESTS)
>> ./gzfht_test gzip_vas.c
>> + ./gunz_test gzip_vas.c.nx.gz
>>
>> clean:
>> - rm -f $(TESTS) *.o *~ *.gz
>> + rm -f $(TESTS) *.o *~ *.gz *.gunzip
>> diff --git a/tools/testing/selftests/powerpc/nx-gzip/gunz_test.c b/tools/testing/selftests/powerpc/nx-gzip/gunz_test.c
>> new file mode 100644
>> index 000000000000..653de92698cc
>> --- /dev/null
>> +++ b/tools/testing/selftests/powerpc/nx-gzip/gunz_test.c
>> @@ -0,0 +1,1058 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later
>> + *
>> + * P9 gunzip sample code for demonstrating the P9 NX hardware
>> + * interface. Not intended for productive uses or for performance or
>> + * compression ratio measurements. Note also that /dev/crypto/gzip,
>> + * VAS and skiboot support are required
>> + *
>> + * Copyright 2020 IBM Corp.
>> + *
>> + * Author: Bulent Abali <abali@us.ibm.com>
>> + *
>> + * https://github.com/libnxz/power-gzip for zlib api and other utils
>> + * Definitions of acronyms used here. See
>> + * P9 NX Gzip Accelerator User's Manual for details
>> + *
>> + * adler/crc: 32 bit checksums appended to stream tail
>> + * ce: completion extension
>> + * cpb: coprocessor parameter block (metadata)
>> + * crb: coprocessor request block (command)
>> + * csb: coprocessor status block (status)
>> + * dht: dynamic huffman table
>> + * dde: data descriptor element (address, length)
>> + * ddl: list of ddes
>> + * dh/fh: dynamic and fixed huffman types
>> + * fc: coprocessor function code
>> + * histlen: history/dictionary length
>> + * history: sliding window of up to 32KB of data
>> + * lzcount: Deflate LZ symbol counts
>> + * rembytecnt: remaining byte count
>> + * sfbt: source final block type; last block's type during decomp
>> + * spbc: source processed byte count
>> + * subc: source unprocessed bit count
>> + * tebc: target ending bit count; valid bits in the last byte
>> + * tpbc: target processed byte count
>> + * vas: virtual accelerator switch; the user mode interface
>> + */
>> +
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <string.h>
>> +#include <unistd.h>
>> +#include <stdint.h>
>> +#include <sys/types.h>
>> +#include <sys/stat.h>
>> +#include <sys/time.h>
>> +#include <sys/fcntl.h>
>> +#include <sys/mman.h>
>> +#include <endian.h>
>> +#include <bits/endian.h>
>> +#include <sys/ioctl.h>
>> +#include <assert.h>
>> +#include <errno.h>
>> +#include <signal.h>
>> +#include "nxu.h"
>> +#include "nx.h"
>> +
>> +int nx_dbg = 0;
>> +FILE *nx_gzip_log = NULL;
>> +
>> +#define NX_MIN(X, Y) (((X) < (Y))?(X):(Y))
>> +#define NX_MAX(X, Y) (((X) > (Y))?(X):(Y))
>> +
>> +#define mb() asm volatile("sync" ::: "memory")
>> +#define rmb() asm volatile("lwsync" ::: "memory")
>> +#define wmb() rmb()
>> +
>> +const int fifo_in_len = 1<<24;
>> +const int fifo_out_len = 1<<24;
>> +const int page_sz = 1<<16;
>> +const int line_sz = 1<<7;
>> +const int window_max = 1<<15;
>> +const int retry_max = 50;
>> +
>> +extern void *nx_fault_storage_address;
>> +extern void *nx_function_begin(int function, int pri);
>> +extern int nx_function_end(void *handle);
>> +
>> +/*
>> + * Fault in pages prior to NX job submission. wr=1 may be required to
>> + * touch writeable pages. System zero pages do not fault-in the page as
>> + * intended. Typically set wr=1 for NX target pages and set wr=0 for
>> + * NX source pages.
>> + */
>> +static int nx_touch_pages(void *buf, long buf_len, long page_len, int wr)
>> +{
>> + char *begin = buf;
>> + char *end = (char *) buf + buf_len - 1;
>> + volatile char t;
>> +
>> + assert(buf_len >= 0 && !!buf);
>> +
>> + NXPRT(fprintf(stderr, "touch %p %p len 0x%lx wr=%d\n", buf,
>> + buf + buf_len, buf_len, wr));
>> +
>> + if (buf_len <= 0 || buf == NULL)
>> + return -1;
>> +
>> + do {
>> + t = *begin;
>> + if (wr)
>> + *begin = t;
>> + begin = begin + page_len;
>> + } while (begin < end);
>> +
>> + /* When buf_sz is small or buf tail is in another page. */
>> + t = *end;
>> + if (wr)
>> + *end = t;
>> +
>> + return 0;
>> +}
>> +
>> +void sigsegv_handler(int sig, siginfo_t *info, void *ctx)
>> +{
>> + fprintf(stderr, "%d: Got signal %d si_code %d, si_addr %p\n", getpid(),
>> + sig, info->si_code, info->si_addr);
>> +
>> + nx_fault_storage_address = info->si_addr;
>> +}
>> +
>> +/*
>> + * Adds an (address, len) pair to the list of ddes (ddl) and updates
>> + * the base dde. ddl[0] is the only dde in a direct dde which
>> + * contains a single (addr,len) pair. For more pairs, ddl[0] becomes
>> + * the indirect (base) dde that points to a list of direct ddes.
>> + * See Section 6.4 of the NX-gzip user manual for DDE description.
>> + * Addr=NULL, len=0 clears the ddl[0]. Returns the total number of
>> + * bytes in ddl. Caller is responsible for allocting the array of
>> + * nx_dde_t *ddl. If N addresses are required in the scatter-gather
>> + * list, the ddl array must have N+1 entries minimum.
>> + */
>> +static inline uint32_t nx_append_dde(nx_dde_t *ddl, void *addr, uint32_t len)
>> +{
>> + uint32_t ddecnt;
>> + uint32_t bytes;
>> +
>> + if (addr == NULL && len == 0) {
>> + clearp_dde(ddl);
>> + return 0;
>> + }
>> +
>> + NXPRT(fprintf(stderr, "%d: nx_append_dde addr %p len %x\n", __LINE__,
>> + addr, len));
>> +
>> + /* Number of ddes in the dde list ; == 0 when it is a direct dde */
>> + ddecnt = getpnn(ddl, dde_count);
>> + bytes = getp32(ddl, ddebc);
>> +
>> + if (ddecnt == 0 && bytes == 0) {
>> + /* First dde is unused; make it a direct dde */
>> + bytes = len;
>> + putp32(ddl, ddebc, bytes);
>> + putp64(ddl, ddead, (uint64_t) addr);
>> + } else if (ddecnt == 0) {
>> + /* Converting direct to indirect dde
>> + * ddl[0] becomes head dde of ddl
>> + * copy direct to indirect first.
>> + */
>> + ddl[1] = ddl[0];
>> +
>> + /* Add the new dde next */
>> + clear_dde(ddl[2]);
>> + put32(ddl[2], ddebc, len);
>> + put64(ddl[2], ddead, (uint64_t) addr);
>> +
>> + /* Ddl head points to 2 direct ddes */
>> + ddecnt = 2;
>> + putpnn(ddl, dde_count, ddecnt);
>> + bytes = bytes + len;
>> + putp32(ddl, ddebc, bytes);
>> + /* Pointer to the first direct dde */
>> + putp64(ddl, ddead, (uint64_t) &ddl[1]);
>> + } else {
>> + /* Append a dde to an existing indirect ddl */
>> + ++ddecnt;
>> + clear_dde(ddl[ddecnt]);
>> + put64(ddl[ddecnt], ddead, (uint64_t) addr);
>> + put32(ddl[ddecnt], ddebc, len);
>> +
>> + putpnn(ddl, dde_count, ddecnt);
>> + bytes = bytes + len;
>> + putp32(ddl, ddebc, bytes); /* byte sum of all dde */
>> + }
>> + return bytes;
>> +}
>> +
>> +/*
>> + * Touch specified number of pages represented in number bytes
>> + * beginning from the first buffer in a dde list.
>> + * Do not touch the pages past buf_sz-th byte's page.
>> + *
>> + * Set buf_sz = 0 to touch all pages described by the ddep.
>> + */
>> +static int nx_touch_pages_dde(nx_dde_t *ddep, long buf_sz, long page_sz,
>> + int wr)
>> +{
>> + uint32_t indirect_count;
>> + uint32_t buf_len;
>> + long total;
>> + uint64_t buf_addr;
>> + nx_dde_t *dde_list;
>> + int i;
>> +
>> + assert(!!ddep);
>> +
>> + indirect_count = getpnn(ddep, dde_count);
>> +
>> + NXPRT(fprintf(stderr, "nx_touch_pages_dde dde_count %d request len \
>> + 0x%lx\n", indirect_count, buf_sz));
>> +
>> + if (indirect_count == 0) {
>> + /* Direct dde */
>> + buf_len = getp32(ddep, ddebc);
>> + buf_addr = getp64(ddep, ddead);
>> +
>> + NXPRT(fprintf(stderr, "touch direct ddebc 0x%x ddead %p\n",
>> + buf_len, (void *)buf_addr));
>> +
>> + if (buf_sz == 0)
>> + nx_touch_pages((void *)buf_addr, buf_len, page_sz, wr);
>> + else
>> + nx_touch_pages((void *)buf_addr, NX_MIN(buf_len,
>> + buf_sz), page_sz, wr);
>> +
>> + return ERR_NX_OK;
>> + }
>> +
>> + /* Indirect dde */
>> + if (indirect_count > MAX_DDE_COUNT)
>> + return ERR_NX_EXCESSIVE_DDE;
>> +
>> + /* First address of the list */
>> + dde_list = (nx_dde_t *) getp64(ddep, ddead);
>> +
>> + if (buf_sz == 0)
>> + buf_sz = getp32(ddep, ddebc);
>> +
>> + total = 0;
>> + for (i = 0; i < indirect_count; i++) {
>> + buf_len = get32(dde_list[i], ddebc);
>> + buf_addr = get64(dde_list[i], ddead);
>> + total += buf_len;
>> +
>> + NXPRT(fprintf(stderr, "touch loop len 0x%x ddead %p total \
>> + 0x%lx\n", buf_len, (void *)buf_addr, total));
>> +
>> + /* Touching fewer pages than encoded in the ddebc */
>> + if (total > buf_sz) {
>> + buf_len = NX_MIN(buf_len, total - buf_sz);
>> + nx_touch_pages((void *)buf_addr, buf_len, page_sz, wr);
>> + NXPRT(fprintf(stderr, "touch loop break len 0x%x \
>> + ddead %p\n", buf_len, (void *)buf_addr));
>> + break;
>> + }
>> + nx_touch_pages((void *)buf_addr, buf_len, page_sz, wr);
>> + }
>> + return ERR_NX_OK;
>> +}
>> +
>> +/*
>> + * Src and dst buffers are supplied in scatter gather lists.
>> + * NX function code and other parameters supplied in cmdp.
>> + */
>> +static int nx_submit_job(nx_dde_t *src, nx_dde_t *dst, nx_gzip_crb_cpb_t *cmdp,
>> + void *handle)
>> +{
>> + int cc;
>> + uint64_t csbaddr;
>> +
>> + memset((void *)&cmdp->crb.csb, 0, sizeof(cmdp->crb.csb));
>> +
>> + cmdp->crb.source_dde = *src;
>> + cmdp->crb.target_dde = *dst;
>> +
>> + /* Status, output byte count in tpbc */
>> + csbaddr = ((uint64_t) &cmdp->crb.csb) & csb_address_mask;
>> + put64(cmdp->crb, csb_address, csbaddr);
>> +
>> + /* NX reports input bytes in spbc; cleared */
>> + cmdp->cpb.out_spbc_comp_wrap = 0;
>> + cmdp->cpb.out_spbc_comp_with_count = 0;
>> + cmdp->cpb.out_spbc_decomp = 0;
>> +
>> + /* Clear output */
>> + put32(cmdp->cpb, out_crc, INIT_CRC);
>> + put32(cmdp->cpb, out_adler, INIT_ADLER);
>> +
>> + cc = nxu_run_job(cmdp, handle);
>> +
>> + if (!cc)
>> + cc = getnn(cmdp->crb.csb, csb_cc); /* CC Table 6-8 */
>> +
>> + return cc;
>> +}
>> +
>> +/* fifo queue management */
>> +#define fifo_used_bytes(used) (used)
>> +#define fifo_free_bytes(used, len) ((len)-(used))
>> +/* amount of free bytes in the first and last parts */
>> +#define fifo_free_first_bytes(cur, used, len) ((((cur)+(used)) <= (len)) \
>> + ? (len)-((cur)+(used)) : 0)
>> +#define fifo_free_last_bytes(cur, used, len) ((((cur)+(used)) <= (len)) \
>> + ? (cur) : (len)-(used))
>> +/* amount of used bytes in the first and last parts */
>> +#define fifo_used_first_bytes(cur, used, len) ((((cur)+(used)) <= (len)) \
>> + ? (used) : (len)-(cur))
>> +#define fifo_used_last_bytes(cur, used, len) ((((cur)+(used)) <= (len)) \
>> + ? 0 : ((used)+(cur))-(len))
>> +/* first and last free parts start here */
>> +#define fifo_free_first_offset(cur, used) ((cur)+(used))
>> +#define fifo_free_last_offset(cur, used, len) \
>> + fifo_used_last_bytes(cur, used, len)
>> +/* first and last used parts start here */
>> +#define fifo_used_first_offset(cur) (cur)
>> +#define fifo_used_last_offset(cur) (0)
>> +
>> +int decompress_file(int argc, char **argv, void *devhandle)
>> +{
>> + FILE *inpf;
>> + FILE *outf;
>> +
>> + int c, expect, i, cc, rc = 0;
>> + char gzfname[1024];
>> +
>> + /* Queuing, file ops, byte counting */
>> + char *fifo_in, *fifo_out;
>> + int used_in, cur_in, used_out, cur_out, read_sz, n;
>> + int first_free, last_free, first_used, last_used;
>> + int first_offset, last_offset;
>> + int write_sz, free_space, source_sz;
>> + int source_sz_estimate, target_sz_estimate;
>> + uint64_t last_comp_ratio; /* 1000 max */
>> + uint64_t total_out;
>> + int is_final, is_eof;
>> +
>> + /* nx hardware */
>> + int sfbt, subc, spbc, tpbc, nx_ce, fc, resuming = 0;
>> + int history_len = 0;
>> + nx_gzip_crb_cpb_t cmd, *cmdp;
>> + nx_dde_t *ddl_in;
>> + nx_dde_t dde_in[6] __attribute__((aligned (128)));
>> + nx_dde_t *ddl_out;
>> + nx_dde_t dde_out[6] __attribute__((aligned (128)));
>> + int pgfault_retries;
>> +
>> + /* when using mmap'ed files */
>> + off_t input_file_offset;
>> +
>> + if (argc > 2) {
>> + fprintf(stderr, "usage: %s <fname> or stdin\n", argv[0]);
>> + fprintf(stderr, " writes to stdout or <fname>.nx.gunzip\n");
>> + return -1;
>> + }
>> +
>> + if (argc == 1) {
>> + inpf = stdin;
>> + outf = stdout;
>> + } else if (argc == 2) {
>> + char w[1024];
>> + char *wp;
>> + inpf = fopen(argv[1], "r");
>> + if (inpf == NULL) {
>> + perror(argv[1]);
>> + return -1;
>> + }
>> +
>> + /* Make a new file name to write to. Ignoring '.gz' */
>> + wp = (NULL != (wp = strrchr(argv[1], '/'))) ? ++wp : argv[1];
>> + strcpy(w, wp);
>> + strcat(w, ".nx.gunzip");
>> +
>> + outf = fopen(w, "w");
>> + if (outf == NULL) {
>> + perror(w);
>> + return -1;
>> + }
>> + }
>> +
>> +#define GETINPC(X) fgetc(X)
>> +
>> + /* Decode the gzip header */
>> + c = GETINPC(inpf); expect = 0x1f; /* ID1 */
>> + if (c != expect)
>> + goto err1;
>> +
>> + c = GETINPC(inpf); expect = 0x8b; /* ID2 */
>> + if (c != expect)
>> + goto err1;
>> +
>> + c = GETINPC(inpf); expect = 0x08; /* CM */
>> + if (c != expect)
>> + goto err1;
>> +
>> + int flg = GETINPC(inpf); /* FLG */
>> + if (flg & 0b11100000 || flg & 0b100)
>> + goto err2;
>> +
>> + fprintf(stderr, "gzHeader FLG %x\n", flg);
>> +
>> + /* Read 6 bytes; ignoring the MTIME, XFL, OS fields in this
>> + * sample code.
>> + */
>> + for (i = 0; i < 6; i++) {
>> + char tmp[10];
>> + if (EOF == (tmp[i] = GETINPC(inpf)))
>> + goto err3;
>> + fprintf(stderr, "%02x ", tmp[i]);
>> + if (i == 5)
>> + fprintf(stderr, "\n");
>> + }
>> + fprintf(stderr, "gzHeader MTIME, XFL, OS ignored\n");
>> +
>> + /* FNAME */
>> + if (flg & 0b1000) {
>> + int k = 0;
>> + do {
>> + if (EOF == (c = GETINPC(inpf)))
>> + goto err3;
>> + gzfname[k++] = c;
>> + } while (c);
>> + fprintf(stderr, "gzHeader FNAME: %s\n", gzfname);
>> + }
>> +
>> + /* FHCRC */
>> + if (flg & 0b10) {
>> + c = GETINPC(inpf); c = GETINPC(inpf);
>> + fprintf(stderr, "gzHeader FHCRC: ignored\n");
>> + }
>> +
>> + used_in = cur_in = used_out = cur_out = 0;
>> + is_final = is_eof = 0;
>> +
>> + /* Allocate one page larger to prevent page faults due to NX
>> + * overfetching.
>> + * Either do this (char*)(uintptr_t)aligned_alloc or use
>> + * -std=c11 flag to make the int-to-pointer warning go away.
>> + */
>> + assert((fifo_in = (char *)(uintptr_t)aligned_alloc(line_sz,
>> + fifo_in_len + page_sz)) != NULL);
>> + assert((fifo_out = (char *)(uintptr_t)aligned_alloc(line_sz,
>> + fifo_out_len + page_sz + line_sz)) != NULL);
>> + /* Leave unused space due to history rounding rules */
>> + fifo_out = fifo_out + line_sz;
>> + nx_touch_pages(fifo_out, fifo_out_len, page_sz, 1);
>> +
>> + ddl_in = &dde_in[0];
>> + ddl_out = &dde_out[0];
>> + cmdp = &cmd;
>> + memset(&cmdp->crb, 0, sizeof(cmdp->crb));
>> +
>> +read_state:
>> +
>> + /* Read from .gz file */
>> +
>> + NXPRT(fprintf(stderr, "read_state:\n"));
>> +
>> + if (is_eof != 0)
>> + goto write_state;
>> +
>> + /* We read in to fifo_in in two steps: first: read in to from
>> + * cur_in to the end of the buffer. last: if free space wrapped
>> + * around, read from fifo_in offset 0 to offset cur_in.
>> + */
>> +
>> + /* Reset fifo head to reduce unnecessary wrap arounds */
>> + cur_in = (used_in == 0) ? 0 : cur_in;
>> +
>> + /* Free space total is reduced by a gap */
>> + free_space = NX_MAX(0, fifo_free_bytes(used_in, fifo_in_len)
>> + - line_sz);
>> +
>> + /* Free space may wrap around as first and last */
>> + first_free = fifo_free_first_bytes(cur_in, used_in, fifo_in_len);
>> + last_free = fifo_free_last_bytes(cur_in, used_in, fifo_in_len);
>> +
>> + /* Start offsets of the free memory */
>> + first_offset = fifo_free_first_offset(cur_in, used_in);
>> + last_offset = fifo_free_last_offset(cur_in, used_in, fifo_in_len);
>> +
>> + /* Reduce read_sz because of the line_sz gap */
>> + read_sz = NX_MIN(free_space, first_free);
>> + n = 0;
>> + if (read_sz > 0) {
>> + /* Read in to offset cur_in + used_in */
>> + n = fread(fifo_in + first_offset, 1, read_sz, inpf);
>> + used_in = used_in + n;
>> + free_space = free_space - n;
>> + assert(n <= read_sz);
>> + if (n != read_sz) {
>> + /* Either EOF or error; exit the read loop */
>> + is_eof = 1;
>> + goto write_state;
>> + }
>> + }
>> +
>> + /* If free space wrapped around */
>> + if (last_free > 0) {
>> + /* Reduce read_sz because of the line_sz gap */
>> + read_sz = NX_MIN(free_space, last_free);
>> + n = 0;
>> + if (read_sz > 0) {
>> + n = fread(fifo_in + last_offset, 1, read_sz, inpf);
>> + used_in = used_in + n; /* Increase used space */
>> + free_space = free_space - n; /* Decrease free space */
>> + assert(n <= read_sz);
>> + if (n != read_sz) {
>> + /* Either EOF or error; exit the read loop */
>> + is_eof = 1;
>> + goto write_state;
>> + }
>> + }
>> + }
>> +
>> + /* At this point we have used_in bytes in fifo_in with the
>> + * data head starting at cur_in and possibly wrapping around.
>> + */
>> +
>> +write_state:
>> +
>> + /* Write decompressed data to output file */
>> +
>> + NXPRT(fprintf(stderr, "write_state:\n"));
>> +
>> + if (used_out == 0)
>> + goto decomp_state;
>> +
>> + /* If fifo_out has data waiting, write it out to the file to
>> + * make free target space for the accelerator used bytes in
>> + * the first and last parts of fifo_out.
>> + */
>> +
>> + first_used = fifo_used_first_bytes(cur_out, used_out, fifo_out_len);
>> + last_used = fifo_used_last_bytes(cur_out, used_out, fifo_out_len);
>> +
>> + write_sz = first_used;
>> +
>> + n = 0;
>> + if (write_sz > 0) {
>> + n = fwrite(fifo_out + cur_out, 1, write_sz, outf);
>> + used_out = used_out - n;
>> + /* Move head of the fifo */
>> + cur_out = (cur_out + n) % fifo_out_len;
>> + assert(n <= write_sz);
>> + if (n != write_sz) {
>> + fprintf(stderr, "error: write\n");
>> + rc = -1;
>> + goto err5;
>> + }
>> + }
>> +
>> + if (last_used > 0) { /* If more data available in the last part */
>> + write_sz = last_used; /* Keep it here for later */
>> + n = 0;
>> + if (write_sz > 0) {
>> + n = fwrite(fifo_out, 1, write_sz, outf);
>> + used_out = used_out - n;
>> + cur_out = (cur_out + n) % fifo_out_len;
>> + assert(n <= write_sz);
>> + if (n != write_sz) {
>> + fprintf(stderr, "error: write\n");
>> + rc = -1;
>> + goto err5;
>> + }
>> + }
>> + }
>> +
>> +decomp_state:
>> +
>> + /* NX decompresses input data */
>> +
>> + NXPRT(fprintf(stderr, "decomp_state:\n"));
>> +
>> + if (is_final)
>> + goto finish_state;
>> +
>> + /* Address/len lists */
>> + clearp_dde(ddl_in);
>> + clearp_dde(ddl_out);
>> +
>> + /* FC, CRC, HistLen, Table 6-6 */
>> + if (resuming) {
>> + /* Resuming a partially decompressed input.
>> + * The key to resume is supplying the 32KB
>> + * dictionary (history) to NX, which is basically
>> + * the last 32KB of output produced.
>> + */
>> + fc = GZIP_FC_DECOMPRESS_RESUME;
>> +
>> + cmdp->cpb.in_crc = cmdp->cpb.out_crc;
>> + cmdp->cpb.in_adler = cmdp->cpb.out_adler;
>> +
>> + /* Round up the history size to quadword. Section 2.10 */
>> + history_len = (history_len + 15) / 16;
>> + putnn(cmdp->cpb, in_histlen, history_len);
>> + history_len = history_len * 16; /* bytes */
>> +
>> + if (history_len > 0) {
>> + /* Chain in the history buffer to the DDE list */
>> + if (cur_out >= history_len) {
>> + nx_append_dde(ddl_in, fifo_out
>> + + (cur_out - history_len),
>> + history_len);
>> + } else {
>> + nx_append_dde(ddl_in, fifo_out
>> + + ((fifo_out_len + cur_out)
>> + - history_len),
>> + history_len - cur_out);
>> + /* Up to 32KB history wraps around fifo_out */
>> + nx_append_dde(ddl_in, fifo_out, cur_out);
>> + }
>> +
>> + }
>> + } else {
>> + /* First decompress job */
>> + fc = GZIP_FC_DECOMPRESS;
>> +
>> + history_len = 0;
>> + /* Writing 0 clears out subc as well */
>> + cmdp->cpb.in_histlen = 0;
>> + total_out = 0;
>> +
>> + put32(cmdp->cpb, in_crc, INIT_CRC);
>> + put32(cmdp->cpb, in_adler, INIT_ADLER);
>> + put32(cmdp->cpb, out_crc, INIT_CRC);
>> + put32(cmdp->cpb, out_adler, INIT_ADLER);
>> +
>> + /* Assuming 10% compression ratio initially; use the
>> + * most recently measured compression ratio as a
>> + * heuristic to estimate the input and output
>> + * sizes. If we give too much input, the target buffer
>> + * overflows and NX cycles are wasted, and then we
>> + * must retry with smaller input size. 1000 is 100%.
>> + */
>> + last_comp_ratio = 100UL;
>> + }
>> + cmdp->crb.gzip_fc = 0;
>> + putnn(cmdp->crb, gzip_fc, fc);
>> +
>> + /*
>> + * NX source buffers
>> + */
>> + first_used = fifo_used_first_bytes(cur_in, used_in, fifo_in_len);
>> + last_used = fifo_used_last_bytes(cur_in, used_in, fifo_in_len);
>> +
>> + if (first_used > 0)
>> + nx_append_dde(ddl_in, fifo_in + cur_in, first_used);
>> +
>> + if (last_used > 0)
>> + nx_append_dde(ddl_in, fifo_in, last_used);
>> +
>> + /*
>> + * NX target buffers
>> + */
>> + first_free = fifo_free_first_bytes(cur_out, used_out, fifo_out_len);
>> + last_free = fifo_free_last_bytes(cur_out, used_out, fifo_out_len);
>> +
>> + /* Reduce output free space amount not to overwrite the history */
>> + int target_max = NX_MAX(0, fifo_free_bytes(used_out, fifo_out_len)
>> + - (1<<16));
>> +
>> + NXPRT(fprintf(stderr, "target_max %d (0x%x)\n", target_max,
>> + target_max));
>> +
>> + first_free = NX_MIN(target_max, first_free);
>> + if (first_free > 0) {
>> + first_offset = fifo_free_first_offset(cur_out, used_out);
>> + nx_append_dde(ddl_out, fifo_out + first_offset, first_free);
>> + }
>> +
>> + if (last_free > 0) {
>> + last_free = NX_MIN(target_max - first_free, last_free);
>> + if (last_free > 0) {
>> + last_offset = fifo_free_last_offset(cur_out, used_out,
>> + fifo_out_len);
>> + nx_append_dde(ddl_out, fifo_out + last_offset,
>> + last_free);
>> + }
>> + }
>> +
>> + /* Target buffer size is used to limit the source data size
>> + * based on previous measurements of compression ratio.
>> + */
>> +
>> + /* source_sz includes history */
>> + source_sz = getp32(ddl_in, ddebc);
>> + assert(source_sz > history_len);
>> + source_sz = source_sz - history_len;
>> +
>> + /* Estimating how much source is needed to 3/4 fill a
>> + * target_max size target buffer. If we overshoot, then NX
>> + * must repeat the job with smaller input and we waste
>> + * bandwidth. If we undershoot then we use more NX calls than
>> + * necessary.
>> + */
>> +
>> + source_sz_estimate = ((uint64_t)target_max * last_comp_ratio * 3UL)
>> + / 4000;
>> +
>> + if (source_sz_estimate < source_sz) {
>> + /* Target might be small, therefore limiting the
>> + * source data.
>> + */
>> + source_sz = source_sz_estimate;
>> + target_sz_estimate = target_max;
>> + } else {
>> + /* Source file might be small, therefore limiting target
>> + * touch pages to a smaller value to save processor cycles.
>> + */
>> + target_sz_estimate = ((uint64_t)source_sz * 1000UL)
>> + / (last_comp_ratio + 1);
>> + target_sz_estimate = NX_MIN(2 * target_sz_estimate,
>> + target_max);
>> + }
>> +
>> + source_sz = source_sz + history_len;
>> +
>> + /* Some NX condition codes require submitting the NX job again.
>> + * Kernel doesn't handle NX page faults. Expects user code to
>> + * touch pages.
>> + */
>> + pgfault_retries = retry_max;
>> +
>> +restart_nx:
>> +
>> + putp32(ddl_in, ddebc, source_sz);
>> +
>> + /* Fault in pages */
>> + nx_touch_pages_dde(ddl_in, 0, page_sz, 0);
>> + nx_touch_pages_dde(ddl_out, target_sz_estimate, page_sz, 1);
>> +
>> + /* Send job to NX */
>> + cc = nx_submit_job(ddl_in, ddl_out, cmdp, devhandle);
>> +
>> + switch (cc) {
>> +
>> + case ERR_NX_TRANSLATION:
>> +
>> + /* We touched the pages ahead of time. In the most common case
>> + * we shouldn't be here. But may be some pages were paged out.
>> + * Kernel should have placed the faulting address to fsaddr.
>> + */
>> + NXPRT(fprintf(stderr, "ERR_NX_TRANSLATION %p\n",
>> + (void *)cmdp->crb.csb.fsaddr));
>> +
>> + /* Touch 1 byte, read-only */
>> + nx_touch_pages((void *)cmdp->crb.csb.fsaddr, 1, page_sz, 0);
>> +
>> + if (pgfault_retries == retry_max) {
>> + /* Try once with exact number of pages */
>> + --pgfault_retries;
>> + goto restart_nx;
>> + } else if (pgfault_retries > 0) {
>> + /* If still faulting try fewer input pages
>> + * assuming memory outage
>> + */
>> + if (source_sz > page_sz)
>> + source_sz = NX_MAX(source_sz / 2, page_sz);
>> + --pgfault_retries;
>> + goto restart_nx;
>> + } else {
>> + fprintf(stderr, "cannot make progress; too many page \
>> + fault retries cc= %d\n", cc);
>> + rc = -1;
>> + goto err5;
>> + }
>> +
>> + case ERR_NX_DATA_LENGTH:
>> +
>> + NXPRT(fprintf(stderr, "ERR_NX_DATA_LENGTH; not an error \
>> + usually; stream may have trailing data\n"));
>> +
>> + /* Not an error in the most common case; it just says
>> + * there is trailing data that we must examine.
>> + *
>> + * CC=3 CE(1)=0 CE(0)=1 indicates partial completion
>> + * Fig.6-7 and Table 6-8.
>> + */
>> + nx_ce = get_csb_ce_ms3b(cmdp->crb.csb);
>> +
>> + if (!csb_ce_termination(nx_ce) &&
>> + csb_ce_partial_completion(nx_ce)) {
>> + /* Check CPB for more information
>> + * spbc and tpbc are valid
>> + */
>> + sfbt = getnn(cmdp->cpb, out_sfbt); /* Table 6-4 */
>> + subc = getnn(cmdp->cpb, out_subc); /* Table 6-4 */
>> + spbc = get32(cmdp->cpb, out_spbc_decomp);
>> + tpbc = get32(cmdp->crb.csb, tpbc);
>> + assert(target_max >= tpbc);
>> +
>> + goto ok_cc3; /* not an error */
>> + } else {
>> + /* History length error when CE(1)=1 CE(0)=0. */
>> + rc = -1;
>> + fprintf(stderr, "history length error cc= %d\n", cc);
>> + goto err5;
>> + }
>> +
>> + case ERR_NX_TARGET_SPACE:
>> +
>> + /* Target buffer not large enough; retry smaller input
>> + * data; give at least 1 byte. SPBC/TPBC are not valid.
>> + */
>> + assert(source_sz > history_len);
>> + source_sz = ((source_sz - history_len + 2) / 2) + history_len;
>> + NXPRT(fprintf(stderr, "ERR_NX_TARGET_SPACE; retry with \
>> + smaller input data src %d hist %d\n", source_sz,
>> + history_len));
>> + goto restart_nx;
>> +
>> + case ERR_NX_OK:
>> +
>> + /* This should not happen for gzip formatted data;
>> + * we need trailing crc and isize
>> + */
>> + fprintf(stderr, "ERR_NX_OK\n");
>> + spbc = get32(cmdp->cpb, out_spbc_decomp);
>> + tpbc = get32(cmdp->crb.csb, tpbc);
>> + assert(target_max >= tpbc);
>> + assert(spbc >= history_len);
>> + source_sz = spbc - history_len;
>> + goto offsets_state;
>> +
>> + default:
>> + fprintf(stderr, "error: cc= %d\n", cc);
>> + rc = -1;
>> + goto err5;
>> + }
>> +
>> +ok_cc3:
>> +
>> + NXPRT(fprintf(stderr, "cc3: sfbt: %x\n", sfbt));
>> +
>> + assert(spbc > history_len);
>> + source_sz = spbc - history_len;
>> +
>> + /* Table 6-4: Source Final Block Type (SFBT) describes the
>> + * last processed deflate block and clues the software how to
>> + * resume the next job. SUBC indicates how many input bits NX
>> + * consumed but did not process. SPBC indicates how many
>> + * bytes of source were given to the accelerator including
>> + * history bytes.
>> + */
>> +
>> + switch (sfbt) {
>> + int dhtlen;
>> +
>> + case 0b0000: /* Deflate final EOB received */
>> +
>> + /* Calculating the checksum start position. */
>> +
>> + source_sz = source_sz - subc / 8;
>> + is_final = 1;
>> + break;
>> +
>> + /* Resume decompression cases are below. Basically
>> + * indicates where NX has suspended and how to resume
>> + * the input stream.
>> + */
>> +
>> + case 0b1000: /* Within a literal block; use rembytecount */
>> + case 0b1001: /* Within a literal block; use rembytecount; bfinal=1 */
>> +
>> + /* Supply the partially processed source byte again */
>> + source_sz = source_sz - ((subc + 7) / 8);
>> +
>> + /* SUBC LS 3bits: number of bits in the first source byte need
>> + * to be processed.
>> + * 000 means all 8 bits; Table 6-3
>> + * Clear subc, histlen, sfbt, rembytecnt, dhtlen
>> + */
>> + cmdp->cpb.in_subc = 0;
>> + cmdp->cpb.in_sfbt = 0;
>> + putnn(cmdp->cpb, in_subc, subc % 8);
>> + putnn(cmdp->cpb, in_sfbt, sfbt);
>> + putnn(cmdp->cpb, in_rembytecnt, getnn(cmdp->cpb,
>> + out_rembytecnt));
>> + break;
>> +
>> + case 0b1010: /* Within a FH block; */
>> + case 0b1011: /* Within a FH block; bfinal=1 */
>> +
>> + source_sz = source_sz - ((subc + 7) / 8);
>> +
>> + /* Clear subc, histlen, sfbt, rembytecnt, dhtlen */
>> + cmdp->cpb.in_subc = 0;
>> + cmdp->cpb.in_sfbt = 0;
>> + putnn(cmdp->cpb, in_subc, subc % 8);
>> + putnn(cmdp->cpb, in_sfbt, sfbt);
>> + break;
>> +
>> + case 0b1100: /* Within a DH block; */
>> + case 0b1101: /* Within a DH block; bfinal=1 */
>> +
>> + source_sz = source_sz - ((subc + 7) / 8);
>> +
>> + /* Clear subc, histlen, sfbt, rembytecnt, dhtlen */
>> + cmdp->cpb.in_subc = 0;
>> + cmdp->cpb.in_sfbt = 0;
>> + putnn(cmdp->cpb, in_subc, subc % 8);
>> + putnn(cmdp->cpb, in_sfbt, sfbt);
>> +
>> + dhtlen = getnn(cmdp->cpb, out_dhtlen);
>> + putnn(cmdp->cpb, in_dhtlen, dhtlen);
>> + assert(dhtlen >= 42);
>> +
>> + /* Round up to a qword */
>> + dhtlen = (dhtlen + 127) / 128;
>> +
>> + while (dhtlen > 0) { /* Copy dht from cpb.out to cpb.in */
>> + --dhtlen;
>> + cmdp->cpb.in_dht[dhtlen] = cmdp->cpb.out_dht[dhtlen];
>> + }
>> + break;
>> +
>> + case 0b1110: /* Within a block header; bfinal=0; */
>> + /* Also given if source data exactly ends (SUBC=0) with
>> + * EOB code with BFINAL=0. Means the next byte will
>> + * contain a block header.
>> + */
>> + case 0b1111: /* within a block header with BFINAL=1. */
>> +
>> + source_sz = source_sz - ((subc + 7) / 8);
>> +
>> + /* Clear subc, histlen, sfbt, rembytecnt, dhtlen */
>> + cmdp->cpb.in_subc = 0;
>> + cmdp->cpb.in_sfbt = 0;
>> + putnn(cmdp->cpb, in_subc, subc % 8);
>> + putnn(cmdp->cpb, in_sfbt, sfbt);
>> + }
>> +
>> +offsets_state:
>> +
>> + /* Adjust the source and target buffer offsets and lengths */
>> +
>> + NXPRT(fprintf(stderr, "offsets_state:\n"));
>> +
>> + /* Delete input data from fifo_in */
>> + used_in = used_in - source_sz;
>> + cur_in = (cur_in + source_sz) % fifo_in_len;
>> + input_file_offset = input_file_offset + source_sz;
>> +
>> + /* Add output data to fifo_out */
>> + used_out = used_out + tpbc;
>> +
>> + assert(used_out <= fifo_out_len);
>> +
>> + total_out = total_out + tpbc;
>> +
>> + /* Deflate history is 32KB max. No need to supply more
>> + * than 32KB on a resume.
>> + */
>> + history_len = (total_out > window_max) ? window_max : total_out;
>> +
>> + /* To estimate expected expansion in the next NX job; 500 means 50%.
>> + * Deflate best case is around 1 to 1000.
>> + */
>> + last_comp_ratio = (1000UL * ((uint64_t)source_sz + 1))
>> + / ((uint64_t)tpbc + 1);
>> + last_comp_ratio = NX_MAX(NX_MIN(1000UL, last_comp_ratio), 1);
>> + NXPRT(fprintf(stderr, "comp_ratio %ld source_sz %d spbc %d tpbc %d\n",
>> + last_comp_ratio, source_sz, spbc, tpbc));
>> +
>> + resuming = 1;
>> +
>> +finish_state:
>> +
>> + NXPRT(fprintf(stderr, "finish_state:\n"));
>> +
>> + if (is_final) {
>> + if (used_out)
>> + goto write_state; /* More data to write out */
>> + else if (used_in < 8) {
>> + /* Need at least 8 more bytes containing gzip crc
>> + * and isize.
>> + */
>> + rc = -1;
>> + goto err4;
>> + } else {
>> + /* Compare checksums and exit */
>> + int i;
>> + char tail[8];
>> + uint32_t cksum, isize;
>> + for (i = 0; i < 8; i++)
>> + tail[i] = fifo_in[(cur_in + i) % fifo_in_len];
>> + fprintf(stderr, "computed checksum %08x isize %08x\n",
>> + cmdp->cpb.out_crc, (uint32_t) (total_out
>> + % (1ULL<<32)));
>> + cksum = (tail[0] | tail[1]<<8 | tail[2]<<16
>> + | tail[3]<<24);
>> + isize = (tail[4] | tail[5]<<8 | tail[6]<<16
>> + | tail[7]<<24);
>> + fprintf(stderr, "stored checksum %08x isize %08x\n",
>> + cksum, isize);
>> +
>> + if (cksum == cmdp->cpb.out_crc && isize == (uint32_t)
>> + (total_out % (1ULL<<32))) {
>> + rc = 0; goto ok1;
>> + } else {
>> + rc = -1; goto err4;
>> + }
>> + }
>> + } else
>> + goto read_state;
>> +
>> + return -1;
>> +
>> +err1:
>> + fprintf(stderr, "error: not a gzip file, expect %x, read %x\n",
>> + expect, c);
>> + return -1;
>> +
>> +err2:
>> + fprintf(stderr, "error: the FLG byte is wrong or not handled by this \
>> + code sample\n");
>> + return -1;
>> +
>> +err3:
>> + fprintf(stderr, "error: gzip header\n");
>> + return -1;
>> +
>> +err4:
>> + fprintf(stderr, "error: checksum\n");
>> +
>> +err5:
>> +ok1:
>> + fprintf(stderr, "decomp is complete: fclose\n");
>> + fclose(outf);
>> +
>> + return rc;
>> +}
>> +
>> +
>> +int main(int argc, char **argv)
>> +{
>> + int rc;
>> + struct sigaction act;
>> + void *handle;
>> +
>> + act.sa_handler = 0;
>> + act.sa_sigaction = sigsegv_handler;
>> + act.sa_flags = SA_SIGINFO;
>> + act.sa_restorer = 0;
>> + sigemptyset(&act.sa_mask);
>> + sigaction(SIGSEGV, &act, NULL);
>> +
>> + handle = nx_function_begin(NX_FUNC_COMP_GZIP, 0);
>> + if (!handle) {
>> + fprintf(stderr, "Unable to init NX, errno %d\n", errno);
>> + exit(-1);
>> + }
>> +
>> + rc = decompress_file(argc, argv, handle);
>> +
>> + nx_function_end(handle);
>> +
>> + return rc;
>> +}
>> --
>> 2.21.0
--
Raphael Moreira Zinsly
IBM
Linux on Power Toolchain
^ permalink raw reply
* Re: [PATCH 14/15] powerpc/watchpoint/xmon: Don't allow breakpoint overwriting
From: Christophe Leroy @ 2020-03-18 13:31 UTC (permalink / raw)
To: Ravi Bangoria
Cc: apopple, mikey, peterz, oleg, npiggin, linux-kernel, paulus,
jolsa, fweisbec, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <56ee9190-2363-efbe-fd94-0b42194e6586@linux.ibm.com>
Le 18/03/2020 à 13:37, Ravi Bangoria a écrit :
>
>
> On 3/17/20 4:40 PM, Christophe Leroy wrote:
>>
>>
>> Le 09/03/2020 à 09:58, Ravi Bangoria a écrit :
>>> Xmon allows overwriting breakpoints because it's supported by only
>>> one dawr. But with multiple dawrs, overwriting becomes ambiguous
>>> or unnecessary complicated. So let's not allow it.
>>
>> Could we drop this completely (I mean the functionnality, not the patch).
>
> Not sure I follow. Isn't the same thing I'm doing?
>
Yes, I think I misunderstood the patch. That seems ok.
Christophe
^ permalink raw reply
* Re: [PATCH v2 0/8] mm/memory_hotplug: allow to specify a default online_type
From: David Hildenbrand @ 2020-03-18 13:50 UTC (permalink / raw)
To: Baoquan He
Cc: Yumei Huang, linux-hyperv, Michal Hocko, Rafael J. Wysocki,
Michal Hocko, linux-mm, Paul Mackerras, K. Y. Srinivasan, Wei Liu,
Stephen Hemminger, Eduardo Habkost, Haiyang Zhang, Wei Yang,
Andrew Morton, Oscar Salvador, Greg Kroah-Hartman, linux-kernel,
Milan Zamazal, Igor Mammedov, Vitaly Kuznetsov, linuxppc-dev
In-Reply-To: <20200318130517.GC30899@MiWiFi-R3L-srv>
On 18.03.20 14:05, Baoquan He wrote:
> On 03/17/20 at 11:49am, David Hildenbrand wrote:
>> Distributions nowadays use udev rules ([1] [2]) to specify if and
>> how to online hotplugged memory. The rules seem to get more complex with
>> many special cases. Due to the various special cases,
>> CONFIG_MEMORY_HOTPLUG_DEFAULT_ONLINE cannot be used. All memory hotplug
>> is handled via udev rules.
>>
>> Everytime we hotplug memory, the udev rule will come to the same
>> conclusion. Especially Hyper-V (but also soon virtio-mem) add a lot of
>> memory in separate memory blocks and wait for memory to get onlined by user
>> space before continuing to add more memory blocks (to not add memory faster
>> than it is getting onlined). This of course slows down the whole memory
>> hotplug process.
>>
>> To make the job of distributions easier and to avoid udev rules that get
>> more and more complicated, let's extend the mechanism provided by
>> - /sys/devices/system/memory/auto_online_blocks
>> - "memhp_default_state=" on the kernel cmdline
>> to be able to specify also "online_movable" as well as "online_kernel"
>
> This patch series looks good, thanks. Since Andrew has merged it to -mm again,
> I won't add my Reviewed-by to bother.
>
> Hi David, Vitaly
>
> There are several things unclear to me.
>
> So, these improved interfaces are used to alleviate the burden of the
> existing udev rules, or try to replace it? As you know, we have been
At least in RHEL, my plan is to replace it / use a udev rules as a
fallback on older kernels (see the example scripts below). But other
distribution can handle it as they want.
> using udev rules to interact between kernel and user space on bare metal,
> and guests who want to hot add/remove.>
> And also the OOM issue in hyperV when onlining pages after adding memory
> block. I am not a virt devel expert, could this happen on bare metal
> system?
Don't think it's relevant on bare metal. If you plug a big DIMM, all
memory blocks will be added first in one shot and then all memory blocks
will be onlined. So it doesn't matter "how fast" you online that memory.
In contrast, Hyper-V (and virtio-mem) add one (or a limited number of)
memory block at a time and wait for them to get onlined.
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: [PATCH v2 0/8] mm/memory_hotplug: allow to specify a default online_type
From: Michal Hocko @ 2020-03-18 13:54 UTC (permalink / raw)
To: Baoquan He
Cc: Yumei Huang, linux-hyperv, David Hildenbrand, Wei Yang, linux-mm,
Paul Mackerras, K. Y. Srinivasan, Wei Liu, Stephen Hemminger,
Rafael J. Wysocki, Eduardo Habkost, Haiyang Zhang, Andrew Morton,
Oscar Salvador, Milan Zamazal, linux-kernel, Greg Kroah-Hartman,
Igor Mammedov, Vitaly Kuznetsov, linuxppc-dev
In-Reply-To: <20200318130517.GC30899@MiWiFi-R3L-srv>
On Wed 18-03-20 21:05:17, Baoquan He wrote:
> On 03/17/20 at 11:49am, David Hildenbrand wrote:
> > Distributions nowadays use udev rules ([1] [2]) to specify if and
> > how to online hotplugged memory. The rules seem to get more complex with
> > many special cases. Due to the various special cases,
> > CONFIG_MEMORY_HOTPLUG_DEFAULT_ONLINE cannot be used. All memory hotplug
> > is handled via udev rules.
> >
> > Everytime we hotplug memory, the udev rule will come to the same
> > conclusion. Especially Hyper-V (but also soon virtio-mem) add a lot of
> > memory in separate memory blocks and wait for memory to get onlined by user
> > space before continuing to add more memory blocks (to not add memory faster
> > than it is getting onlined). This of course slows down the whole memory
> > hotplug process.
> >
> > To make the job of distributions easier and to avoid udev rules that get
> > more and more complicated, let's extend the mechanism provided by
> > - /sys/devices/system/memory/auto_online_blocks
> > - "memhp_default_state=" on the kernel cmdline
> > to be able to specify also "online_movable" as well as "online_kernel"
>
> This patch series looks good, thanks. Since Andrew has merged it to -mm again,
> I won't add my Reviewed-by to bother.
JFYI, Andrew usually adds R-b or A-b tags as they are posted.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH v2 0/8] mm/memory_hotplug: allow to specify a default online_type
From: Vitaly Kuznetsov @ 2020-03-18 13:58 UTC (permalink / raw)
To: Baoquan He, David Hildenbrand
Cc: Yumei Huang, linux-hyperv, Michal Hocko, Rafael J. Wysocki,
Wei Yang, linux-mm, Paul Mackerras, K. Y. Srinivasan, Wei Liu,
Stephen Hemminger, Eduardo Habkost, Haiyang Zhang, Michal Hocko,
Oscar Salvador, Greg Kroah-Hartman, linux-kernel, Milan Zamazal,
Igor Mammedov, Andrew Morton, linuxppc-dev
In-Reply-To: <20200318130517.GC30899@MiWiFi-R3L-srv>
Baoquan He <bhe@redhat.com> writes:
> On 03/17/20 at 11:49am, David Hildenbrand wrote:
>> Distributions nowadays use udev rules ([1] [2]) to specify if and
>> how to online hotplugged memory. The rules seem to get more complex with
>> many special cases. Due to the various special cases,
>> CONFIG_MEMORY_HOTPLUG_DEFAULT_ONLINE cannot be used. All memory hotplug
>> is handled via udev rules.
>>
>> Everytime we hotplug memory, the udev rule will come to the same
>> conclusion. Especially Hyper-V (but also soon virtio-mem) add a lot of
>> memory in separate memory blocks and wait for memory to get onlined by user
>> space before continuing to add more memory blocks (to not add memory faster
>> than it is getting onlined). This of course slows down the whole memory
>> hotplug process.
>>
>> To make the job of distributions easier and to avoid udev rules that get
>> more and more complicated, let's extend the mechanism provided by
>> - /sys/devices/system/memory/auto_online_blocks
>> - "memhp_default_state=" on the kernel cmdline
>> to be able to specify also "online_movable" as well as "online_kernel"
>
> This patch series looks good, thanks. Since Andrew has merged it to -mm again,
> I won't add my Reviewed-by to bother.
>
> Hi David, Vitaly
>
> There are several things unclear to me.
>
> So, these improved interfaces are used to alleviate the burden of the
> existing udev rules, or try to replace it? As you know, we have been
> using udev rules to interact between kernel and user space on bare metal,
> and guests who want to hot add/remove.
With 'auto_online_blocks' interface you don't need the udev rule. David
is trying to make it more versatile.
>
> And also the OOM issue in hyperV when onlining pages after adding memory
> block. I am not a virt devel expert, could this happen on bare metal
> system?
Yes - in theory, very unlikely - in practice.
The root cause of the problem here is adding more memory to the system
requires memory (page tables, memmaps,..) so if your system is low on
memory and you're trying to hotplug A LOT you may run into OOM before
you're able to online anything. With bare metal it's usualy not the
case: servers, which are able to hotplug memory, are usually booted with
enough memory and memory hotplug is a manual action (you need to insert
DIMMs!). But, if you boot your server with e.g. 4G, almost exhaust it
and then try to hotplug e.g. 256G ... well, OOM is almost guaranteed.
With virtual machines it's very common (e.g. with Hyper-V VMs) to boot
them with low memory and hotplug it (automatically, by some management
software) when neededm thus the problem is way more common.
--
Vitaly
^ permalink raw reply
* Re: [PATCH v2 0/8] mm/memory_hotplug: allow to specify a default online_type
From: Baoquan He @ 2020-03-18 14:41 UTC (permalink / raw)
To: Vitaly Kuznetsov
Cc: Yumei Huang, linux-hyperv, Michal Hocko, David Hildenbrand,
Michal Hocko, linux-mm, Paul Mackerras, K. Y. Srinivasan, Wei Liu,
Stephen Hemminger, Rafael J. Wysocki, Eduardo Habkost,
Haiyang Zhang, Wei Yang, Oscar Salvador, Milan Zamazal,
linux-kernel, Greg Kroah-Hartman, Igor Mammedov, Andrew Morton,
linuxppc-dev
In-Reply-To: <87d0993gto.fsf@vitty.brq.redhat.com>
On 03/18/20 at 02:58pm, Vitaly Kuznetsov wrote:
> Baoquan He <bhe@redhat.com> writes:
>
> > On 03/17/20 at 11:49am, David Hildenbrand wrote:
> >> Distributions nowadays use udev rules ([1] [2]) to specify if and
> >> how to online hotplugged memory. The rules seem to get more complex with
> >> many special cases. Due to the various special cases,
> >> CONFIG_MEMORY_HOTPLUG_DEFAULT_ONLINE cannot be used. All memory hotplug
> >> is handled via udev rules.
> >>
> >> Everytime we hotplug memory, the udev rule will come to the same
> >> conclusion. Especially Hyper-V (but also soon virtio-mem) add a lot of
> >> memory in separate memory blocks and wait for memory to get onlined by user
> >> space before continuing to add more memory blocks (to not add memory faster
> >> than it is getting onlined). This of course slows down the whole memory
> >> hotplug process.
> >>
> >> To make the job of distributions easier and to avoid udev rules that get
> >> more and more complicated, let's extend the mechanism provided by
> >> - /sys/devices/system/memory/auto_online_blocks
> >> - "memhp_default_state=" on the kernel cmdline
> >> to be able to specify also "online_movable" as well as "online_kernel"
> >
> > This patch series looks good, thanks. Since Andrew has merged it to -mm again,
> > I won't add my Reviewed-by to bother.
> >
> > Hi David, Vitaly
> >
> > There are several things unclear to me.
> >
> > So, these improved interfaces are used to alleviate the burden of the
> > existing udev rules, or try to replace it? As you know, we have been
> > using udev rules to interact between kernel and user space on bare metal,
> > and guests who want to hot add/remove.
>
> With 'auto_online_blocks' interface you don't need the udev rule. David
> is trying to make it more versatile.
>
> >
> > And also the OOM issue in hyperV when onlining pages after adding memory
> > block. I am not a virt devel expert, could this happen on bare metal
> > system?
>
> Yes - in theory, very unlikely - in practice.
>
> The root cause of the problem here is adding more memory to the system
> requires memory (page tables, memmaps,..) so if your system is low on
> memory and you're trying to hotplug A LOT you may run into OOM before
> you're able to online anything. With bare metal it's usualy not the
> case: servers, which are able to hotplug memory, are usually booted with
> enough memory and memory hotplug is a manual action (you need to insert
> DIMMs!). But, if you boot your server with e.g. 4G, almost exhaust it
> and then try to hotplug e.g. 256G ... well, OOM is almost guaranteed.
Thanks for this detailed explanation.
I finally know why this is a problem in hyperV. But with the current
mechanism, it will happen on any system if thing is done like this.
Is there a reason hyperV need boot with small memory, then enlarge it
with huge memory? Since it's a real case in hyperV, I guess there must
be reason, I am just curious.
> With virtual machines it's very common (e.g. with Hyper-V VMs) to boot
> them with low memory and hotplug it (automatically, by some management
> software) when neededm thus the problem is way more common.
^ permalink raw reply
* Re: [PATCH v2 0/8] mm/memory_hotplug: allow to specify a default online_type
From: Baoquan He @ 2020-03-18 14:41 UTC (permalink / raw)
To: Michal Hocko
Cc: Yumei Huang, linux-hyperv, David Hildenbrand, Wei Yang, linux-mm,
Paul Mackerras, K. Y. Srinivasan, Wei Liu, Stephen Hemminger,
Rafael J. Wysocki, Eduardo Habkost, Haiyang Zhang, Andrew Morton,
Oscar Salvador, Milan Zamazal, linux-kernel, Greg Kroah-Hartman,
Igor Mammedov, Vitaly Kuznetsov, linuxppc-dev
In-Reply-To: <20200318135408.GP21362@dhcp22.suse.cz>
On 03/18/20 at 02:54pm, Michal Hocko wrote:
> On Wed 18-03-20 21:05:17, Baoquan He wrote:
> > On 03/17/20 at 11:49am, David Hildenbrand wrote:
> > > Distributions nowadays use udev rules ([1] [2]) to specify if and
> > > how to online hotplugged memory. The rules seem to get more complex with
> > > many special cases. Due to the various special cases,
> > > CONFIG_MEMORY_HOTPLUG_DEFAULT_ONLINE cannot be used. All memory hotplug
> > > is handled via udev rules.
> > >
> > > Everytime we hotplug memory, the udev rule will come to the same
> > > conclusion. Especially Hyper-V (but also soon virtio-mem) add a lot of
> > > memory in separate memory blocks and wait for memory to get onlined by user
> > > space before continuing to add more memory blocks (to not add memory faster
> > > than it is getting onlined). This of course slows down the whole memory
> > > hotplug process.
> > >
> > > To make the job of distributions easier and to avoid udev rules that get
> > > more and more complicated, let's extend the mechanism provided by
> > > - /sys/devices/system/memory/auto_online_blocks
> > > - "memhp_default_state=" on the kernel cmdline
> > > to be able to specify also "online_movable" as well as "online_kernel"
> >
> > This patch series looks good, thanks. Since Andrew has merged it to -mm again,
> > I won't add my Reviewed-by to bother.
>
> JFYI, Andrew usually adds R-b or A-b tags as they are posted.
Got it, thanks for telling.
^ permalink raw reply
* [RFC 1/2] mm, slub: prevent kmalloc_node crashes and memory leaks
From: Vlastimil Babka @ 2020-03-18 14:42 UTC (permalink / raw)
To: linux-mm
Cc: Sachin Sant, Nathan Lynch, Srikar Dronamraju, linuxppc-dev,
Bharata B Rao, Pekka Enberg, Kirill Tkhai, David Rientjes,
Christopher Lameter, Michal Hocko, Mel Gorman, Joonsoo Kim,
Vlastimil Babka
Sachin reports [1] a crash in SLUB __slab_alloc():
BUG: Kernel NULL pointer dereference on read at 0x000073b0
Faulting instruction address: 0xc0000000003d55f4
Oops: Kernel access of bad area, sig: 11 [#1]
LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
Modules linked in:
CPU: 19 PID: 1 Comm: systemd Not tainted 5.6.0-rc2-next-20200218-autotest #1
NIP: c0000000003d55f4 LR: c0000000003d5b94 CTR: 0000000000000000
REGS: c0000008b37836d0 TRAP: 0300 Not tainted (5.6.0-rc2-next-20200218-autotest)
MSR: 8000000000009033 <SF,EE,ME,IR,DR,RI,LE> CR: 24004844 XER: 00000000
CFAR: c00000000000dec4 DAR: 00000000000073b0 DSISR: 40000000 IRQMASK: 1
GPR00: c0000000003d5b94 c0000008b3783960 c00000000155d400 c0000008b301f500
GPR04: 0000000000000dc0 0000000000000002 c0000000003443d8 c0000008bb398620
GPR08: 00000008ba2f0000 0000000000000001 0000000000000000 0000000000000000
GPR12: 0000000024004844 c00000001ec52a00 0000000000000000 0000000000000000
GPR16: c0000008a1b20048 c000000001595898 c000000001750c18 0000000000000002
GPR20: c000000001750c28 c000000001624470 0000000fffffffe0 5deadbeef0000122
GPR24: 0000000000000001 0000000000000dc0 0000000000000002 c0000000003443d8
GPR28: c0000008b301f500 c0000008bb398620 0000000000000000 c00c000002287180
NIP [c0000000003d55f4] ___slab_alloc+0x1f4/0x760
LR [c0000000003d5b94] __slab_alloc+0x34/0x60
Call Trace:
[c0000008b3783960] [c0000000003d5734] ___slab_alloc+0x334/0x760 (unreliable)
[c0000008b3783a40] [c0000000003d5b94] __slab_alloc+0x34/0x60
[c0000008b3783a70] [c0000000003d6fa0] __kmalloc_node+0x110/0x490
[c0000008b3783af0] [c0000000003443d8] kvmalloc_node+0x58/0x110
[c0000008b3783b30] [c0000000003fee38] mem_cgroup_css_online+0x108/0x270
[c0000008b3783b90] [c000000000235aa8] online_css+0x48/0xd0
[c0000008b3783bc0] [c00000000023eaec] cgroup_apply_control_enable+0x2ec/0x4d0
[c0000008b3783ca0] [c000000000242318] cgroup_mkdir+0x228/0x5f0
[c0000008b3783d10] [c00000000051e170] kernfs_iop_mkdir+0x90/0xf0
[c0000008b3783d50] [c00000000043dc00] vfs_mkdir+0x110/0x230
[c0000008b3783da0] [c000000000441c90] do_mkdirat+0xb0/0x1a0
[c0000008b3783e20] [c00000000000b278] system_call+0x5c/0x68
This is a PowerPC platform with following NUMA topology:
available: 2 nodes (0-1)
node 0 cpus:
node 0 size: 0 MB
node 0 free: 0 MB
node 1 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
node 1 size: 35247 MB
node 1 free: 30907 MB
node distances:
node 0 1
0: 10 40
1: 40 10
possible numa nodes: 0-31
This only happens with a mmotm patch "mm/memcontrol.c: allocate shrinker_map on
appropriate NUMA node" [2] which effectively calls kmalloc_node for each
possible node. SLUB however only allocates kmem_cache_node on online nodes
with present memory, and relies on node_to_mem_node to return such valid node
for other nodes since commit a561ce00b09e ("slub: fall back to
node_to_mem_node() node if allocating on memoryless node"). This is however not
true in this configuration where the _node_numa_mem_ array is not initialized
for nodes 0 and 2-31, thus it contains zeroes and get_partial() ends up
accessing non-allocated kmem_cache_node.
A related issue was reported by Bharata [3] where a similar PowerPC
configuration, but without patch [2] ends up allocating large amounts of pages
by kmalloc-1k kmalloc-512. This seems to have the same underlying issue with
node_to_mem_node() not behaving as expected, and might probably also lead
to an infinite loop with CONFIG_SLUB_CPU_PARTIAL.
This patch should fix both issues by not relying on node_to_mem_node() anymore
and instead simply falling back to NUMA_NO_NODE, when kmalloc_node(node) is
attempted for a node that's not online or has no pages. Also in case
alloc_slab_page() is reached with a non-online node, fallback as well, until
we have a guarantee that all possible nodes have valid NODE_DATA with proper
zonelist for fallback.
[1] https://lore.kernel.org/linux-next/3381CD91-AB3D-4773-BA04-E7A072A63968@linux.vnet.ibm.com/
[2] https://lore.kernel.org/linux-mm/fff0e636-4c36-ed10-281c-8cdb0687c839@virtuozzo.com/
[3] https://lore.kernel.org/linux-mm/20200317092624.GB22538@in.ibm.com/
[4] https://lore.kernel.org/linux-mm/088b5996-faae-8a56-ef9c-5b567125ae54@suse.cz/
Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Reported-by: Bharata B Rao <bharata@linux.ibm.com>
Debugged-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Christopher Lameter <cl@linux.com>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
---
Hi, this is my alternate solution of the SLUB issues to the series [1]. Could
Sachin and Bharata please test whether it fixes the issues?
1) on plain mainline (Bharata) or next (Sachin)
2) the same but with [PATCH 0/3] Offline memoryless cpuless node 0 [2] as I
assume the series was not related to the SLUB issues and will be kept?
Thanks!
[1] https://lore.kernel.org/linux-mm/20200318072810.9735-1-srikar@linux.vnet.ibm.com/
[2] https://lore.kernel.org/linux-mm/20200311110237.5731-1-srikar@linux.vnet.ibm.com/
mm/slub.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/mm/slub.c b/mm/slub.c
index 17dc00e33115..4d798cacdae1 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1511,7 +1511,7 @@ static inline struct page *alloc_slab_page(struct kmem_cache *s,
struct page *page;
unsigned int order = oo_order(oo);
- if (node == NUMA_NO_NODE)
+ if (node == NUMA_NO_NODE || !node_online(node))
page = alloc_pages(flags, order);
else
page = __alloc_pages_node(node, flags, order);
@@ -1973,8 +1973,6 @@ static void *get_partial(struct kmem_cache *s, gfp_t flags, int node,
if (node == NUMA_NO_NODE)
searchnode = numa_mem_id();
- else if (!node_present_pages(node))
- searchnode = node_to_mem_node(node);
object = get_partial_node(s, get_node(s, searchnode), c, flags);
if (object || node != NUMA_NO_NODE)
@@ -2568,12 +2566,15 @@ static void *___slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node,
redo:
if (unlikely(!node_match(page, node))) {
- int searchnode = node;
-
- if (node != NUMA_NO_NODE && !node_present_pages(node))
- searchnode = node_to_mem_node(node);
-
- if (unlikely(!node_match(page, searchnode))) {
+ /*
+ * node_match() false implies node != NUMA_NO_NODE
+ * but if the node is not online or has no pages, just
+ * ignore the constraint
+ */
+ if ((!node_online(node) || !node_present_pages(node))) {
+ node = NUMA_NO_NODE;
+ goto redo;
+ } else {
stat(s, ALLOC_NODE_MISMATCH);
deactivate_slab(s, page, c->freelist, c);
goto new_slab;
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v2 0/8] mm/memory_hotplug: allow to specify a default online_type
From: Baoquan He @ 2020-03-18 14:50 UTC (permalink / raw)
To: David Hildenbrand
Cc: Yumei Huang, linux-hyperv, Michal Hocko, Rafael J. Wysocki,
Michal Hocko, linux-mm, Paul Mackerras, K. Y. Srinivasan, Wei Liu,
Stephen Hemminger, Eduardo Habkost, Haiyang Zhang, Wei Yang,
Andrew Morton, Oscar Salvador, Greg Kroah-Hartman, linux-kernel,
Milan Zamazal, Igor Mammedov, Vitaly Kuznetsov, linuxppc-dev
In-Reply-To: <67a054f6-df07-e4fb-dd4b-e503cb767276@redhat.com>
On 03/18/20 at 02:50pm, David Hildenbrand wrote:
> On 18.03.20 14:05, Baoquan He wrote:
> > On 03/17/20 at 11:49am, David Hildenbrand wrote:
> >> Distributions nowadays use udev rules ([1] [2]) to specify if and
> >> how to online hotplugged memory. The rules seem to get more complex with
> >> many special cases. Due to the various special cases,
> >> CONFIG_MEMORY_HOTPLUG_DEFAULT_ONLINE cannot be used. All memory hotplug
> >> is handled via udev rules.
> >>
> >> Everytime we hotplug memory, the udev rule will come to the same
> >> conclusion. Especially Hyper-V (but also soon virtio-mem) add a lot of
> >> memory in separate memory blocks and wait for memory to get onlined by user
> >> space before continuing to add more memory blocks (to not add memory faster
> >> than it is getting onlined). This of course slows down the whole memory
> >> hotplug process.
> >>
> >> To make the job of distributions easier and to avoid udev rules that get
> >> more and more complicated, let's extend the mechanism provided by
> >> - /sys/devices/system/memory/auto_online_blocks
> >> - "memhp_default_state=" on the kernel cmdline
> >> to be able to specify also "online_movable" as well as "online_kernel"
> >
> > This patch series looks good, thanks. Since Andrew has merged it to -mm again,
> > I won't add my Reviewed-by to bother.
> >
> > Hi David, Vitaly
> >
> > There are several things unclear to me.
> >
> > So, these improved interfaces are used to alleviate the burden of the
> > existing udev rules, or try to replace it? As you know, we have been
>
> At least in RHEL, my plan is to replace it / use a udev rules as a
> fallback on older kernels (see the example scripts below). But other
Ok, got it. Didn't notice the script and the systemd service are your
part of plan, thought you are demonstrating the status. Thanks.
> distribution can handle it as they want.
>
> > using udev rules to interact between kernel and user space on bare metal,
> > and guests who want to hot add/remove.>
> > And also the OOM issue in hyperV when onlining pages after adding memory
> > block. I am not a virt devel expert, could this happen on bare metal
> > system?
>
> Don't think it's relevant on bare metal. If you plug a big DIMM, all
> memory blocks will be added first in one shot and then all memory blocks
> will be onlined. So it doesn't matter "how fast" you online that memory.
>
> In contrast, Hyper-V (and virtio-mem) add one (or a limited number of)
> memory block at a time and wait for them to get onlined.
>
> --
> Thanks,
>
> David / dhildenb
^ permalink raw reply
* Re: [PATCH v2 0/8] mm/memory_hotplug: allow to specify a default online_type
From: Vitaly Kuznetsov @ 2020-03-18 15:00 UTC (permalink / raw)
To: Baoquan He
Cc: Yumei Huang, linux-hyperv, Michal Hocko, David Hildenbrand,
Michal Hocko, linux-mm, Paul Mackerras, K. Y. Srinivasan, Wei Liu,
Stephen Hemminger, Rafael J. Wysocki, Eduardo Habkost,
Haiyang Zhang, Wei Yang, Oscar Salvador, Milan Zamazal,
linux-kernel, Greg Kroah-Hartman, Igor Mammedov, Andrew Morton,
linuxppc-dev
In-Reply-To: <20200318144119.GD30899@MiWiFi-R3L-srv>
Baoquan He <bhe@redhat.com> writes:
> Is there a reason hyperV need boot with small memory, then enlarge it
> with huge memory? Since it's a real case in hyperV, I guess there must
> be reason, I am just curious.
>
It doesn't really *need* to but this can be utilized in e.g. 'hot
standby' schemes I believe. Also, it may be enough if the administrator
is just trying to e.g. double the size of RAM but the VM is already
under memory pressure. I wouldn't say that these cases are common but
afair bugs like 'I tried adding more memory to my VM and it just OOMed'
were reported in the past.
--
Vitaly
^ permalink raw reply
* Re: [PATCH v5 1/3] powerpc/powernv: Interface to define support and preference for a SPR
From: Vaidyanathan Srinivasan @ 2020-03-18 15:17 UTC (permalink / raw)
To: Pratik Rajesh Sampat
Cc: ego, pratik.r.sampat, linuxram, linux-kernel, linuxppc-dev,
psampat
In-Reply-To: <20200317141018.42380-2-psampat@linux.ibm.com>
* Pratik Rajesh Sampat <psampat@linux.ibm.com> [2020-03-17 19:40:16]:
> Define a bitmask interface to determine support for the Self Restore,
> Self Save or both.
>
> Also define an interface to determine the preference of that SPR to
> be strictly saved or restored or encapsulated with an order of preference.
>
> The preference bitmask is shown as below:
> ----------------------------
> |... | 2nd pref | 1st pref |
> ----------------------------
> MSB LSB
>
> The preference from higher to lower is from LSB to MSB with a shift of 8
> bits.
> Example:
> Prefer self save first, if not available then prefer self
> restore
> The preference mask for this scenario will be seen as below.
> ((SELF_RESTORE_STRICT << PREFERENCE_SHIFT) | SELF_SAVE_STRICT)
> ---------------------------------
> |... | Self restore | Self save |
> ---------------------------------
> MSB LSB
>
> Finally, declare a list of preferred SPRs which encapsulate the bitmaks
> for preferred and supported with defaults of both being set to support
> legacy firmware.
>
> This commit also implements using the above interface and retains the
> legacy functionality of self restore.
>
> Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
> Reviewed-by: Ram Pai <linuxram@us.ibm.com>
Reviewed-by: Vaidyanathan Srinivasan <svaidy@linux.ibm.com>
> ---
> arch/powerpc/platforms/powernv/idle.c | 316 +++++++++++++++++++++-----
> 1 file changed, 259 insertions(+), 57 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
> index 78599bca66c2..03fe835aadd1 100644
> --- a/arch/powerpc/platforms/powernv/idle.c
> +++ b/arch/powerpc/platforms/powernv/idle.c
> @@ -32,9 +32,112 @@
> #define P9_STOP_SPR_MSR 2000
> #define P9_STOP_SPR_PSSCR 855
>
> +/* Interface for the stop state supported and preference */
> +#define SELF_RESTORE_TYPE 0
> +#define SELF_SAVE_TYPE 1
> +
> +#define NR_PREFERENCES 2
> +#define PREFERENCE_SHIFT 4
> +#define PREFERENCE_MASK 0xf
> +
> +#define UNSUPPORTED 0x0
> +#define SELF_RESTORE_STRICT 0x1
> +#define SELF_SAVE_STRICT 0x2
> +
> +/*
> + * Bitmask defining the kind of preferences available.
> + * Note : The higher to lower preference is from LSB to MSB, with a shift of
> + * 4 bits.
> + * ----------------------------
> + * | | 2nd pref | 1st pref |
> + * ----------------------------
> + * MSB LSB
> + */
> +/* Prefer Restore if available, otherwise unsupported */
> +#define PREFER_SELF_RESTORE_ONLY SELF_RESTORE_STRICT
> +/* Prefer Save if available, otherwise unsupported */
> +#define PREFER_SELF_SAVE_ONLY SELF_SAVE_STRICT
> +/* Prefer Restore when available, otherwise prefer Save */
> +#define PREFER_RESTORE_SAVE ((SELF_SAVE_STRICT << \
> + PREFERENCE_SHIFT)\
> + | SELF_RESTORE_STRICT)
> +/* Prefer Save when available, otherwise prefer Restore*/
> +#define PREFER_SAVE_RESTORE ((SELF_RESTORE_STRICT <<\
> + PREFERENCE_SHIFT)\
> + | SELF_SAVE_STRICT)
> static u32 supported_cpuidle_states;
> struct pnv_idle_states_t *pnv_idle_states;
> int nr_pnv_idle_states;
> +/* Caching the lpcr & ptcr support to use later */
> +static bool is_lpcr_self_save;
> +static bool is_ptcr_self_save;
> +
> +struct preferred_sprs {
> + u64 spr;
> + u32 preferred_mode;
> + u32 supported_mode;
> +};
> +
> +/*
> + * Preferred mode: Order of precedence when both self-save and self-restore
> + * supported
> + * Supported mode: Default support. Can be overwritten during system
> + * initialization
> + */
> +struct preferred_sprs preferred_sprs[] = {
> + {
> + .spr = SPRN_HSPRG0,
> + .preferred_mode = PREFER_RESTORE_SAVE,
> + .supported_mode = SELF_RESTORE_STRICT,
> + },
> + {
> + .spr = SPRN_LPCR,
> + .preferred_mode = PREFER_RESTORE_SAVE,
> + .supported_mode = SELF_RESTORE_STRICT,
> + },
> + {
> + .spr = SPRN_PTCR,
> + .preferred_mode = PREFER_SAVE_RESTORE,
> + .supported_mode = SELF_RESTORE_STRICT,
> + },
> + {
> + .spr = SPRN_HMEER,
> + .preferred_mode = PREFER_RESTORE_SAVE,
> + .supported_mode = SELF_RESTORE_STRICT,
> + },
> + {
> + .spr = SPRN_HID0,
> + .preferred_mode = PREFER_RESTORE_SAVE,
> + .supported_mode = SELF_RESTORE_STRICT,
> + },
> + {
> + .spr = P9_STOP_SPR_MSR,
> + .preferred_mode = PREFER_RESTORE_SAVE,
> + .supported_mode = SELF_RESTORE_STRICT,
> + },
> + {
> + .spr = P9_STOP_SPR_PSSCR,
> + .preferred_mode = PREFER_SAVE_RESTORE,
> + .supported_mode = SELF_RESTORE_STRICT,
> + },
> + {
> + .spr = SPRN_HID1,
> + .preferred_mode = PREFER_RESTORE_SAVE,
> + .supported_mode = SELF_RESTORE_STRICT,
> + },
> + {
> + .spr = SPRN_HID4,
> + .preferred_mode = PREFER_RESTORE_SAVE,
> + .supported_mode = SELF_RESTORE_STRICT,
> + },
> + {
> + .spr = SPRN_HID5,
> + .preferred_mode = PREFER_RESTORE_SAVE,
> + .supported_mode = SELF_RESTORE_STRICT,
> + }
> +};
> +
> +const int nr_preferred_sprs = ARRAY_SIZE(preferred_sprs);
>
> /*
> * The default stop state that will be used by ppc_md.power_save
> @@ -61,78 +164,170 @@ static bool deepest_stop_found;
>
> static unsigned long power7_offline_type;
>
> -static int pnv_save_sprs_for_deep_states(void)
> +static int pnv_self_restore_sprs(u64 pir, int cpu, u64 spr)
> {
> - int cpu;
> + u64 reg_val;
> int rc;
>
> - /*
> - * hid0, hid1, hid4, hid5, hmeer and lpcr values are symmetric across
> - * all cpus at boot. Get these reg values of current cpu and use the
> - * same across all cpus.
> - */
> - uint64_t lpcr_val = mfspr(SPRN_LPCR);
> - uint64_t hid0_val = mfspr(SPRN_HID0);
> - uint64_t hid1_val = mfspr(SPRN_HID1);
> - uint64_t hid4_val = mfspr(SPRN_HID4);
> - uint64_t hid5_val = mfspr(SPRN_HID5);
> - uint64_t hmeer_val = mfspr(SPRN_HMEER);
> - uint64_t msr_val = MSR_IDLE;
> - uint64_t psscr_val = pnv_deepest_stop_psscr_val;
> -
> - for_each_present_cpu(cpu) {
> - uint64_t pir = get_hard_smp_processor_id(cpu);
> - uint64_t hsprg0_val = (uint64_t)paca_ptrs[cpu];
> -
> - rc = opal_slw_set_reg(pir, SPRN_HSPRG0, hsprg0_val);
> + switch (spr) {
> + case SPRN_HSPRG0:
> + reg_val = (uint64_t)paca_ptrs[cpu];
> + rc = opal_slw_set_reg(pir, SPRN_HSPRG0, reg_val);
> if (rc != 0)
> return rc;
> -
> - rc = opal_slw_set_reg(pir, SPRN_LPCR, lpcr_val);
> + break;
> + case SPRN_LPCR:
> + reg_val = mfspr(SPRN_LPCR);
> + rc = opal_slw_set_reg(pir, SPRN_LPCR, reg_val);
> if (rc != 0)
> return rc;
> -
> + break;
> + case P9_STOP_SPR_MSR:
> + reg_val = MSR_IDLE;
> if (cpu_has_feature(CPU_FTR_ARCH_300)) {
> - rc = opal_slw_set_reg(pir, P9_STOP_SPR_MSR, msr_val);
> + rc = opal_slw_set_reg(pir, P9_STOP_SPR_MSR, reg_val);
> if (rc)
> return rc;
> -
> - rc = opal_slw_set_reg(pir,
> - P9_STOP_SPR_PSSCR, psscr_val);
> -
> + }
> + break;
> + case P9_STOP_SPR_PSSCR:
> + reg_val = pnv_deepest_stop_psscr_val;
> + if (cpu_has_feature(CPU_FTR_ARCH_300)) {
> + rc = opal_slw_set_reg(pir, P9_STOP_SPR_PSSCR, reg_val);
> if (rc)
> return rc;
> }
> -
> - /* HIDs are per core registers */
> + break;
> + case SPRN_HMEER:
> + reg_val = mfspr(SPRN_HMEER);
> if (cpu_thread_in_core(cpu) == 0) {
> -
> - rc = opal_slw_set_reg(pir, SPRN_HMEER, hmeer_val);
> - if (rc != 0)
> + rc = opal_slw_set_reg(pir, SPRN_HMEER, reg_val);
> + if (rc)
> return rc;
> -
> - rc = opal_slw_set_reg(pir, SPRN_HID0, hid0_val);
> - if (rc != 0)
> + }
> + break;
> + case SPRN_HID0:
> + reg_val = mfspr(SPRN_HID0);
> + if (cpu_thread_in_core(cpu) == 0) {
> + rc = opal_slw_set_reg(pir, SPRN_HID0, reg_val);
> + if (rc)
> return rc;
> + }
> + break;
> + case SPRN_HID1:
> + reg_val = mfspr(SPRN_HID1);
> + if (cpu_thread_in_core(cpu) == 0 &&
> + !cpu_has_feature(CPU_FTR_ARCH_300)) {
> + rc = opal_slw_set_reg(pir, SPRN_HID1, reg_val);
> + if (rc)
> + return rc;
> + }
> + break;
> + case SPRN_HID4:
> + reg_val = mfspr(SPRN_HID4);
> + if (cpu_thread_in_core(cpu) == 0 &&
> + !cpu_has_feature(CPU_FTR_ARCH_300)) {
> + rc = opal_slw_set_reg(pir, SPRN_HID4, reg_val);
> + if (rc)
> + return rc;
> + }
> + break;
> + case SPRN_HID5:
> + reg_val = mfspr(SPRN_HID5);
> + if (cpu_thread_in_core(cpu) == 0 &&
> + !cpu_has_feature(CPU_FTR_ARCH_300)) {
> + rc = opal_slw_set_reg(pir, SPRN_HID5, reg_val);
> + if (rc)
> + return rc;
> + }
> + break;
> + case SPRN_PTCR:
> + break;
> + default:
> + return -EINVAL;
> + }
> + return 0;
> +}
>
> - /* Only p8 needs to set extra HID regiters */
> - if (!cpu_has_feature(CPU_FTR_ARCH_300)) {
> -
> - rc = opal_slw_set_reg(pir, SPRN_HID1, hid1_val);
> - if (rc != 0)
> - return rc;
> -
> - rc = opal_slw_set_reg(pir, SPRN_HID4, hid4_val);
> - if (rc != 0)
> - return rc;
> -
> - rc = opal_slw_set_reg(pir, SPRN_HID5, hid5_val);
> - if (rc != 0)
> - return rc;
> +static int pnv_self_save_restore_sprs(void)
> +{
> + int rc, index, cpu, k;
> + u64 pir;
> + struct preferred_sprs curr_spr;
> + bool is_initialized;
> + u32 preferred;
> +
> + is_lpcr_self_save = false;
> + is_ptcr_self_save = false;
> + for_each_present_cpu(cpu) {
> + pir = get_hard_smp_processor_id(cpu);
> + for (index = 0; index < nr_preferred_sprs; index++) {
> + curr_spr = preferred_sprs[index];
> + is_initialized = false;
> + /*
> + * Go through each of the preferences
> + * Check if it is preferred as well as supported
> + */
> + for (k = 0; k < NR_PREFERENCES; k++) {
> + preferred = curr_spr.preferred_mode
> + & PREFERENCE_MASK;
> + if (preferred & curr_spr.supported_mode
> + & SELF_RESTORE_STRICT) {
> + is_initialized = true;
> + rc = pnv_self_restore_sprs(pir, cpu,
> + curr_spr.spr);
> + if (rc != 0)
> + return rc;
> + break;
> + }
> + preferred_sprs[index].preferred_mode =
> + preferred_sprs[index].preferred_mode >>
> + PREFERENCE_SHIFT;
> + curr_spr = preferred_sprs[index];
> + }
> + if (!is_initialized) {
> + if (preferred_sprs[index].spr == SPRN_PTCR ||
> + (cpu_has_feature(CPU_FTR_ARCH_300) &&
> + (preferred_sprs[index].spr == SPRN_HID1 ||
> + preferred_sprs[index].spr == SPRN_HID4 ||
> + preferred_sprs[index].spr == SPRN_HID5)))
> + continue;
> + return OPAL_UNSUPPORTED;
> }
> }
> }
> + return 0;
> +}
>
> +static int pnv_save_sprs_for_deep_states(void)
> +{
> + int rc;
> + int index;
> +
> + /*
> + * Iterate over the preffered SPRs and if even one of them is
> + * still unsupported We cut support for deep stop states
> + */
> + for (index = 0; index < nr_preferred_sprs; index++) {
> + if (preferred_sprs[index].supported_mode == UNSUPPORTED) {
> + if (preferred_sprs[index].spr == SPRN_PTCR ||
> + (cpu_has_feature(CPU_FTR_ARCH_300) &&
> + (preferred_sprs[index].spr == SPRN_HID1 ||
> + preferred_sprs[index].spr == SPRN_HID4 ||
> + preferred_sprs[index].spr == SPRN_HID5)))
> + continue;
> + return OPAL_UNSUPPORTED;
> + }
> + }
> + /*
> + * Try to self-restore the registers that can be self restored if self
> + * restore is active, try the same for the registers that
> + * can be self saved too.
> + * Note : If both are supported, self restore is given more priority
> + */
> + rc = pnv_self_save_restore_sprs();
> + if (rc != 0)
> + return rc;
> return 0;
> }
>
> @@ -658,7 +853,8 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
> mmcr0 = mfspr(SPRN_MMCR0);
> }
> if ((psscr & PSSCR_RL_MASK) >= pnv_first_spr_loss_level) {
> - sprs.lpcr = mfspr(SPRN_LPCR);
> + if (!is_lpcr_self_save)
> + sprs.lpcr = mfspr(SPRN_LPCR);
> sprs.hfscr = mfspr(SPRN_HFSCR);
> sprs.fscr = mfspr(SPRN_FSCR);
> sprs.pid = mfspr(SPRN_PID);
> @@ -672,7 +868,8 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
> sprs.mmcr1 = mfspr(SPRN_MMCR1);
> sprs.mmcr2 = mfspr(SPRN_MMCR2);
>
> - sprs.ptcr = mfspr(SPRN_PTCR);
> + if (!is_ptcr_self_save)
> + sprs.ptcr = mfspr(SPRN_PTCR);
> sprs.rpr = mfspr(SPRN_RPR);
> sprs.tscr = mfspr(SPRN_TSCR);
> if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR))
> @@ -756,7 +953,8 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
> goto core_woken;
>
> /* Per-core SPRs */
> - mtspr(SPRN_PTCR, sprs.ptcr);
> + if (!is_ptcr_self_save)
> + mtspr(SPRN_PTCR, sprs.ptcr);
> mtspr(SPRN_RPR, sprs.rpr);
> mtspr(SPRN_TSCR, sprs.tscr);
>
> @@ -777,7 +975,8 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
> atomic_unlock_and_stop_thread_idle();
>
> /* Per-thread SPRs */
> - mtspr(SPRN_LPCR, sprs.lpcr);
> + if (!is_lpcr_self_save)
> + mtspr(SPRN_LPCR, sprs.lpcr);
> mtspr(SPRN_HFSCR, sprs.hfscr);
> mtspr(SPRN_FSCR, sprs.fscr);
> mtspr(SPRN_PID, sprs.pid);
> @@ -956,8 +1155,11 @@ void pnv_program_cpu_hotplug_lpcr(unsigned int cpu, u64 lpcr_val)
> * Program the LPCR via stop-api only if the deepest stop state
> * can lose hypervisor context.
> */
> - if (supported_cpuidle_states & OPAL_PM_LOSE_FULL_CONTEXT)
> - opal_slw_set_reg(pir, SPRN_LPCR, lpcr_val);
> + if (supported_cpuidle_states & OPAL_PM_LOSE_FULL_CONTEXT) {
> + if (!is_lpcr_self_save)
> + opal_slw_set_reg(pir, SPRN_LPCR,
> + lpcr_val);
> + }
> }
>
> /*
This framework provides a flexible interface to exploit microcode
capabilities to save and restore and SPR. The complexity in the
implementation and the various options are mainly to provide backward
compatibility to OPAL and Linux on top of different microcode
capabilities and platforms.
--Vaidy
^ permalink raw reply
* Re: [PATCH v5 2/3] powerpc/powernv: Introduce Self save support
From: Vaidyanathan Srinivasan @ 2020-03-18 15:22 UTC (permalink / raw)
To: Pratik Rajesh Sampat
Cc: ego, pratik.r.sampat, linuxram, linux-kernel, linuxppc-dev,
psampat
In-Reply-To: <20200317141018.42380-3-psampat@linux.ibm.com>
* Pratik Rajesh Sampat <psampat@linux.ibm.com> [2020-03-17 19:40:17]:
> This commit introduces and leverages the Self save API which OPAL now
> supports.
>
> Add the new Self Save OPAL API call in the list of OPAL calls.
> Implement the self saving of the SPRs based on the support populated
> while respecting it's preferences.
>
> This implementation allows mixing of support for the SPRs, which
> means that a SPR can be self restored while another SPR be self saved if
> they support and prefer it to be so.
>
> Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
> Reviewed-by: Ram Pai <linuxram@us.ibm.com>
Reviewed-by: Vaidyanathan Srinivasan <svaidy@linux.ibm.com>
> ---
> arch/powerpc/include/asm/opal-api.h | 3 ++-
> arch/powerpc/include/asm/opal.h | 1 +
> arch/powerpc/platforms/powernv/idle.c | 22 ++++++++++++++++++++++
> arch/powerpc/platforms/powernv/opal-call.c | 1 +
> 4 files changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
> index c1f25a760eb1..1b6e1a68d431 100644
> --- a/arch/powerpc/include/asm/opal-api.h
> +++ b/arch/powerpc/include/asm/opal-api.h
> @@ -214,7 +214,8 @@
> #define OPAL_SECVAR_GET 176
> #define OPAL_SECVAR_GET_NEXT 177
> #define OPAL_SECVAR_ENQUEUE_UPDATE 178
> -#define OPAL_LAST 178
> +#define OPAL_SLW_SELF_SAVE_REG 181
> +#define OPAL_LAST 181
>
> #define QUIESCE_HOLD 1 /* Spin all calls at entry */
> #define QUIESCE_REJECT 2 /* Fail all calls with OPAL_BUSY */
> diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
> index 9986ac34b8e2..389a85b63805 100644
> --- a/arch/powerpc/include/asm/opal.h
> +++ b/arch/powerpc/include/asm/opal.h
> @@ -203,6 +203,7 @@ int64_t opal_handle_hmi(void);
> int64_t opal_handle_hmi2(__be64 *out_flags);
> int64_t opal_register_dump_region(uint32_t id, uint64_t start, uint64_t end);
> int64_t opal_unregister_dump_region(uint32_t id);
> +int64_t opal_slw_self_save_reg(uint64_t cpu_pir, uint64_t sprn);
> int64_t opal_slw_set_reg(uint64_t cpu_pir, uint64_t sprn, uint64_t val);
> int64_t opal_config_cpu_idle_state(uint64_t state, uint64_t flag);
> int64_t opal_pci_set_phb_cxl_mode(uint64_t phb_id, uint64_t mode, uint64_t pe_number);
> diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
> index 03fe835aadd1..97aeb45e897b 100644
> --- a/arch/powerpc/platforms/powernv/idle.c
> +++ b/arch/powerpc/platforms/powernv/idle.c
> @@ -279,6 +279,26 @@ static int pnv_self_save_restore_sprs(void)
> if (rc != 0)
> return rc;
> break;
> + } else if (preferred & curr_spr.supported_mode
> + & SELF_SAVE_STRICT) {
> + is_initialized = true;
> + if (curr_spr.spr == SPRN_HMEER &&
> + cpu_thread_in_core(cpu) != 0) {
> + continue;
> + }
> + rc = opal_slw_self_save_reg(pir,
> + curr_spr.spr);
> + if (rc != 0)
> + return rc;
> + switch (curr_spr.spr) {
> + case SPRN_LPCR:
> + is_lpcr_self_save = true;
> + break;
> + case SPRN_PTCR:
> + is_ptcr_self_save = true;
> + break;
> + }
> + break;
> }
> preferred_sprs[index].preferred_mode =
> preferred_sprs[index].preferred_mode >>
> @@ -1159,6 +1179,8 @@ void pnv_program_cpu_hotplug_lpcr(unsigned int cpu, u64 lpcr_val)
> if (!is_lpcr_self_save)
> opal_slw_set_reg(pir, SPRN_LPCR,
> lpcr_val);
> + else
> + opal_slw_self_save_reg(pir, SPRN_LPCR);
> }
> }
>
> diff --git a/arch/powerpc/platforms/powernv/opal-call.c b/arch/powerpc/platforms/powernv/opal-call.c
> index 5cd0f52d258f..11e0ceb90de0 100644
> --- a/arch/powerpc/platforms/powernv/opal-call.c
> +++ b/arch/powerpc/platforms/powernv/opal-call.c
> @@ -223,6 +223,7 @@ OPAL_CALL(opal_handle_hmi, OPAL_HANDLE_HMI);
> OPAL_CALL(opal_handle_hmi2, OPAL_HANDLE_HMI2);
> OPAL_CALL(opal_config_cpu_idle_state, OPAL_CONFIG_CPU_IDLE_STATE);
> OPAL_CALL(opal_slw_set_reg, OPAL_SLW_SET_REG);
> +OPAL_CALL(opal_slw_self_save_reg, OPAL_SLW_SELF_SAVE_REG);
> OPAL_CALL(opal_register_dump_region, OPAL_REGISTER_DUMP_REGION);
> OPAL_CALL(opal_unregister_dump_region, OPAL_UNREGISTER_DUMP_REGION);
> OPAL_CALL(opal_pci_set_phb_cxl_mode, OPAL_PCI_SET_PHB_CAPI_MODE);
> --
The new opal_slw_self_save_reg() call and related interface are more
ideal to provide backward compatibility and simplifies implementation
for future platforms.
--Vaidy
^ permalink raw reply
* Re: [PATCH v5 3/3] powerpc/powernv: Parse device tree, population of SPR support
From: Vaidyanathan Srinivasan @ 2020-03-18 15:24 UTC (permalink / raw)
To: Pratik Rajesh Sampat
Cc: ego, pratik.r.sampat, linuxram, linux-kernel, linuxppc-dev,
psampat
In-Reply-To: <20200317141018.42380-4-psampat@linux.ibm.com>
* Pratik Rajesh Sampat <psampat@linux.ibm.com> [2020-03-17 19:40:18]:
> Parse the device tree for nodes self-save, self-restore and populate
> support for the preferred SPRs based what was advertised by the device
> tree.
>
> Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
> Reviewed-by: Ram Pai <linuxram@us.ibm.com>
Reviewed-by: Vaidyanathan Srinivasan <svaidy@linux.ibm.com>
>
> ---
> .../bindings/powerpc/opal/power-mgt.txt | 10 +++
> arch/powerpc/platforms/powernv/idle.c | 78 +++++++++++++++++++
> 2 files changed, 88 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt b/Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt
> index 9d619e955576..093cb5fe3d2d 100644
> --- a/Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt
> +++ b/Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt
> @@ -116,3 +116,13 @@ otherwise. The length of all the property arrays must be the same.
> which of the fields of the PMICR are set in the corresponding
> entries in ibm,cpu-idle-state-pmicr. This is an optional
> property on POWER8 and is absent on POWER9.
> +
> +- self-restore:
> + Array of unsigned 64-bit values containing a property for sprn-mask
> + with each bit indicating the index of the supported SPR for the
> + functionality. This is an optional property for both Power8 and Power9
> +
> +- self-save:
> + Array of unsigned 64-bit values containing a property for sprn-mask
> + with each bit indicating the index of the supported SPR for the
> + functionality. This is an optional property for both Power8 and Power9
> diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
> index 97aeb45e897b..c39111b338ff 100644
> --- a/arch/powerpc/platforms/powernv/idle.c
> +++ b/arch/powerpc/platforms/powernv/idle.c
> @@ -1436,6 +1436,81 @@ static void __init pnv_probe_idle_states(void)
> supported_cpuidle_states |= pnv_idle_states[i].flags;
> }
>
> +/*
> + * Extracts and populates the self save or restore capabilities
> + * passed from the device tree node
> + */
> +static int extract_save_restore_state_dt(struct device_node *np, int type)
> +{
> + int nr_sprns = 0, i, bitmask_index;
> + u64 *temp_u64;
> + u64 bit_pos;
> +
> + nr_sprns = of_property_count_u64_elems(np, "sprn-bitmask");
> + if (nr_sprns <= 0)
> + return -EINVAL;
> + temp_u64 = kcalloc(nr_sprns, sizeof(u64), GFP_KERNEL);
> + if (of_property_read_u64_array(np, "sprn-bitmask",
> + temp_u64, nr_sprns)) {
> + pr_warn("cpuidle-powernv: failed to find registers in DT\n");
> + kfree(temp_u64);
> + return -EINVAL;
> + }
> + /*
> + * Populate acknowledgment of support for the sprs in the global vector
> + * gotten by the registers supplied by the firmware.
> + * The registers are in a bitmask, bit index within
> + * that specifies the SPR
> + */
> + for (i = 0; i < nr_preferred_sprs; i++) {
> + bitmask_index = BIT_WORD(preferred_sprs[i].spr);
> + bit_pos = BIT_MASK(preferred_sprs[i].spr);
> + if ((temp_u64[bitmask_index] & bit_pos) == 0) {
> + if (type == SELF_RESTORE_TYPE)
> + preferred_sprs[i].supported_mode &=
> + ~SELF_RESTORE_STRICT;
> + else
> + preferred_sprs[i].supported_mode &=
> + ~SELF_SAVE_STRICT;
> + continue;
> + }
> + if (type == SELF_RESTORE_TYPE) {
> + preferred_sprs[i].supported_mode |=
> + SELF_RESTORE_STRICT;
> + } else {
> + preferred_sprs[i].supported_mode |=
> + SELF_SAVE_STRICT;
> + }
> + }
> +
> + kfree(temp_u64);
> + return 0;
> +}
> +
> +static int pnv_parse_deepstate_dt(void)
> +{
> + struct device_node *np;
> + int rc = 0, i;
> +
> + /* Self restore register population */
> + np = of_find_compatible_node(NULL, NULL, "ibm,opal-self-restore");
> + if (np) {
> + rc = extract_save_restore_state_dt(np, SELF_RESTORE_TYPE);
> + if (rc != 0)
> + return rc;
> + }
> + /* Self save register population */
> + np = of_find_compatible_node(NULL, NULL, "ibm,opal-self-save");
> + if (!np) {
> + for (i = 0; i < nr_preferred_sprs; i++)
> + preferred_sprs[i].supported_mode &= ~SELF_SAVE_STRICT;
> + } else {
> + rc = extract_save_restore_state_dt(np, SELF_SAVE_TYPE);
> + }
> + of_node_put(np);
> + return rc;
> +}
> +
> /*
> * This function parses device-tree and populates all the information
> * into pnv_idle_states structure. It also sets up nr_pnv_idle_states
> @@ -1584,6 +1659,9 @@ static int __init pnv_init_idle_states(void)
> return rc;
> pnv_probe_idle_states();
>
> + rc = pnv_parse_deepstate_dt();
> + if (rc)
> + return rc;
> if (!cpu_has_feature(CPU_FTR_ARCH_300)) {
> if (!(supported_cpuidle_states & OPAL_PM_SLEEP_ENABLED_ER1)) {
> power7_fastsleep_workaround_entry = false;
> --
Thanks Michael for the detailed review and feedback. Your review
comments have been addressed by Pratik.
--Vaidy
^ permalink raw reply
* Re: [RFC 1/2] mm, slub: prevent kmalloc_node crashes and memory leaks
From: Bharata B Rao @ 2020-03-18 16:06 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Sachin Sant, Nathan Lynch, Srikar Dronamraju, linuxppc-dev,
Michal Hocko, Pekka Enberg, linux-mm, Kirill Tkhai,
David Rientjes, Christopher Lameter, Mel Gorman, Joonsoo Kim
In-Reply-To: <20200318144220.18083-1-vbabka@suse.cz>
On Wed, Mar 18, 2020 at 03:42:19PM +0100, Vlastimil Babka wrote:
> This is a PowerPC platform with following NUMA topology:
>
> available: 2 nodes (0-1)
> node 0 cpus:
> node 0 size: 0 MB
> node 0 free: 0 MB
> node 1 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
> node 1 size: 35247 MB
> node 1 free: 30907 MB
> node distances:
> node 0 1
> 0: 10 40
> 1: 40 10
>
> possible numa nodes: 0-31
>
> A related issue was reported by Bharata [3] where a similar PowerPC
> configuration, but without patch [2] ends up allocating large amounts of pages
> by kmalloc-1k kmalloc-512. This seems to have the same underlying issue with
> node_to_mem_node() not behaving as expected, and might probably also lead
> to an infinite loop with CONFIG_SLUB_CPU_PARTIAL.
This patch doesn't fix the issue of kmalloc caches consuming more
memory for the above mentioned topology. Also CONFIG_SLUB_CPU_PARTIAL is set
here and I have not observed infinite loop till now.
Or, are you expecting your fix to work on top of Srikar's other patchset
https://lore.kernel.org/linuxppc-dev/20200311110237.5731-1-srikar@linux.vnet.ibm.com/t/#u ?
With the above patchset, no fix is required to address increased memory
consumption of kmalloc caches because this patchset prevents such
topology from occuring thereby making it impossible for the problem
to surface (or at least impossible for the specific topology that I
mentioned)
> diff --git a/mm/slub.c b/mm/slub.c
> index 17dc00e33115..4d798cacdae1 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -1511,7 +1511,7 @@ static inline struct page *alloc_slab_page(struct kmem_cache *s,
> struct page *page;
> unsigned int order = oo_order(oo);
>
> - if (node == NUMA_NO_NODE)
> + if (node == NUMA_NO_NODE || !node_online(node))
> page = alloc_pages(flags, order);
> else
> page = __alloc_pages_node(node, flags, order);
> @@ -1973,8 +1973,6 @@ static void *get_partial(struct kmem_cache *s, gfp_t flags, int node,
>
> if (node == NUMA_NO_NODE)
> searchnode = numa_mem_id();
> - else if (!node_present_pages(node))
> - searchnode = node_to_mem_node(node);
We still come here with memory-less node=0 (and not NUMA_NO_NODE), fail to
find partial slab, go back and allocate a new one thereby continuosly
increasing the number of newly allocated slabs.
>
> object = get_partial_node(s, get_node(s, searchnode), c, flags);
> if (object || node != NUMA_NO_NODE)
> @@ -2568,12 +2566,15 @@ static void *___slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node,
> redo:
>
> if (unlikely(!node_match(page, node))) {
> - int searchnode = node;
> -
> - if (node != NUMA_NO_NODE && !node_present_pages(node))
> - searchnode = node_to_mem_node(node);
> -
> - if (unlikely(!node_match(page, searchnode))) {
> + /*
> + * node_match() false implies node != NUMA_NO_NODE
> + * but if the node is not online or has no pages, just
> + * ignore the constraint
> + */
> + if ((!node_online(node) || !node_present_pages(node))) {
> + node = NUMA_NO_NODE;
> + goto redo;
Many calls for allocating slab object from memory-less node 0 in my case
don't even hit the above check because they get short circuited by
goto new_slab label which is present a few lines above. Hence I don't see
any reduction in the amount of slab memory with this fix.
Regards,
Bharata.
^ permalink raw reply
* Re: [RFC 1/2] mm, slub: prevent kmalloc_node crashes and memory leaks
From: Vlastimil Babka @ 2020-03-18 16:10 UTC (permalink / raw)
To: bharata
Cc: Sachin Sant, Nathan Lynch, Srikar Dronamraju, linuxppc-dev,
Michal Hocko, Pekka Enberg, linux-mm, Kirill Tkhai,
David Rientjes, Christopher Lameter, Mel Gorman, Joonsoo Kim
In-Reply-To: <20200318160610.GD26049@in.ibm.com>
On 3/18/20 5:06 PM, Bharata B Rao wrote:
> On Wed, Mar 18, 2020 at 03:42:19PM +0100, Vlastimil Babka wrote:
>> This is a PowerPC platform with following NUMA topology:
>>
>> available: 2 nodes (0-1)
>> node 0 cpus:
>> node 0 size: 0 MB
>> node 0 free: 0 MB
>> node 1 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
>> node 1 size: 35247 MB
>> node 1 free: 30907 MB
>> node distances:
>> node 0 1
>> 0: 10 40
>> 1: 40 10
>>
>> possible numa nodes: 0-31
>>
>> A related issue was reported by Bharata [3] where a similar PowerPC
>> configuration, but without patch [2] ends up allocating large amounts of pages
>> by kmalloc-1k kmalloc-512. This seems to have the same underlying issue with
>> node_to_mem_node() not behaving as expected, and might probably also lead
>> to an infinite loop with CONFIG_SLUB_CPU_PARTIAL.
>
> This patch doesn't fix the issue of kmalloc caches consuming more
> memory for the above mentioned topology. Also CONFIG_SLUB_CPU_PARTIAL is set
> here and I have not observed infinite loop till now.
OK that means something is wrong with my analysis.
> Or, are you expecting your fix to work on top of Srikar's other patchset
> https://lore.kernel.org/linuxppc-dev/20200311110237.5731-1-srikar@linux.vnet.ibm.com/t/#u ?
No, I hoped it would work on mainline.
> With the above patchset, no fix is required to address increased memory
> consumption of kmalloc caches because this patchset prevents such
> topology from occuring thereby making it impossible for the problem
> to surface (or at least impossible for the specific topology that I
> mentioned)
Right, I hope to fix it nevertheless.
>> diff --git a/mm/slub.c b/mm/slub.c
>> index 17dc00e33115..4d798cacdae1 100644
>> --- a/mm/slub.c
>> +++ b/mm/slub.c
>> @@ -1511,7 +1511,7 @@ static inline struct page *alloc_slab_page(struct kmem_cache *s,
>> struct page *page;
>> unsigned int order = oo_order(oo);
>>
>> - if (node == NUMA_NO_NODE)
>> + if (node == NUMA_NO_NODE || !node_online(node))
>> page = alloc_pages(flags, order);
>> else
>> page = __alloc_pages_node(node, flags, order);
>> @@ -1973,8 +1973,6 @@ static void *get_partial(struct kmem_cache *s, gfp_t flags, int node,
>>
>> if (node == NUMA_NO_NODE)
>> searchnode = numa_mem_id();
>> - else if (!node_present_pages(node))
>> - searchnode = node_to_mem_node(node);
>
> We still come here with memory-less node=0 (and not NUMA_NO_NODE), fail to
> find partial slab, go back and allocate a new one thereby continuosly
> increasing the number of newly allocated slabs.
>
>>
>> object = get_partial_node(s, get_node(s, searchnode), c, flags);
>> if (object || node != NUMA_NO_NODE)
>> @@ -2568,12 +2566,15 @@ static void *___slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node,
>> redo:
>>
>> if (unlikely(!node_match(page, node))) {
>> - int searchnode = node;
>> -
>> - if (node != NUMA_NO_NODE && !node_present_pages(node))
>> - searchnode = node_to_mem_node(node);
>> -
>> - if (unlikely(!node_match(page, searchnode))) {
>> + /*
>> + * node_match() false implies node != NUMA_NO_NODE
>> + * but if the node is not online or has no pages, just
>> + * ignore the constraint
>> + */
>> + if ((!node_online(node) || !node_present_pages(node))) {
>> + node = NUMA_NO_NODE;
>> + goto redo;
>
> Many calls for allocating slab object from memory-less node 0 in my case
> don't even hit the above check because they get short circuited by
> goto new_slab label which is present a few lines above. Hence I don't see
> any reduction in the amount of slab memory with this fix.
Thanks a lot for the info, I will try again :)
> Regards,
> Bharata.
>
^ permalink raw reply
* Re: [RFC 1/2] mm, slub: prevent kmalloc_node crashes and memory leaks
From: Vlastimil Babka @ 2020-03-18 16:51 UTC (permalink / raw)
To: bharata
Cc: Sachin Sant, Nathan Lynch, Srikar Dronamraju, linuxppc-dev,
Michal Hocko, Pekka Enberg, linux-mm, Kirill Tkhai,
David Rientjes, Christopher Lameter, Mel Gorman, Joonsoo Kim
In-Reply-To: <20200318160610.GD26049@in.ibm.com>
On 3/18/20 5:06 PM, Bharata B Rao wrote:
>> @@ -2568,12 +2566,15 @@ static void *___slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node,
>> redo:
>>
>> if (unlikely(!node_match(page, node))) {
>> - int searchnode = node;
>> -
>> - if (node != NUMA_NO_NODE && !node_present_pages(node))
>> - searchnode = node_to_mem_node(node);
>> -
>> - if (unlikely(!node_match(page, searchnode))) {
>> + /*
>> + * node_match() false implies node != NUMA_NO_NODE
>> + * but if the node is not online or has no pages, just
>> + * ignore the constraint
>> + */
>> + if ((!node_online(node) || !node_present_pages(node))) {
>> + node = NUMA_NO_NODE;
>> + goto redo;
>
> Many calls for allocating slab object from memory-less node 0 in my case
> don't even hit the above check because they get short circuited by
> goto new_slab label which is present a few lines above. Hence I don't see
> any reduction in the amount of slab memory with this fix.
>
> Regards,
> Bharata.
OK how about this version? It's somewhat ugly, but important is that the fast
path case (c->page exists) is unaffected and another common case (c->page is
NULL, but node is NUMA_NO_NODE) is just one extra check - impossible to avoid at
some point anyway.
----8<----
From d1675363c2ddc3758e5c0d0f435ca46818219194 Mon Sep 17 00:00:00 2001
From: Vlastimil Babka <vbabka@suse.cz>
Date: Wed, 18 Mar 2020 14:47:33 +0100
Subject: [RFC 1/2] mm, slub: prevent kmalloc_node crashes and memory leaks
Sachin reports [1] a crash in SLUB __slab_alloc():
BUG: Kernel NULL pointer dereference on read at 0x000073b0
Faulting instruction address: 0xc0000000003d55f4
Oops: Kernel access of bad area, sig: 11 [#1]
LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
Modules linked in:
CPU: 19 PID: 1 Comm: systemd Not tainted 5.6.0-rc2-next-20200218-autotest #1
NIP: c0000000003d55f4 LR: c0000000003d5b94 CTR: 0000000000000000
REGS: c0000008b37836d0 TRAP: 0300 Not tainted (5.6.0-rc2-next-20200218-autotest)
MSR: 8000000000009033 <SF,EE,ME,IR,DR,RI,LE> CR: 24004844 XER: 00000000
CFAR: c00000000000dec4 DAR: 00000000000073b0 DSISR: 40000000 IRQMASK: 1
GPR00: c0000000003d5b94 c0000008b3783960 c00000000155d400 c0000008b301f500
GPR04: 0000000000000dc0 0000000000000002 c0000000003443d8 c0000008bb398620
GPR08: 00000008ba2f0000 0000000000000001 0000000000000000 0000000000000000
GPR12: 0000000024004844 c00000001ec52a00 0000000000000000 0000000000000000
GPR16: c0000008a1b20048 c000000001595898 c000000001750c18 0000000000000002
GPR20: c000000001750c28 c000000001624470 0000000fffffffe0 5deadbeef0000122
GPR24: 0000000000000001 0000000000000dc0 0000000000000002 c0000000003443d8
GPR28: c0000008b301f500 c0000008bb398620 0000000000000000 c00c000002287180
NIP [c0000000003d55f4] ___slab_alloc+0x1f4/0x760
LR [c0000000003d5b94] __slab_alloc+0x34/0x60
Call Trace:
[c0000008b3783960] [c0000000003d5734] ___slab_alloc+0x334/0x760 (unreliable)
[c0000008b3783a40] [c0000000003d5b94] __slab_alloc+0x34/0x60
[c0000008b3783a70] [c0000000003d6fa0] __kmalloc_node+0x110/0x490
[c0000008b3783af0] [c0000000003443d8] kvmalloc_node+0x58/0x110
[c0000008b3783b30] [c0000000003fee38] mem_cgroup_css_online+0x108/0x270
[c0000008b3783b90] [c000000000235aa8] online_css+0x48/0xd0
[c0000008b3783bc0] [c00000000023eaec] cgroup_apply_control_enable+0x2ec/0x4d0
[c0000008b3783ca0] [c000000000242318] cgroup_mkdir+0x228/0x5f0
[c0000008b3783d10] [c00000000051e170] kernfs_iop_mkdir+0x90/0xf0
[c0000008b3783d50] [c00000000043dc00] vfs_mkdir+0x110/0x230
[c0000008b3783da0] [c000000000441c90] do_mkdirat+0xb0/0x1a0
[c0000008b3783e20] [c00000000000b278] system_call+0x5c/0x68
This is a PowerPC platform with following NUMA topology:
available: 2 nodes (0-1)
node 0 cpus:
node 0 size: 0 MB
node 0 free: 0 MB
node 1 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
node 1 size: 35247 MB
node 1 free: 30907 MB
node distances:
node 0 1
0: 10 40
1: 40 10
possible numa nodes: 0-31
This only happens with a mmotm patch "mm/memcontrol.c: allocate shrinker_map on
appropriate NUMA node" [2] which effectively calls kmalloc_node for each
possible node. SLUB however only allocates kmem_cache_node on online nodes
with present memory, and relies on node_to_mem_node to return such valid node
for other nodes since commit a561ce00b09e ("slub: fall back to
node_to_mem_node() node if allocating on memoryless node"). This is however not
true in this configuration where the _node_numa_mem_ array is not initialized
for nodes 0 and 2-31, thus it contains zeroes and get_partial() ends up
accessing non-allocated kmem_cache_node.
A related issue was reported by Bharata [3] where a similar PowerPC
configuration, but without patch [2] ends up allocating large amounts of pages
by kmalloc-1k kmalloc-512. This seems to have the same underlying issue with
node_to_mem_node() not behaving as expected, and might probably also lead
to an infinite loop with CONFIG_SLUB_CPU_PARTIAL.
This patch should fix both issues by not relying on node_to_mem_node() anymore
and instead simply falling back to NUMA_NO_NODE, when kmalloc_node(node) is
attempted for a node that's not online or has no pages. Also in case
alloc_slab_page() is reached with a non-online node, fallback as well, until
we have a guarantee that all possible nodes have valid NODE_DATA with proper
zonelist for fallback.
[1] https://lore.kernel.org/linux-next/3381CD91-AB3D-4773-BA04-E7A072A63968@linux.vnet.ibm.com/
[2] https://lore.kernel.org/linux-mm/fff0e636-4c36-ed10-281c-8cdb0687c839@virtuozzo.com/
[3] https://lore.kernel.org/linux-mm/20200317092624.GB22538@in.ibm.com/
[4] https://lore.kernel.org/linux-mm/088b5996-faae-8a56-ef9c-5b567125ae54@suse.cz/
Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Reported-by: Bharata B Rao <bharata@linux.ibm.com>
Debugged-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Christopher Lameter <cl@linux.com>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
---
mm/slub.c | 28 ++++++++++++++++++----------
1 file changed, 18 insertions(+), 10 deletions(-)
diff --git a/mm/slub.c b/mm/slub.c
index 17dc00e33115..60352f80eb33 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1511,7 +1511,7 @@ static inline struct page *alloc_slab_page(struct kmem_cache *s,
struct page *page;
unsigned int order = oo_order(oo);
- if (node == NUMA_NO_NODE)
+ if (node == NUMA_NO_NODE || !node_online(node))
page = alloc_pages(flags, order);
else
page = __alloc_pages_node(node, flags, order);
@@ -1973,8 +1973,6 @@ static void *get_partial(struct kmem_cache *s, gfp_t flags, int node,
if (node == NUMA_NO_NODE)
searchnode = numa_mem_id();
- else if (!node_present_pages(node))
- searchnode = node_to_mem_node(node);
object = get_partial_node(s, get_node(s, searchnode), c, flags);
if (object || node != NUMA_NO_NODE)
@@ -2563,17 +2561,27 @@ static void *___slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node,
struct page *page;
page = c->page;
- if (!page)
+ if (!page) {
+ /*
+ * if the node is not online or has no pages, just
+ * ignore the constraint
+ */
+ if (unlikely(node != NUMA_NO_NODE &&
+ (!node_online(node) || !node_present_pages(node))))
+ node = NUMA_NO_NODE;
goto new_slab;
+ }
redo:
if (unlikely(!node_match(page, node))) {
- int searchnode = node;
-
- if (node != NUMA_NO_NODE && !node_present_pages(node))
- searchnode = node_to_mem_node(node);
-
- if (unlikely(!node_match(page, searchnode))) {
+ /*
+ * same as above but node_match() being false already
+ * implies node != NUMA_NO_NODE
+ */
+ if ((!node_online(node) || !node_present_pages(node))) {
+ node = NUMA_NO_NODE;
+ goto redo;
+ } else {
stat(s, ALLOC_NODE_MISMATCH);
deactivate_slab(s, page, c->freelist, c);
goto new_slab;
--
2.25.1
^ permalink raw reply related
* Re: [PATCH] powerpc/vdso: Fix multiple issues with sys_call_table
From: kbuild test robot @ 2020-03-18 17:06 UTC (permalink / raw)
To: Anton Blanchard; +Cc: kbuild-all, linux-kernel, Nicholas Piggin, linuxppc-dev
In-Reply-To: <20200306135705.7f80fcad@kryten.localdomain>
[-- Attachment #1: Type: text/plain, Size: 4105 bytes --]
Hi Anton,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on powerpc/next]
[also build test WARNING on linux/master linus/master v5.6-rc6 next-20200317]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Anton-Blanchard/powerpc-vdso-Fix-multiple-issues-with-sys_call_table/20200306-134043
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-randconfig-a001-20200318 (attached as .config)
compiler: powerpc-linux-gcc (GCC) 9.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=9.2.0 make.cross ARCH=powerpc
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
arch/powerpc/kernel/vdso.c: In function 'vdso_setup_syscall_map':
>> arch/powerpc/kernel/vdso.c:662:25: warning: comparison between pointer and integer
662 | if (sys_call_table[i] != sys_ni_syscall)
| ^~
vim +662 arch/powerpc/kernel/vdso.c
a7f290dad32ee3 Benjamin Herrenschmidt 2005-11-11 641
a7f290dad32ee3 Benjamin Herrenschmidt 2005-11-11 642 /*
a7f290dad32ee3 Benjamin Herrenschmidt 2005-11-11 643 * Called from setup_arch to initialize the bitmap of available
a7f290dad32ee3 Benjamin Herrenschmidt 2005-11-11 644 * syscalls in the systemcfg page
a7f290dad32ee3 Benjamin Herrenschmidt 2005-11-11 645 */
a7f290dad32ee3 Benjamin Herrenschmidt 2005-11-11 646 static void __init vdso_setup_syscall_map(void)
a7f290dad32ee3 Benjamin Herrenschmidt 2005-11-11 647 {
a7f290dad32ee3 Benjamin Herrenschmidt 2005-11-11 648 unsigned int i;
457c1792bcc570 Anton Blanchard 2020-03-06 649 unsigned long ni_syscall;
a7f290dad32ee3 Benjamin Herrenschmidt 2005-11-11 650
457c1792bcc570 Anton Blanchard 2020-03-06 651 ni_syscall = (unsigned long)dereference_kernel_function_descriptor(sys_ni_syscall);
a7f290dad32ee3 Benjamin Herrenschmidt 2005-11-11 652
f43194e45852b0 Rashmica Gupta 2015-11-19 653 for (i = 0; i < NR_syscalls; i++) {
a7f290dad32ee3 Benjamin Herrenschmidt 2005-11-11 654 #ifdef CONFIG_PPC64
457c1792bcc570 Anton Blanchard 2020-03-06 655 if (sys_call_table[i] != ni_syscall)
a7f290dad32ee3 Benjamin Herrenschmidt 2005-11-11 656 vdso_data->syscall_map_64[i >> 5] |=
a7f290dad32ee3 Benjamin Herrenschmidt 2005-11-11 657 0x80000000UL >> (i & 0x1f);
457c1792bcc570 Anton Blanchard 2020-03-06 658 if (compat_sys_call_table[i] != ni_syscall)
a7f290dad32ee3 Benjamin Herrenschmidt 2005-11-11 659 vdso_data->syscall_map_32[i >> 5] |=
a7f290dad32ee3 Benjamin Herrenschmidt 2005-11-11 660 0x80000000UL >> (i & 0x1f);
a7f290dad32ee3 Benjamin Herrenschmidt 2005-11-11 661 #else /* CONFIG_PPC64 */
a7f290dad32ee3 Benjamin Herrenschmidt 2005-11-11 @662 if (sys_call_table[i] != sys_ni_syscall)
a7f290dad32ee3 Benjamin Herrenschmidt 2005-11-11 663 vdso_data->syscall_map_32[i >> 5] |=
a7f290dad32ee3 Benjamin Herrenschmidt 2005-11-11 664 0x80000000UL >> (i & 0x1f);
a7f290dad32ee3 Benjamin Herrenschmidt 2005-11-11 665 #endif /* CONFIG_PPC64 */
a7f290dad32ee3 Benjamin Herrenschmidt 2005-11-11 666 }
a7f290dad32ee3 Benjamin Herrenschmidt 2005-11-11 667 }
a7f290dad32ee3 Benjamin Herrenschmidt 2005-11-11 668
:::::: The code at line 662 was first introduced by commit
:::::: a7f290dad32ee34d931561b7943c858fe2aae503 [PATCH] powerpc: Merge vdso's and add vdso support to 32 bits kernel
:::::: TO: Benjamin Herrenschmidt <benh@kernel.crashing.org>
:::::: CC: Paul Mackerras <paulus@samba.org>
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28069 bytes --]
^ permalink raw reply
* Re: [RFC 00/11] perf: Enhancing perf to export processor hazard information
From: Kim Phillips @ 2020-03-18 17:35 UTC (permalink / raw)
To: maddy, Ravi Bangoria
Cc: Mark Rutland, Andi Kleen, Peter Zijlstra, Jiri Olsa, LKML,
Stephane Eranian, Adrian Hunter, Alexander Shishkin, yao.jin,
Ingo Molnar, Paul Mackerras, Arnaldo Carvalho de Melo,
Robert Richter, Namhyung Kim, linuxppc-dev, Alexey Budankov,
Liang, Kan
In-Reply-To: <0c5e94a3-e86e-f7cb-d668-d542b3a8ae29@linux.ibm.com>
Hi Maddy,
On 3/17/20 1:50 AM, maddy wrote:
> On 3/13/20 4:08 AM, Kim Phillips wrote:
>> On 3/11/20 11:00 AM, Ravi Bangoria wrote:
>>> On 3/6/20 3:36 AM, Kim Phillips wrote:
>>>>> On 3/3/20 3:55 AM, Kim Phillips wrote:
>>>>>> On 3/2/20 2:21 PM, Stephane Eranian wrote:
>>>>>>> On Mon, Mar 2, 2020 at 2:13 AM Peter Zijlstra <peterz@infradead.org> wrote:
>>>>>>>> On Mon, Mar 02, 2020 at 10:53:44AM +0530, Ravi Bangoria wrote:
>>>>>>>>> Modern processors export such hazard data in Performance
>>>>>>>>> Monitoring Unit (PMU) registers. Ex, 'Sampled Instruction Event
>>>>>>>>> Register' on IBM PowerPC[1][2] and 'Instruction-Based Sampling' on
>>>>>>>>> AMD[3] provides similar information.
>>>>>>>>>
>>>>>>>>> Implementation detail:
>>>>>>>>>
>>>>>>>>> A new sample_type called PERF_SAMPLE_PIPELINE_HAZ is introduced.
>>>>>>>>> If it's set, kernel converts arch specific hazard information
>>>>>>>>> into generic format:
>>>>>>>>>
>>>>>>>>> struct perf_pipeline_haz_data {
>>>>>>>>> /* Instruction/Opcode type: Load, Store, Branch .... */
>>>>>>>>> __u8 itype;
>>>>>>>>> /* Instruction Cache source */
>>>>>>>>> __u8 icache;
>>>>>>>>> /* Instruction suffered hazard in pipeline stage */
>>>>>>>>> __u8 hazard_stage;
>>>>>>>>> /* Hazard reason */
>>>>>>>>> __u8 hazard_reason;
>>>>>>>>> /* Instruction suffered stall in pipeline stage */
>>>>>>>>> __u8 stall_stage;
>>>>>>>>> /* Stall reason */
>>>>>>>>> __u8 stall_reason;
>>>>>>>>> __u16 pad;
>>>>>>>>> };
>>>>>>>> Kim, does this format indeed work for AMD IBS?
>>>>>> It's not really 1:1, we don't have these separations of stages
>>>>>> and reasons, for example: we have missed in L2 cache, for example.
>>>>>> So IBS output is flatter, with more cycle latency figures than
>>>>>> IBM's AFAICT.
>>>>> AMD IBS captures pipeline latency data incase Fetch sampling like the
>>>>> Fetch latency, tag to retire latency, completion to retire latency and
>>>>> so on. Yes, Ops sampling do provide more data on load/store centric
>>>>> information. But it also captures more detailed data for Branch instructions.
>>>>> And we also looked at ARM SPE, which also captures more details pipeline
>>>>> data and latency information.
>>>>>
>>>>>>> Personally, I don't like the term hazard. This is too IBM Power
>>>>>>> specific. We need to find a better term, maybe stall or penalty.
>>>>>> Right, IBS doesn't have a filter to only count stalled or otherwise
>>>>>> bad events. IBS' PPR descriptions has one occurrence of the
>>>>>> word stall, and no penalty. The way I read IBS is it's just
>>>>>> reporting more sample data than just the precise IP: things like
>>>>>> hits, misses, cycle latencies, addresses, types, etc., so words
>>>>>> like 'extended', or the 'auxiliary' already used today even
>>>>>> are more appropriate for IBS, although I'm the last person to
>>>>>> bikeshed.
>>>>> We are thinking of using "pipeline" word instead of Hazard.
>>>> Hm, the word 'pipeline' occurs 0 times in IBS documentation.
>>> NP. We thought pipeline is generic hw term so we proposed "pipeline"
>>> word. We are open to term which can be generic enough.
>>>
>>>> I realize there are a couple of core pipeline-specific pieces
>>>> of information coming out of it, but the vast majority
>>>> are addresses, latencies of various components in the memory
>>>> hierarchy, and various component hit/miss bits.
>>> Yes. we should capture core pipeline specific details. For example,
>>> IBS generates Branch unit information(IbsOpData1) and Icahce related
>>> data(IbsFetchCtl) which is something that shouldn't be extended as
>>> part of perf-mem, IMO.
>> Sure, IBS Op-side output is more 'perf mem' friendly, and so it
>> should populate perf_mem_data_src fields, just like POWER9 can:
>>
>> union perf_mem_data_src {
>> ...
>> __u64 mem_rsvd:24,
>> mem_snoopx:2, /* snoop mode, ext */
>> mem_remote:1, /* remote */
>> mem_lvl_num:4, /* memory hierarchy level number */
>> mem_dtlb:7, /* tlb access */
>> mem_lock:2, /* lock instr */
>> mem_snoop:5, /* snoop mode */
>> mem_lvl:14, /* memory hierarchy level */
>> mem_op:5; /* type of opcode */
>>
>>
>> E.g., SIER[LDST] SIER[A_XLATE_SRC] can be used to populate
>> mem_lvl[_num], SIER_TYPE can be used to populate 'mem_op',
>> 'mem_lock', and the Reload Bus Source Encoding bits can
>> be used to populate mem_snoop, right?
> Hi Kim,
>
> Yes. We do expose these data as part of perf-mem for POWER.
OK, I see relevant PERF_MEM_S bits in arch/powerpc/perf/isa207-common.c:
isa207_find_source now, thanks.
>> For IBS, I see PERF_SAMPLE_ADDR and PERF_SAMPLE_PHYS_ADDR can be
>> used for the ld/st target addresses, too.
>>
>>>> What's needed here is a vendor-specific extended
>>>> sample information that all these technologies gather,
>>>> of which things like e.g., 'L1 TLB cycle latency' we
>>>> all should have in common.
>>> Yes. We will include fields to capture the latency cycles (like Issue
>>> latency, Instruction completion latency etc..) along with other pipeline
>>> details in the proposed structure.
>> Latency figures are just an example, and from what I
>> can tell, struct perf_sample_data already has a 'weight' member,
>> used with PERF_SAMPLE_WEIGHT, that is used by intel-pt to
>> transfer memory access latency figures. Granted, that's
>> a bad name given all other vendors don't call latency
>> 'weight'.
>>
>> I didn't see any latency figures coming out of POWER9,
>> and do not expect this patchseries to implement those
>> of other vendors, e.g., AMD's IBS; leave each vendor
>> to amend perf to suit their own h/w output please.
>
> Reference structure proposed in this patchset did not have members
> to capture latency info for that exact reason. But idea here is to
> abstract as vendor specific as possible. So if we include u16 array,
> then this format can also capture data from IBS since it provides
> few latency details.
OK, that sounds a bit different from the 6 x u8's + 1 u16 padded
struct presented in this patchset.
IBS Ops can report e.g.:
15 tag-to-retire cycles bits,
15 completion to retire count bits,
15 L1 DTLB refill latency bits,
15 DC miss latency bits,
5 outstanding memory requests on mem refill bits, and so on.
IBS Fetch reports 15 bits of fetch latency, and another 16
for iTLB latency, among others.
Some of these may/may not be valid simultaneously, and
there are IBS specific rules to establish validity.
>> My main point there, however, was that each vendor should
>> use streamlined record-level code to just copy the data
>> in the proprietary format that their hardware produces,
>> and then then perf tooling can synthesize the events
>> from the raw data at report/script/etc. time.
>>
>>>> I'm not sure why a new PERF_SAMPLE_PIPELINE_HAZ is needed
>>>> either. Can we use PERF_SAMPLE_AUX instead?
>>> We took a look at PERF_SAMPLE_AUX. IIUC, PERF_SAMPLE_AUX is intended when
>>> large volume of data needs to be captured as part of perf.data without
>>> frequent PMIs. But proposed type is to address the capture of pipeline
>> SAMPLE_AUX shouldn't care whether the volume is large, or how frequent
>> PMIs are, even though it may be used in those environments.
>>
>>> information on each sample using PMI at periodic intervals. Hence proposing
>>> PERF_SAMPLE_PIPELINE_HAZ.
>> And that's fine for any extra bits that POWER9 has to convey
>> to its users beyond things already represented by other sample
>> types like PERF_SAMPLE_DATA_SRC, but the capturing of both POWER9
>> and other vendor e.g., AMD IBS data can be made vendor-independent
>> at record time by using SAMPLE_AUX, or SAMPLE_RAW even, which is
>> what IBS currently uses.
>
> My bad. Not sure what you mean by this. We are trying to abstract
> as much vendor specific data as possible with this (like perf-mem).
Perhaps if I say it this way: instead of doing all the
isa207_get_phazard_data() work past the mfspr(SPRN_SIER)
in patch 4/11, rather/instead just put the raw sier value in a
PERF_SAMPLE_RAW or _AUX event, and call perf_event_update_userpage.
Specific SIER capabilities can be written as part of the perf.data
header. Then synthesize the true pipe events from the raw SIER
values later, and in userspace.
I guess it's technically optional, but I think that's how
I'd do it in IBS, since it minimizes the record-time overhead.
Thanks,
Kim
> Maddy
>>
>>>> Take a look at
>>>> commit 98dcf14d7f9c "perf tools: Add kernel AUX area sampling
>>>> definitions". The sample identifier can be used to determine
>>>> which vendor's sampling IP's data is in it, and events can
>>>> be recorded just by copying the content of the SIER, etc.
>>>> registers, and then events get synthesized from the aux
>>>> sample at report/inject/annotate etc. time. This allows
>>>> for less sample recording overhead, and moves all the vendor
>>>> specific decoding and common event conversions for userspace
>>>> to figure out.
>>> When AUX buffer data is structured, tool side changes added to present the
>>> pipeline data can be re-used.
>> Not sure I understand: AUX data would be structured on
>> each vendor's raw h/w register formats.
>>
>> Thanks,
>>
>> Kim
>>
>>>>>>> Also worth considering is the support of ARM SPE (Statistical
>>>>>>> Profiling Extension) which is their version of IBS.
>>>>>>> Whatever gets added need to cover all three with no limitations.
>>>>>> I thought Intel's various LBR, PEBS, and PT supported providing
>>>>>> similar sample data in perf already, like with perf mem/c2c?
>>>>> perf-mem is more of data centric in my opinion. It is more towards
>>>>> memory profiling. So proposal here is to expose pipeline related
>>>>> details like stalls and latencies.
>>>> Like I said, I don't see it that way, I see it as "any particular
>>>> vendor's event's extended details', and these pipeline details
>>>> have overlap with existing infrastructure within perf, e.g., L2
>>>> cache misses.
>>>>
>>>> Kim
>>>>
>
^ permalink raw reply
* [PATCH 0/3] KVM: PPC: Fix host kernel crash with PR KVM
From: Greg Kurz @ 2020-03-18 17:43 UTC (permalink / raw)
To: Paul Mackerras; +Cc: kvm-ppc, Sean Christopherson, Paolo Bonzini, linuxppc-dev
Recent cleanup from Sean Christopherson introduced a use-after-free
condition that crashes the kernel when shutting down the VM with
PR KVM. It went unnoticed so far because PR isn't tested/used much
these days (mostly used for nested on POWER8, not supported on POWER9
where HV should be used for nested), and other KVM implementations for
ppc are unaffected.
This all boils down to the fact that the path that frees the per-vCPU
MMU data goes through a complex set of indirections. This obfuscates
the code to the point that we didn't realize that the MMU data was
now being freed too early. And worse, most of the indirection isn't
needed because only PR KVM has some MMU data to free when the vCPU is
destroyed.
Fix the issue (patch 1) and simplify the code (patch 2 and 3).
--
Greg
---
Greg Kurz (3):
KVM: PPC: Fix kernel crash with PR KVM
KVM: PPC: Move kvmppc_mmu_init() PR KVM
KVM: PPC: Kill kvmppc_ops::mmu_destroy() and kvmppc_mmu_destroy()
arch/powerpc/include/asm/kvm_ppc.h | 3 ---
arch/powerpc/kvm/book3s.c | 5 -----
arch/powerpc/kvm/book3s.h | 1 +
arch/powerpc/kvm/book3s_32_mmu_host.c | 2 +-
arch/powerpc/kvm/book3s_64_mmu_host.c | 2 +-
arch/powerpc/kvm/book3s_hv.c | 6 ------
arch/powerpc/kvm/book3s_pr.c | 4 ++--
arch/powerpc/kvm/booke.c | 5 -----
arch/powerpc/kvm/booke.h | 2 --
arch/powerpc/kvm/e500.c | 1 -
arch/powerpc/kvm/e500_mmu.c | 4 ----
arch/powerpc/kvm/e500mc.c | 1 -
arch/powerpc/kvm/powerpc.c | 2 --
13 files changed, 5 insertions(+), 33 deletions(-)
^ permalink raw reply
* [PATCH 1/3] KVM: PPC: Fix kernel crash with PR KVM
From: Greg Kurz @ 2020-03-18 17:43 UTC (permalink / raw)
To: Paul Mackerras; +Cc: kvm-ppc, Sean Christopherson, Paolo Bonzini, linuxppc-dev
In-Reply-To: <158455340419.178873.11399595021669446372.stgit@bahia.lan>
With PR KVM, shutting down a VM causes the host kernel to crash:
[ 314.219284] BUG: Unable to handle kernel data access on read at 0xc00800000176c638
[ 314.219299] Faulting instruction address: 0xc008000000d4ddb0
cpu 0x0: Vector: 300 (Data Access) at [c00000036da077a0]
pc: c008000000d4ddb0: kvmppc_mmu_pte_flush_all+0x68/0xd0 [kvm_pr]
lr: c008000000d4dd94: kvmppc_mmu_pte_flush_all+0x4c/0xd0 [kvm_pr]
sp: c00000036da07a30
msr: 900000010280b033
dar: c00800000176c638
dsisr: 40000000
current = 0xc00000036d4c0000
paca = 0xc000000001a00000 irqmask: 0x03 irq_happened: 0x01
pid = 1992, comm = qemu-system-ppc
Linux version 5.6.0-master-gku+ (greg@palmb) (gcc version 7.5.0 (Ubuntu 7.5.0-3ubuntu1~18.04)) #17 SMP Wed Mar 18 13:49:29 CET 2020
enter ? for help
[c00000036da07ab0] c008000000d4fbe0 kvmppc_mmu_destroy_pr+0x28/0x60 [kvm_pr]
[c00000036da07ae0] c0080000009eab8c kvmppc_mmu_destroy+0x34/0x50 [kvm]
[c00000036da07b00] c0080000009e50c0 kvm_arch_vcpu_destroy+0x108/0x140 [kvm]
[c00000036da07b30] c0080000009d1b50 kvm_vcpu_destroy+0x28/0x80 [kvm]
[c00000036da07b60] c0080000009e4434 kvm_arch_destroy_vm+0xbc/0x190 [kvm]
[c00000036da07ba0] c0080000009d9c2c kvm_put_kvm+0x1d4/0x3f0 [kvm]
[c00000036da07c00] c0080000009da760 kvm_vm_release+0x38/0x60 [kvm]
[c00000036da07c30] c000000000420be0 __fput+0xe0/0x310
[c00000036da07c90] c0000000001747a0 task_work_run+0x150/0x1c0
[c00000036da07cf0] c00000000014896c do_exit+0x44c/0xd00
[c00000036da07dc0] c0000000001492f4 do_group_exit+0x64/0xd0
[c00000036da07e00] c000000000149384 sys_exit_group+0x24/0x30
[c00000036da07e20] c00000000000b9d0 system_call+0x5c/0x68
This is caused by a use-after-free in kvmppc_mmu_pte_flush_all()
which dereferences vcpu->arch.book3s which was previously freed by
kvmppc_core_vcpu_free_pr(). This happens because kvmppc_mmu_destroy()
is called after kvmppc_core_vcpu_free() since commit ff030fdf5573
("KVM: PPC: Move kvm_vcpu_init() invocation to common code").
The kvmppc_mmu_destroy() helper calls one of the following depending
on the KVM backend:
- kvmppc_mmu_destroy_hv() which does nothing (Book3s HV)
- kvmppc_mmu_destroy_pr() which undoes the effects of
kvmppc_mmu_init() (Book3s PR 32-bit)
- kvmppc_mmu_destroy_pr() which undoes the effects of
kvmppc_mmu_init() (Book3s PR 64-bit)
- kvmppc_mmu_destroy_e500() which does nothing (BookE e500/e500mc)
It turns out that this is only relevant to PR KVM actually. And both
32 and 64 backends need vcpu->arch.book3s to be valid when calling
kvmppc_mmu_destroy_pr(). So instead of calling kvmppc_mmu_destroy()
from kvm_arch_vcpu_destroy(), call kvmppc_mmu_destroy_pr() at the
beginning of kvmppc_core_vcpu_free_pr(). This is consistent with
kvmppc_mmu_init() being the last call in kvmppc_core_vcpu_create_pr().
For the same reason, if kvmppc_core_vcpu_create_pr() returns an
error then this means that kvmppc_mmu_init() was either not called
or failed, in which case kvmppc_mmu_destroy() should not be called.
Drop the line in the error path of kvm_arch_vcpu_create().
Fixes: ff030fdf5573 ("KVM: PPC: Move kvm_vcpu_init() invocation to common code")
Signed-off-by: Greg Kurz <groug@kaod.org>
---
arch/powerpc/kvm/book3s_pr.c | 1 +
arch/powerpc/kvm/powerpc.c | 2 --
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index 729a0f12a752..db3a87319642 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -1817,6 +1817,7 @@ static void kvmppc_core_vcpu_free_pr(struct kvm_vcpu *vcpu)
{
struct kvmppc_vcpu_book3s *vcpu_book3s = to_book3s(vcpu);
+ kvmppc_mmu_destroy_pr(vcpu);
free_page((unsigned long)vcpu->arch.shared & PAGE_MASK);
#ifdef CONFIG_KVM_BOOK3S_32_HANDLER
kfree(vcpu->arch.shadow_vcpu);
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 1af96fb5dc6f..302e9dccdd6d 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -759,7 +759,6 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
return 0;
out_vcpu_uninit:
- kvmppc_mmu_destroy(vcpu);
kvmppc_subarch_vcpu_uninit(vcpu);
return err;
}
@@ -792,7 +791,6 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
kvmppc_core_vcpu_free(vcpu);
- kvmppc_mmu_destroy(vcpu);
kvmppc_subarch_vcpu_uninit(vcpu);
}
^ permalink raw reply related
* [PATCH 2/3] KVM: PPC: Move kvmppc_mmu_init() PR KVM
From: Greg Kurz @ 2020-03-18 17:43 UTC (permalink / raw)
To: Paul Mackerras; +Cc: kvm-ppc, Sean Christopherson, Paolo Bonzini, linuxppc-dev
In-Reply-To: <158455340419.178873.11399595021669446372.stgit@bahia.lan>
This is only relevant to PR KVM. Make it obvious by moving the
function declaration to the Book3s header and rename it with
a _pr suffix.
Signed-off-by: Greg Kurz <groug@kaod.org>
---
arch/powerpc/include/asm/kvm_ppc.h | 1 -
arch/powerpc/kvm/book3s.h | 1 +
arch/powerpc/kvm/book3s_32_mmu_host.c | 2 +-
arch/powerpc/kvm/book3s_64_mmu_host.c | 2 +-
arch/powerpc/kvm/book3s_pr.c | 2 +-
5 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
index bc2494e5710a..399a657c1bf3 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -108,7 +108,6 @@ extern void kvmppc_mmu_map(struct kvm_vcpu *vcpu, u64 gvaddr, gpa_t gpaddr,
extern void kvmppc_mmu_priv_switch(struct kvm_vcpu *vcpu, int usermode);
extern void kvmppc_mmu_switch_pid(struct kvm_vcpu *vcpu, u32 pid);
extern void kvmppc_mmu_destroy(struct kvm_vcpu *vcpu);
-extern int kvmppc_mmu_init(struct kvm_vcpu *vcpu);
extern int kvmppc_mmu_dtlb_index(struct kvm_vcpu *vcpu, gva_t eaddr);
extern int kvmppc_mmu_itlb_index(struct kvm_vcpu *vcpu, gva_t eaddr);
extern gpa_t kvmppc_mmu_xlate(struct kvm_vcpu *vcpu, unsigned int gtlb_index,
diff --git a/arch/powerpc/kvm/book3s.h b/arch/powerpc/kvm/book3s.h
index 3a4613985949..eae259ee49af 100644
--- a/arch/powerpc/kvm/book3s.h
+++ b/arch/powerpc/kvm/book3s.h
@@ -16,6 +16,7 @@ extern int kvm_age_hva_hv(struct kvm *kvm, unsigned long start,
extern int kvm_test_age_hva_hv(struct kvm *kvm, unsigned long hva);
extern void kvm_set_spte_hva_hv(struct kvm *kvm, unsigned long hva, pte_t pte);
+extern int kvmppc_mmu_init_pr(struct kvm_vcpu *vcpu);
extern void kvmppc_mmu_destroy_pr(struct kvm_vcpu *vcpu);
extern int kvmppc_core_emulate_op_pr(struct kvm_run *run, struct kvm_vcpu *vcpu,
unsigned int inst, int *advance);
diff --git a/arch/powerpc/kvm/book3s_32_mmu_host.c b/arch/powerpc/kvm/book3s_32_mmu_host.c
index d4cb3bcf41b6..e8e7b2c530d1 100644
--- a/arch/powerpc/kvm/book3s_32_mmu_host.c
+++ b/arch/powerpc/kvm/book3s_32_mmu_host.c
@@ -356,7 +356,7 @@ void kvmppc_mmu_destroy_pr(struct kvm_vcpu *vcpu)
/* From mm/mmu_context_hash32.c */
#define CTX_TO_VSID(c, id) ((((c) * (897 * 16)) + (id * 0x111)) & 0xffffff)
-int kvmppc_mmu_init(struct kvm_vcpu *vcpu)
+int kvmppc_mmu_init_pr(struct kvm_vcpu *vcpu)
{
struct kvmppc_vcpu_book3s *vcpu3s = to_book3s(vcpu);
int err;
diff --git a/arch/powerpc/kvm/book3s_64_mmu_host.c b/arch/powerpc/kvm/book3s_64_mmu_host.c
index 044dd49eeb9d..e452158a18d7 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_host.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_host.c
@@ -384,7 +384,7 @@ void kvmppc_mmu_destroy_pr(struct kvm_vcpu *vcpu)
__destroy_context(to_book3s(vcpu)->context_id[0]);
}
-int kvmppc_mmu_init(struct kvm_vcpu *vcpu)
+int kvmppc_mmu_init_pr(struct kvm_vcpu *vcpu)
{
struct kvmppc_vcpu_book3s *vcpu3s = to_book3s(vcpu);
int err;
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index db3a87319642..28e63a68a3dc 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -1795,7 +1795,7 @@ static int kvmppc_core_vcpu_create_pr(struct kvm_vcpu *vcpu)
vcpu->arch.shadow_msr = MSR_USER64 & ~MSR_LE;
- err = kvmppc_mmu_init(vcpu);
+ err = kvmppc_mmu_init_pr(vcpu);
if (err < 0)
goto free_shared_page;
^ permalink raw reply related
* [PATCH 3/3] KVM: PPC: Kill kvmppc_ops::mmu_destroy() and kvmppc_mmu_destroy()
From: Greg Kurz @ 2020-03-18 17:43 UTC (permalink / raw)
To: Paul Mackerras; +Cc: kvm-ppc, Sean Christopherson, Paolo Bonzini, linuxppc-dev
In-Reply-To: <158455340419.178873.11399595021669446372.stgit@bahia.lan>
These are only used by HV KVM and BookE, and in both cases they are
nops.
Signed-off-by: Greg Kurz <groug@kaod.org>
---
arch/powerpc/include/asm/kvm_ppc.h | 2 --
arch/powerpc/kvm/book3s.c | 5 -----
arch/powerpc/kvm/book3s_hv.c | 6 ------
arch/powerpc/kvm/book3s_pr.c | 1 -
arch/powerpc/kvm/booke.c | 5 -----
arch/powerpc/kvm/booke.h | 2 --
arch/powerpc/kvm/e500.c | 1 -
arch/powerpc/kvm/e500_mmu.c | 4 ----
arch/powerpc/kvm/e500mc.c | 1 -
9 files changed, 27 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
index 399a657c1bf3..be627367e3bd 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -107,7 +107,6 @@ extern void kvmppc_mmu_map(struct kvm_vcpu *vcpu, u64 gvaddr, gpa_t gpaddr,
unsigned int gtlb_idx);
extern void kvmppc_mmu_priv_switch(struct kvm_vcpu *vcpu, int usermode);
extern void kvmppc_mmu_switch_pid(struct kvm_vcpu *vcpu, u32 pid);
-extern void kvmppc_mmu_destroy(struct kvm_vcpu *vcpu);
extern int kvmppc_mmu_dtlb_index(struct kvm_vcpu *vcpu, gva_t eaddr);
extern int kvmppc_mmu_itlb_index(struct kvm_vcpu *vcpu, gva_t eaddr);
extern gpa_t kvmppc_mmu_xlate(struct kvm_vcpu *vcpu, unsigned int gtlb_index,
@@ -290,7 +289,6 @@ struct kvmppc_ops {
int (*age_hva)(struct kvm *kvm, unsigned long start, unsigned long end);
int (*test_age_hva)(struct kvm *kvm, unsigned long hva);
void (*set_spte_hva)(struct kvm *kvm, unsigned long hva, pte_t pte);
- void (*mmu_destroy)(struct kvm_vcpu *vcpu);
void (*free_memslot)(struct kvm_memory_slot *free,
struct kvm_memory_slot *dont);
int (*create_memslot)(struct kvm_memory_slot *slot,
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index d07a8e12fa15..19ccb019eb3b 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -858,11 +858,6 @@ int kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte)
return 0;
}
-void kvmppc_mmu_destroy(struct kvm_vcpu *vcpu)
-{
- vcpu->kvm->arch.kvm_ops->mmu_destroy(vcpu);
-}
-
int kvmppc_core_init_vm(struct kvm *kvm)
{
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 2cefd071b848..48d0bce16164 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -4558,11 +4558,6 @@ void kvmppc_update_lpcr(struct kvm *kvm, unsigned long lpcr, unsigned long mask)
}
}
-static void kvmppc_mmu_destroy_hv(struct kvm_vcpu *vcpu)
-{
- return;
-}
-
void kvmppc_setup_partition_table(struct kvm *kvm)
{
unsigned long dw0, dw1;
@@ -5526,7 +5521,6 @@ static struct kvmppc_ops kvm_ops_hv = {
.age_hva = kvm_age_hva_hv,
.test_age_hva = kvm_test_age_hva_hv,
.set_spte_hva = kvm_set_spte_hva_hv,
- .mmu_destroy = kvmppc_mmu_destroy_hv,
.free_memslot = kvmppc_core_free_memslot_hv,
.create_memslot = kvmppc_core_create_memslot_hv,
.init_vm = kvmppc_core_init_vm_hv,
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index 28e63a68a3dc..5cc88203f435 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -2098,7 +2098,6 @@ static struct kvmppc_ops kvm_ops_pr = {
.age_hva = kvm_age_hva_pr,
.test_age_hva = kvm_test_age_hva_pr,
.set_spte_hva = kvm_set_spte_hva_pr,
- .mmu_destroy = kvmppc_mmu_destroy_pr,
.free_memslot = kvmppc_core_free_memslot_pr,
.create_memslot = kvmppc_core_create_memslot_pr,
.init_vm = kvmppc_core_init_vm_pr,
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index 7b27604adadf..8a516d947405 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -2074,11 +2074,6 @@ void kvmppc_booke_vcpu_put(struct kvm_vcpu *vcpu)
kvmppc_clear_dbsr();
}
-void kvmppc_mmu_destroy(struct kvm_vcpu *vcpu)
-{
- vcpu->kvm->arch.kvm_ops->mmu_destroy(vcpu);
-}
-
int kvmppc_core_init_vm(struct kvm *kvm)
{
return kvm->arch.kvm_ops->init_vm(kvm);
diff --git a/arch/powerpc/kvm/booke.h b/arch/powerpc/kvm/booke.h
index 9d3169fbce55..65b4d337d337 100644
--- a/arch/powerpc/kvm/booke.h
+++ b/arch/powerpc/kvm/booke.h
@@ -94,7 +94,6 @@ enum int_class {
void kvmppc_set_pending_interrupt(struct kvm_vcpu *vcpu, enum int_class type);
-extern void kvmppc_mmu_destroy_e500(struct kvm_vcpu *vcpu);
extern int kvmppc_core_emulate_op_e500(struct kvm_run *run,
struct kvm_vcpu *vcpu,
unsigned int inst, int *advance);
@@ -102,7 +101,6 @@ extern int kvmppc_core_emulate_mtspr_e500(struct kvm_vcpu *vcpu, int sprn,
ulong spr_val);
extern int kvmppc_core_emulate_mfspr_e500(struct kvm_vcpu *vcpu, int sprn,
ulong *spr_val);
-extern void kvmppc_mmu_destroy_e500(struct kvm_vcpu *vcpu);
extern int kvmppc_core_emulate_op_e500(struct kvm_run *run,
struct kvm_vcpu *vcpu,
unsigned int inst, int *advance);
diff --git a/arch/powerpc/kvm/e500.c b/arch/powerpc/kvm/e500.c
index f2b4feaff6d2..7e8b69015d20 100644
--- a/arch/powerpc/kvm/e500.c
+++ b/arch/powerpc/kvm/e500.c
@@ -490,7 +490,6 @@ static struct kvmppc_ops kvm_ops_e500 = {
.vcpu_put = kvmppc_core_vcpu_put_e500,
.vcpu_create = kvmppc_core_vcpu_create_e500,
.vcpu_free = kvmppc_core_vcpu_free_e500,
- .mmu_destroy = kvmppc_mmu_destroy_e500,
.init_vm = kvmppc_core_init_vm_e500,
.destroy_vm = kvmppc_core_destroy_vm_e500,
.emulate_op = kvmppc_core_emulate_op_e500,
diff --git a/arch/powerpc/kvm/e500_mmu.c b/arch/powerpc/kvm/e500_mmu.c
index 2d910b87e441..e131fbecdcc4 100644
--- a/arch/powerpc/kvm/e500_mmu.c
+++ b/arch/powerpc/kvm/e500_mmu.c
@@ -533,10 +533,6 @@ gpa_t kvmppc_mmu_xlate(struct kvm_vcpu *vcpu, unsigned int index,
return get_tlb_raddr(gtlbe) | (eaddr & pgmask);
}
-void kvmppc_mmu_destroy_e500(struct kvm_vcpu *vcpu)
-{
-}
-
/*****************************************/
static void free_gtlb(struct kvmppc_vcpu_e500 *vcpu_e500)
diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c
index e6b06cb2b92c..1c189b5aadcc 100644
--- a/arch/powerpc/kvm/e500mc.c
+++ b/arch/powerpc/kvm/e500mc.c
@@ -376,7 +376,6 @@ static struct kvmppc_ops kvm_ops_e500mc = {
.vcpu_put = kvmppc_core_vcpu_put_e500mc,
.vcpu_create = kvmppc_core_vcpu_create_e500mc,
.vcpu_free = kvmppc_core_vcpu_free_e500mc,
- .mmu_destroy = kvmppc_mmu_destroy_e500,
.init_vm = kvmppc_core_init_vm_e500mc,
.destroy_vm = kvmppc_core_destroy_vm_e500mc,
.emulate_op = kvmppc_core_emulate_op_e500,
^ permalink raw reply related
* Re: [PATCH 1/3] KVM: PPC: Fix kernel crash with PR KVM
From: Sean Christopherson @ 2020-03-18 18:16 UTC (permalink / raw)
To: Greg Kurz; +Cc: kvm-ppc, Paolo Bonzini, linuxppc-dev
In-Reply-To: <158455341029.178873.15248663726399374882.stgit@bahia.lan>
On Wed, Mar 18, 2020 at 06:43:30PM +0100, Greg Kurz wrote:
> It turns out that this is only relevant to PR KVM actually. And both
> 32 and 64 backends need vcpu->arch.book3s to be valid when calling
> kvmppc_mmu_destroy_pr(). So instead of calling kvmppc_mmu_destroy()
> from kvm_arch_vcpu_destroy(), call kvmppc_mmu_destroy_pr() at the
> beginning of kvmppc_core_vcpu_free_pr(). This is consistent with
> kvmppc_mmu_init() being the last call in kvmppc_core_vcpu_create_pr().
>
> For the same reason, if kvmppc_core_vcpu_create_pr() returns an
> error then this means that kvmppc_mmu_init() was either not called
> or failed, in which case kvmppc_mmu_destroy() should not be called.
> Drop the line in the error path of kvm_arch_vcpu_create().
>
> Fixes: ff030fdf5573 ("KVM: PPC: Move kvm_vcpu_init() invocation to common code")
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
Dang, I see where I went wrong. Sorry :-(
Reviewed-by: Sean Christopherson <sean.j.christopherson@intel.com>
^ 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