2021-09-16 13:39:53

by Imran Khan

[permalink] [raw]
Subject: [PATCH RESEND v3 0/3] lib, stackdepot: check stackdepot handle before accessing slabs

Changes in v3:
- Fixed build error [1], due to missing EXPORT_SYMBOL_GPL in patch-3

[1] https://patchwork.freedesktop.org/series/94696/#rev2

Changes in v2:
- Fixed compilation error [1] due to typo in patch-3 (stack_depot_print
used in place of stack_depot_snprint)
This compilation error appears with CONFIG_DRM_I915_DEBUG_RUNTIME_PM=y
and this was missed by my test config (x86_64_defconfig)

[1] https://patchwork.freedesktop.org/series/94696/

Original cover letter
------------------------------------------
This patch series consolidates the changes submitted and reviewed at [1]
and [2].
The patches at [1] and [2] were submitted separarely, but they have some
inter dependency (later patches were created on top of earlier ones).
As both sets are still under review, I have put them in a single
change set here, so that it can be reviewed/included together and also
to avoid automation build failures where git am fails because of absent
parent.

I have included Acked-by (from Vlastimil) and Reviewed-by (from Alexander)
tags obtained so far for these changes and have also addressed last review
comment from Vlastimil [3].

To summarize, the changes in this set are as follows:

PATCH-1: Checks validity of a stackdepot handle before proceeding
to access stackdepot slab/objects.

PATCH-2: Adds a helper in stackdepot, to allow users to print
stack entries just by specifying the stackdepot handle. It also
changes such users to use this new interface.

PATCH-3: Adds a helper in stackdepot, to allow users to print
stack entries into buffers just by specifying the stackdepot handle and
destination buffer. It also changes such users to use this new interface.

[1] https://lore.kernel.org/lkml/[email protected]/
[2] https://lore.kernel.org/lkml/[email protected]/
[3] https://lore.kernel.org/lkml/[email protected]/

Imran Khan (3):
lib, stackdepot: check stackdepot handle before accessing slabs.
lib, stackdepot: Add helper to print stack entries.
lib, stackdepot: Add helper to print stack entries into buffer.

drivers/gpu/drm/drm_dp_mst_topology.c | 5 +--
drivers/gpu/drm/drm_mm.c | 5 +--
drivers/gpu/drm/i915/i915_vma.c | 5 +--
drivers/gpu/drm/i915/intel_runtime_pm.c | 20 +++--------
include/linux/stackdepot.h | 5 +++
lib/stackdepot.c | 46 +++++++++++++++++++++++++
mm/kasan/report.c | 15 ++------
mm/page_owner.c | 18 +++-------
8 files changed, 67 insertions(+), 52 deletions(-)


base-commit: 70ced02f322fd5bde59e805e77b19c811950d165
--
2.30.2


2021-09-16 13:40:28

by Imran Khan

[permalink] [raw]
Subject: [PATCH RESEND v3 2/3] lib, stackdepot: Add helper to print stack entries.

To print a stack entries, users of stackdepot, first
use stack_depot_fetch to get a list of stack entries
and then use stack_trace_print to print this list.
Provide a helper in stackdepot to print stack entries
based on stackdepot handle.
Also change above mentioned users to use this helper.

Signed-off-by: Imran Khan <[email protected]>
Suggested-by: Vlastimil Babka <[email protected]>

Acked-by: Vlastimil Babka <[email protected]>
Reviewed-by: Alexander Potapenko <[email protected]>
---
include/linux/stackdepot.h | 2 ++
lib/stackdepot.c | 18 ++++++++++++++++++
mm/kasan/report.c | 15 +++------------
mm/page_owner.c | 13 ++++---------
4 files changed, 27 insertions(+), 21 deletions(-)

diff --git a/include/linux/stackdepot.h b/include/linux/stackdepot.h
index 6bb4bc1a5f54..d77a30543dd4 100644
--- a/include/linux/stackdepot.h
+++ b/include/linux/stackdepot.h
@@ -19,6 +19,8 @@ depot_stack_handle_t stack_depot_save(unsigned long *entries,
unsigned int stack_depot_fetch(depot_stack_handle_t handle,
unsigned long **entries);

+void stack_depot_print(depot_stack_handle_t stack);
+
unsigned int filter_irq_stacks(unsigned long *entries, unsigned int nr_entries);

#ifdef CONFIG_STACKDEPOT
diff --git a/lib/stackdepot.c b/lib/stackdepot.c
index 67439c082490..354fe1b62017 100644
--- a/lib/stackdepot.c
+++ b/lib/stackdepot.c
@@ -214,6 +214,24 @@ static inline struct stack_record *find_stack(struct stack_record *bucket,
return NULL;
}

+/**
+ * stack_depot_print - print stack entries from a depot
+ *
+ * @stack: Stack depot handle which was returned from
+ * stack_depot_save().
+ *
+ */
+void stack_depot_print(depot_stack_handle_t stack)
+{
+ unsigned long *entries;
+ unsigned int nr_entries;
+
+ nr_entries = stack_depot_fetch(stack, &entries);
+ if (nr_entries > 0)
+ stack_trace_print(entries, nr_entries, 0);
+}
+EXPORT_SYMBOL_GPL(stack_depot_print);
+
/**
* stack_depot_fetch - Fetch stack entries from a depot
*
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 884a950c7026..3239fd8f8747 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -132,20 +132,11 @@ static void end_report(unsigned long *flags, unsigned long addr)
kasan_enable_current();
}

-static void print_stack(depot_stack_handle_t stack)
-{
- unsigned long *entries;
- unsigned int nr_entries;
-
- nr_entries = stack_depot_fetch(stack, &entries);
- stack_trace_print(entries, nr_entries, 0);
-}
-
static void print_track(struct kasan_track *track, const char *prefix)
{
pr_err("%s by task %u:\n", prefix, track->pid);
if (track->stack) {
- print_stack(track->stack);
+ stack_depot_print(track->stack);
} else {
pr_err("(stack is not available)\n");
}
@@ -214,12 +205,12 @@ static void describe_object_stacks(struct kmem_cache *cache, void *object,
return;
if (alloc_meta->aux_stack[0]) {
pr_err("Last potentially related work creation:\n");
- print_stack(alloc_meta->aux_stack[0]);
+ stack_depot_print(alloc_meta->aux_stack[0]);
pr_err("\n");
}
if (alloc_meta->aux_stack[1]) {
pr_err("Second to last potentially related work creation:\n");
- print_stack(alloc_meta->aux_stack[1]);
+ stack_depot_print(alloc_meta->aux_stack[1]);
pr_err("\n");
}
#endif
diff --git a/mm/page_owner.c b/mm/page_owner.c
index d24ed221357c..7918770c2b2b 100644
--- a/mm/page_owner.c
+++ b/mm/page_owner.c
@@ -394,8 +394,6 @@ void __dump_page_owner(const struct page *page)
struct page_ext *page_ext = lookup_page_ext(page);
struct page_owner *page_owner;
depot_stack_handle_t handle;
- unsigned long *entries;
- unsigned int nr_entries;
gfp_t gfp_mask;
int mt;

@@ -423,20 +421,17 @@ void __dump_page_owner(const struct page *page)
page_owner->pid, page_owner->ts_nsec, page_owner->free_ts_nsec);

handle = READ_ONCE(page_owner->handle);
- if (!handle) {
+ if (!handle)
pr_alert("page_owner allocation stack trace missing\n");
- } else {
- nr_entries = stack_depot_fetch(handle, &entries);
- stack_trace_print(entries, nr_entries, 0);
- }
+ else
+ stack_depot_print(handle);

handle = READ_ONCE(page_owner->free_handle);
if (!handle) {
pr_alert("page_owner free stack trace missing\n");
} else {
- nr_entries = stack_depot_fetch(handle, &entries);
pr_alert("page last free stack trace:\n");
- stack_trace_print(entries, nr_entries, 0);
+ stack_depot_print(handle);
}

if (page_owner->last_migrate_reason != -1)
--
2.30.2

2021-09-16 18:00:25

by Imran Khan

[permalink] [raw]
Subject: [PATCH RESEND v3 3/3] lib, stackdepot: Add helper to print stack entries into buffer.

To print stack entries into a buffer, users of stackdepot,
first get a list of stack entries using stack_depot_fetch
and then print this list into a buffer using stack_trace_snprint.
Provide a helper in stackdepot for this purpose.
Also change above mentioned users to use this helper.

Signed-off-by: Imran Khan <[email protected]>
Suggested-by: Vlastimil Babka <[email protected]>

Acked-by: Vlastimil Babka <[email protected]>
---
drivers/gpu/drm/drm_dp_mst_topology.c | 5 +----
drivers/gpu/drm/drm_mm.c | 5 +----
drivers/gpu/drm/i915/i915_vma.c | 5 +----
drivers/gpu/drm/i915/intel_runtime_pm.c | 20 +++++---------------
include/linux/stackdepot.h | 3 +++
lib/stackdepot.c | 25 +++++++++++++++++++++++++
mm/page_owner.c | 5 +----
7 files changed, 37 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 86d13d6bc463..2d1adab9e360 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1668,13 +1668,10 @@ __dump_topology_ref_history(struct drm_dp_mst_topology_ref_history *history,
for (i = 0; i < history->len; i++) {
const struct drm_dp_mst_topology_ref_entry *entry =
&history->entries[i];
- ulong *entries;
- uint nr_entries;
u64 ts_nsec = entry->ts_nsec;
u32 rem_nsec = do_div(ts_nsec, 1000000000);

- nr_entries = stack_depot_fetch(entry->backtrace, &entries);
- stack_trace_snprint(buf, PAGE_SIZE, entries, nr_entries, 4);
+ stack_depot_snprint(entry->backtrace, buf, PAGE_SIZE, 4);

drm_printf(&p, " %d %ss (last at %5llu.%06u):\n%s",
entry->count,
diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
index 93d48a6f04ab..7d1c578388d3 100644
--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -118,8 +118,6 @@ static noinline void save_stack(struct drm_mm_node *node)
static void show_leaks(struct drm_mm *mm)
{
struct drm_mm_node *node;
- unsigned long *entries;
- unsigned int nr_entries;
char *buf;

buf = kmalloc(BUFSZ, GFP_KERNEL);
@@ -133,8 +131,7 @@ static void show_leaks(struct drm_mm *mm)
continue;
}

- nr_entries = stack_depot_fetch(node->stack, &entries);
- stack_trace_snprint(buf, BUFSZ, entries, nr_entries, 0);
+ stack_depot_snprint(node->stack, buf, BUFSZ, 0);
DRM_ERROR("node [%08llx + %08llx]: inserted at\n%s",
node->start, node->size, buf);
}
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 4b7fc4647e46..f2d9ed375109 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -56,8 +56,6 @@ void i915_vma_free(struct i915_vma *vma)

static void vma_print_allocator(struct i915_vma *vma, const char *reason)
{
- unsigned long *entries;
- unsigned int nr_entries;
char buf[512];

if (!vma->node.stack) {
@@ -66,8 +64,7 @@ static void vma_print_allocator(struct i915_vma *vma, const char *reason)
return;
}

- nr_entries = stack_depot_fetch(vma->node.stack, &entries);
- stack_trace_snprint(buf, sizeof(buf), entries, nr_entries, 0);
+ stack_depot_snprint(vma->node.stack, buf, sizeof(buf), 0);
DRM_DEBUG_DRIVER("vma.node [%08llx + %08llx] %s: inserted at %s\n",
vma->node.start, vma->node.size, reason, buf);
}
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index eaf7688f517d..0d85f3c5c526 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -65,16 +65,6 @@ static noinline depot_stack_handle_t __save_depot_stack(void)
return stack_depot_save(entries, n, GFP_NOWAIT | __GFP_NOWARN);
}

-static void __print_depot_stack(depot_stack_handle_t stack,
- char *buf, int sz, int indent)
-{
- unsigned long *entries;
- unsigned int nr_entries;
-
- nr_entries = stack_depot_fetch(stack, &entries);
- stack_trace_snprint(buf, sz, entries, nr_entries, indent);
-}
-
static void init_intel_runtime_pm_wakeref(struct intel_runtime_pm *rpm)
{
spin_lock_init(&rpm->debug.lock);
@@ -146,12 +136,12 @@ static void untrack_intel_runtime_pm_wakeref(struct intel_runtime_pm *rpm,
if (!buf)
return;

- __print_depot_stack(stack, buf, PAGE_SIZE, 2);
+ stack_depot_snprint(stack, buf, PAGE_SIZE, 2);
DRM_DEBUG_DRIVER("wakeref %x from\n%s", stack, buf);

stack = READ_ONCE(rpm->debug.last_release);
if (stack) {
- __print_depot_stack(stack, buf, PAGE_SIZE, 2);
+ stack_depot_snprint(stack, buf, PAGE_SIZE, 2);
DRM_DEBUG_DRIVER("wakeref last released at\n%s", buf);
}

@@ -183,12 +173,12 @@ __print_intel_runtime_pm_wakeref(struct drm_printer *p,
return;

if (dbg->last_acquire) {
- __print_depot_stack(dbg->last_acquire, buf, PAGE_SIZE, 2);
+ stack_depot_snprint(dbg->last_acquire, buf, PAGE_SIZE, 2);
drm_printf(p, "Wakeref last acquired:\n%s", buf);
}

if (dbg->last_release) {
- __print_depot_stack(dbg->last_release, buf, PAGE_SIZE, 2);
+ stack_depot_snprint(dbg->last_release, buf, PAGE_SIZE, 2);
drm_printf(p, "Wakeref last released:\n%s", buf);
}

@@ -203,7 +193,7 @@ __print_intel_runtime_pm_wakeref(struct drm_printer *p,
rep = 1;
while (i + 1 < dbg->count && dbg->owners[i + 1] == stack)
rep++, i++;
- __print_depot_stack(stack, buf, PAGE_SIZE, 2);
+ stack_depot_snprint(stack, buf, PAGE_SIZE, 2);
drm_printf(p, "Wakeref x%lu taken at:\n%s", rep, buf);
}

diff --git a/include/linux/stackdepot.h b/include/linux/stackdepot.h
index d77a30543dd4..88b0b4cc9906 100644
--- a/include/linux/stackdepot.h
+++ b/include/linux/stackdepot.h
@@ -19,6 +19,9 @@ depot_stack_handle_t stack_depot_save(unsigned long *entries,
unsigned int stack_depot_fetch(depot_stack_handle_t handle,
unsigned long **entries);

+int stack_depot_snprint(depot_stack_handle_t handle, char *buf, size_t size,
+ int spaces);
+
void stack_depot_print(depot_stack_handle_t stack);

unsigned int filter_irq_stacks(unsigned long *entries, unsigned int nr_entries);
diff --git a/lib/stackdepot.c b/lib/stackdepot.c
index 354fe1b62017..f953752aa885 100644
--- a/lib/stackdepot.c
+++ b/lib/stackdepot.c
@@ -214,6 +214,31 @@ static inline struct stack_record *find_stack(struct stack_record *bucket,
return NULL;
}

+/**
+ * stack_depot_snprint - print stack entries from a depot into a buffer
+ *
+ * @handle: Stack depot handle which was returned from
+ * stack_depot_save().
+ * @buf: Pointer to the print buffer
+ *
+ * @size: Size of the print buffer
+ *
+ * @spaces: Number of leading spaces to print
+ *
+ * Return: Number of bytes printed.
+ */
+int stack_depot_snprint(depot_stack_handle_t handle, char *buf, size_t size,
+ int spaces)
+{
+ unsigned long *entries;
+ unsigned int nr_entries;
+
+ nr_entries = stack_depot_fetch(handle, &entries);
+ return nr_entries ? stack_trace_snprint(buf, size, entries, nr_entries,
+ spaces) : 0;
+}
+EXPORT_SYMBOL_GPL(stack_depot_snprint);
+
/**
* stack_depot_print - print stack entries from a depot
*
diff --git a/mm/page_owner.c b/mm/page_owner.c
index 7918770c2b2b..a83f546c06b5 100644
--- a/mm/page_owner.c
+++ b/mm/page_owner.c
@@ -329,8 +329,6 @@ print_page_owner(char __user *buf, size_t count, unsigned long pfn,
depot_stack_handle_t handle)
{
int ret, pageblock_mt, page_mt;
- unsigned long *entries;
- unsigned int nr_entries;
char *kbuf;

count = min_t(size_t, count, PAGE_SIZE);
@@ -361,8 +359,7 @@ print_page_owner(char __user *buf, size_t count, unsigned long pfn,
if (ret >= count)
goto err;

- nr_entries = stack_depot_fetch(handle, &entries);
- ret += stack_trace_snprint(kbuf + ret, count - ret, entries, nr_entries, 0);
+ ret += stack_depot_snprint(handle, kbuf + ret, count - ret, 0);
if (ret >= count)
goto err;

--
2.30.2