* [Bug 206203] kmemleak reports various leaks in drivers/of/unittest.c
From: bugzilla-daemon @ 2020-02-27 22:39 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-206203-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=206203
--- Comment #7 from Erhard F. (erhard_f@mailbox.org) ---
(In reply to mpe from comment #6)
> Can you attach the /sys/kernel/debug/kmemleak output please.
>
> cheers
I already did:
"kmemleak output (kernel 5.6-rc3, PowerMac G5 11,2) (91.35 KB, text/plain)"
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* Re: [PATCH v4 13/13] powerpc/ptrace: move ptrace_triggered() into hw_breakpoint.c
From: Michael Ellerman @ 2020-02-27 22:16 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras, mikey,
Russell Currey
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <4e528bf2-2b53-ae93-cdcc-0c80953f40f2@c-s.fr>
Christophe Leroy <christophe.leroy@c-s.fr> writes:
> Russel,
>
> Le 27/02/2020 à 12:49, Christophe Leroy a écrit :
>> ptrace_triggered() is declared in asm/hw_breakpoint.h and
>> only needed when CONFIG_HW_BREAKPOINT is set, so move it
>> into hw_breakpoint.c
>
> My series v4 is definitely buggy (I included ptrace_decl.h instead
> instead of ptrace-decl.h), how can Snowpatch say build succeeded
> (https://patchwork.ozlabs.org/patch/1245807/) ?
Which links to:
https://openpower.xyz/job/snowpatch/job/snowpatch-linux-sparse/15895//artifact/linux/report.txt
The actual build log of which is:
https://openpower.xyz/job/snowpatch/job/snowpatch-linux-sparse/15895/artifact/linux/build_new.log
Which contains:
scripts/Makefile.build:267: recipe for target 'arch/powerpc/kernel/ptrace/ptrace-altivec.o' failed
make[3]: *** [arch/powerpc/kernel/ptrace/ptrace-altivec.o] Error 1
make[3]: *** Waiting for unfinished jobs....
scripts/Makefile.build:505: recipe for target 'arch/powerpc/kernel/ptrace' failed
make[2]: *** [arch/powerpc/kernel/ptrace] Error 2
make[2]: *** Waiting for unfinished jobs....
scripts/Makefile.build:505: recipe for target 'arch/powerpc/kernel' failed
make[1]: *** [arch/powerpc/kernel] Error 2
make[1]: *** Waiting for unfinished jobs....
Makefile:1681: recipe for target 'arch/powerpc' failed
make: *** [arch/powerpc] Error 2
make: *** Waiting for unfinished jobs....
Same for ppc64le:
https://openpower.xyz/job/snowpatch/job/snowpatch-linux-sparse/15896/artifact/linux/build_new.log
So it seems like snowpatch always reports the build as succeeded even
when it fails.
cheers
^ permalink raw reply
* [Bug 206203] kmemleak reports various leaks in drivers/of/unittest.c
From: bugzilla-daemon @ 2020-02-27 22:07 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-206203-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=206203
--- Comment #6 from mpe@ellerman.id.au ---
bugzilla-daemon@bugzilla.kernel.org writes:
> https://bugzilla.kernel.org/show_bug.cgi?id=206203
>
> --- Comment #3 from Erhard F. (erhard_f@mailbox.org) ---
> Created attachment 287671
> --> https://bugzilla.kernel.org/attachment.cgi?id=287671&action=edit
> kmemleak output (kernel 5.6-rc3, PowerMac G5 11,2)
>
> Same on a PowerMac G5 11,2 (kernel 5.6-rc3).
Can you attach the /sys/kernel/debug/kmemleak output please.
cheers
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* Re: [Bug 206203] kmemleak reports various leaks in drivers/of/unittest.c
From: Michael Ellerman @ 2020-02-27 22:06 UTC (permalink / raw)
To: bugzilla-daemon, linuxppc-dev
In-Reply-To: <bug-206203-206035-Iux2rayMYJ@https.bugzilla.kernel.org/>
bugzilla-daemon@bugzilla.kernel.org writes:
> https://bugzilla.kernel.org/show_bug.cgi?id=206203
>
> --- Comment #3 from Erhard F. (erhard_f@mailbox.org) ---
> Created attachment 287671
> --> https://bugzilla.kernel.org/attachment.cgi?id=287671&action=edit
> kmemleak output (kernel 5.6-rc3, PowerMac G5 11,2)
>
> Same on a PowerMac G5 11,2 (kernel 5.6-rc3).
Can you attach the /sys/kernel/debug/kmemleak output please.
cheers
^ permalink raw reply
* [Bug 206203] kmemleak reports various leaks in drivers/of/unittest.c
From: bugzilla-daemon @ 2020-02-27 21:31 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-206203-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=206203
--- Comment #5 from Erhard F. (erhard_f@mailbox.org) ---
Created attachment 287675
--> https://bugzilla.kernel.org/attachment.cgi?id=287675&action=edit
kernel .config (kernel 5.6-rc3, PowerMac G5 11,2)
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [Bug 206203] kmemleak reports various leaks in drivers/of/unittest.c
From: bugzilla-daemon @ 2020-02-27 21:20 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-206203-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=206203
--- Comment #4 from Erhard F. (erhard_f@mailbox.org) ---
Created attachment 287673
--> https://bugzilla.kernel.org/attachment.cgi?id=287673&action=edit
dmesg (kernel 5.6-rc3, PowerMac G5 11,2)
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [Bug 206203] kmemleak reports various leaks in drivers/of/unittest.c
From: bugzilla-daemon @ 2020-02-27 21:19 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-206203-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=206203
--- Comment #3 from Erhard F. (erhard_f@mailbox.org) ---
Created attachment 287671
--> https://bugzilla.kernel.org/attachment.cgi?id=287671&action=edit
kmemleak output (kernel 5.6-rc3, PowerMac G5 11,2)
Same on a PowerMac G5 11,2 (kernel 5.6-rc3).
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* Re: [PATCH v3 10/27] powerpc: Add driver for OpenCAPI Persistent Memory
From: Frederic Barrat @ 2020-02-27 20:44 UTC (permalink / raw)
To: Alastair D'Silva, alastair
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
Aneesh Kumar K . V, Krzysztof Kozlowski, Anju T Sudhakar,
Mahesh Salgaonkar, Andrew Donnellan, Arnd Bergmann, Greg Kurz,
Nicholas Piggin, Cédric Le Goater, Dan Williams,
Hari Bathini, linux-mm, Greg Kroah-Hartman, linux-kernel,
Vishal Verma, Paul Mackerras, Andrew Morton, linuxppc-dev,
David S. Miller
In-Reply-To: <20200221032720.33893-11-alastair@au1.ibm.com>
Le 21/02/2020 à 04:27, Alastair D'Silva a écrit :
> From: Alastair D'Silva <alastair@d-silva.org>
>
> This driver exposes LPC memory on OpenCAPI pmem cards
> as an NVDIMM, allowing the existing nvram infrastructure
> to be used.
>
> Namespace metadata is stored on the media itself, so
> scm_reserve_metadata() maps 1 section's worth of PMEM storage
> at the start to hold this. The rest of the PMEM range is registered
> with libnvdimm as an nvdimm. scm_ndctl_config_read/write/size() provide
> callbacks to libnvdimm to access the metadata.
>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
> arch/powerpc/platforms/powernv/Kconfig | 3 +
> arch/powerpc/platforms/powernv/Makefile | 1 +
> arch/powerpc/platforms/powernv/pmem/Kconfig | 15 +
> arch/powerpc/platforms/powernv/pmem/Makefile | 7 +
> arch/powerpc/platforms/powernv/pmem/ocxl.c | 473 ++++++++++++++++++
> .../platforms/powernv/pmem/ocxl_internal.h | 28 ++
> 6 files changed, 527 insertions(+)
> create mode 100644 arch/powerpc/platforms/powernv/pmem/Kconfig
> create mode 100644 arch/powerpc/platforms/powernv/pmem/Makefile
> create mode 100644 arch/powerpc/platforms/powernv/pmem/ocxl.c
> create mode 100644 arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
>
> diff --git a/arch/powerpc/platforms/powernv/Kconfig b/arch/powerpc/platforms/powernv/Kconfig
> index 938803eab0ad..fc8976af0e52 100644
> --- a/arch/powerpc/platforms/powernv/Kconfig
> +++ b/arch/powerpc/platforms/powernv/Kconfig
> @@ -50,3 +50,6 @@ config PPC_VAS
> config SCOM_DEBUGFS
> bool "Expose SCOM controllers via debugfs"
> depends on DEBUG_FS
> +
> +source "arch/powerpc/platforms/powernv/pmem/Kconfig"
> +
> diff --git a/arch/powerpc/platforms/powernv/Makefile b/arch/powerpc/platforms/powernv/Makefile
> index c0f8120045c3..0bbd72988b6f 100644
> --- a/arch/powerpc/platforms/powernv/Makefile
> +++ b/arch/powerpc/platforms/powernv/Makefile
> @@ -21,3 +21,4 @@ obj-$(CONFIG_PPC_VAS) += vas.o vas-window.o vas-debug.o
> obj-$(CONFIG_OCXL_BASE) += ocxl.o
> obj-$(CONFIG_SCOM_DEBUGFS) += opal-xscom.o
> obj-$(CONFIG_PPC_SECURE_BOOT) += opal-secvar.o
> +obj-$(CONFIG_LIBNVDIMM) += pmem/
> diff --git a/arch/powerpc/platforms/powernv/pmem/Kconfig b/arch/powerpc/platforms/powernv/pmem/Kconfig
> new file mode 100644
> index 000000000000..c5d927520920
> --- /dev/null
> +++ b/arch/powerpc/platforms/powernv/pmem/Kconfig
> @@ -0,0 +1,15 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +if LIBNVDIMM
> +
> +config OCXL_PMEM
> + tristate "OpenCAPI Persistent Memory"
> + depends on LIBNVDIMM && PPC_POWERNV && PCI && EEH && ZONE_DEVICE && OCXL
> + help
> + Exposes devices that implement the OpenCAPI Storage Class Memory
> + specification as persistent memory regions. You may also want
> + DEV_DAX, DEV_DAX_PMEM & FS_DAX if you plan on using DAX devices
> + stacked on top of this driver.
> +
> + Select N if unsure.
> +
> +endif
> diff --git a/arch/powerpc/platforms/powernv/pmem/Makefile b/arch/powerpc/platforms/powernv/pmem/Makefile
> new file mode 100644
> index 000000000000..1c55c4193175
> --- /dev/null
> +++ b/arch/powerpc/platforms/powernv/pmem/Makefile
> @@ -0,0 +1,7 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +ccflags-$(CONFIG_PPC_WERROR) += -Werror
> +
> +obj-$(CONFIG_OCXL_PMEM) += ocxlpmem.o
> +
> +ocxlpmem-y := ocxl.o
> diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl.c b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> new file mode 100644
> index 000000000000..3c4eeb5dcc0f
> --- /dev/null
> +++ b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> @@ -0,0 +1,473 @@
> +// SPDX-License-Id
> +// Copyright 2019 IBM Corp.
> +
> +/*
> + * A driver for OpenCAPI devices that implement the Storage Class
> + * Memory specification.
> + */
> +
> +#include <linux/module.h>
> +#include <misc/ocxl.h>
> +#include <linux/ndctl.h>
> +#include <linux/mm_types.h>
> +#include <linux/memory_hotplug.h>
> +#include "ocxl_internal.h"
> +
> +
> +static const struct pci_device_id ocxlpmem_pci_tbl[] = {
> + { PCI_DEVICE(PCI_VENDOR_ID_IBM, 0x0625), },
> + { }
> +};
> +
> +MODULE_DEVICE_TABLE(pci, ocxlpmem_pci_tbl);
> +
> +#define NUM_MINORS 256 // Total to reserve
> +
> +static dev_t ocxlpmem_dev;
> +static struct class *ocxlpmem_class;
> +static struct mutex minors_idr_lock;
> +static struct idr minors_idr;
> +
> +/**
> + * ndctl_config_write() - Handle a ND_CMD_SET_CONFIG_DATA command from ndctl
> + * @ocxlpmem: the device metadata
> + * @command: the incoming data to write
> + * Return: 0 on success, negative on failure
> + */
> +static int ndctl_config_write(struct ocxlpmem *ocxlpmem,
> + struct nd_cmd_set_config_hdr *command)
> +{
> + if (command->in_offset + command->in_length > LABEL_AREA_SIZE)
> + return -EINVAL;
> +
> + memcpy_flushcache(ocxlpmem->metadata_addr + command->in_offset, command->in_buf,
> + command->in_length);
> +
> + return 0;
> +}
> +
> +/**
> + * ndctl_config_read() - Handle a ND_CMD_GET_CONFIG_DATA command from ndctl
> + * @ocxlpmem: the device metadata
> + * @command: the read request
> + * Return: 0 on success, negative on failure
> + */
> +static int ndctl_config_read(struct ocxlpmem *ocxlpmem,
> + struct nd_cmd_get_config_data_hdr *command)
> +{
> + if (command->in_offset + command->in_length > LABEL_AREA_SIZE)
> + return -EINVAL;
> +
> + memcpy_mcsafe(command->out_buf, ocxlpmem->metadata_addr + command->in_offset,
> + command->in_length);
> +
> + return 0;
> +}
> +
> +/**
> + * ndctl_config_size() - Handle a ND_CMD_GET_CONFIG_SIZE command from ndctl
> + * @command: the read request
> + * Return: 0 on success, negative on failure
> + */
> +static int ndctl_config_size(struct nd_cmd_get_config_size *command)
> +{
> + command->status = 0;
> + command->config_size = LABEL_AREA_SIZE;
> + command->max_xfer = PAGE_SIZE;
> +
> + return 0;
> +}
> +
> +static int ndctl(struct nvdimm_bus_descriptor *nd_desc,
> + struct nvdimm *nvdimm,
> + unsigned int cmd, void *buf, unsigned int buf_len, int *cmd_rc)
> +{
> + struct ocxlpmem *ocxlpmem = container_of(nd_desc, struct ocxlpmem, bus_desc);
> +
> + switch (cmd) {
> + case ND_CMD_GET_CONFIG_SIZE:
> + *cmd_rc = ndctl_config_size(buf);
> + return 0;
> +
> + case ND_CMD_GET_CONFIG_DATA:
> + *cmd_rc = ndctl_config_read(ocxlpmem, buf);
> + return 0;
> +
> + case ND_CMD_SET_CONFIG_DATA:
> + *cmd_rc = ndctl_config_write(ocxlpmem, buf);
> + return 0;
> +
> + default:
> + return -ENOTTY;
> + }
> +}
> +
> +/**
> + * reserve_metadata() - Reserve space for nvdimm metadata
> + * @ocxlpmem: the device metadata
> + * @lpc_mem: The resource representing the LPC memory of the OpenCAPI device
> + */
> +static int reserve_metadata(struct ocxlpmem *ocxlpmem,
> + struct resource *lpc_mem)
> +{
> + ocxlpmem->metadata_addr = devm_memremap(&ocxlpmem->dev, lpc_mem->start,
> + LABEL_AREA_SIZE, MEMREMAP_WB);
> + if (IS_ERR(ocxlpmem->metadata_addr))
> + return PTR_ERR(ocxlpmem->metadata_addr);
> +
> + return 0;
> +}
> +
> +/**
> + * register_lpc_mem() - Discover persistent memory on a device and register it with the NVDIMM subsystem
> + * @ocxlpmem: the device metadata
> + * Return: 0 on success
> + */
> +static int register_lpc_mem(struct ocxlpmem *ocxlpmem)
> +{
> + struct nd_region_desc region_desc;
> + struct nd_mapping_desc nd_mapping_desc;
> + struct resource *lpc_mem;
> + const struct ocxl_afu_config *config;
> + const struct ocxl_fn_config *fn_config;
> + int rc;
> + unsigned long nvdimm_cmd_mask = 0;
> + unsigned long nvdimm_flags = 0;
> + int target_node;
> + char serial[16+1];
> +
> + // Set up the reserved metadata area
> + rc = ocxl_afu_map_lpc_mem(ocxlpmem->ocxl_afu);
> + if (rc < 0)
> + return rc;
> +
> + lpc_mem = ocxl_afu_lpc_mem(ocxlpmem->ocxl_afu);
> + if (lpc_mem == NULL || lpc_mem->start == 0)
> + return -EINVAL;
> +
> + config = ocxl_afu_config(ocxlpmem->ocxl_afu);
> + fn_config = ocxl_function_config(ocxlpmem->ocxl_fn);
> +
> + rc = reserve_metadata(ocxlpmem, lpc_mem);
> + if (rc)
> + return rc;
> +
> + ocxlpmem->bus_desc.provider_name = "ocxl-pmem";
> + ocxlpmem->bus_desc.ndctl = ndctl;
> + ocxlpmem->bus_desc.module = THIS_MODULE;
> +
> + ocxlpmem->nvdimm_bus = nvdimm_bus_register(&ocxlpmem->dev,
> + &ocxlpmem->bus_desc);
> + if (!ocxlpmem->nvdimm_bus)
> + return -EINVAL;
> +
> + ocxlpmem->pmem_res.start = (u64)lpc_mem->start + LABEL_AREA_SIZE;
> + ocxlpmem->pmem_res.end = (u64)lpc_mem->start + config->lpc_mem_size - 1;
> + ocxlpmem->pmem_res.name = "OpenCAPI persistent memory";
> +
> + set_bit(ND_CMD_GET_CONFIG_SIZE, &nvdimm_cmd_mask);
> + set_bit(ND_CMD_GET_CONFIG_DATA, &nvdimm_cmd_mask);
> + set_bit(ND_CMD_SET_CONFIG_DATA, &nvdimm_cmd_mask);
> +
> + set_bit(NDD_ALIASING, &nvdimm_flags);
> +
> + snprintf(serial, sizeof(serial), "%llx", fn_config->serial);
> + nd_mapping_desc.nvdimm = nvdimm_create(ocxlpmem->nvdimm_bus, ocxlpmem,
> + NULL, nvdimm_flags, nvdimm_cmd_mask,
> + 0, NULL);
> + if (!nd_mapping_desc.nvdimm)
> + return -ENOMEM;
> +
> + if (nvdimm_bus_check_dimm_count(ocxlpmem->nvdimm_bus, 1))
> + return -EINVAL;
> +
> + nd_mapping_desc.start = ocxlpmem->pmem_res.start;
> + nd_mapping_desc.size = resource_size(&ocxlpmem->pmem_res);
> + nd_mapping_desc.position = 0;
> +
> + ocxlpmem->nd_set.cookie1 = fn_config->serial + 1; // allow for empty serial
> + ocxlpmem->nd_set.cookie2 = fn_config->serial + 1;
> +
> + target_node = of_node_to_nid(ocxlpmem->pdev->dev.of_node);
> +
> + memset(®ion_desc, 0, sizeof(region_desc));
> + region_desc.res = &ocxlpmem->pmem_res;
> + region_desc.numa_node = NUMA_NO_NODE;
> + region_desc.target_node = target_node;
> + region_desc.num_mappings = 1;
> + region_desc.mapping = &nd_mapping_desc;
> + region_desc.nd_set = &ocxlpmem->nd_set;
> +
> + set_bit(ND_REGION_PAGEMAP, ®ion_desc.flags);
> + /*
> + * NB: libnvdimm copies the data from ndr_desc into it's own
> + * structures so passing a stack pointer is fine.
> + */
> + ocxlpmem->nd_region = nvdimm_pmem_region_create(ocxlpmem->nvdimm_bus,
> + ®ion_desc);
> + if (!ocxlpmem->nd_region)
> + return -EINVAL;
> +
> + dev_info(&ocxlpmem->dev,
> + "Onlining %lluMB of persistent memory\n",
> + nd_mapping_desc.size / SZ_1M);
> +
> + return 0;
> +}
> +
> +/**
> + * allocate_minor() - Allocate a minor number to use for an OpenCAPI pmem device
> + * @ocxlpmem: the device metadata
> + * Return: the allocated minor number
> + */
> +static int allocate_minor(struct ocxlpmem *ocxlpmem)
> +{
> + int minor;
> +
> + mutex_lock(&minors_idr_lock);
> + minor = idr_alloc(&minors_idr, ocxlpmem, 0, NUM_MINORS, GFP_KERNEL);
> + mutex_unlock(&minors_idr_lock);
> + return minor;
> +}
> +
> +static void free_minor(struct ocxlpmem *ocxlpmem)
> +{
> + mutex_lock(&minors_idr_lock);
> + idr_remove(&minors_idr, MINOR(ocxlpmem->dev.devt));
> + mutex_unlock(&minors_idr_lock);
> +}
> +
> +/**
> + * free_ocxlpmem() - Free all members of an ocxlpmem struct
> + * @ocxlpmem: the device struct to clear
> + */
> +static void free_ocxlpmem(struct ocxlpmem *ocxlpmem)
> +{
> + int rc;
> +
> + if (ocxlpmem->nvdimm_bus)
> + nvdimm_bus_unregister(ocxlpmem->nvdimm_bus);
> +
> + free_minor(ocxlpmem);
> +
> + if (ocxlpmem->metadata_addr)
> + devm_memunmap(&ocxlpmem->dev, ocxlpmem->metadata_addr);
> +
> + if (ocxlpmem->ocxl_context) {
> + rc = ocxl_context_detach(ocxlpmem->ocxl_context);
> + if (rc == -EBUSY)
> + dev_warn(&ocxlpmem->dev, "Timeout detaching ocxl context\n");
> + else
> + ocxl_context_free(ocxlpmem->ocxl_context);
> +
> + }
> +
> + if (ocxlpmem->ocxl_afu)
> + ocxl_afu_put(ocxlpmem->ocxl_afu);
> +
> + if (ocxlpmem->ocxl_fn)
> + ocxl_function_close(ocxlpmem->ocxl_fn);
> +
> + kfree(ocxlpmem);
> +}
> +
> +/**
> + * free_ocxlpmem_dev() - Free an OpenCAPI persistent memory device
> + * @dev: The device struct
> + */
> +static void free_ocxlpmem_dev(struct device *dev)
> +{
> + struct ocxlpmem *ocxlpmem = container_of(dev, struct ocxlpmem, dev);
> +
> + free_ocxlpmem(ocxlpmem);
> +}
> +
> +/**
> + * ocxlpmem_register() - Register an OpenCAPI pmem device with the kernel
> + * @ocxlpmem: the device metadata
> + * Return: 0 on success, negative on failure
> + */
> +static int ocxlpmem_register(struct ocxlpmem *ocxlpmem)
> +{
> + int rc;
> + int minor = allocate_minor(ocxlpmem);
> +
> + if (minor < 0)
> + return minor;
> +
> + ocxlpmem->dev.release = free_ocxlpmem_dev;
> + rc = dev_set_name(&ocxlpmem->dev, "ocxlpmem%d", minor);
> + if (rc < 0)
> + return rc;
> +
> + ocxlpmem->dev.devt = MKDEV(MAJOR(ocxlpmem_dev), minor);
> + ocxlpmem->dev.class = ocxlpmem_class;
This function, as well as allocate_minor() and free_minor() above
reference resources (the IDR, the file class, ...) which are not
initialized yet. The function file_init() is coming in a later patch.
> + ocxlpmem->dev.parent = &ocxlpmem->pdev->dev;
> +
> + return device_register(&ocxlpmem->dev);
> +}
> +
> +/**
> + * ocxlpmem_remove() - Free an OpenCAPI persistent memory device
> + * @pdev: the PCI device information struct
> + */
> +static void ocxlpmem_remove(struct pci_dev *pdev)
> +{
> + if (PCI_FUNC(pdev->devfn) == 0) {
> + struct ocxlpmem_function0 *func0 = pci_get_drvdata(pdev);
> +
> + if (func0) {
> + ocxl_function_close(func0->ocxl_fn);
> + func0->ocxl_fn = NULL;
> + }
The struct ocxlpmem_function0 allocated on probe() should be freed.
> + } else {
> + struct ocxlpmem *ocxlpmem = pci_get_drvdata(pdev);
> +
> + if (ocxlpmem)
> + device_unregister(&ocxlpmem->dev);
> + }
> +}
> +
> +/**
> + * probe_function0() - Set up function 0 for an OpenCAPI persistent memory device
> + * This is important as it enables templates higher than 0 across all other functions,
> + * which in turn enables higher bandwidth accesses
> + * @pdev: the PCI device information struct
> + * Return: 0 on success, negative on failure
> + */
> +static int probe_function0(struct pci_dev *pdev)
> +{
> + struct ocxlpmem_function0 *func0 = NULL;
> + struct ocxl_fn *fn;
> +
> + func0 = kzalloc(sizeof(*func0), GFP_KERNEL);
> + if (!func0)
> + return -ENOMEM;
> +
> + func0->pdev = pdev;
Storing the struct pci_dev for function 0 appears to be useless.
> + fn = ocxl_function_open(pdev);
> + if (IS_ERR(fn)) {
> + kfree(func0);
> + dev_err(&pdev->dev, "failed to open OCXL function\n");
> + return PTR_ERR(fn);
> + }
> + func0->ocxl_fn = fn;
> +
> + pci_set_drvdata(pdev, func0);
> +
> + return 0;
> +}
> +
> +/**
> + * probe() - Init an OpenCAPI persistent memory device
> + * @pdev: the PCI device information struct
> + * @ent: The entry from ocxlpmem_pci_tbl
> + * Return: 0 on success, negative on failure
> + */
> +static int probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> +{
> + struct ocxlpmem *ocxlpmem;
> + int rc;
> +
> + if (PCI_FUNC(pdev->devfn) == 0)
> + return probe_function0(pdev);
> + else if (PCI_FUNC(pdev->devfn) != 1)
> + return 0;
> +
> + ocxlpmem = kzalloc(sizeof(*ocxlpmem), GFP_KERNEL);
> + if (!ocxlpmem) {
> + dev_err(&pdev->dev, "Could not allocate OpenCAPI persistent memory metadata\n");
> + rc = -ENOMEM;
> + goto err;
> + }
> + ocxlpmem->pdev = pdev;
We should probably call pci_dev_get() here if we store the struct
pci_dev pointer. We could debate how useful it really is, considering
we're registering a device, which will also take a reference, but it
looks like the safe thing to do considering all those resources don't
have exactly the same life cycle and it is standard practice to
guarantee that we won't have a dangling pointer.
> +
> + pci_set_drvdata(pdev, ocxlpmem);
> +
> + ocxlpmem->ocxl_fn = ocxl_function_open(pdev);
> + if (IS_ERR(ocxlpmem->ocxl_fn)) {
> + kfree(ocxlpmem);
ocxlpmem is freed...
> + pci_set_drvdata(pdev, NULL);
> + dev_err(&pdev->dev, "failed to open OCXL function\n");
> + rc = PTR_ERR(ocxlpmem->ocxl_fn);
... and then referenced.
> + goto err;
> + }
> +
> + ocxlpmem->ocxl_afu = ocxl_function_fetch_afu(ocxlpmem->ocxl_fn, 0);
> + if (ocxlpmem->ocxl_afu == NULL) {
> + dev_err(&pdev->dev, "Could not get OCXL AFU from function\n");
The error path here should match the above, to free struct ocxlpmem.
> + rc = -ENXIO;
> + goto err;
> + }
> +
> + ocxl_afu_get(ocxlpmem->ocxl_afu);
> +
> + // Resources allocated below here are cleaned up in the release handler
> +
> + rc = ocxlpmem_register(ocxlpmem);
> + if (rc) {
> + dev_err(&pdev->dev, "Could not register OpenCAPI persistent memory device with the kernel\n");
> + goto err;
> + }
> +
> + rc = ocxl_context_alloc(&ocxlpmem->ocxl_context, ocxlpmem->ocxl_afu, NULL);
> + if (rc) {
> + dev_err(&pdev->dev, "Could not allocate OCXL context\n");
> + goto err;
> + }
> +
> + rc = ocxl_context_attach(ocxlpmem->ocxl_context, 0, NULL);
> + if (rc) {
> + dev_err(&pdev->dev, "Could not attach ocxl context\n");
> + goto err;
> + }
> +
> + rc = register_lpc_mem(ocxlpmem);
> + if (rc) {
> + dev_err(&pdev->dev, "Could not register OpenCAPI persistent memory with libnvdimm\n");
> + goto err;
> + }
> +
> + return 0;
> +
> +err:
> + /*
> + * Further cleanup is done in the release handler via free_ocxlpmem()
> + * This allows us to keep the character device live to handle IOCTLs to
> + * investigate issues if the card has an error
> + */
If we fail probe, we don't call device_unregister() and the data
structures will never be freed. The comment seems to indicate it's done
on purpose but that looks suprising and wrong. If we fail probe, the
kernel thinks the driver it _not_ handling the device, so we need to
exit probe() cleanly. We're not supposed to be able to make some debug
ioctl calls. Once we fail probe, the kernel is free to do whatever it
wants with the pci device. If you manage to extract some debug info
during development, then fine, but it's not something we can rely on and
upstream.
If the card enters an error state after probe(), then we don't need that
anyway. We have all the time in the world to call ioctl's, as long as we
don't call the remove callback of the driver.
> +
> + dev_err(&pdev->dev,
> + "Error detected, will not register OpenCAPI persistent memory\n");
> + return rc;
> +}
> +
> +static struct pci_driver pci_driver = {
> + .name = "ocxl-pmem",
> + .id_table = ocxlpmem_pci_tbl,
> + .probe = probe,
> + .remove = ocxlpmem_remove,
> + .shutdown = ocxlpmem_remove,
nitpick: why doesn't the probe callback follow the same naming
convention? It's all static and doesn't really matter, but...
Fred
> +};
> +
> +static int __init ocxlpmem_init(void)
> +{
> + int rc = 0;
> +
> + rc = pci_register_driver(&pci_driver);
> + if (rc)
> + return rc;
> +
> + return 0;
> +}
> +
> +static void ocxlpmem_exit(void)
> +{
> + pci_unregister_driver(&pci_driver);
> +}
> +
> +module_init(ocxlpmem_init);
> +module_exit(ocxlpmem_exit);
> +
> +MODULE_DESCRIPTION("OpenCAPI Persistent Memory");
> +MODULE_LICENSE("GPL");
> diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> new file mode 100644
> index 000000000000..0faf3740e9b8
> --- /dev/null
> +++ b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> @@ -0,0 +1,28 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +// Copyright 2019 IBM Corp.
> +
> +#include <linux/pci.h>
> +#include <misc/ocxl.h>
> +#include <linux/libnvdimm.h>
> +#include <linux/mm.h>
> +
> +#define LABEL_AREA_SIZE (1UL << PA_SECTION_SHIFT)
> +
> +struct ocxlpmem_function0 {
> + struct pci_dev *pdev;
> + struct ocxl_fn *ocxl_fn;
> +};
> +
> +struct ocxlpmem {
> + struct device dev;
> + struct pci_dev *pdev;
> + struct ocxl_fn *ocxl_fn;
> + struct nd_interleave_set nd_set;
> + struct nvdimm_bus_descriptor bus_desc;
> + struct nvdimm_bus *nvdimm_bus;
> + struct ocxl_afu *ocxl_afu;
> + struct ocxl_context *ocxl_context;
> + void *metadata_addr;
> + struct resource pmem_res;
> + struct nd_region *nd_region;
> +};
>
^ permalink raw reply
* Re: [PATCH] ima: add a new CONFIG for loading arch-specific policies
From: Mimi Zohar @ 2020-02-27 19:38 UTC (permalink / raw)
To: Lakshmi Ramasubramanian, Nayna Jain, linux-integrity,
linuxppc-dev, linux-efi, linux-s390
Cc: Martin Schwidefsky, Philipp Rudo, Ard Biesheuvel, linux-kernel
In-Reply-To: <1582749379.10443.246.camel@linux.ibm.com>
On Wed, 2020-02-26 at 15:36 -0500, Mimi Zohar wrote:
> On Wed, 2020-02-26 at 11:21 -0800, Lakshmi Ramasubramanian wrote:
> > Hi Nayna,
> >
> > > +
> > > +config IMA_SECURE_AND_OR_TRUSTED_BOOT
> > > + bool
> > > + depends on IMA
> > > + depends on IMA_ARCH_POLICY
> > > + default n
> > > + help
> > > + This option is selected by architectures to enable secure and/or
> > > + trusted boot based on IMA runtime policies.
> > >
> >
> > Why is the default for this new config "n"?
> > Is there any reason to not turn on this config if both IMA and
> > IMA_ARCH_POLICY are set to y?
>
> Good catch. Having "IMA_SECURE_AND_OR_TRUSTED_BOOT" depend on
> "IMA_ARCH_POLICY" doesn't make sense. "IMA_ARCH_POLICY" needs to be
> selected.
After discussing this some more with Nayna, the new Kconfig indicates
that the architecture defines the arch_ima_get_secureboot() and
arch_get_ima_policy() functions, but doesn't automatically enable
IMA_ARCH_POLICY. The decision to enable IMA_ARCH_POLICY is left up to
whoever is building the kernel. The patch, at least this aspect of
it, is correct.
Mimi
^ permalink raw reply
* Re: [PATCH v3 2/4] ASoC: fsl_asrc: Move common definition to fsl_asrc_common
From: Nicolin Chen @ 2020-02-27 18:39 UTC (permalink / raw)
To: Shengjiu Wang
Cc: mark.rutland, devicetree, alsa-devel, timur, Xiubo.Lee,
linuxppc-dev, tiwai, lgirdwood, robh+dt, perex, broonie, festevam,
linux-kernel
In-Reply-To: <d9aebf5ef9a92623db10dc537161b3ecdb1d8b18.1582770784.git.shengjiu.wang@nxp.com>
On Thu, Feb 27, 2020 at 10:41:56AM +0800, Shengjiu Wang wrote:
> There is a new ASRC included in i.MX serial platform, there
> are some common definition can be shared with each other.
> So move the common definition to a separate header file.
>
> And add fsl_asrc_pair_internal and fsl_asrc_internal for
> the variable specific for the module, which can be used
> internally.
I think we can just call it "priv", instead of "internal", since
it's passed by the "void *private" pointer.
And it'd be nicer to have an extra preparational patch to rename
existing "struct fsl_asrc *asrc_priv" to "struct fsl_asrc *asrc".
Something like:
struct fsl_asrc *asrc = yyyy;
struct fsl_asrc_pair *pair = xxxx;
struct fsl_asrc_priv *asrc_priv = asrc->private;
struct fsl_asrc_pair_priv *pair_priv = pair->private;
Thanks
------
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> ---
> sound/soc/fsl/fsl_asrc.c | 81 +++++++++++++++---------
> sound/soc/fsl/fsl_asrc.h | 74 ++--------------------
> sound/soc/fsl/fsl_asrc_common.h | 105 ++++++++++++++++++++++++++++++++
> sound/soc/fsl/fsl_asrc_dma.c | 25 ++++----
> 4 files changed, 176 insertions(+), 109 deletions(-)
> create mode 100644 sound/soc/fsl/fsl_asrc_common.h
>
> diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
> index 2b6a1643573c..7f862d220a8e 100644
> --- a/sound/soc/fsl/fsl_asrc.c
> +++ b/sound/soc/fsl/fsl_asrc.c
> @@ -308,8 +308,10 @@ static int fsl_asrc_set_ideal_ratio(struct fsl_asrc_pair *pair,
> */
> static int fsl_asrc_config_pair(struct fsl_asrc_pair *pair, bool use_ideal_rate)
> {
> - struct asrc_config *config = pair->config;
> + struct fsl_asrc_pair_internal *pair_internal = pair->private;
> + struct asrc_config *config = pair_internal->config;
> struct fsl_asrc *asrc_priv = pair->asrc_priv;
> + struct fsl_asrc_internal *asrc_internal = asrc_priv->private;
> enum asrc_pair_index index = pair->index;
> enum asrc_word_width input_word_width;
> enum asrc_word_width output_word_width;
> @@ -392,11 +394,11 @@ static int fsl_asrc_config_pair(struct fsl_asrc_pair *pair, bool use_ideal_rate)
> }
>
> /* Validate input and output clock sources */
> - clk_index[IN] = asrc_priv->clk_map[IN][config->inclk];
> - clk_index[OUT] = asrc_priv->clk_map[OUT][config->outclk];
> + clk_index[IN] = asrc_internal->clk_map[IN][config->inclk];
> + clk_index[OUT] = asrc_internal->clk_map[OUT][config->outclk];
>
> /* We only have output clock for ideal ratio mode */
> - clk = asrc_priv->asrck_clk[clk_index[ideal ? OUT : IN]];
> + clk = asrc_internal->asrck_clk[clk_index[ideal ? OUT : IN]];
>
> clk_rate = clk_get_rate(clk);
> rem[IN] = do_div(clk_rate, inrate);
> @@ -417,7 +419,7 @@ static int fsl_asrc_config_pair(struct fsl_asrc_pair *pair, bool use_ideal_rate)
>
> div[IN] = min_t(u32, 1024, div[IN]);
>
> - clk = asrc_priv->asrck_clk[clk_index[OUT]];
> + clk = asrc_internal->asrck_clk[clk_index[OUT]];
> clk_rate = clk_get_rate(clk);
> if (ideal && use_ideal_rate)
> rem[OUT] = do_div(clk_rate, IDEAL_RATIO_RATE);
> @@ -437,13 +439,13 @@ static int fsl_asrc_config_pair(struct fsl_asrc_pair *pair, bool use_ideal_rate)
> /* Set the channel number */
> channels = config->channel_num;
>
> - if (asrc_priv->soc->channel_bits < 4)
> + if (asrc_internal->soc->channel_bits < 4)
> channels /= 2;
>
> /* Update channels for current pair */
> regmap_update_bits(asrc_priv->regmap, REG_ASRCNCR,
> - ASRCNCR_ANCi_MASK(index, asrc_priv->soc->channel_bits),
> - ASRCNCR_ANCi(index, channels, asrc_priv->soc->channel_bits));
> + ASRCNCR_ANCi_MASK(index, asrc_internal->soc->channel_bits),
> + ASRCNCR_ANCi(index, channels, asrc_internal->soc->channel_bits));
>
> /* Default setting: Automatic selection for processing mode */
> regmap_update_bits(asrc_priv->regmap, REG_ASRCTR,
> @@ -568,9 +570,10 @@ static int fsl_asrc_dai_startup(struct snd_pcm_substream *substream,
> struct snd_soc_dai *dai)
> {
> struct fsl_asrc *asrc_priv = snd_soc_dai_get_drvdata(dai);
> + struct fsl_asrc_internal *asrc_internal = asrc_priv->private;
>
> /* Odd channel number is not valid for older ASRC (channel_bits==3) */
> - if (asrc_priv->soc->channel_bits == 3)
> + if (asrc_internal->soc->channel_bits == 3)
> snd_pcm_hw_constraint_step(substream->runtime, 0,
> SNDRV_PCM_HW_PARAM_CHANNELS, 2);
>
> @@ -586,6 +589,7 @@ static int fsl_asrc_dai_hw_params(struct snd_pcm_substream *substream,
> struct fsl_asrc *asrc_priv = snd_soc_dai_get_drvdata(dai);
> struct snd_pcm_runtime *runtime = substream->runtime;
> struct fsl_asrc_pair *pair = runtime->private_data;
> + struct fsl_asrc_pair_internal *pair_internal = pair->private;
> unsigned int channels = params_channels(params);
> unsigned int rate = params_rate(params);
> struct asrc_config config;
> @@ -597,7 +601,7 @@ static int fsl_asrc_dai_hw_params(struct snd_pcm_substream *substream,
> return ret;
> }
>
> - pair->config = &config;
> + pair_internal->config = &config;
>
> config.pair = pair->index;
> config.channel_num = channels;
> @@ -931,10 +935,16 @@ static irqreturn_t fsl_asrc_isr(int irq, void *dev_id)
> return IRQ_HANDLED;
> }
>
> +static int fsl_asrc_get_fifo_addr(u8 dir, enum asrc_pair_index index)
> +{
> + return REG_ASRDx(dir, index);
> +}
> +
> static int fsl_asrc_probe(struct platform_device *pdev)
> {
> struct device_node *np = pdev->dev.of_node;
> struct fsl_asrc *asrc_priv;
> + struct fsl_asrc_internal *asrc_internal;
> struct resource *res;
> void __iomem *regs;
> int irq, ret, i;
> @@ -946,7 +956,12 @@ static int fsl_asrc_probe(struct platform_device *pdev)
> if (!asrc_priv)
> return -ENOMEM;
>
> + asrc_internal = devm_kzalloc(&pdev->dev, sizeof(*asrc_internal), GFP_KERNEL);
> + if (!asrc_internal)
> + return -ENOMEM;
> +
> asrc_priv->pdev = pdev;
> + asrc_priv->private = asrc_internal;
>
> /* Get the addresses and IRQ */
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> @@ -992,25 +1007,31 @@ static int fsl_asrc_probe(struct platform_device *pdev)
>
> for (i = 0; i < ASRC_CLK_MAX_NUM; i++) {
> sprintf(tmp, "asrck_%x", i);
> - asrc_priv->asrck_clk[i] = devm_clk_get(&pdev->dev, tmp);
> - if (IS_ERR(asrc_priv->asrck_clk[i])) {
> + asrc_internal->asrck_clk[i] = devm_clk_get(&pdev->dev, tmp);
> + if (IS_ERR(asrc_internal->asrck_clk[i])) {
> dev_err(&pdev->dev, "failed to get %s clock\n", tmp);
> - return PTR_ERR(asrc_priv->asrck_clk[i]);
> + return PTR_ERR(asrc_internal->asrck_clk[i]);
> }
> }
>
> - asrc_priv->soc = of_device_get_match_data(&pdev->dev);
> - if (!asrc_priv->soc) {
> + asrc_internal->soc = of_device_get_match_data(&pdev->dev);
> + if (!asrc_internal->soc) {
> dev_err(&pdev->dev, "failed to get soc data\n");
> return -ENODEV;
> }
>
> + asrc_priv->use_edma = asrc_internal->soc->use_edma;
> + asrc_priv->get_dma_channel = fsl_asrc_get_dma_channel;
> + asrc_priv->request_pair = fsl_asrc_request_pair;
> + asrc_priv->release_pair = fsl_asrc_release_pair;
> + asrc_priv->get_fifo_addr = fsl_asrc_get_fifo_addr;
> +
> if (of_device_is_compatible(np, "fsl,imx35-asrc")) {
> - asrc_priv->clk_map[IN] = input_clk_map_imx35;
> - asrc_priv->clk_map[OUT] = output_clk_map_imx35;
> + asrc_internal->clk_map[IN] = input_clk_map_imx35;
> + asrc_internal->clk_map[OUT] = output_clk_map_imx35;
> } else if (of_device_is_compatible(np, "fsl,imx53-asrc")) {
> - asrc_priv->clk_map[IN] = input_clk_map_imx53;
> - asrc_priv->clk_map[OUT] = output_clk_map_imx53;
> + asrc_internal->clk_map[IN] = input_clk_map_imx53;
> + asrc_internal->clk_map[OUT] = output_clk_map_imx53;
> } else if (of_device_is_compatible(np, "fsl,imx8qm-asrc") ||
> of_device_is_compatible(np, "fsl,imx8qxp-asrc")) {
> ret = of_property_read_u32(np, "fsl,asrc-clk-map", &map_idx);
> @@ -1024,11 +1045,11 @@ static int fsl_asrc_probe(struct platform_device *pdev)
> return -EINVAL;
> }
> if (of_device_is_compatible(np, "fsl,imx8qm-asrc")) {
> - asrc_priv->clk_map[IN] = clk_map_imx8qm[map_idx];
> - asrc_priv->clk_map[OUT] = clk_map_imx8qm[map_idx];
> + asrc_internal->clk_map[IN] = clk_map_imx8qm[map_idx];
> + asrc_internal->clk_map[OUT] = clk_map_imx8qm[map_idx];
> } else {
> - asrc_priv->clk_map[IN] = clk_map_imx8qxp[map_idx];
> - asrc_priv->clk_map[OUT] = clk_map_imx8qxp[map_idx];
> + asrc_internal->clk_map[IN] = clk_map_imx8qxp[map_idx];
> + asrc_internal->clk_map[OUT] = clk_map_imx8qxp[map_idx];
> }
> }
>
> @@ -1081,6 +1102,7 @@ static int fsl_asrc_probe(struct platform_device *pdev)
> static int fsl_asrc_runtime_resume(struct device *dev)
> {
> struct fsl_asrc *asrc_priv = dev_get_drvdata(dev);
> + struct fsl_asrc_internal *asrc_internal = asrc_priv->private;
> int i, ret;
>
> ret = clk_prepare_enable(asrc_priv->mem_clk);
> @@ -1095,7 +1117,7 @@ static int fsl_asrc_runtime_resume(struct device *dev)
> goto disable_ipg_clk;
> }
> for (i = 0; i < ASRC_CLK_MAX_NUM; i++) {
> - ret = clk_prepare_enable(asrc_priv->asrck_clk[i]);
> + ret = clk_prepare_enable(asrc_internal->asrck_clk[i]);
> if (ret)
> goto disable_asrck_clk;
> }
> @@ -1104,7 +1126,7 @@ static int fsl_asrc_runtime_resume(struct device *dev)
>
> disable_asrck_clk:
> for (i--; i >= 0; i--)
> - clk_disable_unprepare(asrc_priv->asrck_clk[i]);
> + clk_disable_unprepare(asrc_internal->asrck_clk[i]);
> if (!IS_ERR(asrc_priv->spba_clk))
> clk_disable_unprepare(asrc_priv->spba_clk);
> disable_ipg_clk:
> @@ -1117,10 +1139,11 @@ static int fsl_asrc_runtime_resume(struct device *dev)
> static int fsl_asrc_runtime_suspend(struct device *dev)
> {
> struct fsl_asrc *asrc_priv = dev_get_drvdata(dev);
> + struct fsl_asrc_internal *asrc_internal = asrc_priv->private;
> int i;
>
> for (i = 0; i < ASRC_CLK_MAX_NUM; i++)
> - clk_disable_unprepare(asrc_priv->asrck_clk[i]);
> + clk_disable_unprepare(asrc_internal->asrck_clk[i]);
> if (!IS_ERR(asrc_priv->spba_clk))
> clk_disable_unprepare(asrc_priv->spba_clk);
> clk_disable_unprepare(asrc_priv->ipg_clk);
> @@ -1134,9 +1157,10 @@ static int fsl_asrc_runtime_suspend(struct device *dev)
> static int fsl_asrc_suspend(struct device *dev)
> {
> struct fsl_asrc *asrc_priv = dev_get_drvdata(dev);
> + struct fsl_asrc_internal *asrc_internal = asrc_priv->private;
>
> regmap_read(asrc_priv->regmap, REG_ASRCFG,
> - &asrc_priv->regcache_cfg);
> + &asrc_internal->regcache_cfg);
>
> regcache_cache_only(asrc_priv->regmap, true);
> regcache_mark_dirty(asrc_priv->regmap);
> @@ -1147,6 +1171,7 @@ static int fsl_asrc_suspend(struct device *dev)
> static int fsl_asrc_resume(struct device *dev)
> {
> struct fsl_asrc *asrc_priv = dev_get_drvdata(dev);
> + struct fsl_asrc_internal *asrc_internal = asrc_priv->private;
> u32 asrctr;
>
> /* Stop all pairs provisionally */
> @@ -1160,7 +1185,7 @@ static int fsl_asrc_resume(struct device *dev)
>
> regmap_update_bits(asrc_priv->regmap, REG_ASRCFG,
> ASRCFG_NDPRi_ALL_MASK | ASRCFG_POSTMODi_ALL_MASK |
> - ASRCFG_PREMODi_ALL_MASK, asrc_priv->regcache_cfg);
> + ASRCFG_PREMODi_ALL_MASK, asrc_internal->regcache_cfg);
>
> /* Restart enabled pairs */
> regmap_update_bits(asrc_priv->regmap, REG_ASRCTR,
> diff --git a/sound/soc/fsl/fsl_asrc.h b/sound/soc/fsl/fsl_asrc.h
> index 4940fa0a7542..9d02bcab1a88 100644
> --- a/sound/soc/fsl/fsl_asrc.h
> +++ b/sound/soc/fsl/fsl_asrc.h
> @@ -10,8 +10,7 @@
> #ifndef _FSL_ASRC_H
> #define _FSL_ASRC_H
>
> -#define IN 0
> -#define OUT 1
> +#include "fsl_asrc_common.h"
>
> #define ASRC_DMA_BUFFER_NUM 2
> #define ASRC_INPUTFIFO_THRESHOLD 32
> @@ -283,14 +282,6 @@
> #define ASRMCR1i_OW16_MASK (1 << ASRMCR1i_OW16_SHIFT)
> #define ASRMCR1i_OW16(v) ((v) << ASRMCR1i_OW16_SHIFT)
>
> -
> -enum asrc_pair_index {
> - ASRC_INVALID_PAIR = -1,
> - ASRC_PAIR_A = 0,
> - ASRC_PAIR_B = 1,
> - ASRC_PAIR_C = 2,
> -};
> -
> #define ASRC_PAIR_MAX_NUM (ASRC_PAIR_C + 1)
>
> enum asrc_inclk {
> @@ -446,83 +437,28 @@ struct fsl_asrc_soc_data {
> };
>
> /**
> - * fsl_asrc_pair: ASRC Pair private data
> + * fsl_asrc_pair_internal: ASRC Pair private data
> *
> - * @asrc_priv: pointer to its parent module
> * @config: configuration profile
> - * @error: error record
> - * @index: pair index (ASRC_PAIR_A, ASRC_PAIR_B, ASRC_PAIR_C)
> - * @channels: occupied channel number
> - * @desc: input and output dma descriptors
> - * @dma_chan: inputer and output DMA channels
> - * @dma_data: private dma data
> - * @pos: hardware pointer position
> - * @private: pair private area
> */
> -struct fsl_asrc_pair {
> - struct fsl_asrc *asrc_priv;
> +struct fsl_asrc_pair_internal {
> struct asrc_config *config;
> - unsigned int error;
> -
> - enum asrc_pair_index index;
> - unsigned int channels;
> -
> - struct dma_async_tx_descriptor *desc[2];
> - struct dma_chan *dma_chan[2];
> - struct imx_dma_data dma_data;
> - unsigned int pos;
> -
> - void *private;
> };
>
> /**
> - * fsl_asrc_pair: ASRC private data
> + * fsl_asrc_internal: ASRC private data
> *
> - * @dma_params_rx: DMA parameters for receive channel
> - * @dma_params_tx: DMA parameters for transmit channel
> - * @pdev: platform device pointer
> - * @regmap: regmap handler
> - * @paddr: physical address to the base address of registers
> - * @mem_clk: clock source to access register
> - * @ipg_clk: clock source to drive peripheral
> - * @spba_clk: SPBA clock (optional, depending on SoC design)
> * @asrck_clk: clock sources to driver ASRC internal logic
> - * @lock: spin lock for resource protection
> - * @pair: pair pointers
> * @soc: soc specific data
> - * @channel_avail: non-occupied channel numbers
> * @clk_map: clock map for input/output clock
> - * @asrc_rate: default sample rate for ASoC Back-Ends
> - * @asrc_format: default sample format for ASoC Back-Ends
> * @regcache_cfg: store register value of REG_ASRCFG
> */
> -struct fsl_asrc {
> - struct snd_dmaengine_dai_dma_data dma_params_rx;
> - struct snd_dmaengine_dai_dma_data dma_params_tx;
> - struct platform_device *pdev;
> - struct regmap *regmap;
> - unsigned long paddr;
> - struct clk *mem_clk;
> - struct clk *ipg_clk;
> - struct clk *spba_clk;
> +struct fsl_asrc_internal {
> struct clk *asrck_clk[ASRC_CLK_MAX_NUM];
> - spinlock_t lock;
> -
> - struct fsl_asrc_pair *pair[ASRC_PAIR_MAX_NUM];
> const struct fsl_asrc_soc_data *soc;
> - unsigned int channel_avail;
> unsigned char *clk_map[2];
>
> - int asrc_rate;
> - snd_pcm_format_t asrc_format;
> -
> u32 regcache_cfg;
> };
>
> -#define DRV_NAME "fsl-asrc-dai"
> -extern struct snd_soc_component_driver fsl_asrc_component;
> -struct dma_chan *fsl_asrc_get_dma_channel(struct fsl_asrc_pair *pair, bool dir);
> -int fsl_asrc_request_pair(int channels, struct fsl_asrc_pair *pair);
> -void fsl_asrc_release_pair(struct fsl_asrc_pair *pair);
> -
> #endif /* _FSL_ASRC_H */
> diff --git a/sound/soc/fsl/fsl_asrc_common.h b/sound/soc/fsl/fsl_asrc_common.h
> new file mode 100644
> index 000000000000..737712e8ae76
> --- /dev/null
> +++ b/sound/soc/fsl/fsl_asrc_common.h
> @@ -0,0 +1,105 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright 2019 NXP
> + *
> + */
> +
> +#ifndef _FSL_ASRC_COMMON_H
> +#define _FSL_ASRC_COMMON_H
> +
> +/* directions */
> +#define IN 0
> +#define OUT 1
> +
> +enum asrc_pair_index {
> + ASRC_INVALID_PAIR = -1,
> + ASRC_PAIR_A = 0,
> + ASRC_PAIR_B = 1,
> + ASRC_PAIR_C = 2,
> + ASRC_PAIR_D = 3,
> +};
> +
> +#define PAIR_CTX_NUM 0x4
> +#define PAIR_PRIVAT_SIZE 0x400
> +
> +/**
> + * fsl_asrc_pair: ASRC Pair common data
> + *
> + * @asrc_priv: pointer to its parent module
> + * @error: error record
> + * @index: pair index (ASRC_PAIR_A, ASRC_PAIR_B, ASRC_PAIR_C)
> + * @channels: occupied channel number
> + * @desc: input and output dma descriptors
> + * @dma_chan: inputer and output DMA channels
> + * @dma_data: private dma data
> + * @pos: hardware pointer position
> + * @private: pair private area
> + */
> +struct fsl_asrc_pair {
> + struct fsl_asrc *asrc_priv;
> + unsigned int error;
> +
> + enum asrc_pair_index index;
> + unsigned int channels;
> +
> + struct dma_async_tx_descriptor *desc[2];
> + struct dma_chan *dma_chan[2];
> + struct imx_dma_data dma_data;
> + unsigned int pos;
> +
> + void *private;
> +};
> +
> +/**
> + * fsl_asrc_pair: ASRC common data
> + *
> + * @dma_params_rx: DMA parameters for receive channel
> + * @dma_params_tx: DMA parameters for transmit channel
> + * @pdev: platform device pointer
> + * @regmap: regmap handler
> + * @paddr: physical address to the base address of registers
> + * @mem_clk: clock source to access register
> + * @ipg_clk: clock source to drive peripheral
> + * @spba_clk: SPBA clock (optional, depending on SoC design)
> + * @lock: spin lock for resource protection
> + * @pair: pair pointers
> + * @channel_avail: non-occupied channel numbers
> + * @asrc_rate: default sample rate for ASoC Back-Ends
> + * @asrc_format: default sample format for ASoC Back-Ends
> + * @use_edma: edma is used
> + * @get_dma_channel: function pointer
> + * @request_pair: function pointer
> + * @release_pair: function pointer
> + * @get_fifo_addr: function pointer
> + * @private: private data structure
> + */
> +struct fsl_asrc {
> + struct snd_dmaengine_dai_dma_data dma_params_rx;
> + struct snd_dmaengine_dai_dma_data dma_params_tx;
> + struct platform_device *pdev;
> + struct regmap *regmap;
> + unsigned long paddr;
> + struct clk *mem_clk;
> + struct clk *ipg_clk;
> + struct clk *spba_clk;
> + spinlock_t lock; /* spin lock for resource protection */
> +
> + struct fsl_asrc_pair *pair[PAIR_CTX_NUM];
> + unsigned int channel_avail;
> +
> + int asrc_rate;
> + snd_pcm_format_t asrc_format;
> + bool use_edma;
> +
> + struct dma_chan *(*get_dma_channel)(struct fsl_asrc_pair *pair, bool dir);
> + int (*request_pair)(int channels, struct fsl_asrc_pair *pair);
> + void (*release_pair)(struct fsl_asrc_pair *pair);
> + int (*get_fifo_addr)(u8 dir, enum asrc_pair_index index);
> +
> + void *private;
> +};
> +
> +#define DRV_NAME "fsl-asrc-dai"
> +extern struct snd_soc_component_driver fsl_asrc_component;
> +
> +#endif /* _FSL_ASRC_COMMON_H */
> diff --git a/sound/soc/fsl/fsl_asrc_dma.c b/sound/soc/fsl/fsl_asrc_dma.c
> index f344360dcd24..5f687a54993c 100644
> --- a/sound/soc/fsl/fsl_asrc_dma.c
> +++ b/sound/soc/fsl/fsl_asrc_dma.c
> @@ -12,7 +12,7 @@
> #include <sound/dmaengine_pcm.h>
> #include <sound/pcm_params.h>
>
> -#include "fsl_asrc.h"
> +#include "fsl_asrc_common.h"
>
> #define FSL_ASRC_DMABUF_SIZE (256 * 1024)
>
> @@ -170,10 +170,10 @@ static int fsl_asrc_dma_hw_params(struct snd_soc_component *component,
>
> /* Override dma_data of the Front-End and config its dmaengine */
> dma_params_fe = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream);
> - dma_params_fe->addr = asrc_priv->paddr + REG_ASRDx(!dir, index);
> + dma_params_fe->addr = asrc_priv->paddr + asrc_priv->get_fifo_addr(!dir, index);
> dma_params_fe->maxburst = dma_params_be->maxburst;
>
> - pair->dma_chan[!dir] = fsl_asrc_get_dma_channel(pair, !dir);
> + pair->dma_chan[!dir] = asrc_priv->get_dma_channel(pair, !dir);
> if (!pair->dma_chan[!dir]) {
> dev_err(dev, "failed to request DMA channel\n");
> return -EINVAL;
> @@ -203,7 +203,7 @@ static int fsl_asrc_dma_hw_params(struct snd_soc_component *component,
> * need to configure dma_request and dma_request2, but get dma_chan via
> * dma_request_slave_channel directly with dma name of Front-End device
> */
> - if (!asrc_priv->soc->use_edma) {
> + if (!asrc_priv->use_edma) {
> /* Get DMA request of Back-End */
> tmp_chan = dma_request_slave_channel(dev_be, tx ? "tx" : "rx");
> tmp_data = tmp_chan->private;
> @@ -211,7 +211,7 @@ static int fsl_asrc_dma_hw_params(struct snd_soc_component *component,
> dma_release_channel(tmp_chan);
>
> /* Get DMA request of Front-End */
> - tmp_chan = fsl_asrc_get_dma_channel(pair, dir);
> + tmp_chan = asrc_priv->get_dma_channel(pair, dir);
> tmp_data = tmp_chan->private;
> pair->dma_data.dma_request2 = tmp_data->dma_request;
> pair->dma_data.peripheral_type = tmp_data->peripheral_type;
> @@ -222,7 +222,7 @@ static int fsl_asrc_dma_hw_params(struct snd_soc_component *component,
> dma_request_channel(mask, filter, &pair->dma_data);
> } else {
> pair->dma_chan[dir] =
> - fsl_asrc_get_dma_channel(pair, dir);
> + asrc_priv->get_dma_channel(pair, dir);
> }
>
> if (!pair->dma_chan[dir]) {
> @@ -242,10 +242,10 @@ static int fsl_asrc_dma_hw_params(struct snd_soc_component *component,
> config_be.dst_maxburst = dma_params_be->maxburst;
>
> if (tx) {
> - config_be.src_addr = asrc_priv->paddr + REG_ASRDO(index);
> + config_be.src_addr = asrc_priv->paddr + asrc_priv->get_fifo_addr(OUT, index);
> config_be.dst_addr = dma_params_be->addr;
> } else {
> - config_be.dst_addr = asrc_priv->paddr + REG_ASRDI(index);
> + config_be.dst_addr = asrc_priv->paddr + asrc_priv->get_fifo_addr(IN, index);
> config_be.src_addr = dma_params_be->addr;
> }
>
> @@ -302,11 +302,12 @@ static int fsl_asrc_dma_startup(struct snd_soc_component *component,
> return ret;
> }
>
> - pair = kzalloc(sizeof(struct fsl_asrc_pair), GFP_KERNEL);
> + pair = kzalloc(sizeof(struct fsl_asrc_pair) + PAIR_PRIVAT_SIZE, GFP_KERNEL);
> if (!pair)
> return -ENOMEM;
>
> pair->asrc_priv = asrc_priv;
> + pair->private = (void *)pair + sizeof(struct fsl_asrc_pair);
>
> runtime->private_data = pair;
>
> @@ -314,14 +315,14 @@ static int fsl_asrc_dma_startup(struct snd_soc_component *component,
> * Request pair function needs channel num as input, for this
> * dummy pair, we just request "1" channel temporarily.
> */
> - ret = fsl_asrc_request_pair(1, pair);
> + ret = asrc_priv->request_pair(1, pair);
> if (ret < 0) {
> dev_err(dev, "failed to request asrc pair\n");
> goto req_pair_err;
> }
>
> /* Request a dummy dma channel, which will be released later. */
> - tmp_chan = fsl_asrc_get_dma_channel(pair, dir);
> + tmp_chan = asrc_priv->get_dma_channel(pair, dir);
> if (!tmp_chan) {
> dev_err(dev, "failed to get dma channel\n");
> ret = -EINVAL;
> @@ -347,7 +348,7 @@ static int fsl_asrc_dma_startup(struct snd_soc_component *component,
> dma_release_channel(tmp_chan);
>
> dma_chan_err:
> - fsl_asrc_release_pair(pair);
> + asrc_priv->release_pair(pair);
>
> req_pair_err:
> if (release_pair)
> --
> 2.21.0
>
^ permalink raw reply
* Re: [5.6.0-rc2-next-20200218/powerpc] Boot failure on POWER9
From: Michal Hocko @ 2020-02-27 18:26 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Sachin Sant, Pekka Enberg, Linux-Next Mailing List,
Paul Mackerras, David Rientjes, Christopher Lameter, linuxppc-dev,
Joonsoo Kim, Kirill Tkhai
In-Reply-To: <9a86f865-50b5-7483-9257-dbb08fecd62b@suse.cz>
[Cc ppc maintainers]
On Thu 27-02-20 17:16:41, Vlastimil Babka wrote:
> On 2/27/20 5:00 PM, Sachin Sant wrote:
> >
> >
> >> On 27-Feb-2020, at 5:42 PM, Michal Hocko <mhocko@kernel.org> wrote:
> >>
> >> A very good hint indeed. I would do this
> >> diff --git a/include/linux/topology.h b/include/linux/topology.h
> >> index eb2fe6edd73c..d9f1b6737e4d 100644
> >> --- a/include/linux/topology.h
> >> +++ b/include/linux/topology.h
> >> @@ -137,6 +137,8 @@ static inline void set_numa_mem(int node)
> >> {
> >> this_cpu_write(_numa_mem_, node);
> >> _node_numa_mem_[numa_node_id()] = node;
> >> + pr_info("%s %d -> %d\n", __FUNCTION__, numa_node_id(), node);
> >> + dump_stack();
> >> }
> >> #endif
> >>
> >> Btw. it would be also helpful to get
> >> `faddr2line ___slab_alloc+0x334' from your kernel Sachin.
> >
> > [linux-next]# ./scripts/faddr2line ./vmlinux ___slab_alloc+0x334
> > ___slab_alloc+0x334/0x760:
> > new_slab_objects at mm/slub.c:2478
> > (inlined by) ___slab_alloc at mm/slub.c:2628
> > [linux-next]#
>
> Hmm that doesn't look relevant, but that address was marked as unreliable, no?
> Don't we actually need this one?
>
> [ 8.768727] NIP [c0000000003d55f4] ___slab_alloc+0x1f4/0x760
>
> > I have also attached boot log with a kernel that include about change.
> > I see the following o/p during boot:
> >
> > [ 0.005269] set_numa_mem 1 -> 1
>
> So there's no "set_numa_mem 0 -> X", specifically not
> "set_numa_mem 0 -> 1" which I would have expected. That seems to confirm my
> suspicion that the arch code doesn't set up the memoryless node 0 properly.
Please have a look at http://lkml.kernel.org/r/52EF4673-7292-4C4C-B459-AF583951BA48@linux.vnet.ibm.com
for the boot log with the debugging patch which tracks set_numa_mem.
This seems to lead to a crash in the slab allocator bebcause
node_to_mem_node(0) for memory less node resolves to the memory less
node http://lkml.kernel.org/r/dd450314-d428-6776-af07-f92c04c7b967@suse.cz.
The original report is http://lkml.kernel.org/r/3381CD91-AB3D-4773-BA04-E7A072A63968@linux.vnet.ibm.com
>
> > [ 0.005270] CPU: 12 PID: 0 Comm: swapper/12 Not tainted 5.6.0-rc3-next-20200227-autotest+ #6
> > [ 0.005271] Call Trace:
> > [ 0.005272] [c0000008b37dfe80] [c000000000b5d948] dump_stack+0xbc/0x104 (unreliable)
> > [ 0.005274] [c0000008b37dfec0] [c000000000059320] start_secondary+0x600/0x6e0
> > [ 0.005277] [c0000008b37dff90] [c00000000000ac54] start_secondary_prolog+0x10/0x14
> >
> > Thanks
> > -Sachin
> >
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH v3 0/7] Allow setting caching mode in arch_add_memory() for P2PDMA
From: Dan Williams @ 2020-02-27 18:08 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: linux-ia64, Linux-sh, Peter Zijlstra, Dave Hansen,
platform-driver-x86, Linux MM, Will Deacon, Christoph Hellwig,
linux-s390, David Hildenbrand, Ingo Molnar, Catalin Marinas,
Borislav Petkov, Andy Lutomirski, Thomas Gleixner, Michal Hocko,
Linux ARM, linuxppc-dev, Eric Badger, Linux Kernel Mailing List,
Andrew Morton, Logan Gunthorpe
In-Reply-To: <20200227180346.GM31668@ziepe.ca>
On Thu, Feb 27, 2020 at 10:03 AM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> On Thu, Feb 27, 2020 at 09:55:04AM -0800, Dan Williams wrote:
> > On Thu, Feb 27, 2020 at 9:43 AM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> > >
> > > On Thu, Feb 27, 2020 at 10:21:50AM -0700, Logan Gunthorpe wrote:
> > > >
> > > >
> > > > On 2020-02-27 10:17 a.m., Jason Gunthorpe wrote:
> > > > >> Instead of this, this series proposes a change to arch_add_memory()
> > > > >> to take the pgprot required by the mapping which allows us to
> > > > >> explicitly set pagetable entries for P2PDMA memory to WC.
> > > > >
> > > > > Is there a particular reason why WC was selected here? I thought for
> > > > > the p2pdma cases there was no kernel user that touched the memory?
> > > >
> > > > Yes, that's correct. I choose WC here because the existing users are
> > > > registering memory blocks without side effects which fit the WC
> > > > semantics well.
> > >
> > > Hm, AFAIK WC memory is not compatible with the spinlocks/mutexs/etc in
> > > Linux, so while it is true the memory has no side effects, there would
> > > be surprising concurrency risks if anything in the kernel tried to
> > > write to it.
> > >
> > > Not compatible means the locks don't contain stores to WC memory the
> > > way you would expect. AFAIK on many CPUs extra barriers are required
> > > to keep WC stores ordered, the same way ARM already has extra barriers
> > > to keep UC stores ordered with locking..
> > >
> > > The spinlocks are defined to contain UC stores though.
> >
> > How are spinlocks and mutexes getting into p2pdma ranges in the first
> > instance? Even with UC, the system has bigger problems if it's trying
> > to send bus locks targeting PCI, see the flurry of activity of trying
> > to trigger faults on split locks [1].
>
> This is not what I was trying to explain.
>
> Consider
>
> static spinlock lock; // CPU DRAM
> static idx = 0;
> u64 *wc_memory = [..];
>
> spin_lock(&lock);
> wc_memory[0] = idx++;
> spin_unlock(&lock);
>
> You'd expect that the PCI device will observe stores where idx is
> strictly increasing, but this is not guarenteed. idx may decrease, idx
> may skip. It just won't duplicate.
>
> Or perhaps
>
> wc_memory[0] = foo;
> writel(doorbell)
>
> foo is not guarenteed observable by the device before doorbell reaches
> the device.
>
> All of these are things that do not happen with UC or NC memory, and
> are surprising violations of our programming model.
>
> Generic kernel code should never touch WC memory unless the code is
> specifically designed to handle it.
Ah, yes, agree.
^ permalink raw reply
* Re: [PATCH v3 0/7] Allow setting caching mode in arch_add_memory() for P2PDMA
From: Jason Gunthorpe @ 2020-02-27 18:03 UTC (permalink / raw)
To: Dan Williams
Cc: linux-ia64, Linux-sh, Peter Zijlstra, Dave Hansen,
platform-driver-x86, Linux MM, Will Deacon, Christoph Hellwig,
linux-s390, David Hildenbrand, Ingo Molnar, Catalin Marinas,
Borislav Petkov, Andy Lutomirski, Thomas Gleixner, Michal Hocko,
Linux ARM, linuxppc-dev, Eric Badger, Linux Kernel Mailing List,
Andrew Morton, Logan Gunthorpe
In-Reply-To: <CAPcyv4iek=EmNk9JgXq=-HcZjd9Kz4m2+qXMhnDWMshFKFZPXQ@mail.gmail.com>
On Thu, Feb 27, 2020 at 09:55:04AM -0800, Dan Williams wrote:
> On Thu, Feb 27, 2020 at 9:43 AM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> >
> > On Thu, Feb 27, 2020 at 10:21:50AM -0700, Logan Gunthorpe wrote:
> > >
> > >
> > > On 2020-02-27 10:17 a.m., Jason Gunthorpe wrote:
> > > >> Instead of this, this series proposes a change to arch_add_memory()
> > > >> to take the pgprot required by the mapping which allows us to
> > > >> explicitly set pagetable entries for P2PDMA memory to WC.
> > > >
> > > > Is there a particular reason why WC was selected here? I thought for
> > > > the p2pdma cases there was no kernel user that touched the memory?
> > >
> > > Yes, that's correct. I choose WC here because the existing users are
> > > registering memory blocks without side effects which fit the WC
> > > semantics well.
> >
> > Hm, AFAIK WC memory is not compatible with the spinlocks/mutexs/etc in
> > Linux, so while it is true the memory has no side effects, there would
> > be surprising concurrency risks if anything in the kernel tried to
> > write to it.
> >
> > Not compatible means the locks don't contain stores to WC memory the
> > way you would expect. AFAIK on many CPUs extra barriers are required
> > to keep WC stores ordered, the same way ARM already has extra barriers
> > to keep UC stores ordered with locking..
> >
> > The spinlocks are defined to contain UC stores though.
>
> How are spinlocks and mutexes getting into p2pdma ranges in the first
> instance? Even with UC, the system has bigger problems if it's trying
> to send bus locks targeting PCI, see the flurry of activity of trying
> to trigger faults on split locks [1].
This is not what I was trying to explain.
Consider
static spinlock lock; // CPU DRAM
static idx = 0;
u64 *wc_memory = [..];
spin_lock(&lock);
wc_memory[0] = idx++;
spin_unlock(&lock);
You'd expect that the PCI device will observe stores where idx is
strictly increasing, but this is not guarenteed. idx may decrease, idx
may skip. It just won't duplicate.
Or perhaps
wc_memory[0] = foo;
writel(doorbell)
foo is not guarenteed observable by the device before doorbell reaches
the device.
All of these are things that do not happen with UC or NC memory, and
are surprising violations of our programming model.
Generic kernel code should never touch WC memory unless the code is
specifically designed to handle it.
Jason
^ permalink raw reply
* Re: [PATCH v3 0/7] Allow setting caching mode in arch_add_memory() for P2PDMA
From: Dan Williams @ 2020-02-27 17:55 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: linux-ia64, Linux-sh, Peter Zijlstra, Dave Hansen,
platform-driver-x86, Linux MM, Will Deacon, Christoph Hellwig,
linux-s390, David Hildenbrand, Ingo Molnar, Catalin Marinas,
Borislav Petkov, Andy Lutomirski, Thomas Gleixner, Michal Hocko,
Linux ARM, linuxppc-dev, Eric Badger, Linux Kernel Mailing List,
Andrew Morton, Logan Gunthorpe
In-Reply-To: <20200227174311.GL31668@ziepe.ca>
On Thu, Feb 27, 2020 at 9:43 AM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> On Thu, Feb 27, 2020 at 10:21:50AM -0700, Logan Gunthorpe wrote:
> >
> >
> > On 2020-02-27 10:17 a.m., Jason Gunthorpe wrote:
> > >> Instead of this, this series proposes a change to arch_add_memory()
> > >> to take the pgprot required by the mapping which allows us to
> > >> explicitly set pagetable entries for P2PDMA memory to WC.
> > >
> > > Is there a particular reason why WC was selected here? I thought for
> > > the p2pdma cases there was no kernel user that touched the memory?
> >
> > Yes, that's correct. I choose WC here because the existing users are
> > registering memory blocks without side effects which fit the WC
> > semantics well.
>
> Hm, AFAIK WC memory is not compatible with the spinlocks/mutexs/etc in
> Linux, so while it is true the memory has no side effects, there would
> be surprising concurrency risks if anything in the kernel tried to
> write to it.
>
> Not compatible means the locks don't contain stores to WC memory the
> way you would expect. AFAIK on many CPUs extra barriers are required
> to keep WC stores ordered, the same way ARM already has extra barriers
> to keep UC stores ordered with locking..
>
> The spinlocks are defined to contain UC stores though.
How are spinlocks and mutexes getting into p2pdma ranges in the first
instance? Even with UC, the system has bigger problems if it's trying
to send bus locks targeting PCI, see the flurry of activity of trying
to trigger faults on split locks [1].
This does raise a question about separating the cacheability of the
'struct page' memmap from the BAR range. You get this for free if the
memmap is dynamically allocated from "System RAM", but perhaps
memremap_pages() should explicitly prevent altmap configurations that
try to place the map in PCI space?
> If there is no actual need today for WC I would suggest using UC as
> the default.
That's reasonable, but it still seems to be making a broken
configuration marginally less broken. I'd be more interested in
safeguards that prevent p2pdma mappings from being used for any cpu
atomic cycles.
[1]: https://lwn.net/Articles/784864/
^ permalink raw reply
* Re: [PATCH v3 0/7] Allow setting caching mode in arch_add_memory() for P2PDMA
From: Logan Gunthorpe @ 2020-02-27 17:54 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: linux-ia64, linux-sh, Peter Zijlstra, Catalin Marinas,
Dave Hansen, platform-driver-x86, linux-mm, Will Deacon,
Christoph Hellwig, linux-s390, David Hildenbrand, Ingo Molnar,
Dan Williams, Borislav Petkov, Andy Lutomirski, Thomas Gleixner,
Michal Hocko, linux-arm-kernel, Eric Badger, linux-kernel,
Andrew Morton, linuxppc-dev
In-Reply-To: <20200227174311.GL31668@ziepe.ca>
On 2020-02-27 10:43 a.m., Jason Gunthorpe wrote:
> Hm, AFAIK WC memory is not compatible with the spinlocks/mutexs/etc in
> Linux, so while it is true the memory has no side effects, there would
> be surprising concurrency risks if anything in the kernel tried to
> write to it.
>
> Not compatible means the locks don't contain stores to WC memory the
> way you would expect. AFAIK on many CPUs extra barriers are required
> to keep WC stores ordered, the same way ARM already has extra barriers
> to keep UC stores ordered with locking..
>
> The spinlocks are defined to contain UC stores though.
>
> If there is no actual need today for WC I would suggest using UC as
> the default.
Ok, that sounds sensible. I'll do that in the next revision.
Thanks,
Logan
^ permalink raw reply
* Re: [PATCH v3 0/7] Allow setting caching mode in arch_add_memory() for P2PDMA
From: Logan Gunthorpe @ 2020-02-27 17:21 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: linux-ia64, linux-sh, Peter Zijlstra, Catalin Marinas,
Dave Hansen, platform-driver-x86, linux-mm, Will Deacon,
Christoph Hellwig, linux-s390, David Hildenbrand, Ingo Molnar,
Dan Williams, Borislav Petkov, Andy Lutomirski, Thomas Gleixner,
Michal Hocko, linux-arm-kernel, Eric Badger, linux-kernel,
Andrew Morton, linuxppc-dev
In-Reply-To: <20200227171704.GK31668@ziepe.ca>
On 2020-02-27 10:17 a.m., Jason Gunthorpe wrote:
>> Instead of this, this series proposes a change to arch_add_memory()
>> to take the pgprot required by the mapping which allows us to
>> explicitly set pagetable entries for P2PDMA memory to WC.
>
> Is there a particular reason why WC was selected here? I thought for
> the p2pdma cases there was no kernel user that touched the memory?
Yes, that's correct. I choose WC here because the existing users are
registering memory blocks without side effects which fit the WC
semantics well.
> I definitely forsee devices where we want UC instead.
Yes. My expectation is that once we have a kernel user that needs this,
we'd wire the option through struct dev_pagemap so the caller can choose
the mapping that makes sense.
Logan
^ permalink raw reply
* Re: [PATCH v3 1/4] ASoC: fsl_asrc: Change asrc_width to asrc_format
From: Nicolin Chen @ 2020-02-27 17:45 UTC (permalink / raw)
To: Shengjiu Wang
Cc: Mark Rutland,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux-ALSA, Timur Tabi, Xiubo Li, Fabio Estevam, Shengjiu Wang,
Takashi Iwai, Liam Girdwood, Rob Herring, Mark Brown,
linuxppc-dev, linux-kernel
In-Reply-To: <CAA+D8AMzqpC35_CR2dCG6a_h4FzvZ6orXkPSYh_1o1d8hv+BMg@mail.gmail.com>
On Thu, Feb 27, 2020 at 01:10:19PM +0800, Shengjiu Wang wrote:
> On Thu, Feb 27, 2020 at 11:43 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
> >
> > On Thu, Feb 27, 2020 at 10:41:55AM +0800, Shengjiu Wang wrote:
> > > asrc_format is more inteligent variable, which is align
> > > with the alsa definition snd_pcm_format_t.
> > >
> > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > ---
> > > sound/soc/fsl/fsl_asrc.c | 23 +++++++++++------------
> > > sound/soc/fsl/fsl_asrc.h | 4 ++--
> > > sound/soc/fsl/fsl_asrc_dma.c | 2 +-
> > > 3 files changed, 14 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
> > > index 0dcebc24c312..2b6a1643573c 100644
> > > --- a/sound/soc/fsl/fsl_asrc.c
> > > +++ b/sound/soc/fsl/fsl_asrc.c
> >
> > > @@ -600,11 +599,6 @@ static int fsl_asrc_dai_hw_params(struct snd_pcm_substream *substream,
> > >
> > > pair->config = &config;
> > >
> > > - if (asrc_priv->asrc_width == 16)
> > > - format = SNDRV_PCM_FORMAT_S16_LE;
> > > - else
> > > - format = SNDRV_PCM_FORMAT_S24_LE;
> >
> > It feels better to me that we have format settings in hw_params().
> >
> > Why not let fsl_easrc align with this? Any reason that I'm missing?
>
> because the asrc_width is not formal, in the future we can direct
Hmm..that's our DT binding. And I don't feel it is a problem
to be ASoC irrelative.
> input the format from the dts. format involve the info about width.
Is there such any formal ASoC binding? I don't see those PCM
formats under include/dt-bindings/ folder. How are we going
to involve those formats in DT?
^ permalink raw reply
* Re: [PATCH v3 0/7] Allow setting caching mode in arch_add_memory() for P2PDMA
From: Jason Gunthorpe @ 2020-02-27 17:43 UTC (permalink / raw)
To: Logan Gunthorpe
Cc: linux-ia64, linux-sh, Peter Zijlstra, Catalin Marinas,
Dave Hansen, platform-driver-x86, linux-mm, Will Deacon,
Christoph Hellwig, linux-s390, David Hildenbrand, Ingo Molnar,
Dan Williams, Borislav Petkov, Andy Lutomirski, Thomas Gleixner,
Michal Hocko, linux-arm-kernel, Eric Badger, linux-kernel,
Andrew Morton, linuxppc-dev
In-Reply-To: <e8781f85-3fc7-b9ce-c751-606803cbdc77@deltatee.com>
On Thu, Feb 27, 2020 at 10:21:50AM -0700, Logan Gunthorpe wrote:
>
>
> On 2020-02-27 10:17 a.m., Jason Gunthorpe wrote:
> >> Instead of this, this series proposes a change to arch_add_memory()
> >> to take the pgprot required by the mapping which allows us to
> >> explicitly set pagetable entries for P2PDMA memory to WC.
> >
> > Is there a particular reason why WC was selected here? I thought for
> > the p2pdma cases there was no kernel user that touched the memory?
>
> Yes, that's correct. I choose WC here because the existing users are
> registering memory blocks without side effects which fit the WC
> semantics well.
Hm, AFAIK WC memory is not compatible with the spinlocks/mutexs/etc in
Linux, so while it is true the memory has no side effects, there would
be surprising concurrency risks if anything in the kernel tried to
write to it.
Not compatible means the locks don't contain stores to WC memory the
way you would expect. AFAIK on many CPUs extra barriers are required
to keep WC stores ordered, the same way ARM already has extra barriers
to keep UC stores ordered with locking..
The spinlocks are defined to contain UC stores though.
If there is no actual need today for WC I would suggest using UC as
the default.
Jason
^ permalink raw reply
* Re: [PATCH v3 0/7] Allow setting caching mode in arch_add_memory() for P2PDMA
From: Jason Gunthorpe @ 2020-02-27 17:17 UTC (permalink / raw)
To: Logan Gunthorpe
Cc: linux-ia64, linux-sh, Peter Zijlstra, Catalin Marinas,
Dave Hansen, platform-driver-x86, linux-mm, Will Deacon,
Christoph Hellwig, linux-s390, David Hildenbrand, Ingo Molnar,
Dan Williams, Borislav Petkov, Andy Lutomirski, Thomas Gleixner,
Michal Hocko, linux-arm-kernel, Eric Badger, linux-kernel,
Andrew Morton, linuxppc-dev
In-Reply-To: <20200221182503.28317-1-logang@deltatee.com>
On Fri, Feb 21, 2020 at 11:24:56AM -0700, Logan Gunthorpe wrote:
> Hi,
>
> This is v3 of the patchset which cleans up a number of minor issues
> from the feedback of v2 and rebases onto v5.6-rc2. Additional feedback
> is welcome.
>
> Thanks,
>
> Logan
>
> --
>
> Changes in v3:
> * Rebased onto v5.6-rc2
> * Rename mhp_modifiers to mhp_params per David with an updated kernel
> doc per Dan
> * Drop support for s390 per David seeing it does not support
> ZONE_DEVICE yet and there was a potential problem with huge pages.
> * Added WARN_ON_ONCE in cases where arches recieve non PAGE_KERNEL
> parameters
> * Collected David and Micheal's Reviewed-By and Acked-by Tags
>
> Changes in v2:
> * Rebased onto v5.5-rc5
> * Renamed mhp_restrictions to mhp_modifiers and added the pgprot field
> to that structure instead of using an argument for
> arch_add_memory().
> * Add patch to drop the unused flags field in mhp_restrictions
>
> A git branch is available here:
>
> https://github.com/sbates130272/linux-p2pmem remap_pages_cache_v3
>
> --
>
> Currently, the page tables created using memremap_pages() are always
> created with the PAGE_KERNEL cacheing mode. However, the P2PDMA code
> is creating pages for PCI BAR memory which should never be accessed
> through the cache and instead use either WC or UC. This still works in
> most cases, on x86, because the MTRR registers typically override the
> caching settings in the page tables for all of the IO memory to be
> UC-. However, this tends not to work so well on other arches or
> some rare x86 machines that have firmware which does not setup the
> MTRR registers in this way.
>
> Instead of this, this series proposes a change to arch_add_memory()
> to take the pgprot required by the mapping which allows us to
> explicitly set pagetable entries for P2PDMA memory to WC.
Is there a particular reason why WC was selected here? I thought for
the p2pdma cases there was no kernel user that touched the memory?
I definitely forsee devices where we want UC instead.
Even so, the whole idea looks like the right direction to me.
Jason
^ permalink raw reply
* Re: [PATCH v4 13/13] powerpc/ptrace: move ptrace_triggered() into hw_breakpoint.c
From: Christophe Leroy @ 2020-02-27 17:09 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, mikey,
Russell Currey
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <d45c91cf5f83424b8f3989b7ead28c50d8d765a9.1582803998.git.christophe.leroy@c-s.fr>
Russel,
Le 27/02/2020 à 12:49, Christophe Leroy a écrit :
> ptrace_triggered() is declared in asm/hw_breakpoint.h and
> only needed when CONFIG_HW_BREAKPOINT is set, so move it
> into hw_breakpoint.c
My series v4 is definitely buggy (I included ptrace_decl.h instead
instead of ptrace-decl.h), how can Snowpatch say build succeeded
(https://patchwork.ozlabs.org/patch/1245807/) ?
It fails at least on pmac32_defconfig and ppc64_defconfig, see:
http://kisskb.ellerman.id.au/kisskb/head/d45c91cf5f83424b8f3989b7ead28c50d8d765a9/
Christophe
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> v4: removing inclusing of hw_breakpoint.h now. Previously it was done too early.
> ---
> arch/powerpc/kernel/hw_breakpoint.c | 16 ++++++++++++++++
> arch/powerpc/kernel/ptrace/ptrace.c | 19 -------------------
> 2 files changed, 16 insertions(+), 19 deletions(-)
>
> diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
> index 2462cd7c565c..2c0be9d941cf 100644
> --- a/arch/powerpc/kernel/hw_breakpoint.c
> +++ b/arch/powerpc/kernel/hw_breakpoint.c
> @@ -427,3 +427,19 @@ void hw_breakpoint_pmu_read(struct perf_event *bp)
> {
> /* TODO */
> }
> +
> +void ptrace_triggered(struct perf_event *bp,
> + struct perf_sample_data *data, struct pt_regs *regs)
> +{
> + struct perf_event_attr attr;
> +
> + /*
> + * Disable the breakpoint request here since ptrace has defined a
> + * one-shot behaviour for breakpoint exceptions in PPC64.
> + * The SIGTRAP signal is generated automatically for us in do_dabr().
> + * We don't have to do anything about that here
> + */
> + attr = bp->attr;
> + attr.disabled = true;
> + modify_user_hw_breakpoint(bp, &attr);
> +}
> diff --git a/arch/powerpc/kernel/ptrace/ptrace.c b/arch/powerpc/kernel/ptrace/ptrace.c
> index a44f6e5e05ff..f6e51be47c6e 100644
> --- a/arch/powerpc/kernel/ptrace/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace/ptrace.c
> @@ -18,7 +18,6 @@
> #include <linux/regset.h>
> #include <linux/tracehook.h>
> #include <linux/audit.h>
> -#include <linux/hw_breakpoint.h>
> #include <linux/context_tracking.h>
> #include <linux/syscalls.h>
>
> @@ -31,24 +30,6 @@
>
> #include "ptrace-decl.h"
>
> -#ifdef CONFIG_HAVE_HW_BREAKPOINT
> -void ptrace_triggered(struct perf_event *bp,
> - struct perf_sample_data *data, struct pt_regs *regs)
> -{
> - struct perf_event_attr attr;
> -
> - /*
> - * Disable the breakpoint request here since ptrace has defined a
> - * one-shot behaviour for breakpoint exceptions in PPC64.
> - * The SIGTRAP signal is generated automatically for us in do_dabr().
> - * We don't have to do anything about that here
> - */
> - attr = bp->attr;
> - attr.disabled = true;
> - modify_user_hw_breakpoint(bp, &attr);
> -}
> -#endif /* CONFIG_HAVE_HW_BREAKPOINT */
> -
> /*
> * Called by kernel/ptrace.c when detaching..
> *
>
^ permalink raw reply
* Re: [PATCH] selftests/vm: Fix map_hugetlb length used for testing read and write
From: Christophe Leroy @ 2020-02-27 17:03 UTC (permalink / raw)
To: Shuah Khan; +Cc: linuxppc-dev, linux-kernel, linux-kselftest, linux-mm
In-Reply-To: <9a404a13c871c4bd0ba9ede68f69a1225180dd7e.1580978385.git.christophe.leroy@c-s.fr>
Shuah,
Le 06/02/2020 à 09:42, Christophe Leroy a écrit :
> Commit fa7b9a805c79 ("tools/selftest/vm: allow choosing mem size and
> page size in map_hugetlb") added the possibility to change the size
> of memory mapped for the test, but left the read and write test using
> the default value. This is unnoticed when mapping a length greater
> than the default one, but segfaults otherwise.
>
> Fix read_bytes() and write_bytes() by giving them the real length.
>
> Also fix the call to munmap().
>
> Fixes: fa7b9a805c79 ("tools/selftest/vm: allow choosing mem size and page size in map_hugetlb")
> Cc: stable@vger.kernel.org
Can you also consider this one for next rc ?
Thanks
Christophe
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> tools/testing/selftests/vm/map_hugetlb.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/tools/testing/selftests/vm/map_hugetlb.c b/tools/testing/selftests/vm/map_hugetlb.c
> index 5a2d7b8efc40..6af951900aa3 100644
> --- a/tools/testing/selftests/vm/map_hugetlb.c
> +++ b/tools/testing/selftests/vm/map_hugetlb.c
> @@ -45,20 +45,20 @@ static void check_bytes(char *addr)
> printf("First hex is %x\n", *((unsigned int *)addr));
> }
>
> -static void write_bytes(char *addr)
> +static void write_bytes(char *addr, size_t length)
> {
> unsigned long i;
>
> - for (i = 0; i < LENGTH; i++)
> + for (i = 0; i < length; i++)
> *(addr + i) = (char)i;
> }
>
> -static int read_bytes(char *addr)
> +static int read_bytes(char *addr, size_t length)
> {
> unsigned long i;
>
> check_bytes(addr);
> - for (i = 0; i < LENGTH; i++)
> + for (i = 0; i < length; i++)
> if (*(addr + i) != (char)i) {
> printf("Mismatch at %lu\n", i);
> return 1;
> @@ -96,11 +96,11 @@ int main(int argc, char **argv)
>
> printf("Returned address is %p\n", addr);
> check_bytes(addr);
> - write_bytes(addr);
> - ret = read_bytes(addr);
> + write_bytes(addr, length);
> + ret = read_bytes(addr, length);
>
> /* munmap() length of MAP_HUGETLB memory must be hugepage aligned */
> - if (munmap(addr, LENGTH)) {
> + if (munmap(addr, length)) {
> perror("munmap");
> exit(1);
> }
>
^ permalink raw reply
* Re: [PATCH v3 15/27] powerpc/powernv/pmem: Add support for near storage commands
From: Dan Williams @ 2020-02-27 17:02 UTC (permalink / raw)
To: Alastair D'Silva
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Mahesh Salgaonkar,
Masahiro Yamada, Oliver O'Halloran, Mauro Carvalho Chehab,
Ira Weiny, Rob Herring, Dave Jiang, linux-nvdimm,
Aneesh Kumar K . V, Krzysztof Kozlowski, Anju T Sudhakar,
alastair, Andrew Donnellan, Arnd Bergmann, Greg Kurz,
Nicholas Piggin, Cédric Le Goater, Thomas Gleixner,
Hari Bathini, Linux MM, Greg Kroah-Hartman,
Linux Kernel Mailing List, Vishal Verma, Frederic Barrat,
Paul Mackerras, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20200221032720.33893-16-alastair@au1.ibm.com>
On Thu, Feb 20, 2020 at 7:28 PM Alastair D'Silva <alastair@au1.ibm.com> wrote:
>
> From: Alastair D'Silva <alastair@d-silva.org>
>
> Similar to the previous patch, this adds support for near storage commands.
Similar comment as the last patch. This changelog does not give the
reviewer any frame of reference to review the patch.
^ permalink raw reply
* Re: [PATCH v3 14/27] powerpc/powernv/pmem: Add support for Admin commands
From: Dan Williams @ 2020-02-27 17:01 UTC (permalink / raw)
To: Alastair D'Silva
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Mahesh Salgaonkar,
Masahiro Yamada, Oliver O'Halloran, Mauro Carvalho Chehab,
Ira Weiny, Rob Herring, Dave Jiang, linux-nvdimm,
Aneesh Kumar K . V, Krzysztof Kozlowski, Anju T Sudhakar,
alastair, Andrew Donnellan, Arnd Bergmann, Greg Kurz,
Nicholas Piggin, Cédric Le Goater, Thomas Gleixner,
Hari Bathini, Linux MM, Greg Kroah-Hartman,
Linux Kernel Mailing List, Vishal Verma, Frederic Barrat,
Paul Mackerras, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20200221032720.33893-15-alastair@au1.ibm.com>
On Thu, Feb 20, 2020 at 7:28 PM Alastair D'Silva <alastair@au1.ibm.com> wrote:
>
> From: Alastair D'Silva <alastair@d-silva.org>
>
> This patch requests the metadata required to issue admin commands, as well
> as some helper functions to construct and check the completion of the
> commands.
What are the admin commands? Any pointer to a spec? Why does Linux
need to support these commands?
^ permalink raw reply
* Re: [PATCH] selftest/lkdtm: Use local .gitignore
From: Christophe Leroy @ 2020-02-27 16:53 UTC (permalink / raw)
To: Shuah Khan, Kees Cook
Cc: linux-mm, linuxppc-dev, linux-kernel, linux-kselftest
In-Reply-To: <f804d7bc-134d-27f8-daa5-6bbf9f28b54d@linuxfoundation.org>
Le 27/02/2020 à 17:45, Shuah Khan a écrit :
> On 2/27/20 9:17 AM, Kees Cook wrote:
>> On Thu, Feb 27, 2020 at 02:07:10PM +0000, Christophe Leroy wrote:
>>> Commit 68ca0fd272da ("selftest/lkdtm: Don't pollute 'git status'")
>>> introduced patterns for git to ignore files generated in
>>> tools/testing/selftests/lkdtm/
>>>
>>> Use local .gitignore file instead of using the root one.
>>>
>>> Fixes: 68ca0fd272da ("selftest/lkdtm: Don't pollute 'git status'")
>>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>>
>> Yeah, that's better. Thanks!
>>
>> Acked-by: Kees Cook <keescook@chromium.org>
>>
>
> I will apply it for next rc.
>
> Thanks. I should have noticed the problem in the previous version.
> It slipped by me. :(
>
My fault, I didn't even know we could have .gitignore in subdirectories.
Christophe
^ permalink raw reply
* Re: [PATCH] selftest/lkdtm: Use local .gitignore
From: Shuah Khan @ 2020-02-27 16:45 UTC (permalink / raw)
To: Kees Cook, Christophe Leroy
Cc: linux-mm, linuxppc-dev, linux-kernel, linux-kselftest, Shuah Khan
In-Reply-To: <202002270817.1C32C98@keescook>
On 2/27/20 9:17 AM, Kees Cook wrote:
> On Thu, Feb 27, 2020 at 02:07:10PM +0000, Christophe Leroy wrote:
>> Commit 68ca0fd272da ("selftest/lkdtm: Don't pollute 'git status'")
>> introduced patterns for git to ignore files generated in
>> tools/testing/selftests/lkdtm/
>>
>> Use local .gitignore file instead of using the root one.
>>
>> Fixes: 68ca0fd272da ("selftest/lkdtm: Don't pollute 'git status'")
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>
> Yeah, that's better. Thanks!
>
> Acked-by: Kees Cook <keescook@chromium.org>
>
I will apply it for next rc.
Thanks. I should have noticed the problem in the previous version.
It slipped by me. :(
thanks,
-- Shuah
^ 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