* Re: [RFC PATCH 0/7] Memory hotplug/hotremove at subsection size
From: David Hildenbrand @ 2021-05-12 16:14 UTC (permalink / raw)
To: Zi Yan, Michal Hocko
Cc: linux-ia64, Wei Yang, Anshuman Khandual, Rafael J . Wysocki, x86,
Dan Williams, linux-kernel, linux-mm, Andy Lutomirski,
Thomas Gleixner, linuxppc-dev, Andrew Morton, Mike Rapoport,
Oscar Salvador
In-Reply-To: <746780E5-0288-494D-8B19-538049F1B891@nvidia.com>
>>
>> As stated somewhere here already, we'll have to look into making alloc_contig_range() (and main users CMA and virtio-mem) independent of MAX_ORDER and mainly rely on pageblock_order. The current handling in alloc_contig_range() is far from optimal as we have to isolate a whole MAX_ORDER - 1 page -- and on ZONE_NORMAL we'll fail easily if any part contains something unmovable although we don't even want to allocate that part. I actually have that on my list (to be able to fully support pageblock_order instead of MAX_ORDER -1 chunks in virtio-mem), however didn't have time to look into it.
>
> So in your mind, for gigantic page allocation (> MAX_ORDER), alloc_contig_range()
> should be used instead of buddy allocator while pageblock_order is kept at a small
> granularity like 2MB. Is that the case? Isn’t it going to have high fail rate
> when any of the pageblocks within a gigantic page range (like 1GB) becomes unmovable?
> Are you thinking additional mechanism/policy to prevent such thing happening as
> an additional step for gigantic page allocation? Like your ZONE_PREFER_MOVABLE idea?
>
I am not fully sure yet where the journey will go , I guess nobody
knows. Ultimately, having buddy support for >= current MAX_ORDER (IOW,
increasing MAX_ORDER) will most probably happen, so it would be worth
investigating what has to be done to get that running as a first step.
Of course, we could temporarily think about wiring it up in the buddy like
if (order < MAX_ORDER)
__alloc_pages()...
else
alloc_contig_pages()
but it doesn't really improve the situation IMHO, just an API change.
So I think we should look into increasing MAX_ORDER, seeing what needs
to be done to have that part running while keeping the section size and
the pageblock order as is. I know that at least memory
onlining/offlining, cma, alloc_contig_range(), ... needs tweaking,
especially when we don't increase the section size (but also if we would
due to the way page isolation is currently handled). Having a MAX_ORDER
-1 page being partially in different nodes might be another thing to
look into (I heard that it can already happen right now, but I don't
remember the details).
The next step after that would then be better fragmentation avoidance
for larger granularity like 1G THP.
>>
>> Further, page onlining / offlining code and early init code most probably also needs care if MAX_ORDER - 1 crosses sections. Memory holes we might suddenly have in MAX_ORDER - 1 pages might become a problem and will have to be handled. Not sure which other code has to be tweaked (compaction? page isolation?).
>
> Can you elaborate it a little more? From what I understand, memory holes mean valid
> PFNs are not contiguous before and after a hole, so pfn++ will not work, but
> struct pages are still virtually contiguous assuming SPARSE_VMEMMAP, meaning page++
> would still work. So when MAX_ORDER - 1 crosses sections, additional code would be
> needed instead of simple pfn++. Is there anything I am missing?
I think there are two cases when talking about MAX_ORDER and memory holes:
1. Hole with a valid memmap: the memmap is initialize to PageReserved()
and the pages are not given to the buddy. pfn_valid() and
pfn_to_page() works as expected.
2. Hole without a valid memmam: we have that CONFIG_HOLES_IN_ZONE thing
already, see include/linux/mmzone.h. pfn_valid_within() checks are
required. Doesn't win a beauty contest, but gets the job done in
existing setups that seem to care.
"If it is possible to have holes within a MAX_ORDER_NR_PAGES, then we
need to check pfn validity within that MAX_ORDER_NR_PAGES block.
pfn_valid_within() should be used in this case; we optimise this away
when we have no holes within a MAX_ORDER_NR_PAGES block."
CONFIG_HOLES_IN_ZONE is just a bad name for this.
(increasing the section size implies that we waste more memory for the
memmap in holes. increasing MAX_ORDER means that we might have to deal
with holes within MAX_ORDER chunks)
We don't have too many pfn_valid_within() checks. I wonder if we could
add something that is optimized for "holes are a power of two and
properly aligned", because pfn_valid_within() right not deals with holes
of any kind which makes it somewhat inefficient IIRC.
>
> BTW, to test a system with memory holes, do you know is there an easy of adding
> random memory holes to an x86_64 VM, which can help reveal potential missing pieces
> in the code? Changing BIOS-e820 table might be one way, but I have no idea on
> how to do it on QEMU.
It might not be very easy that way. But I heard that some arm64 systems
have crazy memory layouts -- maybe there, it's easier to get something
nasty running? :)
https://lkml.kernel.org/r/YJpEwF2cGjS5mKma@kernel.org
I remember there was a way to define the e820 completely on kernel
cmdline, but I might be wrong ...
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: [PATCH] lockdown,selinux: fix bogus SELinux lockdown permission checks
From: Casey Schaufler @ 2021-05-12 16:17 UTC (permalink / raw)
To: Ondrej Mosnacek
Cc: SElinux list, network dev, Stephen Smalley, James Morris,
Steven Rostedt, Linux kernel mailing list, Casey Schaufler,
Linux Security Module list, Ingo Molnar, Linux FS Devel, bpf,
linuxppc-dev
In-Reply-To: <CAFqZXNtr1YjzRg7fTm+j=0oZF+7C5xEu5J0mCZynP-dgEzvyUg@mail.gmail.com>
On 5/12/2021 6:21 AM, Ondrej Mosnacek wrote:
> On Sat, May 8, 2021 at 12:17 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> On 5/7/2021 4:40 AM, Ondrej Mosnacek wrote:
>>> Commit 59438b46471a ("security,lockdown,selinux: implement SELinux
>>> lockdown") added an implementation of the locked_down LSM hook to
>>> SELinux, with the aim to restrict which domains are allowed to perform
>>> operations that would breach lockdown.
>>>
>>> However, in several places the security_locked_down() hook is called in
>>> situations where the current task isn't doing any action that would
>>> directly breach lockdown, leading to SELinux checks that are basically
>>> bogus.
>>>
>>> Since in most of these situations converting the callers such that
>>> security_locked_down() is called in a context where the current task
>>> would be meaningful for SELinux is impossible or very non-trivial (and
>>> could lead to TOCTOU issues for the classic Lockdown LSM
>>> implementation), fix this by adding a separate hook
>>> security_locked_down_globally()
>> This is a poor solution to the stated problem. Rather than adding
>> a new hook you should add the task as a parameter to the existing hook
>> and let the security modules do as they will based on its value.
>> If the caller does not have an appropriate task it should pass NULL.
>> The lockdown LSM can ignore the task value and SELinux can make its
>> own decision based on the task value passed.
> The problem with that approach is that all callers would then need to
> be updated and I intended to keep the patch small as I'd like it to go
> to stable kernels as well.
>
> But it does seem to be a better long-term solution - would it work for
> you (and whichever maintainer would be taking the patch(es)) if I just
> added another patch that refactors it to use the task parameter?
I can't figure out what you're suggesting. Are you saying that you
want to add a new hook *and* add the task parameter?
>
> --
> Ondrej Mosnacek
> Software Engineer, Linux Security - SELinux kernel
> Red Hat, Inc.
>
^ permalink raw reply
* [RFC 1/4] drivers/nvdimm: Add perf interface to expose nvdimm performance stats
From: Kajol Jain @ 2021-05-12 16:38 UTC (permalink / raw)
To: mpe, linuxppc-dev, linux-nvdimm, linux-kernel
Cc: santosh, maddy, peterz, aneesh.kumar, atrajeev, kjain, vaibhav,
dan.j.williams, ira.weiny, tglx
In-Reply-To: <20210512163824.255370-1-kjain@linux.ibm.com>
Patch adds performance stats reporting support for nvdimm.
Added interface includes support for a pmu register function and
callbacks to be used by the arch/platform specific drivers.
User could use the standard perf tool to access perf events exposed
via pmu.
A structure is added called nvdimm_pmu which can be used to add
platform specific data like supported events and callbacks to pmu
functions like event_init/add/delete/read. It also adds
unregister_nvdimm_pmu function to handle unregistering of a pmu device.
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
drivers/nvdimm/Makefile | 1 +
drivers/nvdimm/nd_perf.c | 111 +++++++++++++++++++++++++++++++++++++++
include/linux/nd.h | 31 +++++++++++
3 files changed, 143 insertions(+)
create mode 100644 drivers/nvdimm/nd_perf.c
diff --git a/drivers/nvdimm/Makefile b/drivers/nvdimm/Makefile
index 29203f3d3069..25dba6095612 100644
--- a/drivers/nvdimm/Makefile
+++ b/drivers/nvdimm/Makefile
@@ -18,6 +18,7 @@ nd_e820-y := e820.o
libnvdimm-y := core.o
libnvdimm-y += bus.o
libnvdimm-y += dimm_devs.o
+libnvdimm-y += nd_perf.o
libnvdimm-y += dimm.o
libnvdimm-y += region_devs.o
libnvdimm-y += region.o
diff --git a/drivers/nvdimm/nd_perf.c b/drivers/nvdimm/nd_perf.c
new file mode 100644
index 000000000000..d28bec2b61a2
--- /dev/null
+++ b/drivers/nvdimm/nd_perf.c
@@ -0,0 +1,111 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * nd_perf.c: NVDIMM Device Performance Monitoring Unit support
+ *
+ * Perf interface to expose nvdimm performance stats.
+ *
+ * Copyright (C) 2021 IBM Corporation
+ */
+
+#define pr_fmt(fmt) "nvdimm_pmu: " fmt
+
+#include <linux/nd.h>
+
+#define to_nvdimm_pmu(_pmu) container_of(_pmu, struct nvdimm_pmu, pmu)
+
+static int nvdimm_pmu_event_init(struct perf_event *event)
+{
+ struct nvdimm_pmu *nd_pmu = to_nvdimm_pmu(event->pmu);
+
+ /* test the event attr type for PMU enumeration */
+ if (event->attr.type != event->pmu->type)
+ return -ENOENT;
+
+ /* it does not support event sampling mode */
+ if (is_sampling_event(event))
+ return -EINVAL;
+
+ /* no branch sampling */
+ if (has_branch_stack(event))
+ return -EOPNOTSUPP;
+
+ /* jump to arch/platform specific callbacks if any */
+ if (nd_pmu && nd_pmu->event_init)
+ return nd_pmu->event_init(event, nd_pmu->dev);
+
+ return 0;
+}
+
+static void nvdimm_pmu_read(struct perf_event *event)
+{
+ struct nvdimm_pmu *nd_pmu = to_nvdimm_pmu(event->pmu);
+
+ /* jump to arch/platform specific callbacks if any */
+ if (nd_pmu && nd_pmu->read)
+ nd_pmu->read(event, nd_pmu->dev);
+}
+
+static void nvdimm_pmu_del(struct perf_event *event, int flags)
+{
+ struct nvdimm_pmu *nd_pmu = to_nvdimm_pmu(event->pmu);
+
+ /* jump to arch/platform specific callbacks if any */
+ if (nd_pmu && nd_pmu->del)
+ nd_pmu->del(event, flags, nd_pmu->dev);
+}
+
+static int nvdimm_pmu_add(struct perf_event *event, int flags)
+{
+ struct nvdimm_pmu *nd_pmu = to_nvdimm_pmu(event->pmu);
+
+ if (flags & PERF_EF_START)
+ /* jump to arch/platform specific callbacks if any */
+ if (nd_pmu && nd_pmu->add)
+ return nd_pmu->add(event, flags, nd_pmu->dev);
+ return 0;
+}
+
+int register_nvdimm_pmu(struct nvdimm_pmu *nd_pmu, struct platform_device *pdev)
+{
+ int rc;
+
+ if (!nd_pmu || !pdev)
+ return -EINVAL;
+
+ nd_pmu->pmu.task_ctx_nr = perf_invalid_context;
+ nd_pmu->pmu.event_init = nvdimm_pmu_event_init;
+ nd_pmu->pmu.add = nvdimm_pmu_add;
+ nd_pmu->pmu.del = nvdimm_pmu_del;
+ nd_pmu->pmu.read = nvdimm_pmu_read;
+ nd_pmu->pmu.name = nd_pmu->name;
+ nd_pmu->pmu.attr_groups = nd_pmu->attr_groups;
+ nd_pmu->pmu.capabilities = PERF_PMU_CAP_NO_INTERRUPT |
+ PERF_PMU_CAP_NO_EXCLUDE;
+
+ /*
+ * Adding platform_device->dev pointer to nvdimm_pmu, so that we can
+ * access that device data in PMU callbacks and also pass it to
+ * arch/platform specific code.
+ */
+ nd_pmu->dev = &pdev->dev;
+
+ rc = perf_pmu_register(&nd_pmu->pmu, nd_pmu->name, -1);
+ if (rc)
+ return rc;
+
+ pr_info("%s NVDIMM performance monitor support registered\n",
+ nd_pmu->name);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(register_nvdimm_pmu);
+
+void unregister_nvdimm_pmu(struct pmu *nd_pmu)
+{
+ /*
+ * nd_pmu will get free in arch/platform specific code once
+ * corresponding pmu get unregistered.
+ */
+ perf_pmu_unregister(nd_pmu);
+}
+EXPORT_SYMBOL_GPL(unregister_nvdimm_pmu);
diff --git a/include/linux/nd.h b/include/linux/nd.h
index ee9ad76afbba..fa6e60b2b368 100644
--- a/include/linux/nd.h
+++ b/include/linux/nd.h
@@ -8,6 +8,8 @@
#include <linux/ndctl.h>
#include <linux/device.h>
#include <linux/badblocks.h>
+#include <linux/platform_device.h>
+#include <linux/perf_event.h>
enum nvdimm_event {
NVDIMM_REVALIDATE_POISON,
@@ -23,6 +25,35 @@ enum nvdimm_claim_class {
NVDIMM_CCLASS_UNKNOWN,
};
+/**
+ * struct nvdimm_pmu - data structure for nvdimm perf driver
+ *
+ * @name: name of the nvdimm pmu device.
+ * @pmu: pmu data structure for nvdimm performance stats.
+ * @cpu: designated cpu for counter access.
+ * @dev: nvdimm device pointer.
+ * @functions(event_init/add/del/read): platform specific callbacks.
+ * @attr_groups: data structure for events/formats/cpumask.
+ * @node: node for cpu hotplug notifier link.
+ * @cpuhp_state: state for cpu hotplug notification.
+ */
+struct nvdimm_pmu {
+ const char *name;
+ struct pmu pmu;
+ int cpu;
+ struct device *dev;
+ int (*event_init)(struct perf_event *event, struct device *dev);
+ int (*add)(struct perf_event *event, int flags, struct device *dev);
+ void (*del)(struct perf_event *event, int flags, struct device *dev);
+ void (*read)(struct perf_event *event, struct device *dev);
+ const struct attribute_group **attr_groups;
+ struct hlist_node node;
+ enum cpuhp_state cpuhp_state;
+};
+
+int register_nvdimm_pmu(struct nvdimm_pmu *nvdimm, struct platform_device *pdev);
+void unregister_nvdimm_pmu(struct pmu *pmu);
+
struct nd_device_driver {
struct device_driver drv;
unsigned long type;
--
2.27.0
^ permalink raw reply related
* [RFC 0/4] Add perf interface to expose nvdimm performance stats
From: Kajol Jain @ 2021-05-12 16:38 UTC (permalink / raw)
To: mpe, linuxppc-dev, linux-nvdimm, linux-kernel
Cc: santosh, maddy, peterz, aneesh.kumar, atrajeev, kjain, vaibhav,
dan.j.williams, ira.weiny, tglx
Patchset adds performance stats reporting support for nvdimm.
Added interface includes support for a pmu register function and
callbacks to be used by arch/platform specific drivers.
User could use standard perf tool to access perf events exposed via pmu.
Patchset adds a structure called nvdimm_pmu which can
be used to add platform specific data like supported event list and
callbacks to pmu functions like event_init/add/delete/read.
Patchset includes an implements the to expose IBM pseries platform nmem*
device performance stats using this interface.
Result from power9 pseries lpar with 2 nvdimm device:
command:# perf list nmem
nmem0/cchrhcnt/ [Kernel PMU event]
nmem0/cchwhcnt/ [Kernel PMU event]
nmem0/critrscu/ [Kernel PMU event]
nmem0/ctlresct/ [Kernel PMU event]
nmem0/ctlrestm/ [Kernel PMU event]
nmem0/fastwcnt/ [Kernel PMU event]
nmem0/hostlcnt/ [Kernel PMU event]
nmem0/hostldur/ [Kernel PMU event]
nmem0/hostscnt/ [Kernel PMU event]
nmem0/hostsdur/ [Kernel PMU event]
nmem0/medrcnt/ [Kernel PMU event]
nmem0/medrdur/ [Kernel PMU event]
nmem0/medwcnt/ [Kernel PMU event]
nmem0/medwdur/ [Kernel PMU event]
nmem0/memlife/ [Kernel PMU event]
nmem0/noopstat/ [Kernel PMU event]
nmem0/ponsecs/ [Kernel PMU event]
nmem1/cchrhcnt/ [Kernel PMU event]
nmem1/cchwhcnt/ [Kernel PMU event]
nmem1/critrscu/ [Kernel PMU event]
...
nmem1/noopstat/ [Kernel PMU event]
nmem1/ponsecs/ [Kernel PMU event]
Patch1:
Introduces the nvdimm_pmu structure, common function for pmu
register along with callback routine check.
Pacth2
Add code in arch/powerpc/platform/pseries/papr_scm.c to expose
nmem* pmu. It fills in the nvdimm_pmu structure with event attrs
and event callback functions and then registers the pmu by adding
callback to register_nvdimm_pmu.
Patch3:
Sysfs documentation patch
Patch4:
Adds cpuhotplug support.
Kajol Jain (4):
drivers/nvdimm: Add perf interface to expose nvdimm performance stats
powerpc/papr_scm: Add perf interface support
powerpc/papr_scm: Document papr_scm sysfs event format entries
powerpc/papr_scm: Add cpu hotplug support for nvdimm pmu device
Documentation/ABI/testing/sysfs-bus-papr-pmem | 31 ++
arch/powerpc/include/asm/device.h | 5 +
arch/powerpc/platforms/pseries/papr_scm.c | 346 +++++++++++++++++-
drivers/nvdimm/Makefile | 1 +
drivers/nvdimm/nd_perf.c | 111 ++++++
include/linux/nd.h | 31 ++
6 files changed, 524 insertions(+), 1 deletion(-)
create mode 100644 drivers/nvdimm/nd_perf.c
--
2.27.0
^ permalink raw reply
* [RFC 2/4] powerpc/papr_scm: Add perf interface support
From: Kajol Jain @ 2021-05-12 16:38 UTC (permalink / raw)
To: mpe, linuxppc-dev, linux-nvdimm, linux-kernel
Cc: santosh, maddy, peterz, aneesh.kumar, atrajeev, kjain, vaibhav,
dan.j.williams, ira.weiny, tglx
In-Reply-To: <20210512163824.255370-1-kjain@linux.ibm.com>
This patch adds support for performance monitoring of papr
nvdimm devices via perf interface. It adds callbacks functions
like add/del/read/event_init for nvdimm_pmu structure.
Patch adds a new parameter 'priv' in pdev_archdata structure to save
nvdimm_pmu device pointer, to handle the unregistering of pmu device.
papr_scm_pmu_register function populates the nvdimm_pmu structure
with events, attribute groups along with event handling functions.
Event handling functions internally uses hcall to get events and
counter data. Finally the populated nvdimm_pmu structure is passed
to register the pmu device.
Result in power9 machine with 2 nvdimm device:
Ex: List all event by perf list
command:# perf list nmem
nmem0/cchrhcnt/ [Kernel PMU event]
nmem0/cchwhcnt/ [Kernel PMU event]
nmem0/critrscu/ [Kernel PMU event]
nmem0/ctlresct/ [Kernel PMU event]
nmem0/ctlrestm/ [Kernel PMU event]
nmem0/fastwcnt/ [Kernel PMU event]
nmem0/hostlcnt/ [Kernel PMU event]
nmem0/hostldur/ [Kernel PMU event]
nmem0/hostscnt/ [Kernel PMU event]
nmem0/hostsdur/ [Kernel PMU event]
nmem0/medrcnt/ [Kernel PMU event]
nmem0/medrdur/ [Kernel PMU event]
nmem0/medwcnt/ [Kernel PMU event]
nmem0/medwdur/ [Kernel PMU event]
nmem0/memlife/ [Kernel PMU event]
nmem0/noopstat/ [Kernel PMU event]
nmem0/ponsecs/ [Kernel PMU event]
nmem1/cchrhcnt/ [Kernel PMU event]
nmem1/cchwhcnt/ [Kernel PMU event]
nmem1/critrscu/ [Kernel PMU event]
...
nmem1/noopstat/ [Kernel PMU event]
nmem1/ponsecs/ [Kernel PMU event]
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
arch/powerpc/include/asm/device.h | 5 +
arch/powerpc/platforms/pseries/papr_scm.c | 284 +++++++++++++++++++++-
2 files changed, 288 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/device.h b/arch/powerpc/include/asm/device.h
index 219559d65864..47ed639f3b8f 100644
--- a/arch/powerpc/include/asm/device.h
+++ b/arch/powerpc/include/asm/device.h
@@ -48,6 +48,11 @@ struct dev_archdata {
struct pdev_archdata {
u64 dma_mask;
+ /*
+ * Pointer to nvdimm_pmu structure, to handle the unregistering
+ * of pmu device
+ */
+ void *priv;
};
#endif /* _ASM_POWERPC_DEVICE_H */
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index ef26fe40efb0..997d379094d0 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -18,6 +18,8 @@
#include <asm/plpar_wrappers.h>
#include <asm/papr_pdsm.h>
#include <asm/mce.h>
+#include <linux/perf_event.h>
+#include <linux/ctype.h>
#define BIND_ANY_ADDR (~0ul)
@@ -116,6 +118,9 @@ struct papr_scm_priv {
/* length of the stat buffer as expected by phyp */
size_t stat_buffer_len;
+
+ /* array to have event_code and stat_id mappings */
+ char **nvdimm_events_map;
};
static int papr_scm_pmem_flush(struct nd_region *nd_region,
@@ -329,6 +334,271 @@ static ssize_t drc_pmem_query_stats(struct papr_scm_priv *p,
return 0;
}
+static struct attribute_group nvdimm_pmu_events_group = {
+ .name = "events",
+ /* .attrs is set in papr_scm_pmu_check_events function */
+};
+
+PMU_FORMAT_ATTR(event, "config:0-37");
+
+static struct attribute *nvdimm_pmu_format_attr[] = {
+ &format_attr_event.attr,
+ NULL,
+};
+
+static struct attribute_group nvdimm_pmu_format_group = {
+ .name = "format",
+ .attrs = nvdimm_pmu_format_attr,
+};
+
+static const struct attribute_group *nvdimm_pmu_attr_groups[] = {
+ &nvdimm_pmu_format_group,
+ &nvdimm_pmu_events_group,
+ NULL,
+};
+
+static void papr_scm_pmu_get_value(struct perf_event *event, struct device *dev, u64 *count)
+{
+ struct papr_scm_perf_stat *stat;
+ struct papr_scm_perf_stats *stats;
+ struct papr_scm_priv *p = (struct papr_scm_priv *)dev->driver_data;
+ int rc, size;
+ u64 statval;
+
+ /* Allocate buffer to hold single performance stat */
+ size = sizeof(struct papr_scm_perf_stats) +
+ sizeof(struct papr_scm_perf_stat);
+
+ if (!p->nvdimm_events_map)
+ return;
+
+ stats = kzalloc(size, GFP_KERNEL);
+ if (!stats)
+ return;
+
+ stat = &stats->scm_statistic[0];
+ memcpy(&stat->stat_id,
+ p->nvdimm_events_map[event->attr.config - 1],
+ sizeof(stat->stat_id));
+ stat->stat_val = 0;
+
+ rc = drc_pmem_query_stats(p, stats, 1);
+ if (rc < 0) {
+ kfree(stats);
+ return;
+ }
+
+ statval = be64_to_cpu(stat->stat_val);
+ *count = statval;
+ kfree(stats);
+}
+
+static int papr_scm_pmu_add(struct perf_event *event, int flags, struct device *dev)
+{
+ u64 count = 0;
+
+ papr_scm_pmu_get_value(event, dev, &count);
+ local64_set(&event->hw.prev_count, count);
+ return 0;
+}
+
+static void papr_scm_pmu_read(struct perf_event *event, struct device *dev)
+{
+ u64 prev, now = 0;
+
+ papr_scm_pmu_get_value(event, dev, &now);
+ prev = local64_xchg(&event->hw.prev_count, now);
+
+ if (now - prev >= 0)
+ local64_add(now - prev, &event->count);
+}
+
+static void papr_scm_pmu_del(struct perf_event *event, int flags, struct device *dev)
+{
+ papr_scm_pmu_read(event, dev);
+}
+
+static void nvdimm_pmu_uinit(struct nvdimm_pmu *nd_pmu)
+{
+ unregister_nvdimm_pmu(&nd_pmu->pmu);
+ kfree(nd_pmu);
+}
+
+static int papr_scm_pmu_register(struct papr_scm_priv *p)
+{
+ struct nvdimm_pmu *papr_scm_pmu;
+ int rc;
+
+ papr_scm_pmu = devm_kzalloc(&p->pdev->dev, sizeof(*papr_scm_pmu), GFP_KERNEL);
+ if (!papr_scm_pmu)
+ return -ENOMEM;
+
+ papr_scm_pmu->name = nvdimm_name(p->nvdimm);
+ papr_scm_pmu->read = papr_scm_pmu_read;
+ papr_scm_pmu->add = papr_scm_pmu_add;
+ papr_scm_pmu->del = papr_scm_pmu_del;
+ papr_scm_pmu->attr_groups = nvdimm_pmu_attr_groups;
+
+ rc = register_nvdimm_pmu(papr_scm_pmu, p->pdev);
+ if (rc)
+ goto pmu_register_err;
+
+ /*
+ * Set archdata.priv value to nvdimm_pmu structure, to handle the
+ * unregistering of pmu device.
+ */
+ p->pdev->archdata.priv = papr_scm_pmu;
+ return 0;
+
+pmu_register_err:
+ kfree(papr_scm_pmu);
+ return rc;
+}
+
+static ssize_t device_show_string(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct perf_pmu_events_attr *d;
+
+ d = container_of(attr, struct perf_pmu_events_attr, attr);
+
+ return sysfs_emit(buf, "%s\n", (char *)d->event_str);
+}
+
+static char *strtolower(char *updated_name)
+{
+ int i = 0;
+
+ while (updated_name[i]) {
+ if (isupper(updated_name[i]))
+ updated_name[i] = tolower(updated_name[i]);
+ i++;
+ }
+ updated_name[i] = '\0';
+ return strim(updated_name);
+}
+
+/* device_str_attr_create : Populate event "name" and string "str" in attribute */
+static struct attribute *device_str_attr_create_(char *name, char *str)
+{
+ struct perf_pmu_events_attr *attr;
+
+ attr = kzalloc(sizeof(*attr), GFP_KERNEL);
+
+ if (!attr)
+ return NULL;
+
+ sysfs_attr_init(&attr->attr.attr);
+ attr->event_str = str;
+ attr->attr.attr.name = strtolower(name);
+ attr->attr.attr.mode = 0444;
+ attr->attr.show = device_show_string;
+
+ return &attr->attr.attr;
+}
+
+static int papr_scm_pmu_check_events(struct papr_scm_priv *p)
+{
+ struct papr_scm_perf_stat *stat;
+ struct papr_scm_perf_stats *stats, *single_stats;
+ int index, size, rc, attrs;
+ u32 total_events;
+ struct attribute **events;
+ char *eventcode, *eventname, *statid;
+
+ if (!p->stat_buffer_len)
+ return -ENOENT;
+
+ total_events = (p->stat_buffer_len - sizeof(struct papr_scm_perf_stats))
+ / sizeof(struct papr_scm_perf_stat);
+
+ /* Allocate the buffer for phyp where stats are written */
+ stats = kzalloc(p->stat_buffer_len, GFP_KERNEL);
+ if (!stats)
+ return -ENOMEM;
+
+ /* Allocate memory to nvdimm_event_map */
+ p->nvdimm_events_map = kcalloc(total_events, sizeof(char *), GFP_KERNEL);
+ if (!p->nvdimm_events_map) {
+ rc = -ENOMEM;
+ goto out_stats;
+ }
+
+ /* Called to get list of events supported */
+ rc = drc_pmem_query_stats(p, stats, 0);
+ if (rc)
+ goto out_nvdimm_events_map;
+
+ /* Allocate buffer to hold single performance stat */
+ size = sizeof(struct papr_scm_perf_stats) + sizeof(struct papr_scm_perf_stat);
+
+ single_stats = kzalloc(size, GFP_KERNEL);
+ if (!single_stats) {
+ rc = -ENOMEM;
+ goto out_nvdimm_events_map;
+ }
+
+ events = kzalloc(total_events * sizeof(struct attribute *), GFP_KERNEL);
+ if (!events) {
+ rc = -ENOMEM;
+ goto out_single_stats;
+ }
+
+ for (index = 0, stat = stats->scm_statistic, attrs = 0;
+ index < total_events; index++, ++stat) {
+
+ single_stats->scm_statistic[0] = *stat;
+ rc = drc_pmem_query_stats(p, single_stats, 1);
+
+ if (rc < 0) {
+ pr_info("Event not supported %s for device %s\n",
+ stat->stat_id, nvdimm_name(p->nvdimm));
+ } else {
+ eventcode = kasprintf(GFP_KERNEL, "event=0x%x", attrs + 1);
+ eventname = kzalloc(strlen(stat->stat_id) + 1, GFP_KERNEL);
+ statid = kzalloc(strlen(stat->stat_id) + 1, GFP_KERNEL);
+
+ if (!eventname || !statid || !eventcode)
+ goto out;
+
+ strcpy(eventname, stat->stat_id);
+ events[attrs] = device_str_attr_create_(eventname,
+ eventcode);
+ if (!events[attrs])
+ goto out;
+
+ strcpy(statid, stat->stat_id);
+ p->nvdimm_events_map[attrs] = statid;
+ attrs++;
+ continue;
+out:
+ kfree(eventcode);
+ kfree(eventname);
+ kfree(statid);
+ }
+ }
+ events[attrs] = NULL;
+ p->nvdimm_events_map[attrs] = NULL;
+
+ if (!attrs)
+ goto out_events;
+
+ nvdimm_pmu_events_group.attrs = events;
+ kfree(single_stats);
+ kfree(stats);
+ return 0;
+
+out_events:
+ kfree(events);
+out_single_stats:
+ kfree(single_stats);
+out_nvdimm_events_map:
+ kfree(p->nvdimm_events_map);
+out_stats:
+ kfree(stats);
+ return rc;
+}
+
/*
* Issue hcall to retrieve dimm health info and populate papr_scm_priv with the
* health information.
@@ -923,7 +1193,7 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
struct nd_mapping_desc mapping;
struct nd_region_desc ndr_desc;
unsigned long dimm_flags;
- int target_nid, online_nid;
+ int target_nid, online_nid, rc;
ssize_t stat_size;
p->bus_desc.ndctl = papr_scm_ndctl;
@@ -1015,6 +1285,15 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
p->stat_buffer_len = stat_size;
dev_dbg(&p->pdev->dev, "Max perf-stat size %lu-bytes\n",
p->stat_buffer_len);
+
+ rc = papr_scm_pmu_check_events(p);
+ if (rc) {
+ dev_info(&p->pdev->dev, "nvdimm pmu check events failed, rc=%d\n", rc);
+ } else {
+ rc = papr_scm_pmu_register(p);
+ if (rc)
+ dev_info(&p->pdev->dev, "nvdimm pmu didn't register rc=%d\n", rc);
+ }
} else {
dev_info(&p->pdev->dev, "Dimm performance stats unavailable\n");
}
@@ -1195,7 +1474,10 @@ static int papr_scm_remove(struct platform_device *pdev)
nvdimm_bus_unregister(p->bus);
drc_pmem_unbind(p);
+ nvdimm_pmu_uinit(pdev->archdata.priv);
+ pdev->archdata.priv = NULL;
kfree(p->bus_desc.provider_name);
+ kfree(p->nvdimm_events_map);
kfree(p);
return 0;
--
2.27.0
^ permalink raw reply related
* [RFC 4/4] powerpc/papr_scm: Add cpu hotplug support for nvdimm pmu device
From: Kajol Jain @ 2021-05-12 16:38 UTC (permalink / raw)
To: mpe, linuxppc-dev, linux-nvdimm, linux-kernel
Cc: santosh, maddy, peterz, aneesh.kumar, atrajeev, kjain, vaibhav,
dan.j.williams, ira.weiny, tglx
In-Reply-To: <20210512163824.255370-1-kjain@linux.ibm.com>
Patch here adds cpu hotplug functions to nvdimm pmu.
It adds cpumask to designate a cpu to make HCALL to
collect the counter data for the nvdimm device and
update ABI documentation accordingly.
Result in power9 lpar system:
command:# cat /sys/devices/nmem0/cpumask
0
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
Documentation/ABI/testing/sysfs-bus-papr-pmem | 6 ++
arch/powerpc/platforms/pseries/papr_scm.c | 62 +++++++++++++++++++
2 files changed, 68 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-papr-pmem b/Documentation/ABI/testing/sysfs-bus-papr-pmem
index 216f70deca7e..a40fbec683a8 100644
--- a/Documentation/ABI/testing/sysfs-bus-papr-pmem
+++ b/Documentation/ABI/testing/sysfs-bus-papr-pmem
@@ -76,6 +76,12 @@ Description: (RO) Attribute group to describe the magic bits
For example::
noopstat = "event=0x1"
+What: /sys/devices/nmemX/cpumask
+Date: May 2021
+Contact: linuxppc-dev <linuxppc-dev@lists.ozlabs.org>, linux-nvdimm@lists.01.org,
+Description: (RO) This sysfs file exposes the cpumask which is designated to make
+ HCALLs to retrieve nvdimm pmu event counter data.
+
What: /sys/devices/nmemX/events
Date: May 2021
Contact: linuxppc-dev <linuxppc-dev@lists.ozlabs.org>, linux-nvdimm@lists.01.org,
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index 997d379094d0..6d94c2f260aa 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -334,6 +334,28 @@ static ssize_t drc_pmem_query_stats(struct papr_scm_priv *p,
return 0;
}
+static ssize_t cpumask_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct pmu *pmu = dev_get_drvdata(dev);
+ struct nvdimm_pmu *nd_pmu;
+
+ nd_pmu = container_of(pmu, struct nvdimm_pmu, pmu);
+
+ return cpumap_print_to_pagebuf(true, buf, cpumask_of(nd_pmu->cpu));
+}
+
+static DEVICE_ATTR_RO(cpumask);
+
+static struct attribute *nvdimm_cpumask_attrs[] = {
+ &dev_attr_cpumask.attr,
+ NULL,
+};
+
+static const struct attribute_group nvdimm_pmu_cpumask_group = {
+ .attrs = nvdimm_cpumask_attrs,
+};
+
static struct attribute_group nvdimm_pmu_events_group = {
.name = "events",
/* .attrs is set in papr_scm_pmu_check_events function */
@@ -354,6 +376,7 @@ static struct attribute_group nvdimm_pmu_format_group = {
static const struct attribute_group *nvdimm_pmu_attr_groups[] = {
&nvdimm_pmu_format_group,
&nvdimm_pmu_events_group,
+ &nvdimm_pmu_cpumask_group,
NULL,
};
@@ -418,10 +441,30 @@ static void papr_scm_pmu_del(struct perf_event *event, int flags, struct device
papr_scm_pmu_read(event, dev);
}
+static int nvdimm_pmu_offline_cpu(unsigned int cpu, struct hlist_node *node)
+{
+ struct nvdimm_pmu *pmu;
+ int target;
+
+ pmu = hlist_entry_safe(node, struct nvdimm_pmu, node);
+
+ if (cpu != pmu->cpu)
+ return 0;
+
+ target = cpumask_last(cpu_active_mask);
+ if (target < 0 || target >= nr_cpu_ids)
+ return -1;
+
+ pmu->cpu = target;
+ return 0;
+}
+
static void nvdimm_pmu_uinit(struct nvdimm_pmu *nd_pmu)
{
unregister_nvdimm_pmu(&nd_pmu->pmu);
kfree(nd_pmu);
+ cpuhp_state_remove_instance_nocalls(nd_pmu->cpuhp_state, &nd_pmu->node);
+ cpuhp_remove_multi_state(nd_pmu->cpuhp_state);
}
static int papr_scm_pmu_register(struct papr_scm_priv *p)
@@ -438,6 +481,22 @@ static int papr_scm_pmu_register(struct papr_scm_priv *p)
papr_scm_pmu->add = papr_scm_pmu_add;
papr_scm_pmu->del = papr_scm_pmu_del;
papr_scm_pmu->attr_groups = nvdimm_pmu_attr_groups;
+ papr_scm_pmu->cpu = raw_smp_processor_id();
+
+ rc = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN,
+ "perf/nvdimm:online",
+ NULL, nvdimm_pmu_offline_cpu);
+ if (rc < 0) {
+ kfree(papr_scm_pmu);
+ return rc;
+ }
+
+ papr_scm_pmu->cpuhp_state = rc;
+
+ /* Register the pmu instance for cpu hotplug */
+ rc = cpuhp_state_add_instance_nocalls(papr_scm_pmu->cpuhp_state, &papr_scm_pmu->node);
+ if (rc)
+ goto cpuhp_instance_err;
rc = register_nvdimm_pmu(papr_scm_pmu, p->pdev);
if (rc)
@@ -451,6 +510,9 @@ static int papr_scm_pmu_register(struct papr_scm_priv *p)
return 0;
pmu_register_err:
+ cpuhp_state_remove_instance_nocalls(papr_scm_pmu->cpuhp_state, &papr_scm_pmu->node);
+cpuhp_instance_err:
+ cpuhp_remove_multi_state(papr_scm_pmu->cpuhp_state);
kfree(papr_scm_pmu);
return rc;
}
--
2.27.0
^ permalink raw reply related
* [RFC 3/4] powerpc/papr_scm: Document papr_scm sysfs event format entries
From: Kajol Jain @ 2021-05-12 16:38 UTC (permalink / raw)
To: mpe, linuxppc-dev, linux-nvdimm, linux-kernel
Cc: santosh, maddy, peterz, aneesh.kumar, atrajeev, kjain, vaibhav,
dan.j.williams, ira.weiny, tglx
In-Reply-To: <20210512163824.255370-1-kjain@linux.ibm.com>
This patch add event format and events details in ABI
documentation
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
Documentation/ABI/testing/sysfs-bus-papr-pmem | 25 +++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-papr-pmem b/Documentation/ABI/testing/sysfs-bus-papr-pmem
index 8316c33862a0..216f70deca7e 100644
--- a/Documentation/ABI/testing/sysfs-bus-papr-pmem
+++ b/Documentation/ABI/testing/sysfs-bus-papr-pmem
@@ -59,3 +59,28 @@ Description:
* "CchRHCnt" : Cache Read Hit Count
* "CchWHCnt" : Cache Write Hit Count
* "FastWCnt" : Fast Write Count
+
+What: /sys/devices/nmemX/format
+Date: May 2021
+Contact: linuxppc-dev <linuxppc-dev@lists.ozlabs.org>, linux-nvdimm@lists.01.org,
+Description: (RO) Attribute group to describe the magic bits
+ that go into perf_event_attr.config for a particular pmu.
+ (See ABI/testing/sysfs-bus-event_source-devices-format).
+
+ Each attribute under this group defines a bit range of the
+ perf_event_attr.config. Supported attributes is listed
+ below::
+
+ event = "config:0-37" - event ID
+
+ For example::
+ noopstat = "event=0x1"
+
+What: /sys/devices/nmemX/events
+Date: May 2021
+Contact: linuxppc-dev <linuxppc-dev@lists.ozlabs.org>, linux-nvdimm@lists.01.org,
+Description: (RO) Attribute group to describe performance monitoring
+ events specific to papr_pmem. Each attribute in this group describes
+ a single performance monitoring event supported by this nvdimm pmu.
+ The name of the file is the name of the event.
+ (See ABI/testing/sysfs-bus-event_source-devices-events).
--
2.27.0
^ permalink raw reply related
* Re: [PATCH] lockdown, selinux: fix bogus SELinux lockdown permission checks
From: Ondrej Mosnacek @ 2021-05-12 16:44 UTC (permalink / raw)
To: Casey Schaufler
Cc: SElinux list, network dev, Stephen Smalley, James Morris,
Steven Rostedt, Linux kernel mailing list,
Linux Security Module list, Ingo Molnar, Linux FS Devel, bpf,
linuxppc-dev
In-Reply-To: <24a61ff1-e415-adf8-17e8-d212364d4b97@schaufler-ca.com>
On Wed, May 12, 2021 at 6:18 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 5/12/2021 6:21 AM, Ondrej Mosnacek wrote:
> > On Sat, May 8, 2021 at 12:17 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >> On 5/7/2021 4:40 AM, Ondrej Mosnacek wrote:
> >>> Commit 59438b46471a ("security,lockdown,selinux: implement SELinux
> >>> lockdown") added an implementation of the locked_down LSM hook to
> >>> SELinux, with the aim to restrict which domains are allowed to perform
> >>> operations that would breach lockdown.
> >>>
> >>> However, in several places the security_locked_down() hook is called in
> >>> situations where the current task isn't doing any action that would
> >>> directly breach lockdown, leading to SELinux checks that are basically
> >>> bogus.
> >>>
> >>> Since in most of these situations converting the callers such that
> >>> security_locked_down() is called in a context where the current task
> >>> would be meaningful for SELinux is impossible or very non-trivial (and
> >>> could lead to TOCTOU issues for the classic Lockdown LSM
> >>> implementation), fix this by adding a separate hook
> >>> security_locked_down_globally()
> >> This is a poor solution to the stated problem. Rather than adding
> >> a new hook you should add the task as a parameter to the existing hook
> >> and let the security modules do as they will based on its value.
> >> If the caller does not have an appropriate task it should pass NULL.
> >> The lockdown LSM can ignore the task value and SELinux can make its
> >> own decision based on the task value passed.
> > The problem with that approach is that all callers would then need to
> > be updated and I intended to keep the patch small as I'd like it to go
> > to stable kernels as well.
> >
> > But it does seem to be a better long-term solution - would it work for
> > you (and whichever maintainer would be taking the patch(es)) if I just
> > added another patch that refactors it to use the task parameter?
>
> I can't figure out what you're suggesting. Are you saying that you
> want to add a new hook *and* add the task parameter?
No, just to keep this patch as-is (and let it go to stable in this
form) and post another (non-stable) patch on top of it that undoes the
new hook and re-implements the fix using your suggestion. (Yeah, it'll
look weird, but I'm not sure how better to handle such situation - I'm
open to doing it whatever different way the maintainers prefer.)
--
Ondrej Mosnacek
Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.
^ permalink raw reply
* Re: [PATCH] lockdown,selinux: fix bogus SELinux lockdown permission checks
From: Casey Schaufler @ 2021-05-12 17:12 UTC (permalink / raw)
To: Ondrej Mosnacek
Cc: SElinux list, network dev, Stephen Smalley, James Morris,
Steven Rostedt, Linux kernel mailing list, Casey Schaufler,
Linux Security Module list, Ingo Molnar, Linux FS Devel, bpf,
linuxppc-dev
In-Reply-To: <CAFqZXNvB-EyPz1Qz3cCRTr1u1+D+xT-dp7cUxFocYM1AOYSuxw@mail.gmail.com>
On 5/12/2021 9:44 AM, Ondrej Mosnacek wrote:
> On Wed, May 12, 2021 at 6:18 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> On 5/12/2021 6:21 AM, Ondrej Mosnacek wrote:
>>> On Sat, May 8, 2021 at 12:17 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
>>>> On 5/7/2021 4:40 AM, Ondrej Mosnacek wrote:
>>>>> Commit 59438b46471a ("security,lockdown,selinux: implement SELinux
>>>>> lockdown") added an implementation of the locked_down LSM hook to
>>>>> SELinux, with the aim to restrict which domains are allowed to perform
>>>>> operations that would breach lockdown.
>>>>>
>>>>> However, in several places the security_locked_down() hook is called in
>>>>> situations where the current task isn't doing any action that would
>>>>> directly breach lockdown, leading to SELinux checks that are basically
>>>>> bogus.
>>>>>
>>>>> Since in most of these situations converting the callers such that
>>>>> security_locked_down() is called in a context where the current task
>>>>> would be meaningful for SELinux is impossible or very non-trivial (and
>>>>> could lead to TOCTOU issues for the classic Lockdown LSM
>>>>> implementation), fix this by adding a separate hook
>>>>> security_locked_down_globally()
>>>> This is a poor solution to the stated problem. Rather than adding
>>>> a new hook you should add the task as a parameter to the existing hook
>>>> and let the security modules do as they will based on its value.
>>>> If the caller does not have an appropriate task it should pass NULL.
>>>> The lockdown LSM can ignore the task value and SELinux can make its
>>>> own decision based on the task value passed.
>>> The problem with that approach is that all callers would then need to
>>> be updated and I intended to keep the patch small as I'd like it to go
>>> to stable kernels as well.
>>>
>>> But it does seem to be a better long-term solution - would it work for
>>> you (and whichever maintainer would be taking the patch(es)) if I just
>>> added another patch that refactors it to use the task parameter?
>> I can't figure out what you're suggesting. Are you saying that you
>> want to add a new hook *and* add the task parameter?
> No, just to keep this patch as-is (and let it go to stable in this
> form) and post another (non-stable) patch on top of it that undoes the
> new hook and re-implements the fix using your suggestion. (Yeah, it'll
> look weird, but I'm not sure how better to handle such situation - I'm
> open to doing it whatever different way the maintainers prefer.)
James gets to make the call on this one. If it was my call I would
tell you to make the task parameter change and accept the backport
pain. I think that as a security developer community we spend way too
much time and effort trying to avoid being noticed in source trees.
^ permalink raw reply
* Re: [RFC 1/4] drivers/nvdimm: Add perf interface to expose nvdimm performance stats
From: Peter Zijlstra @ 2021-05-12 17:27 UTC (permalink / raw)
To: Kajol Jain
Cc: santosh, maddy, ira.weiny, linux-nvdimm, linux-kernel, atrajeev,
aneesh.kumar, vaibhav, dan.j.williams, linuxppc-dev, tglx
In-Reply-To: <20210512163824.255370-2-kjain@linux.ibm.com>
On Wed, May 12, 2021 at 10:08:21PM +0530, Kajol Jain wrote:
> +static void nvdimm_pmu_read(struct perf_event *event)
> +{
> + struct nvdimm_pmu *nd_pmu = to_nvdimm_pmu(event->pmu);
> +
> + /* jump to arch/platform specific callbacks if any */
> + if (nd_pmu && nd_pmu->read)
> + nd_pmu->read(event, nd_pmu->dev);
> +}
> +
> +static void nvdimm_pmu_del(struct perf_event *event, int flags)
> +{
> + struct nvdimm_pmu *nd_pmu = to_nvdimm_pmu(event->pmu);
> +
> + /* jump to arch/platform specific callbacks if any */
> + if (nd_pmu && nd_pmu->del)
> + nd_pmu->del(event, flags, nd_pmu->dev);
> +}
> +
> +static int nvdimm_pmu_add(struct perf_event *event, int flags)
> +{
> + struct nvdimm_pmu *nd_pmu = to_nvdimm_pmu(event->pmu);
> +
> + if (flags & PERF_EF_START)
> + /* jump to arch/platform specific callbacks if any */
> + if (nd_pmu && nd_pmu->add)
> + return nd_pmu->add(event, flags, nd_pmu->dev);
> + return 0;
> +}
What's the value add here? Why can't you directly set driver pointers? I
also don't really believe ->{add,del,read} can be optional and still
have a sane driver.
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/kprobes: Replace ppc_optinsn by common optinsn
From: kernel test robot @ 2021-05-12 17:29 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, naveen.n.rao, anil.s.keshavamurthy, davem,
mhiramat
Cc: linuxppc-dev, kbuild-all, linux-kernel, clang-built-linux
In-Reply-To: <46998c941d0a5664daaeb92998391aace015eddf.1620829724.git.christophe.leroy@csgroup.eu>
[-- Attachment #1: Type: text/plain, Size: 2373 bytes --]
Hi Christophe,
I love your patch! Perhaps something to improve:
[auto build test WARNING on powerpc/next]
[also build test WARNING on linus/master v5.13-rc1 next-20210512]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Christophe-Leroy/kprobes-Allow-architectures-to-override-optinsn-page-allocation/20210512-223121
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-allyesconfig (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/1cc4bb503e4ee4839247b767883c8b860b26413c
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Christophe-Leroy/kprobes-Allow-architectures-to-override-optinsn-page-allocation/20210512-223121
git checkout 1cc4bb503e4ee4839247b767883c8b860b26413c
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross W=1 ARCH=powerpc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> arch/powerpc/kernel/optprobes.c:36:7: warning: no previous prototype for 'alloc_optinsn_page' [-Wmissing-prototypes]
36 | void *alloc_optinsn_page(void)
| ^~~~~~~~~~~~~~~~~~
>> arch/powerpc/kernel/optprobes.c:44:6: warning: no previous prototype for 'free_optinsn_page' [-Wmissing-prototypes]
44 | void free_optinsn_page(void *page)
| ^~~~~~~~~~~~~~~~~
vim +/alloc_optinsn_page +36 arch/powerpc/kernel/optprobes.c
35
> 36 void *alloc_optinsn_page(void)
37 {
38 if (insn_page_in_use)
39 return NULL;
40 insn_page_in_use = true;
41 return &optinsn_slot;
42 }
43
> 44 void free_optinsn_page(void *page)
45 {
46 insn_page_in_use = false;
47 }
48
---
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: 73383 bytes --]
^ permalink raw reply
* Re: [PATCH] powerpc: Force inlining of csum_add()
From: Segher Boessenkool @ 2021-05-12 18:21 UTC (permalink / raw)
To: Christophe Leroy; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <2623fe98-7a73-f7a2-bcba-2d668d00ffd0@csgroup.eu>
On Wed, May 12, 2021 at 04:43:33PM +0200, Christophe Leroy wrote:
> Le 12/05/2021 à 16:31, Segher Boessenkool a écrit :
> >On Wed, May 12, 2021 at 02:56:56PM +0200, Christophe Leroy wrote:
> >>Le 11/05/2021 à 12:51, Segher Boessenkool a écrit :
> >>>Something seems to have decided this asm is more expensive than it is.
> >>>That isn't always avoidable -- the compiler cannot look inside asms --
> >>>but it seems it could be improved here.
> >>>
> >>>Do you have (or can make) a self-contained testcase?
> >>
> >>I have not tried, and I fear it might be difficult, because on a kernel
> >>build with dozens of calls to csum_add(), only ip6_tunnel.o exhibits such
> >>an issue.
> >
> >Yeah. Sometimes you can force some of the decisions, but that usually
> >requires knowing too many GCC internals :-/
> >
> >>>>And there is even one completely unused instance of csum_add().
> >>>
> >>>That is strange, that should never happen.
> >>
> >>It seems that several .o include unused versions of csum_add. After the
> >>final link, one remains (in addition to the used one) in vmlinux.
> >
> >But it is a static function, so it should not end up in any object file
> >where it isn't used.
>
> Well .... did I dream ?
>
> Now I only find one extra .o with unused csum_add() : That's
> net/ipv6/exthdrs.o
> It matches the one found in vmlinux.
>
> Are you interested in -fdump-tree-einline-all for that one as well ?
Sure. Hopefully it will show more :-)
Segher
^ permalink raw reply
* Re: [PATCH 1/2] kprobes: Allow architectures to override optinsn page allocation
From: kernel test robot @ 2021-05-12 19:04 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, naveen.n.rao, anil.s.keshavamurthy, davem,
mhiramat
Cc: linuxppc-dev, kbuild-all, linux-kernel, clang-built-linux
In-Reply-To: <472c597cc8cc8f74f037acf971f7799fb381a647.1620829724.git.christophe.leroy@csgroup.eu>
[-- Attachment #1: Type: text/plain, Size: 2131 bytes --]
Hi Christophe,
I love your patch! Perhaps something to improve:
[auto build test WARNING on powerpc/next]
[also build test WARNING on linus/master v5.13-rc1 next-20210512]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Christophe-Leroy/kprobes-Allow-architectures-to-override-optinsn-page-allocation/20210512-223121
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: i386-randconfig-r012-20210512 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/2a1f135a9ce3c4d86d3bdefed561aa17760f430f
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Christophe-Leroy/kprobes-Allow-architectures-to-override-optinsn-page-allocation/20210512-223121
git checkout 2a1f135a9ce3c4d86d3bdefed561aa17760f430f
# save the attached .config to linux build tree
make W=1 W=1 ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> kernel/kprobes.c:324:14: warning: no previous prototype for 'alloc_optinsn_page' [-Wmissing-prototypes]
324 | void __weak *alloc_optinsn_page(void)
| ^~~~~~~~~~~~~~~~~~
>> kernel/kprobes.c:329:13: warning: no previous prototype for 'free_optinsn_page' [-Wmissing-prototypes]
329 | void __weak free_optinsn_page(void *page)
| ^~~~~~~~~~~~~~~~~
vim +/alloc_optinsn_page +324 kernel/kprobes.c
322
323 #ifdef CONFIG_OPTPROBES
> 324 void __weak *alloc_optinsn_page(void)
325 {
326 return alloc_insn_page();
327 }
328
> 329 void __weak free_optinsn_page(void *page)
330 {
331 free_insn_page(page);
332 }
333
---
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: 34430 bytes --]
^ permalink raw reply
* Re: [PATCH v2] powerpc/papr_scm: Make 'perf_stats' invisible if perf-stats unavailable
From: Dan Williams @ 2021-05-12 19:48 UTC (permalink / raw)
To: Vaibhav Jain
Cc: Santosh Sivaraj, Ira Weiny, linux-nvdimm, Aneesh Kumar K . V,
linuxppc-dev
In-Reply-To: <20210507113948.38950-1-vaibhav@linux.ibm.com>
On Fri, May 7, 2021 at 4:40 AM Vaibhav Jain <vaibhav@linux.ibm.com> wrote:
>
> In case performance stats for an nvdimm are not available, reading the
> 'perf_stats' sysfs file returns an -ENOENT error. A better approach is
> to make the 'perf_stats' file entirely invisible to indicate that
> performance stats for an nvdimm are unavailable.
>
> So this patch updates 'papr_nd_attribute_group' to add a 'is_visible'
> callback implemented as newly introduced 'papr_nd_attribute_visible()'
> that returns an appropriate mode in case performance stats aren't
> supported in a given nvdimm.
>
> Also the initialization of 'papr_scm_priv.stat_buffer_len' is moved
> from papr_scm_nvdimm_init() to papr_scm_probe() so that it value is
> available when 'papr_nd_attribute_visible()' is called during nvdimm
> initialization.
>
> Fixes: 2d02bf835e57('powerpc/papr_scm: Fetch nvdimm performance stats from PHYP')
Since this has been the exposed ABI since v5.9 perhaps a note /
analysis is needed here that the disappearance of this file will not
break any existing scripts/tools.
> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> ---
> Changelog:
>
> v2:
> * Removed a redundant dev_info() from pap_scm_nvdimm_init() [ Aneesh ]
> * Marked papr_nd_attribute_visible() as static which also fixed the
> build warning reported by kernel build robot
> ---
> arch/powerpc/platforms/pseries/papr_scm.c | 35 ++++++++++++++++-------
> 1 file changed, 24 insertions(+), 11 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
> index e2b69cc3beaf..11e7b90a3360 100644
> --- a/arch/powerpc/platforms/pseries/papr_scm.c
> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
> @@ -907,6 +907,20 @@ static ssize_t flags_show(struct device *dev,
> }
> DEVICE_ATTR_RO(flags);
>
> +static umode_t papr_nd_attribute_visible(struct kobject *kobj,
> + struct attribute *attr, int n)
> +{
> + struct device *dev = container_of(kobj, typeof(*dev), kobj);
This can use the kobj_to_dev() helper.
> + struct nvdimm *nvdimm = to_nvdimm(dev);
> + struct papr_scm_priv *p = nvdimm_provider_data(nvdimm);
> +
> + /* For if perf-stats not available remove perf_stats sysfs */
> + if (attr == &dev_attr_perf_stats.attr && p->stat_buffer_len == 0)
> + return 0;
> +
> + return attr->mode;
> +}
> +
> /* papr_scm specific dimm attributes */
> static struct attribute *papr_nd_attributes[] = {
> &dev_attr_flags.attr,
> @@ -916,6 +930,7 @@ static struct attribute *papr_nd_attributes[] = {
>
> static struct attribute_group papr_nd_attribute_group = {
> .name = "papr",
> + .is_visible = papr_nd_attribute_visible,
> .attrs = papr_nd_attributes,
> };
>
> @@ -931,7 +946,6 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
> struct nd_region_desc ndr_desc;
> unsigned long dimm_flags;
> int target_nid, online_nid;
> - ssize_t stat_size;
>
> p->bus_desc.ndctl = papr_scm_ndctl;
> p->bus_desc.module = THIS_MODULE;
> @@ -1016,16 +1030,6 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
> list_add_tail(&p->region_list, &papr_nd_regions);
> mutex_unlock(&papr_ndr_lock);
>
> - /* Try retriving the stat buffer and see if its supported */
> - stat_size = drc_pmem_query_stats(p, NULL, 0);
> - if (stat_size > 0) {
> - p->stat_buffer_len = stat_size;
> - dev_dbg(&p->pdev->dev, "Max perf-stat size %lu-bytes\n",
> - p->stat_buffer_len);
> - } else {
> - dev_info(&p->pdev->dev, "Dimm performance stats unavailable\n");
> - }
> -
> return 0;
>
> err: nvdimm_bus_unregister(p->bus);
> @@ -1102,6 +1106,7 @@ static int papr_scm_probe(struct platform_device *pdev)
> u64 blocks, block_size;
> struct papr_scm_priv *p;
> const char *uuid_str;
> + ssize_t stat_size;
> u64 uuid[2];
> int rc;
>
> @@ -1179,6 +1184,14 @@ static int papr_scm_probe(struct platform_device *pdev)
> p->res.name = pdev->name;
> p->res.flags = IORESOURCE_MEM;
>
> + /* Try retriving the stat buffer and see if its supported */
s/retriving/retrieving/
> + stat_size = drc_pmem_query_stats(p, NULL, 0);
> + if (stat_size > 0) {
> + p->stat_buffer_len = stat_size;
> + dev_dbg(&p->pdev->dev, "Max perf-stat size %lu-bytes\n",
> + p->stat_buffer_len);
> + }
> +
> rc = papr_scm_nvdimm_init(p);
> if (rc)
> goto err2;
> --
> 2.31.1
>
After the minor fixups above you can add:
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
...I assume this will go through the PPC tree.
^ permalink raw reply
* Re: [PATCH v2 2/2] ASoC: dt-bindings: imx-akcodec: Add binding doc for akcodec machine driver
From: Rob Herring @ 2021-05-11 19:00 UTC (permalink / raw)
To: Shengjiu Wang
Cc: linux-arm-kernel, devicetree, alsa-devel, timur, Xiubo.Lee,
shawnguo, s.hauer, linuxppc-dev, tiwai, lgirdwood, nicoleotsuka,
broonie, linux-imx, kernel, perex, festevam, linux-kernel
In-Reply-To: <1620630441-14358-2-git-send-email-shengjiu.wang@nxp.com>
On Mon, May 10, 2021 at 03:07:21PM +0800, Shengjiu Wang wrote:
> Imx-akcodec is a new added machine driver for supporting
> ak4458/ak5558/ak5552/ak4497 codec on i.MX platforms.
>
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> ---
> changes in v2:
> - update doc accoring to Rob's comment, use the common porperties.
>
> .../bindings/sound/imx-audio-akcodec.yaml | 107 ++++++++++++++++++
> 1 file changed, 107 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/sound/imx-audio-akcodec.yaml
>
> diff --git a/Documentation/devicetree/bindings/sound/imx-audio-akcodec.yaml b/Documentation/devicetree/bindings/sound/imx-audio-akcodec.yaml
> new file mode 100644
> index 000000000000..0812c4779b59
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/imx-audio-akcodec.yaml
> @@ -0,0 +1,107 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/imx-audio-akcodec.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP i.MX audio complex with AK4458/AK5558/AK5552/AK4497 codec
> +
> +maintainers:
> + - Shengjiu Wang <shengjiu.wang@nxp.com>
> +
> +properties:
> + compatible:
> + enum:
> + - fsl,imx8mq-audio-akcodec
> + - fsl,imx-audio-akcodec
We gone from individual codecs to a family of codecs. Is there something
specific for the machine driver about this set of codecs? Why can't it
be *any* codec?
> +
> + model:
> + $ref: /schemas/types.yaml#/definitions/string
> + description: User specified audio sound card name
> +
> + audio-routing:
> + $ref: /schemas/types.yaml#/definitions/non-unique-string-array
> + description:
> + A list of the connections between audio components. Each entry is a
> + pair of strings, the first being the connection's sink, the second
> + being the connection's source. Valid names could be power supplies,
> + MicBias of codec and the jacks on the board.
> +
> +patternProperties:
> + ".*-dai-link$":
> + description:
> + Each subnode represents a dai link. Subnodes of each dai links would be
> + cpu/codec dais.
> +
> + type: object
> +
> + properties:
> + link-name:
> + description: Indicates dai-link name and PCM stream name.
> + $ref: /schemas/types.yaml#/definitions/string
> + maxItems: 1
> +
> + format:
> + description: audio format.
> + items:
> + enum:
> + - i2s
> + - dsp_b
> +
> + cpu:
> + description: Holds subnode which indicates cpu dai.
> + type: object
> + properties:
> + sound-dai: true
> +
> + codec:
> + description: Holds subnode which indicates codec dai.
> + type: object
> + properties:
> + sound-dai: true
> +
> + required:
> + - link-name
> + - cpu
> +
> + additionalProperties: false
> +
> +required:
> + - compatible
> + - model
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + sound-ak4458 {
> + compatible = "fsl,imx-audio-akcodec";
> + model = "ak4458-audio";
> + pri-dai-link {
> + link-name = "akcodec";
> + format = "i2s";
> + cpu {
> + sound-dai = <&sai1>;
> + };
> + codec {
> + sound-dai = <&ak4458_1>, <&ak4458_2>;
> + };
> + };
> + fe-dai-link {
> + link-name = "HiFi-ASRC-FE";
> + format = "i2s";
> + cpu {
> + sound-dai = <&easrc>;
> + };
> + };
> + be-dai-link {
> + link-name = "HiFi-ASRC-BE";
> + format = "dsp_b";
> + cpu {
> + sound-dai = <&sai1>;
> + };
> + codec {
> + sound-dai = <&ak4458_1>, <&ak4458_2>;
> + };
> + };
> + };
> --
> 2.27.0
>
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/kprobes: Replace ppc_optinsn by common optinsn
From: kernel test robot @ 2021-05-12 20:11 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, naveen.n.rao, anil.s.keshavamurthy, davem,
mhiramat
Cc: linuxppc-dev, kbuild-all, linux-kernel, clang-built-linux
In-Reply-To: <46998c941d0a5664daaeb92998391aace015eddf.1620829724.git.christophe.leroy@csgroup.eu>
[-- Attachment #1: Type: text/plain, Size: 2419 bytes --]
Hi Christophe,
I love your patch! Yet something to improve:
[auto build test ERROR on powerpc/next]
[also build test ERROR on linus/master v5.13-rc1 next-20210512]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Christophe-Leroy/kprobes-Allow-architectures-to-override-optinsn-page-allocation/20210512-223121
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-ppc64_defconfig (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/1cc4bb503e4ee4839247b767883c8b860b26413c
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Christophe-Leroy/kprobes-Allow-architectures-to-override-optinsn-page-allocation/20210512-223121
git checkout 1cc4bb503e4ee4839247b767883c8b860b26413c
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross W=1 ARCH=powerpc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> arch/powerpc/kernel/optprobes.c:36:7: error: no previous prototype for 'alloc_optinsn_page' [-Werror=missing-prototypes]
36 | void *alloc_optinsn_page(void)
| ^~~~~~~~~~~~~~~~~~
>> arch/powerpc/kernel/optprobes.c:44:6: error: no previous prototype for 'free_optinsn_page' [-Werror=missing-prototypes]
44 | void free_optinsn_page(void *page)
| ^~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
vim +/alloc_optinsn_page +36 arch/powerpc/kernel/optprobes.c
35
> 36 void *alloc_optinsn_page(void)
37 {
38 if (insn_page_in_use)
39 return NULL;
40 insn_page_in_use = true;
41 return &optinsn_slot;
42 }
43
> 44 void free_optinsn_page(void *page)
45 {
46 insn_page_in_use = false;
47 }
48
---
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: 26845 bytes --]
^ permalink raw reply
* [PATCH v2 2/4] powerpc/pseries: check DRCONF_MEM_RESERVED in lmb_is_removable()
From: Daniel Henrique Barboza @ 2021-05-12 20:28 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Daniel Henrique Barboza, david
In-Reply-To: <20210512202809.95363-1-danielhb413@gmail.com>
DRCONF_MEM_RESERVED is a flag that represents the "Reserved Memory"
status in LOPAR v2.10, section 4.2.8. If a LMB is marked as reserved,
quoting LOPAR, "is not to be used or altered by the base OS". This flag
is read only in the kernel, being set by the firmware/hypervisor in the
DT. As an example, QEMU will set this flag in hw/ppc/spapr.c,
spapr_dt_dynamic_memory().
lmb_is_removable() does not check for DRCONF_MEM_RESERVED. This function
is used in dlpar_remove_lmb() as a guard before the removal logic. Since
it is failing to check for !RESERVED, dlpar_remove_lmb() will fail in a
later stage instead of failing in the validation when receiving a
reserved LMB as input.
lmb_is_removable() is also used in dlpar_memory_remove_by_count() to
evaluate if we have enough LMBs to complete the request. The missing
!RESERVED check in this case is causing dlpar_memory_remove_by_count()
to miscalculate the number of elegible LMBs for the removal, and can
make it error out later on instead of failing in the validation with the
'not enough LMBs to satisfy request' message.
Making a DRCONF_MEM_RESERVED check in lmb_is_removable() fixes all these
issues.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
arch/powerpc/platforms/pseries/hotplug-memory.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index bb98574a84a2..c21d9278c1ce 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -348,7 +348,8 @@ static int pseries_remove_mem_node(struct device_node *np)
static bool lmb_is_removable(struct drmem_lmb *lmb)
{
- if (!(lmb->flags & DRCONF_MEM_ASSIGNED))
+ if ((lmb->flags & DRCONF_MEM_RESERVED) ||
+ !(lmb->flags & DRCONF_MEM_ASSIGNED))
return false;
#ifdef CONFIG_FA_DUMP
--
2.31.1
^ permalink raw reply related
* [PATCH v2 1/4] powerpc/pseries: Set UNISOLATE on dlpar_memory_remove_by_ic() error
From: Daniel Henrique Barboza @ 2021-05-12 20:28 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Daniel Henrique Barboza, david
In-Reply-To: <20210512202809.95363-1-danielhb413@gmail.com>
As previously done in dlpar_cpu_remove() for CPUs, this patch changes
dlpar_memory_remove_by_ic() to unisolate the LMB DRC when the LMB is
failed to be removed. The hypervisor, seeing a LMB DRC that was supposed
to be removed being unisolated instead, can do error recovery on its
side.
This change is done in dlpar_memory_remove_by_ic() only because, as of
today, only QEMU is using this code path for error recovery (via the
PSERIES_HP_ELOG_ID_DRC_IC event). phyp treats it as a no-op.
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
arch/powerpc/platforms/pseries/hotplug-memory.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index 8377f1f7c78e..bb98574a84a2 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -551,6 +551,13 @@ static int dlpar_memory_remove_by_ic(u32 lmbs_to_remove, u32 drc_index)
if (!drmem_lmb_reserved(lmb))
continue;
+ /*
+ * Setting the isolation state of an UNISOLATED/CONFIGURED
+ * device to UNISOLATE is a no-op, but the hypervisor can
+ * use it as a hint that the LMB removal failed.
+ */
+ dlpar_unisolate_drc(lmb->drc_index);
+
rc = dlpar_add_lmb(lmb);
if (rc)
pr_err("Failed to add LMB, drc index %x\n",
--
2.31.1
^ permalink raw reply related
* [PATCH v2 0/4] Unisolate LMBs DRC on removal error + cleanups
From: Daniel Henrique Barboza @ 2021-05-12 20:28 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Daniel Henrique Barboza, david
changes from v1:
- patch 1: added David's r-b
- patch 2:
* removed the DRCONF_MEM_RESERVED assumption for
dlpar_memory_remove_by_ic()
* reworded the commit msg
- patch 3: dropped. the differences between dlpar_memory_remove_by_ic()
and dlpar_memory_remove_by_count() makes a helper function too complex
to handle both cases
- (new) patch 3 and 4: minor enhancements
v1 link: https://lore.kernel.org/linuxppc-dev/20210430120917.217951-1-danielhb413@gmail.com/
Daniel Henrique Barboza (4):
powerpc/pseries: Set UNISOLATE on dlpar_memory_remove_by_ic() error
powerpc/pseries: check DRCONF_MEM_RESERVED in lmb_is_removable()
powerpc/pseries: break early in dlpar_memory_remove_by_count() loops
powerpc/pseries: minor enhancements in dlpar_memory_remove_by_ic()
.../platforms/pseries/hotplug-memory.c | 54 ++++++++++++++-----
1 file changed, 40 insertions(+), 14 deletions(-)
--
2.31.1
^ permalink raw reply
* [PATCH v2 3/4] powerpc/pseries: break early in dlpar_memory_remove_by_count() loops
From: Daniel Henrique Barboza @ 2021-05-12 20:28 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Daniel Henrique Barboza, david
In-Reply-To: <20210512202809.95363-1-danielhb413@gmail.com>
After marking the LMBs as reserved depending on dlpar_remove_lmb() rc,
we evaluate whether we need to add the LMBs back or if we can release
the LMB DRCs. In both cases, a for_each_drmem_lmb() loop without a break
condition is used. This means that we're going to cycle through all LMBs
of the partition even after we're done with what we were going to do.
This patch adds break conditions in both loops to avoid this. The
'lmbs_removed' variable was renamed to 'lmbs_reserved', and it's now
being decremented each time a lmb reservation is removed, indicating
that the operation we're doing (adding back LMBs or releasing DRCs) is
completed.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
arch/powerpc/platforms/pseries/hotplug-memory.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index c21d9278c1ce..3c7ce5361ce3 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -402,7 +402,7 @@ static int dlpar_remove_lmb(struct drmem_lmb *lmb)
static int dlpar_memory_remove_by_count(u32 lmbs_to_remove)
{
struct drmem_lmb *lmb;
- int lmbs_removed = 0;
+ int lmbs_reserved = 0;
int lmbs_available = 0;
int rc;
@@ -436,12 +436,12 @@ static int dlpar_memory_remove_by_count(u32 lmbs_to_remove)
*/
drmem_mark_lmb_reserved(lmb);
- lmbs_removed++;
- if (lmbs_removed == lmbs_to_remove)
+ lmbs_reserved++;
+ if (lmbs_reserved == lmbs_to_remove)
break;
}
- if (lmbs_removed != lmbs_to_remove) {
+ if (lmbs_reserved != lmbs_to_remove) {
pr_err("Memory hot-remove failed, adding LMB's back\n");
for_each_drmem_lmb(lmb) {
@@ -454,6 +454,10 @@ static int dlpar_memory_remove_by_count(u32 lmbs_to_remove)
lmb->drc_index);
drmem_remove_lmb_reservation(lmb);
+
+ lmbs_reserved--;
+ if (lmbs_reserved == 0)
+ break;
}
rc = -EINVAL;
@@ -467,6 +471,10 @@ static int dlpar_memory_remove_by_count(u32 lmbs_to_remove)
lmb->base_addr);
drmem_remove_lmb_reservation(lmb);
+
+ lmbs_reserved--;
+ if (lmbs_reserved == 0)
+ break;
}
rc = 0;
}
--
2.31.1
^ permalink raw reply related
* [PATCH v2 4/4] powerpc/pseries: minor enhancements in dlpar_memory_remove_by_ic()
From: Daniel Henrique Barboza @ 2021-05-12 20:28 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Daniel Henrique Barboza, david
In-Reply-To: <20210512202809.95363-1-danielhb413@gmail.com>
We don't need the 'lmbs_available' variable to count the valid LMBs and
to check if we have less than 'lmbs_to_remove'. We must ensure that the
entire LMB range must be removed, so we can error out immediately if any
LMB in the range is marked as reserved.
Add a couple of comments explaining the reasoning behind the differences
we have in this function in contrast to what it is done in its sister
function, dlpar_memory_remove_by_count().
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
.../platforms/pseries/hotplug-memory.c | 28 +++++++++++++------
1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index 3c7ce5361ce3..ee88c1540fba 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -517,7 +517,6 @@ static int dlpar_memory_remove_by_index(u32 drc_index)
static int dlpar_memory_remove_by_ic(u32 lmbs_to_remove, u32 drc_index)
{
struct drmem_lmb *lmb, *start_lmb, *end_lmb;
- int lmbs_available = 0;
int rc;
pr_info("Attempting to hot-remove %u LMB(s) at %x\n",
@@ -530,18 +529,29 @@ static int dlpar_memory_remove_by_ic(u32 lmbs_to_remove, u32 drc_index)
if (rc)
return -EINVAL;
- /* Validate that there are enough LMBs to satisfy the request */
+ /*
+ * Validate that all LMBs in range are not reserved. Note that it
+ * is ok if they are !ASSIGNED since our goal here is to remove the
+ * LMB range, regardless of whether some LMBs were already removed
+ * by any other reason.
+ *
+ * This is a contrast to what is done in remove_by_count() where we
+ * check for both RESERVED and !ASSIGNED (via lmb_is_removable()),
+ * because we want to remove a fixed amount of LMBs in that function.
+ */
for_each_drmem_lmb_in_range(lmb, start_lmb, end_lmb) {
- if (lmb->flags & DRCONF_MEM_RESERVED)
- break;
-
- lmbs_available++;
+ if (lmb->flags & DRCONF_MEM_RESERVED) {
+ pr_err("Memory at %llx (drc index %x) is reserved\n",
+ lmb->base_addr, lmb->drc_index);
+ return -EINVAL;
+ }
}
- if (lmbs_available < lmbs_to_remove)
- return -EINVAL;
-
for_each_drmem_lmb_in_range(lmb, start_lmb, end_lmb) {
+ /*
+ * dlpar_remove_lmb() will error out if the LMB is already
+ * !ASSIGNED, but this case is a no-op for us.
+ */
if (!(lmb->flags & DRCONF_MEM_ASSIGNED))
continue;
--
2.31.1
^ permalink raw reply related
* Re: [PATCH 2/3] hotplug-memory.c: enhance dlpar_memory_remove* LMB checks
From: Daniel Henrique Barboza @ 2021-05-12 20:35 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <YJCdNJRSLCOV59dE@yekko>
On 5/3/21 10:02 PM, David Gibson wrote:
> On Fri, Apr 30, 2021 at 09:09:16AM -0300, Daniel Henrique Barboza wrote:
>> dlpar_memory_remove_by_ic() validates the amount of LMBs to be removed
>> by checking !DRCONF_MEM_RESERVED, and in the following loop before
>> dlpar_remove_lmb() a check for DRCONF_MEM_ASSIGNED is made before
>> removing it. This means that a LMB that is both !DRCONF_MEM_RESERVED and
>> !DRCONF_MEM_ASSIGNED will be counted as valid, but then not being
>> removed. The function will end up not removing all 'lmbs_to_remove'
>> LMBs while also not reporting any errors.
>>
>> Comparing it to dlpar_memory_remove_by_count(), the validation is done
>> via lmb_is_removable(), which checks for DRCONF_MEM_ASSIGNED and fadump
>> constraints. No additional check is made afterwards, and
>> DRCONF_MEM_RESERVED is never checked before dlpar_remove_lmb(). The
>> function doesn't have the same 'check A for validation, then B for
>> removal' issue as remove_by_ic(), but it's not checking if the LMB is
>> reserved.
>>
>> There is no reason for these functions to validate the same operation in
>> two different manners.
>
> Actually, I think there is: remove_by_ic() is handling a request to
> remove a specific range of LMBs. If any are reserved, they can't be
> removed and so this needs to fail. But if they are !ASSIGNED, that
> essentially means they're *already* removed (or never added), so
> "removing" them is, correctly, a no-op.
>
> remove_by_count(), in contrast, is being asked to remove a fixed
> number of LMBs from wherever they can be found, and for that it needs
> to find LMBs that haven't already been removed.
>
> Basically remove_by_ic() is an absolute request: "make this set of
> LMBs be not-plugged", whereas remove_by_count() is a relative request
> "make N less LMBs be plugged".
>
>
> So I think remove_by_ic()s existing handling is correct. I'm less
> sure if remove_by_count() ignoring RESERVED is correct - I couldn't
> quickly find under what circumstances RESERVED gets set.
RESERVED is never set by the kernel. It is written in the DT by the
firmware/hypervisor and the kernel just checks its value. QEMU sets it in
spapr_dt_dynamic_memory() with the following comment:
/*
* LMB information for RMA, boot time RAM and gap b/n RAM and
* device memory region -- all these are marked as reserved
* and as having no valid DRC.
*/
dynamic_memory[0] = cpu_to_be32(addr >> 32);
dynamic_memory[1] = cpu_to_be32(addr & 0xffffffff);
dynamic_memory[2] = cpu_to_be32(0);
dynamic_memory[3] = cpu_to_be32(0); /* reserved */
dynamic_memory[4] = cpu_to_be32(-1);
dynamic_memory[5] = cpu_to_be32(SPAPR_LMB_FLAGS_RESERVED |
SPAPR_LMB_FLAGS_DRC_INVALID);
The flag is formally described in LOPAR section 4.2.8, "Reserved Memory":
"Memory nodes marked with the special value of the “status” property of
“reserved” is not to be used or altered by the base OS."
This makes me confident that we should check DRCONF_MEM_RESERVED in
remove_by_count() as well, since phyp needs do adhere to these semantics and
shouldn't be able to remove a LMB marked as RESERVED.
Thanks,
Daniel
>
>
>> This patch addresses that by changing
>> lmb_is_removable() to also check for DRCONF_MEM_RESERVED to tell if a
>> lmb is removable, making dlpar_memory_remove_by_count() take the
>> reservation state into account when counting the LMBs.
>> lmb_is_removable() is then used in the validation step of
>> dlpar_memory_remove_by_ic(), which is already checking for both states
>> but in different stages, to avoid counting a LMB that is not assigned as
>> eligible for removal. We can then skip the check before
>> dlpar_remove_lmb() since we're validating all LMBs beforehand.
>>
>> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>> ---
>> arch/powerpc/platforms/pseries/hotplug-memory.c | 8 +++-----
>> 1 file changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
>> index bb98574a84a2..4e6d162c3f1a 100644
>> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
>> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
>> @@ -348,7 +348,8 @@ static int pseries_remove_mem_node(struct device_node *np)
>>
>> static bool lmb_is_removable(struct drmem_lmb *lmb)
>> {
>> - if (!(lmb->flags & DRCONF_MEM_ASSIGNED))
>> + if ((lmb->flags & DRCONF_MEM_RESERVED) ||
>> + !(lmb->flags & DRCONF_MEM_ASSIGNED))
>> return false;
>>
>> #ifdef CONFIG_FA_DUMP
>> @@ -523,7 +524,7 @@ static int dlpar_memory_remove_by_ic(u32 lmbs_to_remove, u32 drc_index)
>>
>> /* Validate that there are enough LMBs to satisfy the request */
>> for_each_drmem_lmb_in_range(lmb, start_lmb, end_lmb) {
>> - if (lmb->flags & DRCONF_MEM_RESERVED)
>> + if (!lmb_is_removable(lmb))
>> break;
>>
>> lmbs_available++;
>> @@ -533,9 +534,6 @@ static int dlpar_memory_remove_by_ic(u32 lmbs_to_remove, u32 drc_index)
>> return -EINVAL;
>>
>> for_each_drmem_lmb_in_range(lmb, start_lmb, end_lmb) {
>> - if (!(lmb->flags & DRCONF_MEM_ASSIGNED))
>> - continue;
>> -
>> rc = dlpar_remove_lmb(lmb);
>> if (rc)
>> break;
>
^ permalink raw reply
* Re: [PATCH 1/2] kprobes: Allow architectures to override optinsn page allocation
From: Masami Hiramatsu @ 2021-05-12 23:43 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-kernel, anil.s.keshavamurthy, clang-built-linux,
Paul Mackerras, mhiramat, naveen.n.rao, linuxppc-dev, davem
In-Reply-To: <472c597cc8cc8f74f037acf971f7799fb381a647.1620829724.git.christophe.leroy@csgroup.eu>
On Wed, 12 May 2021 14:29:26 +0000 (UTC)
Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
> Some architectures like powerpc require a non standard
> allocation of optinsn page, because module pages are
> too far from the kernel for direct branches.
>
> Define weak alloc_optinsn_page() and free_optinsn_page(), that
> fall back on alloc_insn_page() and free_insn_page() when not
> overriden by the architecture.
>
Looks good to me :)
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
> Suggested-by: Masami Hiramatsu <mhiramat@kernel.org>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> kernel/kprobes.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 745f08fdd7a6..8c0a6fdef771 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -321,11 +321,21 @@ int kprobe_cache_get_kallsym(struct kprobe_insn_cache *c, unsigned int *symnum,
> }
>
> #ifdef CONFIG_OPTPROBES
> +void __weak *alloc_optinsn_page(void)
> +{
> + return alloc_insn_page();
> +}
> +
> +void __weak free_optinsn_page(void *page)
> +{
> + free_insn_page(page);
> +}
> +
> /* For optimized_kprobe buffer */
> struct kprobe_insn_cache kprobe_optinsn_slots = {
> .mutex = __MUTEX_INITIALIZER(kprobe_optinsn_slots.mutex),
> - .alloc = alloc_insn_page,
> - .free = free_insn_page,
> + .alloc = alloc_optinsn_page,
> + .free = free_optinsn_page,
> .sym = KPROBE_OPTINSN_PAGE_SYM,
> .pages = LIST_HEAD_INIT(kprobe_optinsn_slots.pages),
> /* .insn_size is initialized later */
> --
> 2.25.0
>
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/kprobes: Replace ppc_optinsn by common optinsn
From: Masami Hiramatsu @ 2021-05-12 23:44 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-kernel, anil.s.keshavamurthy, clang-built-linux,
Paul Mackerras, mhiramat, naveen.n.rao, linuxppc-dev, davem
In-Reply-To: <46998c941d0a5664daaeb92998391aace015eddf.1620829724.git.christophe.leroy@csgroup.eu>
On Wed, 12 May 2021 14:29:27 +0000 (UTC)
Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
> Commit 51c9c0843993 ("powerpc/kprobes: Implement Optprobes")
> implemented a powerpc specific version of optinsn in order
> to workaround the 32Mb limitation for direct branches.
>
> Instead of implementing a dedicated powerpc version, use the
> common optinsn and override the allocation and freeing functions.
>
> This also indirectly remove the CLANG warning about
> is_kprobe_ppc_optinsn_slot() not being use, and the powerpc will
> now benefit from commit 5b485629ba0d ("kprobes, extable: Identify
> kprobes trampolines as kernel text area")
>
This looks good to me.
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
> Suggested-by: Masami Hiramatsu <mhiramat@kernel.org>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/kernel/optprobes.c | 23 +++++------------------
> 1 file changed, 5 insertions(+), 18 deletions(-)
>
> diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c
> index cdf87086fa33..a370190cd02a 100644
> --- a/arch/powerpc/kernel/optprobes.c
> +++ b/arch/powerpc/kernel/optprobes.c
> @@ -31,11 +31,9 @@
> #define TMPL_END_IDX \
> (optprobe_template_end - optprobe_template_entry)
>
> -DEFINE_INSN_CACHE_OPS(ppc_optinsn);
> -
> static bool insn_page_in_use;
>
> -static void *__ppc_alloc_insn_page(void)
> +void *alloc_optinsn_page(void)
> {
> if (insn_page_in_use)
> return NULL;
> @@ -43,20 +41,11 @@ static void *__ppc_alloc_insn_page(void)
> return &optinsn_slot;
> }
>
> -static void __ppc_free_insn_page(void *page __maybe_unused)
> +void free_optinsn_page(void *page)
> {
> insn_page_in_use = false;
> }
>
> -struct kprobe_insn_cache kprobe_ppc_optinsn_slots = {
> - .mutex = __MUTEX_INITIALIZER(kprobe_ppc_optinsn_slots.mutex),
> - .pages = LIST_HEAD_INIT(kprobe_ppc_optinsn_slots.pages),
> - /* insn_size initialized later */
> - .alloc = __ppc_alloc_insn_page,
> - .free = __ppc_free_insn_page,
> - .nr_garbage = 0,
> -};
> -
> /*
> * Check if we can optimize this probe. Returns NIP post-emulation if this can
> * be optimized and 0 otherwise.
> @@ -136,7 +125,7 @@ NOKPROBE_SYMBOL(optimized_callback);
> void arch_remove_optimized_kprobe(struct optimized_kprobe *op)
> {
> if (op->optinsn.insn) {
> - free_ppc_optinsn_slot(op->optinsn.insn, 1);
> + free_optinsn_slot(op->optinsn.insn, 1);
> op->optinsn.insn = NULL;
> }
> }
> @@ -203,14 +192,12 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
> unsigned long nip, size;
> int rc, i;
>
> - kprobe_ppc_optinsn_slots.insn_size = MAX_OPTINSN_SIZE;
> -
> nip = can_optimize(p);
> if (!nip)
> return -EILSEQ;
>
> /* Allocate instruction slot for detour buffer */
> - buff = get_ppc_optinsn_slot();
> + buff = get_optinsn_slot();
> if (!buff)
> return -ENOMEM;
>
> @@ -297,7 +284,7 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
> return 0;
>
> error:
> - free_ppc_optinsn_slot(buff, 0);
> + free_optinsn_slot(buff, 0);
> return -ERANGE;
>
> }
> --
> 2.25.0
>
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH 1/2] kprobes: Allow architectures to override optinsn page allocation
From: Masami Hiramatsu @ 2021-05-13 0:33 UTC (permalink / raw)
To: kernel test robot
Cc: kbuild-all, linux-kernel, anil.s.keshavamurthy, clang-built-linux,
Paul Mackerras, naveen.n.rao, linuxppc-dev, davem, mhiramat
In-Reply-To: <202105130218.8xHlehr7-lkp@intel.com>
On Thu, 13 May 2021 03:04:51 +0800
kernel test robot <lkp@intel.com> wrote:
> Hi Christophe,
>
> I love your patch! Perhaps something to improve:
>
> [auto build test WARNING on powerpc/next]
> [also build test WARNING on linus/master v5.13-rc1 next-20210512]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url: https://github.com/0day-ci/linux/commits/Christophe-Leroy/kprobes-Allow-architectures-to-override-optinsn-page-allocation/20210512-223121
> base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
> config: i386-randconfig-r012-20210512 (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> reproduce (this is a W=1 build):
> # https://github.com/0day-ci/linux/commit/2a1f135a9ce3c4d86d3bdefed561aa17760f430f
> git remote add linux-review https://github.com/0day-ci/linux
> git fetch --no-tags linux-review Christophe-Leroy/kprobes-Allow-architectures-to-override-optinsn-page-allocation/20210512-223121
> git checkout 2a1f135a9ce3c4d86d3bdefed561aa17760f430f
> # save the attached .config to linux build tree
> make W=1 W=1 ARCH=i386
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
> >> kernel/kprobes.c:324:14: warning: no previous prototype for 'alloc_optinsn_page' [-Wmissing-prototypes]
> 324 | void __weak *alloc_optinsn_page(void)
> | ^~~~~~~~~~~~~~~~~~
> >> kernel/kprobes.c:329:13: warning: no previous prototype for 'free_optinsn_page' [-Wmissing-prototypes]
> 329 | void __weak free_optinsn_page(void *page)
> | ^~~~~~~~~~~~~~~~~
Ah, we need a prototype for those in include/linux/kprobes.h
as same as alloc_insn_page() and free_insn_page().
Thank you,
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox