2015-02-12 22:16:43

by Stefan Strogin

[permalink] [raw]
Subject: [PATCH 0/4] mm: cma: add some debug information for CMA

Hi all.

Sorry for the long delay. Here is the second attempt to add some facility
for debugging CMA (the first one was "mm: cma: add /proc/cmainfo" [1]).

This patch set is based on v3.19 and Sasha Levin's patch set
"mm: cma: debugfs access to CMA" [2].
It is also available on git:
git://github.com/stefanstrogin/cmainfo -b cmainfo-v2

We want an interface to see a list of currently allocated CMA buffers and
some useful information about them (like /proc/vmallocinfo but for physically
contiguous buffers allocated with CMA).

Here is an example use case when we need it. We want a big (megabytes)
CMA buffer to be allocated in runtime in default CMA region. If someone
already uses CMA then the big allocation can fail. If it happens then with
such an interface we could find who used CMA at the moment of failure, who
caused fragmentation (possibly ftrace also would be helpful here) and so on.

These patches add some files to debugfs when CONFIG_CMA_DEBUGFS is enabled.

/sys/kernel/debug/cma/cma-<N>/buffers contains a list of currently allocated
CMA buffers for each CMA region. Stacktrace saved at the moment of allocation
is used to see who and whence allocated each buffer [3].

cma/cma-<N>/used and cma/cma-<N>/maxchunk are added to show used size and
the biggest free chunk in each CMA region.

Also added trace events for cma_alloc() and cma_release().

Changes from "mm: cma: add /proc/cmainfo" [1]:
- Rebased on v3.19 and Sasha Levin's patch set [2].
- Moved debug code from cma.c to cma_debug.c.
- Moved cmainfo to debugfs and splited it by CMA region.
- Splited 'cmainfo' into 'buffers', 'used' and 'maxchunk'.
- Used CONFIG_CMA_DEBUGFS instead of CONFIG_CMA_DEBUG.
- Added trace events for cma_alloc() and cma_release().
- Don't use seq_files.
- A small change of debug output in cma_release().
- cma_buffer_list_del() now supports releasing chunks which ranges don't match
allocations. E.g. we have buffer1: [0x0, 0x1], buffer2: [0x2, 0x3], then
cma_buffer_list_del(cma, 0x1 /*or 0x0*/, 1 /*(or 2 or 3)*/) should work.
- Various small changes.


[1] https://lkml.org/lkml/2014/12/26/95

[2] https://lkml.org/lkml/2015/1/28/755

[3] E.g.
root@debian:/sys/kernel/debug/cma# cat cma-0/buffers
0x2f400000 - 0x2f417000 (92 kB), allocated by pid 1 (swapper/0)
[<c1142c4b>] cma_alloc+0x1bb/0x200
[<c143d28a>] dma_alloc_from_contiguous+0x3a/0x40
[<c10079d9>] dma_generic_alloc_coherent+0x89/0x160
[<c14456ce>] dmam_alloc_coherent+0xbe/0x100
[<c1487312>] ahci_port_start+0xe2/0x210
[<c146e0e0>] ata_host_start.part.28+0xc0/0x1a0
[<c1473650>] ata_host_activate+0xd0/0x110
[<c14881bf>] ahci_host_activate+0x3f/0x170
[<c14854e4>] ahci_init_one+0x764/0xab0
[<c12e415f>] pci_device_probe+0x6f/0xd0
[<c14378a8>] driver_probe_device+0x68/0x210
[<c1437b09>] __driver_attach+0x79/0x80
[<c1435eef>] bus_for_each_dev+0x4f/0x80
[<c143749e>] driver_attach+0x1e/0x20
[<c1437197>] bus_add_driver+0x157/0x200
[<c14381bd>] driver_register+0x5d/0xf0
<...>
0x2f41b000 - 0x2f41c000 (4 kB), allocated by pid 1264 (NetworkManager)
[<c1142c4b>] cma_alloc+0x1bb/0x200
[<c143d28a>] dma_alloc_from_contiguous+0x3a/0x40
[<c10079d9>] dma_generic_alloc_coherent+0x89/0x160
[<c14c5d13>] e1000_setup_all_tx_resources+0x93/0x540
[<c14c8021>] e1000_open+0x31/0x120
[<c16264cf>] __dev_open+0x9f/0x130
[<c16267ce>] __dev_change_flags+0x8e/0x150
[<c16268b8>] dev_change_flags+0x28/0x60
[<c1633ee0>] do_setlink+0x2a0/0x760
[<c1634acb>] rtnl_newlink+0x60b/0x7b0
[<c16314f4>] rtnetlink_rcv_msg+0x84/0x1f0
[<c164b58e>] netlink_rcv_skb+0x8e/0xb0
[<c1631461>] rtnetlink_rcv+0x21/0x30
[<c164af7a>] netlink_unicast+0x13a/0x1d0
[<c164b250>] netlink_sendmsg+0x240/0x3e0
[<c160cbfd>] do_sock_sendmsg+0xbd/0xe0
<...>


Dmitry Safonov (1):
mm: cma: add functions to get region pages counters

Stefan Strogin (3):
mm: cma: add currently allocated CMA buffers list to debugfs
mm: cma: add number of pages to debug message in cma_release()
mm: cma: add trace events to debug physically-contiguous memory
allocations

include/linux/cma.h | 11 +++
include/trace/events/cma.h | 57 +++++++++++++++
mm/cma.c | 46 +++++++++++-
mm/cma.h | 16 +++++
mm/cma_debug.c | 169 ++++++++++++++++++++++++++++++++++++++++++++-
5 files changed, 297 insertions(+), 2 deletions(-)
create mode 100644 include/trace/events/cma.h

--
2.1.0


2015-02-12 22:16:51

by Stefan Strogin

[permalink] [raw]
Subject: [PATCH 1/4] mm: cma: add currently allocated CMA buffers list to debugfs

/sys/kernel/debug/cma/cma-<N>/buffers contains a list of currently allocated
CMA buffers for CMA region N when CONFIG_CMA_DEBUGFS is enabled.

Format is:

<base_phys_addr> - <end_phys_addr> (<size> kB), allocated by <PID> (<comm>)
<stack backtrace when the buffer had been allocated>

Signed-off-by: Stefan Strogin <[email protected]>
---
include/linux/cma.h | 9 ++++
mm/cma.c | 9 ++++
mm/cma.h | 16 ++++++
mm/cma_debug.c | 145 +++++++++++++++++++++++++++++++++++++++++++++++++++-
4 files changed, 178 insertions(+), 1 deletion(-)

diff --git a/include/linux/cma.h b/include/linux/cma.h
index 9384ba6..4c2c83c 100644
--- a/include/linux/cma.h
+++ b/include/linux/cma.h
@@ -28,4 +28,13 @@ extern int cma_init_reserved_mem(phys_addr_t base,
struct cma **res_cma);
extern struct page *cma_alloc(struct cma *cma, int count, unsigned int align);
extern bool cma_release(struct cma *cma, struct page *pages, int count);
+
+#ifdef CONFIG_CMA_DEBUGFS
+extern int cma_buffer_list_add(struct cma *cma, unsigned long pfn, int count);
+extern void cma_buffer_list_del(struct cma *cma, unsigned long pfn, int count);
+#else
+#define cma_buffer_list_add(cma, pfn, count) { }
+#define cma_buffer_list_del(cma, pfn, count) { }
+#endif
+
#endif
diff --git a/mm/cma.c b/mm/cma.c
index 2609e20..ed269b0 100644
--- a/mm/cma.c
+++ b/mm/cma.c
@@ -34,6 +34,9 @@
#include <linux/cma.h>
#include <linux/highmem.h>
#include <linux/io.h>
+#include <linux/list.h>
+#include <linux/proc_fs.h>
+#include <linux/time.h>

#include "cma.h"

@@ -125,6 +128,8 @@ static int __init cma_activate_area(struct cma *cma)
#ifdef CONFIG_CMA_DEBUGFS
INIT_HLIST_HEAD(&cma->mem_head);
spin_lock_init(&cma->mem_head_lock);
+ INIT_LIST_HEAD(&cma->buffers_list);
+ mutex_init(&cma->list_lock);
#endif

return 0;
@@ -408,6 +413,9 @@ struct page *cma_alloc(struct cma *cma, int count, unsigned int align)
start = bitmap_no + mask + 1;
}

+ if (page)
+ cma_buffer_list_add(cma, pfn, count);
+
pr_debug("%s(): returned %p\n", __func__, page);
return page;
}
@@ -440,6 +448,7 @@ bool cma_release(struct cma *cma, struct page *pages, int count)

free_contig_range(pfn, count);
cma_clear_bitmap(cma, pfn, count);
+ cma_buffer_list_del(cma, pfn, count);

return true;
}
diff --git a/mm/cma.h b/mm/cma.h
index 1132d73..98e5f79 100644
--- a/mm/cma.h
+++ b/mm/cma.h
@@ -1,6 +1,8 @@
#ifndef __MM_CMA_H__
#define __MM_CMA_H__

+#include <linux/sched.h>
+
struct cma {
unsigned long base_pfn;
unsigned long count;
@@ -10,9 +12,23 @@ struct cma {
#ifdef CONFIG_CMA_DEBUGFS
struct hlist_head mem_head;
spinlock_t mem_head_lock;
+ struct list_head buffers_list;
+ struct mutex list_lock;
#endif
};

+#ifdef CONFIG_CMA_DEBUGFS
+struct cma_buffer {
+ unsigned long pfn;
+ unsigned long count;
+ pid_t pid;
+ char comm[TASK_COMM_LEN];
+ unsigned long trace_entries[16];
+ unsigned int nr_entries;
+ struct list_head list;
+};
+#endif
+
extern struct cma cma_areas[MAX_CMA_AREAS];
extern unsigned cma_area_count;

diff --git a/mm/cma_debug.c b/mm/cma_debug.c
index 7e1d325..5acd937 100644
--- a/mm/cma_debug.c
+++ b/mm/cma_debug.c
@@ -2,6 +2,7 @@
* CMA DebugFS Interface
*
* Copyright (c) 2015 Sasha Levin <[email protected]>
+ * Copyright (c) 2015 Stefan Strogin <[email protected]>
*/


@@ -10,6 +11,8 @@
#include <linux/list.h>
#include <linux/kernel.h>
#include <linux/slab.h>
+#include <linux/mm_types.h>
+#include <linux/stacktrace.h>

#include "cma.h"

@@ -21,6 +24,99 @@ struct cma_mem {

static struct dentry *cma_debugfs_root;

+/* Must be called under cma->list_lock */
+static int __cma_buffer_list_add(struct cma *cma, unsigned long pfn, int count)
+{
+ struct cma_buffer *cmabuf;
+ struct stack_trace trace;
+
+ cmabuf = kmalloc(sizeof(*cmabuf), GFP_KERNEL);
+ if (!cmabuf) {
+ pr_warn("%s(page %p, count %d): failed to allocate buffer list entry\n",
+ __func__, pfn_to_page(pfn), count);
+ return -ENOMEM;
+ }
+
+ trace.nr_entries = 0;
+ trace.max_entries = ARRAY_SIZE(cmabuf->trace_entries);
+ trace.entries = &cmabuf->trace_entries[0];
+ trace.skip = 2;
+ save_stack_trace(&trace);
+
+ cmabuf->pfn = pfn;
+ cmabuf->count = count;
+ cmabuf->pid = task_pid_nr(current);
+ cmabuf->nr_entries = trace.nr_entries;
+ get_task_comm(cmabuf->comm, current);
+
+ list_add_tail(&cmabuf->list, &cma->buffers_list);
+
+ return 0;
+}
+
+/**
+ * cma_buffer_list_add() - add a new entry to a list of allocated buffers
+ * @cma: Contiguous memory region for which the allocation is performed.
+ * @pfn: Base PFN of the allocated buffer.
+ * @count: Number of allocated pages.
+ *
+ * This function adds a new entry to the list of allocated contiguous memory
+ * buffers in a CMA area. It uses the CMA area specificated by the device
+ * if available or the default global one otherwise.
+ */
+int cma_buffer_list_add(struct cma *cma, unsigned long pfn, int count)
+{
+ int ret;
+
+ mutex_lock(&cma->list_lock);
+ ret = __cma_buffer_list_add(cma, pfn, count);
+ mutex_unlock(&cma->list_lock);
+
+ return ret;
+}
+
+/**
+ * cma_buffer_list_del() - delete an entry from a list of allocated buffers
+ * @cma: Contiguous memory region for which the allocation was performed.
+ * @pfn: Base PFN of the released buffer.
+ * @count: Number of pages.
+ *
+ * This function deletes a list entry added by cma_buffer_list_add().
+ */
+void cma_buffer_list_del(struct cma *cma, unsigned long pfn, int count)
+{
+ struct cma_buffer *cmabuf, *tmp;
+ int found = 0;
+ unsigned long buf_end_pfn, free_end_pfn = pfn + count;
+
+ mutex_lock(&cma->list_lock);
+ list_for_each_entry_safe(cmabuf, tmp, &cma->buffers_list, list) {
+
+ buf_end_pfn = cmabuf->pfn + cmabuf->count;
+ if (pfn <= cmabuf->pfn && free_end_pfn >= buf_end_pfn) {
+ list_del(&cmabuf->list);
+ kfree(cmabuf);
+ found = 1;
+ } else if (pfn <= cmabuf->pfn && free_end_pfn < buf_end_pfn) {
+ cmabuf->count -= free_end_pfn - cmabuf->pfn;
+ cmabuf->pfn = free_end_pfn;
+ found = 1;
+ } else if (pfn > cmabuf->pfn && pfn < buf_end_pfn) {
+ if (free_end_pfn < buf_end_pfn)
+ __cma_buffer_list_add(cma, free_end_pfn,
+ buf_end_pfn - free_end_pfn);
+ cmabuf->count = pfn - cmabuf->pfn;
+ found = 1;
+ }
+ }
+ mutex_unlock(&cma->list_lock);
+
+ if (!found)
+ pr_err("%s(page %p, count %d): couldn't find buffer list entry\n",
+ __func__, pfn_to_page(pfn), count);
+
+}
+
static int cma_debugfs_get(void *data, u64 *val)
{
unsigned long *p = data;
@@ -125,6 +221,52 @@ static int cma_alloc_write(void *data, u64 val)

DEFINE_SIMPLE_ATTRIBUTE(cma_alloc_fops, NULL, cma_alloc_write, "%llu\n");

+static int cma_buffers_read(struct file *file, char __user *userbuf,
+ size_t count, loff_t *ppos)
+{
+ struct cma *cma = file->private_data;
+ struct cma_buffer *cmabuf;
+ struct stack_trace trace;
+ char *buf;
+ int ret, n = 0;
+
+ if (*ppos < 0 || !count)
+ return -EINVAL;
+
+ buf = kmalloc(count, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ mutex_lock(&cma->list_lock);
+ list_for_each_entry(cmabuf, &cma->buffers_list, list) {
+ n += snprintf(buf + n, count - n,
+ "0x%llx - 0x%llx (%lu kB), allocated by pid %u (%s)\n",
+ (unsigned long long)PFN_PHYS(cmabuf->pfn),
+ (unsigned long long)PFN_PHYS(cmabuf->pfn +
+ cmabuf->count),
+ (cmabuf->count * PAGE_SIZE) >> 10, cmabuf->pid,
+ cmabuf->comm);
+
+ trace.nr_entries = cmabuf->nr_entries;
+ trace.entries = &cmabuf->trace_entries[0];
+
+ n += snprint_stack_trace(buf + n, count - n, &trace, 0);
+ n += snprintf(buf + n, count - n, "\n");
+ }
+ mutex_unlock(&cma->list_lock);
+
+ ret = simple_read_from_buffer(userbuf, count, ppos, buf, n);
+ kfree(buf);
+
+ return ret;
+}
+
+static const struct file_operations cma_buffers_fops = {
+ .open = simple_open,
+ .read = cma_buffers_read,
+ .llseek = default_llseek,
+};
+
static void cma_debugfs_add_one(struct cma *cma, int idx)
{
struct dentry *tmp;
@@ -148,6 +290,8 @@ static void cma_debugfs_add_one(struct cma *cma, int idx)
debugfs_create_file("order_per_bit", S_IRUGO, tmp,
&cma->order_per_bit, &cma_debugfs_fops);

+ debugfs_create_file("buffers", S_IRUGO, tmp, cma, &cma_buffers_fops);
+
u32s = DIV_ROUND_UP(cma_bitmap_maxno(cma), BITS_PER_BYTE * sizeof(u32));
debugfs_create_u32_array("bitmap", S_IRUGO, tmp, (u32*)cma->bitmap, u32s);
}
@@ -166,4 +310,3 @@ static int __init cma_debugfs_init(void)
return 0;
}
late_initcall(cma_debugfs_init);
-
--
2.1.0

2015-02-12 22:16:58

by Stefan Strogin

[permalink] [raw]
Subject: [PATCH 2/4] mm: cma: add functions to get region pages counters

From: Dmitry Safonov <[email protected]>

Here are two functions that provide interface to compute/get used size
and size of biggest free chunk in cma region.
Add that information to debugfs.

Signed-off-by: Dmitry Safonov <[email protected]>
Signed-off-by: Stefan Strogin <[email protected]>
---
include/linux/cma.h | 2 ++
mm/cma.c | 30 ++++++++++++++++++++++++++++++
mm/cma_debug.c | 24 ++++++++++++++++++++++++
3 files changed, 56 insertions(+)

diff --git a/include/linux/cma.h b/include/linux/cma.h
index 4c2c83c..54a2c4d 100644
--- a/include/linux/cma.h
+++ b/include/linux/cma.h
@@ -18,6 +18,8 @@ struct cma;
extern unsigned long totalcma_pages;
extern phys_addr_t cma_get_base(struct cma *cma);
extern unsigned long cma_get_size(struct cma *cma);
+extern unsigned long cma_get_used(struct cma *cma);
+extern unsigned long cma_get_maxchunk(struct cma *cma);

extern int __init cma_declare_contiguous(phys_addr_t base,
phys_addr_t size, phys_addr_t limit,
diff --git a/mm/cma.c b/mm/cma.c
index ed269b0..95e8121 100644
--- a/mm/cma.c
+++ b/mm/cma.c
@@ -54,6 +54,36 @@ unsigned long cma_get_size(struct cma *cma)
return cma->count << PAGE_SHIFT;
}

+unsigned long cma_get_used(struct cma *cma)
+{
+ unsigned long ret = 0;
+
+ mutex_lock(&cma->lock);
+ /* pages counter is smaller than sizeof(int) */
+ ret = bitmap_weight(cma->bitmap, (int)cma->count);
+ mutex_unlock(&cma->lock);
+
+ return ret;
+}
+
+unsigned long cma_get_maxchunk(struct cma *cma)
+{
+ unsigned long maxchunk = 0;
+ unsigned long start, end = 0;
+
+ mutex_lock(&cma->lock);
+ for (;;) {
+ start = find_next_zero_bit(cma->bitmap, cma->count, end);
+ if (start >= cma->count)
+ break;
+ end = find_next_bit(cma->bitmap, cma->count, start);
+ maxchunk = max(end - start, maxchunk);
+ }
+ mutex_unlock(&cma->lock);
+
+ return maxchunk;
+}
+
static unsigned long cma_bitmap_aligned_mask(struct cma *cma, int align_order)
{
if (align_order <= cma->order_per_bit)
diff --git a/mm/cma_debug.c b/mm/cma_debug.c
index 5acd937..9705e86 100644
--- a/mm/cma_debug.c
+++ b/mm/cma_debug.c
@@ -128,6 +128,28 @@ static int cma_debugfs_get(void *data, u64 *val)

DEFINE_SIMPLE_ATTRIBUTE(cma_debugfs_fops, cma_debugfs_get, NULL, "%llu\n");

+static int cma_used_get(void *data, u64 *val)
+{
+ struct cma *cma = data;
+
+ *val = cma_get_used(cma);
+
+ return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(cma_used_fops, cma_used_get, NULL, "%llu\n");
+
+static int cma_maxchunk_get(void *data, u64 *val)
+{
+ struct cma *cma = data;
+
+ *val = cma_get_maxchunk(cma);
+
+ return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(cma_maxchunk_fops, cma_maxchunk_get, NULL, "%llu\n");
+
static void cma_add_to_cma_mem_list(struct cma *cma, struct cma_mem *mem)
{
spin_lock(&cma->mem_head_lock);
@@ -289,6 +311,8 @@ static void cma_debugfs_add_one(struct cma *cma, int idx)
&cma->count, &cma_debugfs_fops);
debugfs_create_file("order_per_bit", S_IRUGO, tmp,
&cma->order_per_bit, &cma_debugfs_fops);
+ debugfs_create_file("used", S_IRUGO, tmp, cma, &cma_used_fops);
+ debugfs_create_file("maxchunk", S_IRUGO, tmp, cma, &cma_maxchunk_fops);

debugfs_create_file("buffers", S_IRUGO, tmp, cma, &cma_buffers_fops);

--
2.1.0

2015-02-12 22:17:05

by Stefan Strogin

[permalink] [raw]
Subject: [PATCH 3/4] mm: cma: add number of pages to debug message in cma_release()

It's more useful to print address and number of pages which are being released,
not olny address.

Signed-off-by: Stefan Strogin <[email protected]>
---
mm/cma.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/cma.c b/mm/cma.c
index 95e8121..c68d383 100644
--- a/mm/cma.c
+++ b/mm/cma.c
@@ -467,7 +467,7 @@ bool cma_release(struct cma *cma, struct page *pages, int count)
if (!cma || !pages)
return false;

- pr_debug("%s(page %p)\n", __func__, (void *)pages);
+ pr_debug("%s(page %p, count %d)\n", __func__, (void *)pages, count);

pfn = page_to_pfn(pages);

--
2.1.0

2015-02-12 22:17:11

by Stefan Strogin

[permalink] [raw]
Subject: [PATCH 4/4] mm: cma: add trace events to debug physically-contiguous memory allocations

Add trace events for cma_alloc() and cma_release().

Signed-off-by: Stefan Strogin <[email protected]>
---
include/trace/events/cma.h | 57 ++++++++++++++++++++++++++++++++++++++++++++++
mm/cma.c | 7 +++++-
2 files changed, 63 insertions(+), 1 deletion(-)
create mode 100644 include/trace/events/cma.h

diff --git a/include/trace/events/cma.h b/include/trace/events/cma.h
new file mode 100644
index 0000000..3fe7a56
--- /dev/null
+++ b/include/trace/events/cma.h
@@ -0,0 +1,57 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM cma
+
+#if !defined(_TRACE_CMA_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_CMA_H
+
+#include <linux/types.h>
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(cma_alloc,
+
+ TP_PROTO(struct cma *cma, unsigned long pfn, int count),
+
+ TP_ARGS(cma, pfn, count),
+
+ TP_STRUCT__entry(
+ __field(unsigned long, pfn)
+ __field(unsigned long, count)
+ ),
+
+ TP_fast_assign(
+ __entry->pfn = pfn;
+ __entry->count = count;
+ ),
+
+ TP_printk("pfn=%lu page=%p count=%lu\n",
+ __entry->pfn,
+ pfn_to_page(__entry->pfn),
+ __entry->count)
+);
+
+TRACE_EVENT(cma_release,
+
+ TP_PROTO(struct cma *cma, unsigned long pfn, int count),
+
+ TP_ARGS(cma, pfn, count),
+
+ TP_STRUCT__entry(
+ __field(unsigned long, pfn)
+ __field(unsigned long, count)
+ ),
+
+ TP_fast_assign(
+ __entry->pfn = pfn;
+ __entry->count = count;
+ ),
+
+ TP_printk("pfn=%lu page=%p count=%lu\n",
+ __entry->pfn,
+ pfn_to_page(__entry->pfn),
+ __entry->count)
+);
+
+#endif /* _TRACE_CMA_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
diff --git a/mm/cma.c b/mm/cma.c
index c68d383..a7bd7f0 100644
--- a/mm/cma.c
+++ b/mm/cma.c
@@ -23,6 +23,7 @@
# define DEBUG
#endif
#endif
+#define CREATE_TRACE_POINTS

#include <linux/memblock.h>
#include <linux/err.h>
@@ -37,6 +38,7 @@
#include <linux/list.h>
#include <linux/proc_fs.h>
#include <linux/time.h>
+#include <trace/events/cma.h>

#include "cma.h"

@@ -443,8 +445,10 @@ struct page *cma_alloc(struct cma *cma, int count, unsigned int align)
start = bitmap_no + mask + 1;
}

- if (page)
+ if (page) {
cma_buffer_list_add(cma, pfn, count);
+ trace_cma_alloc(cma, pfn, count);
+ }

pr_debug("%s(): returned %p\n", __func__, page);
return page;
@@ -478,6 +482,7 @@ bool cma_release(struct cma *cma, struct page *pages, int count)

free_contig_range(pfn, count);
cma_clear_bitmap(cma, pfn, count);
+ trace_cma_release(cma, pfn, count);
cma_buffer_list_del(cma, pfn, count);

return true;
--
2.1.0

2015-02-13 03:00:53

by Joonsoo Kim

[permalink] [raw]
Subject: Re: [PATCH 0/4] mm: cma: add some debug information for CMA

On Fri, Feb 13, 2015 at 01:15:40AM +0300, Stefan Strogin wrote:
> Hi all.
>
> Sorry for the long delay. Here is the second attempt to add some facility
> for debugging CMA (the first one was "mm: cma: add /proc/cmainfo" [1]).
>
> This patch set is based on v3.19 and Sasha Levin's patch set
> "mm: cma: debugfs access to CMA" [2].
> It is also available on git:
> git://github.com/stefanstrogin/cmainfo -b cmainfo-v2
>
> We want an interface to see a list of currently allocated CMA buffers and
> some useful information about them (like /proc/vmallocinfo but for physically
> contiguous buffers allocated with CMA).
>
> Here is an example use case when we need it. We want a big (megabytes)
> CMA buffer to be allocated in runtime in default CMA region. If someone
> already uses CMA then the big allocation can fail. If it happens then with
> such an interface we could find who used CMA at the moment of failure, who
> caused fragmentation (possibly ftrace also would be helpful here) and so on.

Hello,

So, I'm not sure that information about allocated CMA buffer is really
needed to solve your problem. You just want to know who uses default CMA
region and you can know it by adding tracepoint in your 4/4 patch. We
really need this custom allocation tracer? What can we do more with
this custom tracer to solve your problem? Could you more specific
about your problem and how to solve it by using this custom tracer?

>
> These patches add some files to debugfs when CONFIG_CMA_DEBUGFS is enabled.

If this tracer is justifiable, I think that making it conditional is
better than just enabling always on CONFIG_CMA_DEBUGFS. Some users
don't want to this feature although they enable CONFIG_CMA_DEBUGFS.

Thanks.

>
> /sys/kernel/debug/cma/cma-<N>/buffers contains a list of currently allocated
> CMA buffers for each CMA region. Stacktrace saved at the moment of allocation
> is used to see who and whence allocated each buffer [3].
>
> cma/cma-<N>/used and cma/cma-<N>/maxchunk are added to show used size and
> the biggest free chunk in each CMA region.
>
> Also added trace events for cma_alloc() and cma_release().
>
> Changes from "mm: cma: add /proc/cmainfo" [1]:
> - Rebased on v3.19 and Sasha Levin's patch set [2].
> - Moved debug code from cma.c to cma_debug.c.
> - Moved cmainfo to debugfs and splited it by CMA region.
> - Splited 'cmainfo' into 'buffers', 'used' and 'maxchunk'.
> - Used CONFIG_CMA_DEBUGFS instead of CONFIG_CMA_DEBUG.
> - Added trace events for cma_alloc() and cma_release().
> - Don't use seq_files.
> - A small change of debug output in cma_release().
> - cma_buffer_list_del() now supports releasing chunks which ranges don't match
> allocations. E.g. we have buffer1: [0x0, 0x1], buffer2: [0x2, 0x3], then
> cma_buffer_list_del(cma, 0x1 /*or 0x0*/, 1 /*(or 2 or 3)*/) should work.
> - Various small changes.
>
>
> [1] https://lkml.org/lkml/2014/12/26/95
>
> [2] https://lkml.org/lkml/2015/1/28/755
>
> [3] E.g.
> root@debian:/sys/kernel/debug/cma# cat cma-0/buffers
> 0x2f400000 - 0x2f417000 (92 kB), allocated by pid 1 (swapper/0)
> [<c1142c4b>] cma_alloc+0x1bb/0x200
> [<c143d28a>] dma_alloc_from_contiguous+0x3a/0x40
> [<c10079d9>] dma_generic_alloc_coherent+0x89/0x160
> [<c14456ce>] dmam_alloc_coherent+0xbe/0x100
> [<c1487312>] ahci_port_start+0xe2/0x210
> [<c146e0e0>] ata_host_start.part.28+0xc0/0x1a0
> [<c1473650>] ata_host_activate+0xd0/0x110
> [<c14881bf>] ahci_host_activate+0x3f/0x170
> [<c14854e4>] ahci_init_one+0x764/0xab0
> [<c12e415f>] pci_device_probe+0x6f/0xd0
> [<c14378a8>] driver_probe_device+0x68/0x210
> [<c1437b09>] __driver_attach+0x79/0x80
> [<c1435eef>] bus_for_each_dev+0x4f/0x80
> [<c143749e>] driver_attach+0x1e/0x20
> [<c1437197>] bus_add_driver+0x157/0x200
> [<c14381bd>] driver_register+0x5d/0xf0
> <...>
> 0x2f41b000 - 0x2f41c000 (4 kB), allocated by pid 1264 (NetworkManager)
> [<c1142c4b>] cma_alloc+0x1bb/0x200
> [<c143d28a>] dma_alloc_from_contiguous+0x3a/0x40
> [<c10079d9>] dma_generic_alloc_coherent+0x89/0x160
> [<c14c5d13>] e1000_setup_all_tx_resources+0x93/0x540
> [<c14c8021>] e1000_open+0x31/0x120
> [<c16264cf>] __dev_open+0x9f/0x130
> [<c16267ce>] __dev_change_flags+0x8e/0x150
> [<c16268b8>] dev_change_flags+0x28/0x60
> [<c1633ee0>] do_setlink+0x2a0/0x760
> [<c1634acb>] rtnl_newlink+0x60b/0x7b0
> [<c16314f4>] rtnetlink_rcv_msg+0x84/0x1f0
> [<c164b58e>] netlink_rcv_skb+0x8e/0xb0
> [<c1631461>] rtnetlink_rcv+0x21/0x30
> [<c164af7a>] netlink_unicast+0x13a/0x1d0
> [<c164b250>] netlink_sendmsg+0x240/0x3e0
> [<c160cbfd>] do_sock_sendmsg+0xbd/0xe0
> <...>
>
>
> Dmitry Safonov (1):
> mm: cma: add functions to get region pages counters
>
> Stefan Strogin (3):
> mm: cma: add currently allocated CMA buffers list to debugfs
> mm: cma: add number of pages to debug message in cma_release()
> mm: cma: add trace events to debug physically-contiguous memory
> allocations
>
> include/linux/cma.h | 11 +++
> include/trace/events/cma.h | 57 +++++++++++++++
> mm/cma.c | 46 +++++++++++-
> mm/cma.h | 16 +++++
> mm/cma_debug.c | 169 ++++++++++++++++++++++++++++++++++++++++++++-
> 5 files changed, 297 insertions(+), 2 deletions(-)
> create mode 100644 include/trace/events/cma.h
>
> --
> 2.1.0
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to [email protected]. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

2015-02-13 03:07:56

by Joonsoo Kim

[permalink] [raw]
Subject: Re: [PATCH 1/4] mm: cma: add currently allocated CMA buffers list to debugfs

On Fri, Feb 13, 2015 at 01:15:41AM +0300, Stefan Strogin wrote:
> /sys/kernel/debug/cma/cma-<N>/buffers contains a list of currently allocated
> CMA buffers for CMA region N when CONFIG_CMA_DEBUGFS is enabled.
>
> Format is:
>
> <base_phys_addr> - <end_phys_addr> (<size> kB), allocated by <PID> (<comm>)
> <stack backtrace when the buffer had been allocated>
>
> Signed-off-by: Stefan Strogin <[email protected]>
> ---
> include/linux/cma.h | 9 ++++
> mm/cma.c | 9 ++++
> mm/cma.h | 16 ++++++
> mm/cma_debug.c | 145 +++++++++++++++++++++++++++++++++++++++++++++++++++-
> 4 files changed, 178 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/cma.h b/include/linux/cma.h
> index 9384ba6..4c2c83c 100644
> --- a/include/linux/cma.h
> +++ b/include/linux/cma.h
> @@ -28,4 +28,13 @@ extern int cma_init_reserved_mem(phys_addr_t base,
> struct cma **res_cma);
> extern struct page *cma_alloc(struct cma *cma, int count, unsigned int align);
> extern bool cma_release(struct cma *cma, struct page *pages, int count);
> +
> +#ifdef CONFIG_CMA_DEBUGFS
> +extern int cma_buffer_list_add(struct cma *cma, unsigned long pfn, int count);
> +extern void cma_buffer_list_del(struct cma *cma, unsigned long pfn, int count);
> +#else
> +#define cma_buffer_list_add(cma, pfn, count) { }
> +#define cma_buffer_list_del(cma, pfn, count) { }
> +#endif
> +

These could be in mm/cma.h rather than include/linux/cma.h.

> #endif
> diff --git a/mm/cma.c b/mm/cma.c
> index 2609e20..ed269b0 100644
> --- a/mm/cma.c
> +++ b/mm/cma.c
> @@ -34,6 +34,9 @@
> #include <linux/cma.h>
> #include <linux/highmem.h>
> #include <linux/io.h>
> +#include <linux/list.h>
> +#include <linux/proc_fs.h>
> +#include <linux/time.h>
>
> #include "cma.h"
>
> @@ -125,6 +128,8 @@ static int __init cma_activate_area(struct cma *cma)
> #ifdef CONFIG_CMA_DEBUGFS
> INIT_HLIST_HEAD(&cma->mem_head);
> spin_lock_init(&cma->mem_head_lock);
> + INIT_LIST_HEAD(&cma->buffers_list);
> + mutex_init(&cma->list_lock);
> #endif
>
> return 0;
> @@ -408,6 +413,9 @@ struct page *cma_alloc(struct cma *cma, int count, unsigned int align)
> start = bitmap_no + mask + 1;
> }
>
> + if (page)
> + cma_buffer_list_add(cma, pfn, count);
> +
> pr_debug("%s(): returned %p\n", __func__, page);
> return page;
> }
> @@ -440,6 +448,7 @@ bool cma_release(struct cma *cma, struct page *pages, int count)
>
> free_contig_range(pfn, count);
> cma_clear_bitmap(cma, pfn, count);
> + cma_buffer_list_del(cma, pfn, count);
>
> return true;
> }
> diff --git a/mm/cma.h b/mm/cma.h
> index 1132d73..98e5f79 100644
> --- a/mm/cma.h
> +++ b/mm/cma.h
> @@ -1,6 +1,8 @@
> #ifndef __MM_CMA_H__
> #define __MM_CMA_H__
>
> +#include <linux/sched.h>
> +
> struct cma {
> unsigned long base_pfn;
> unsigned long count;
> @@ -10,9 +12,23 @@ struct cma {
> #ifdef CONFIG_CMA_DEBUGFS
> struct hlist_head mem_head;
> spinlock_t mem_head_lock;
> + struct list_head buffers_list;
> + struct mutex list_lock;
> #endif
> };
>
> +#ifdef CONFIG_CMA_DEBUGFS
> +struct cma_buffer {
> + unsigned long pfn;
> + unsigned long count;
> + pid_t pid;
> + char comm[TASK_COMM_LEN];
> + unsigned long trace_entries[16];
> + unsigned int nr_entries;
> + struct list_head list;
> +};
> +#endif
> +
> extern struct cma cma_areas[MAX_CMA_AREAS];
> extern unsigned cma_area_count;
>
> diff --git a/mm/cma_debug.c b/mm/cma_debug.c
> index 7e1d325..5acd937 100644
> --- a/mm/cma_debug.c
> +++ b/mm/cma_debug.c
> @@ -2,6 +2,7 @@
> * CMA DebugFS Interface
> *
> * Copyright (c) 2015 Sasha Levin <[email protected]>
> + * Copyright (c) 2015 Stefan Strogin <[email protected]>
> */
>
>
> @@ -10,6 +11,8 @@
> #include <linux/list.h>
> #include <linux/kernel.h>
> #include <linux/slab.h>
> +#include <linux/mm_types.h>
> +#include <linux/stacktrace.h>
>
> #include "cma.h"
>
> @@ -21,6 +24,99 @@ struct cma_mem {
>
> static struct dentry *cma_debugfs_root;
>
> +/* Must be called under cma->list_lock */
> +static int __cma_buffer_list_add(struct cma *cma, unsigned long pfn, int count)
> +{
> + struct cma_buffer *cmabuf;
> + struct stack_trace trace;
> +
> + cmabuf = kmalloc(sizeof(*cmabuf), GFP_KERNEL);
> + if (!cmabuf) {
> + pr_warn("%s(page %p, count %d): failed to allocate buffer list entry\n",
> + __func__, pfn_to_page(pfn), count);
> + return -ENOMEM;
> + }
> +
> + trace.nr_entries = 0;
> + trace.max_entries = ARRAY_SIZE(cmabuf->trace_entries);
> + trace.entries = &cmabuf->trace_entries[0];
> + trace.skip = 2;
> + save_stack_trace(&trace);
> +
> + cmabuf->pfn = pfn;
> + cmabuf->count = count;
> + cmabuf->pid = task_pid_nr(current);
> + cmabuf->nr_entries = trace.nr_entries;
> + get_task_comm(cmabuf->comm, current);
> +
> + list_add_tail(&cmabuf->list, &cma->buffers_list);
> +
> + return 0;
> +}
> +
> +/**
> + * cma_buffer_list_add() - add a new entry to a list of allocated buffers
> + * @cma: Contiguous memory region for which the allocation is performed.
> + * @pfn: Base PFN of the allocated buffer.
> + * @count: Number of allocated pages.
> + *
> + * This function adds a new entry to the list of allocated contiguous memory
> + * buffers in a CMA area. It uses the CMA area specificated by the device
> + * if available or the default global one otherwise.
> + */
> +int cma_buffer_list_add(struct cma *cma, unsigned long pfn, int count)
> +{
> + int ret;
> +
> + mutex_lock(&cma->list_lock);
> + ret = __cma_buffer_list_add(cma, pfn, count);
> + mutex_unlock(&cma->list_lock);
> +
> + return ret;
> +}
> +
> +/**
> + * cma_buffer_list_del() - delete an entry from a list of allocated buffers
> + * @cma: Contiguous memory region for which the allocation was performed.
> + * @pfn: Base PFN of the released buffer.
> + * @count: Number of pages.
> + *
> + * This function deletes a list entry added by cma_buffer_list_add().
> + */
> +void cma_buffer_list_del(struct cma *cma, unsigned long pfn, int count)
> +{
> + struct cma_buffer *cmabuf, *tmp;
> + int found = 0;
> + unsigned long buf_end_pfn, free_end_pfn = pfn + count;
> +
> + mutex_lock(&cma->list_lock);
> + list_for_each_entry_safe(cmabuf, tmp, &cma->buffers_list, list) {
> +
> + buf_end_pfn = cmabuf->pfn + cmabuf->count;
> + if (pfn <= cmabuf->pfn && free_end_pfn >= buf_end_pfn) {
> + list_del(&cmabuf->list);
> + kfree(cmabuf);
> + found = 1;
> + } else if (pfn <= cmabuf->pfn && free_end_pfn < buf_end_pfn) {
> + cmabuf->count -= free_end_pfn - cmabuf->pfn;
> + cmabuf->pfn = free_end_pfn;
> + found = 1;
> + } else if (pfn > cmabuf->pfn && pfn < buf_end_pfn) {
> + if (free_end_pfn < buf_end_pfn)
> + __cma_buffer_list_add(cma, free_end_pfn,
> + buf_end_pfn - free_end_pfn);
> + cmabuf->count = pfn - cmabuf->pfn;
> + found = 1;
> + }
> + }
> + mutex_unlock(&cma->list_lock);

This linear searching make cma_release() slow if we have many allocated
cma buffers. It wouldn't cause any problem?

Thanks.

2015-02-13 03:09:34

by Joonsoo Kim

[permalink] [raw]
Subject: Re: [PATCH 2/4] mm: cma: add functions to get region pages counters

On Fri, Feb 13, 2015 at 01:15:42AM +0300, Stefan Strogin wrote:
> From: Dmitry Safonov <[email protected]>
>
> Here are two functions that provide interface to compute/get used size
> and size of biggest free chunk in cma region.
> Add that information to debugfs.
>
> Signed-off-by: Dmitry Safonov <[email protected]>
> Signed-off-by: Stefan Strogin <[email protected]>
> ---
> include/linux/cma.h | 2 ++
> mm/cma.c | 30 ++++++++++++++++++++++++++++++
> mm/cma_debug.c | 24 ++++++++++++++++++++++++
> 3 files changed, 56 insertions(+)
>
> diff --git a/include/linux/cma.h b/include/linux/cma.h
> index 4c2c83c..54a2c4d 100644
> --- a/include/linux/cma.h
> +++ b/include/linux/cma.h
> @@ -18,6 +18,8 @@ struct cma;
> extern unsigned long totalcma_pages;
> extern phys_addr_t cma_get_base(struct cma *cma);
> extern unsigned long cma_get_size(struct cma *cma);
> +extern unsigned long cma_get_used(struct cma *cma);
> +extern unsigned long cma_get_maxchunk(struct cma *cma);
>
> extern int __init cma_declare_contiguous(phys_addr_t base,
> phys_addr_t size, phys_addr_t limit,
> diff --git a/mm/cma.c b/mm/cma.c
> index ed269b0..95e8121 100644
> --- a/mm/cma.c
> +++ b/mm/cma.c
> @@ -54,6 +54,36 @@ unsigned long cma_get_size(struct cma *cma)
> return cma->count << PAGE_SHIFT;
> }
>
> +unsigned long cma_get_used(struct cma *cma)
> +{
> + unsigned long ret = 0;
> +
> + mutex_lock(&cma->lock);
> + /* pages counter is smaller than sizeof(int) */
> + ret = bitmap_weight(cma->bitmap, (int)cma->count);
> + mutex_unlock(&cma->lock);
> +
> + return ret;
> +}

Need to consider order_per_bit for returing number of page rather
than number of bits.

> +
> +unsigned long cma_get_maxchunk(struct cma *cma)
> +{
> + unsigned long maxchunk = 0;
> + unsigned long start, end = 0;
> +
> + mutex_lock(&cma->lock);
> + for (;;) {
> + start = find_next_zero_bit(cma->bitmap, cma->count, end);
> + if (start >= cma->count)
> + break;
> + end = find_next_bit(cma->bitmap, cma->count, start);
> + maxchunk = max(end - start, maxchunk);
> + }
> + mutex_unlock(&cma->lock);
> +
> + return maxchunk;
> +}
> +

Same here.

Thanks.

2015-02-13 03:13:58

by Joonsoo Kim

[permalink] [raw]
Subject: Re: [PATCH 1/4] mm: cma: add currently allocated CMA buffers list to debugfs

On Fri, Feb 13, 2015 at 01:15:41AM +0300, Stefan Strogin wrote:
> static int cma_debugfs_get(void *data, u64 *val)
> {
> unsigned long *p = data;
> @@ -125,6 +221,52 @@ static int cma_alloc_write(void *data, u64 val)
>
> DEFINE_SIMPLE_ATTRIBUTE(cma_alloc_fops, NULL, cma_alloc_write, "%llu\n");
>
> +static int cma_buffers_read(struct file *file, char __user *userbuf,
> + size_t count, loff_t *ppos)
> +{
> + struct cma *cma = file->private_data;
> + struct cma_buffer *cmabuf;
> + struct stack_trace trace;
> + char *buf;
> + int ret, n = 0;
> +
> + if (*ppos < 0 || !count)
> + return -EINVAL;
> +
> + buf = kmalloc(count, GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;

Is count limited within proper size boundary for kmalloc()?
If it can exceed page size, using vmalloc() is better than this.

Thanks.

2015-02-13 14:16:38

by SeongJae Park

[permalink] [raw]
Subject: Re: [PATCH 1/4] mm: cma: add currently allocated CMA buffers list to debugfs

Hello, Stefan.

On Fri, 13 Feb 2015, Stefan Strogin wrote:

> /sys/kernel/debug/cma/cma-<N>/buffers contains a list of currently allocated
> CMA buffers for CMA region N when CONFIG_CMA_DEBUGFS is enabled.
>
> Format is:
>
> <base_phys_addr> - <end_phys_addr> (<size> kB), allocated by <PID> (<comm>)
> <stack backtrace when the buffer had been allocated>
>
> Signed-off-by: Stefan Strogin <[email protected]>
> ---
> include/linux/cma.h | 9 ++++
> mm/cma.c | 9 ++++
> mm/cma.h | 16 ++++++
> mm/cma_debug.c | 145 +++++++++++++++++++++++++++++++++++++++++++++++++++-
> 4 files changed, 178 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/cma.h b/include/linux/cma.h
> index 9384ba6..4c2c83c 100644
> --- a/include/linux/cma.h
> +++ b/include/linux/cma.h
> @@ -28,4 +28,13 @@ extern int cma_init_reserved_mem(phys_addr_t base,
> struct cma **res_cma);
> extern struct page *cma_alloc(struct cma *cma, int count, unsigned int align);
> extern bool cma_release(struct cma *cma, struct page *pages, int count);
> +
> +#ifdef CONFIG_CMA_DEBUGFS
> +extern int cma_buffer_list_add(struct cma *cma, unsigned long pfn, int count);
> +extern void cma_buffer_list_del(struct cma *cma, unsigned long pfn, int count);
> +#else
> +#define cma_buffer_list_add(cma, pfn, count) { }
> +#define cma_buffer_list_del(cma, pfn, count) { }
> +#endif
> +
> #endif
> diff --git a/mm/cma.c b/mm/cma.c
> index 2609e20..ed269b0 100644
> --- a/mm/cma.c
> +++ b/mm/cma.c
> @@ -34,6 +34,9 @@
> #include <linux/cma.h>
> #include <linux/highmem.h>
> #include <linux/io.h>
> +#include <linux/list.h>
> +#include <linux/proc_fs.h>
> +#include <linux/time.h>

Looks like `proc_fs.h` and `time.h` are not necessary.

>
> #include "cma.h"
>
> @@ -125,6 +128,8 @@ static int __init cma_activate_area(struct cma *cma)
> #ifdef CONFIG_CMA_DEBUGFS
> INIT_HLIST_HEAD(&cma->mem_head);
> spin_lock_init(&cma->mem_head_lock);
> + INIT_LIST_HEAD(&cma->buffers_list);
> + mutex_init(&cma->list_lock);
> #endif
>
> return 0;
> @@ -408,6 +413,9 @@ struct page *cma_alloc(struct cma *cma, int count, unsigned int align)
> start = bitmap_no + mask + 1;
> }
>
> + if (page)
> + cma_buffer_list_add(cma, pfn, count);
> +
> pr_debug("%s(): returned %p\n", __func__, page);
> return page;
> }
> @@ -440,6 +448,7 @@ bool cma_release(struct cma *cma, struct page *pages, int count)
>
> free_contig_range(pfn, count);
> cma_clear_bitmap(cma, pfn, count);
> + cma_buffer_list_del(cma, pfn, count);
>
> return true;
> }
> diff --git a/mm/cma.h b/mm/cma.h
> index 1132d73..98e5f79 100644
> --- a/mm/cma.h
> +++ b/mm/cma.h
> @@ -1,6 +1,8 @@
> #ifndef __MM_CMA_H__
> #define __MM_CMA_H__
>
> +#include <linux/sched.h>
> +
> struct cma {
> unsigned long base_pfn;
> unsigned long count;
> @@ -10,9 +12,23 @@ struct cma {
> #ifdef CONFIG_CMA_DEBUGFS
> struct hlist_head mem_head;
> spinlock_t mem_head_lock;
> + struct list_head buffers_list;
> + struct mutex list_lock;
> #endif
> };
>
> +#ifdef CONFIG_CMA_DEBUGFS
> +struct cma_buffer {
> + unsigned long pfn;
> + unsigned long count;
> + pid_t pid;
> + char comm[TASK_COMM_LEN];
> + unsigned long trace_entries[16];
> + unsigned int nr_entries;
> + struct list_head list;
> +};
> +#endif
> +
> extern struct cma cma_areas[MAX_CMA_AREAS];
> extern unsigned cma_area_count;
>
> diff --git a/mm/cma_debug.c b/mm/cma_debug.c
> index 7e1d325..5acd937 100644
> --- a/mm/cma_debug.c
> +++ b/mm/cma_debug.c
> @@ -2,6 +2,7 @@
> * CMA DebugFS Interface
> *
> * Copyright (c) 2015 Sasha Levin <[email protected]>
> + * Copyright (c) 2015 Stefan Strogin <[email protected]>
> */
>
>
> @@ -10,6 +11,8 @@
> #include <linux/list.h>
> #include <linux/kernel.h>
> #include <linux/slab.h>
> +#include <linux/mm_types.h>
> +#include <linux/stacktrace.h>
>
> #include "cma.h"
>
> @@ -21,6 +24,99 @@ struct cma_mem {
>
> static struct dentry *cma_debugfs_root;
>
> +/* Must be called under cma->list_lock */
> +static int __cma_buffer_list_add(struct cma *cma, unsigned long pfn, int count)
> +{
> + struct cma_buffer *cmabuf;
> + struct stack_trace trace;
> +
> + cmabuf = kmalloc(sizeof(*cmabuf), GFP_KERNEL);
> + if (!cmabuf) {
> + pr_warn("%s(page %p, count %d): failed to allocate buffer list entry\n",
> + __func__, pfn_to_page(pfn), count);

pfn_to_page() would cause build failure on x86_64. Why don't you include
appropriate header file?


Thanks,
SeongJae Park

> + return -ENOMEM;
> + }
> +
> + trace.nr_entries = 0;
> + trace.max_entries = ARRAY_SIZE(cmabuf->trace_entries);
> + trace.entries = &cmabuf->trace_entries[0];
> + trace.skip = 2;
> + save_stack_trace(&trace);
> +
> + cmabuf->pfn = pfn;
> + cmabuf->count = count;
> + cmabuf->pid = task_pid_nr(current);
> + cmabuf->nr_entries = trace.nr_entries;
> + get_task_comm(cmabuf->comm, current);
> +
> + list_add_tail(&cmabuf->list, &cma->buffers_list);
> +
> + return 0;
> +}
> +
> +/**
> + * cma_buffer_list_add() - add a new entry to a list of allocated buffers
> + * @cma: Contiguous memory region for which the allocation is performed.
> + * @pfn: Base PFN of the allocated buffer.
> + * @count: Number of allocated pages.
> + *
> + * This function adds a new entry to the list of allocated contiguous memory
> + * buffers in a CMA area. It uses the CMA area specificated by the device
> + * if available or the default global one otherwise.
> + */
> +int cma_buffer_list_add(struct cma *cma, unsigned long pfn, int count)
> +{
> + int ret;
> +
> + mutex_lock(&cma->list_lock);
> + ret = __cma_buffer_list_add(cma, pfn, count);
> + mutex_unlock(&cma->list_lock);
> +
> + return ret;
> +}
> +
> +/**
> + * cma_buffer_list_del() - delete an entry from a list of allocated buffers
> + * @cma: Contiguous memory region for which the allocation was performed.
> + * @pfn: Base PFN of the released buffer.
> + * @count: Number of pages.
> + *
> + * This function deletes a list entry added by cma_buffer_list_add().
> + */
> +void cma_buffer_list_del(struct cma *cma, unsigned long pfn, int count)
> +{
> + struct cma_buffer *cmabuf, *tmp;
> + int found = 0;
> + unsigned long buf_end_pfn, free_end_pfn = pfn + count;
> +
> + mutex_lock(&cma->list_lock);
> + list_for_each_entry_safe(cmabuf, tmp, &cma->buffers_list, list) {
> +
> + buf_end_pfn = cmabuf->pfn + cmabuf->count;
> + if (pfn <= cmabuf->pfn && free_end_pfn >= buf_end_pfn) {
> + list_del(&cmabuf->list);
> + kfree(cmabuf);
> + found = 1;
> + } else if (pfn <= cmabuf->pfn && free_end_pfn < buf_end_pfn) {
> + cmabuf->count -= free_end_pfn - cmabuf->pfn;
> + cmabuf->pfn = free_end_pfn;
> + found = 1;
> + } else if (pfn > cmabuf->pfn && pfn < buf_end_pfn) {
> + if (free_end_pfn < buf_end_pfn)
> + __cma_buffer_list_add(cma, free_end_pfn,
> + buf_end_pfn - free_end_pfn);
> + cmabuf->count = pfn - cmabuf->pfn;
> + found = 1;
> + }
> + }
> + mutex_unlock(&cma->list_lock);
> +
> + if (!found)
> + pr_err("%s(page %p, count %d): couldn't find buffer list entry\n",
> + __func__, pfn_to_page(pfn), count);
> +
> +}
> +
> static int cma_debugfs_get(void *data, u64 *val)
> {
> unsigned long *p = data;
> @@ -125,6 +221,52 @@ static int cma_alloc_write(void *data, u64 val)
>
> DEFINE_SIMPLE_ATTRIBUTE(cma_alloc_fops, NULL, cma_alloc_write, "%llu\n");
>
> +static int cma_buffers_read(struct file *file, char __user *userbuf,
> + size_t count, loff_t *ppos)
> +{
> + struct cma *cma = file->private_data;
> + struct cma_buffer *cmabuf;
> + struct stack_trace trace;
> + char *buf;
> + int ret, n = 0;
> +
> + if (*ppos < 0 || !count)
> + return -EINVAL;
> +
> + buf = kmalloc(count, GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;
> +
> + mutex_lock(&cma->list_lock);
> + list_for_each_entry(cmabuf, &cma->buffers_list, list) {
> + n += snprintf(buf + n, count - n,
> + "0x%llx - 0x%llx (%lu kB), allocated by pid %u (%s)\n",
> + (unsigned long long)PFN_PHYS(cmabuf->pfn),
> + (unsigned long long)PFN_PHYS(cmabuf->pfn +
> + cmabuf->count),
> + (cmabuf->count * PAGE_SIZE) >> 10, cmabuf->pid,
> + cmabuf->comm);
> +
> + trace.nr_entries = cmabuf->nr_entries;
> + trace.entries = &cmabuf->trace_entries[0];
> +
> + n += snprint_stack_trace(buf + n, count - n, &trace, 0);
> + n += snprintf(buf + n, count - n, "\n");
> + }
> + mutex_unlock(&cma->list_lock);
> +
> + ret = simple_read_from_buffer(userbuf, count, ppos, buf, n);
> + kfree(buf);
> +
> + return ret;
> +}
> +
> +static const struct file_operations cma_buffers_fops = {
> + .open = simple_open,
> + .read = cma_buffers_read,
> + .llseek = default_llseek,
> +};
> +
> static void cma_debugfs_add_one(struct cma *cma, int idx)
> {
> struct dentry *tmp;
> @@ -148,6 +290,8 @@ static void cma_debugfs_add_one(struct cma *cma, int idx)
> debugfs_create_file("order_per_bit", S_IRUGO, tmp,
> &cma->order_per_bit, &cma_debugfs_fops);
>
> + debugfs_create_file("buffers", S_IRUGO, tmp, cma, &cma_buffers_fops);
> +
> u32s = DIV_ROUND_UP(cma_bitmap_maxno(cma), BITS_PER_BYTE * sizeof(u32));
> debugfs_create_u32_array("bitmap", S_IRUGO, tmp, (u32*)cma->bitmap, u32s);
> }
> @@ -166,4 +310,3 @@ static int __init cma_debugfs_init(void)
> return 0;
> }
> late_initcall(cma_debugfs_init);
> -
> --
> 2.1.0
>
>

2015-02-14 07:32:23

by Gioh Kim

[permalink] [raw]
Subject: Re: [PATCH 2/4] mm: cma: add functions to get region pages counters



2015-02-13 ???? 7:15?? Stefan Strogin ??(??) ?? ??:
> From: Dmitry Safonov <[email protected]>
>
> Here are two functions that provide interface to compute/get used size
> and size of biggest free chunk in cma region.

I usually just try to allocate memory, not check free size before try,
becuase free size can be changed after I check it.

Could you tell me why biggest free chunk size is necessary?

2015-02-14 07:40:40

by Gioh Kim

[permalink] [raw]
Subject: Re: [PATCH 0/4] mm: cma: add some debug information for CMA



2015-02-13 오후 12:03에 Joonsoo Kim 이(가) 쓴 글:
> On Fri, Feb 13, 2015 at 01:15:40AM +0300, Stefan Strogin wrote:
>> Hi all.
>>
>> Sorry for the long delay. Here is the second attempt to add some facility
>> for debugging CMA (the first one was "mm: cma: add /proc/cmainfo" [1]).
>>
>> This patch set is based on v3.19 and Sasha Levin's patch set
>> "mm: cma: debugfs access to CMA" [2].
>> It is also available on git:
>> git://github.com/stefanstrogin/cmainfo -b cmainfo-v2
>>
>> We want an interface to see a list of currently allocated CMA buffers and
>> some useful information about them (like /proc/vmallocinfo but for physically
>> contiguous buffers allocated with CMA).
>>
>> Here is an example use case when we need it. We want a big (megabytes)
>> CMA buffer to be allocated in runtime in default CMA region. If someone
>> already uses CMA then the big allocation can fail. If it happens then with
>> such an interface we could find who used CMA at the moment of failure, who
>> caused fragmentation (possibly ftrace also would be helpful here) and so on.
>
> Hello,
>
> So, I'm not sure that information about allocated CMA buffer is really
> needed to solve your problem. You just want to know who uses default CMA
> region and you can know it by adding tracepoint in your 4/4 patch. We
> really need this custom allocation tracer? What can we do more with
> this custom tracer to solve your problem? Could you more specific
> about your problem and how to solve it by using this custom tracer?
>
>>
>> These patches add some files to debugfs when CONFIG_CMA_DEBUGFS is enabled.
>
> If this tracer is justifiable, I think that making it conditional is
> better than just enabling always on CONFIG_CMA_DEBUGFS. Some users
> don't want to this feature although they enable CONFIG_CMA_DEBUGFS.
>
> Thanks.
>

Hello,

Thanks for your work. It must be helpful to me.

What about add another option to activate stack-trace?
In my platform I know all devices using cma area, so I usually don't need stack-trace.

2015-02-16 18:54:07

by Stefan Strogin

[permalink] [raw]
Subject: Re: [PATCH 0/4] mm: cma: add some debug information for CMA

Hello Joonsoo,

Thank you for your answer.

On 13/02/15 06:03, Joonsoo Kim wrote:
> On Fri, Feb 13, 2015 at 01:15:40AM +0300, Stefan Strogin wrote:
>>
>> Here is an example use case when we need it. We want a big (megabytes)
>> CMA buffer to be allocated in runtime in default CMA region. If someone
>> already uses CMA then the big allocation can fail. If it happens then with
>> such an interface we could find who used CMA at the moment of failure, who
>> caused fragmentation (possibly ftrace also would be helpful here) and so on.
>
> Hello,
>
> So, I'm not sure that information about allocated CMA buffer is really
> needed to solve your problem. You just want to know who uses default CMA
> region and you can know it by adding tracepoint in your 4/4 patch. We
> really need this custom allocation tracer? What can we do more with
> this custom tracer to solve your problem? Could you more specific
> about your problem and how to solve it by using this custom tracer?
>

I think, yes, we could solve the problem using only trace events. We
could get all CMA allocations and releases. But if we want to get
the current state of CMA region, for example to know actual
fragmentation, should we parse the tracer's output or what else? IMHO it
would be easier for testers if they have the list of currently allocated
buffers right away.

>>
>> These patches add some files to debugfs when CONFIG_CMA_DEBUGFS is enabled.
>
> If this tracer is justifiable, I think that making it conditional is
> better than just enabling always on CONFIG_CMA_DEBUGFS. Some users
> don't want to this feature although they enable CONFIG_CMA_DEBUGFS.
>
> Thanks.
>

Thank you. I think, this makes sense because of overhead.

2015-02-16 19:15:09

by Stefan Strogin

[permalink] [raw]
Subject: Re: [PATCH 1/4] mm: cma: add currently allocated CMA buffers list to debugfs

Hello,

On 13/02/15 06:10, Joonsoo Kim wrote:
> On Fri, Feb 13, 2015 at 01:15:41AM +0300, Stefan Strogin wrote:
>
> This linear searching make cma_release() slow if we have many allocated
> cma buffers. It wouldn't cause any problem?
>
> Thanks.
>
>

On my board the usual number of CMA buffers is about 20, and releasing a
buffer isn't a very frequent operation.
But if there could be systems with much more CMA buffers and/or frequent
allocating/releasing them, maybe it would be useful to convert buffers
list to rb_trees?

2015-02-16 19:19:14

by Stefan Strogin

[permalink] [raw]
Subject: Re: [PATCH 1/4] mm: cma: add currently allocated CMA buffers list to debugfs



On 13/02/15 06:10, Joonsoo Kim wrote:
>> @@ -28,4 +28,13 @@ extern int cma_init_reserved_mem(phys_addr_t base,
>> struct cma **res_cma);
>> extern struct page *cma_alloc(struct cma *cma, int count, unsigned int align);
>> extern bool cma_release(struct cma *cma, struct page *pages, int count);
>> +
>> +#ifdef CONFIG_CMA_DEBUGFS
>> +extern int cma_buffer_list_add(struct cma *cma, unsigned long pfn, int count);
>> +extern void cma_buffer_list_del(struct cma *cma, unsigned long pfn, int count);
>> +#else
>> +#define cma_buffer_list_add(cma, pfn, count) { }
>> +#define cma_buffer_list_del(cma, pfn, count) { }
>> +#endif
>> +
>
> These could be in mm/cma.h rather than include/linux/cma.h.
>

Thank you. I'll correct it.

2015-02-16 19:25:39

by Stefan Strogin

[permalink] [raw]
Subject: Re: [PATCH 1/4] mm: cma: add currently allocated CMA buffers list to debugfs

Hello SeongJae,

On 13/02/15 17:18, SeongJae Park wrote:
> Hello, Stefan.
>
> On Fri, 13 Feb 2015, Stefan Strogin wrote:
>
>> #include <linux/io.h>
>> +#include <linux/list.h>
>> +#include <linux/proc_fs.h>
>> +#include <linux/time.h>
>
> Looks like `proc_fs.h` and `time.h` are not necessary.
>

Yes, of course. Thanks.


>> + pr_warn("%s(page %p, count %d): failed to allocate buffer
>> list entry\n",
>> + __func__, pfn_to_page(pfn), count);
>
> pfn_to_page() would cause build failure on x86_64. Why don't you include
> appropriate header file?
>

Indeed. Because I tested it only on arm and x86. Sorry :( Thank you.

2015-02-16 19:29:32

by Stefan Strogin

[permalink] [raw]
Subject: Re: [PATCH 0/4] mm: cma: add some debug information for CMA

Hello Gioh,

Thank you for your answer.

On 14/02/15 10:40, Gioh Kim wrote:
>>
>> If this tracer is justifiable, I think that making it conditional is
>> better than just enabling always on CONFIG_CMA_DEBUGFS. Some users
>> don't want to this feature although they enable CONFIG_CMA_DEBUGFS.
>>
>> Thanks.
>>
>
> Hello,
>
> Thanks for your work. It must be helpful to me.
>
> What about add another option to activate stack-trace?
> In my platform I know all devices using cma area, so I usually don't
> need stack-trace.
>

So Joonsoo suggests to add an option for buffer list, and you suggest to
add another in addition to the first one (and also add CONFIG_STACKTRACE
to dependences) right?

2015-02-18 10:06:25

by Stefan Strogin

[permalink] [raw]
Subject: Re: [PATCH 1/4] mm: cma: add currently allocated CMA buffers list to debugfs

Hello

On 13/02/15 06:16, Joonsoo Kim wrote:
> On Fri, Feb 13, 2015 at 01:15:41AM +0300, Stefan Strogin wrote:
>> static int cma_debugfs_get(void *data, u64 *val)
>> {
>> unsigned long *p = data;
>> @@ -125,6 +221,52 @@ static int cma_alloc_write(void *data, u64 val)
>>
>> DEFINE_SIMPLE_ATTRIBUTE(cma_alloc_fops, NULL, cma_alloc_write, "%llu\n");
>>
>> +static int cma_buffers_read(struct file *file, char __user *userbuf,
>> + size_t count, loff_t *ppos)
>> +{
>> + struct cma *cma = file->private_data;
>> + struct cma_buffer *cmabuf;
>> + struct stack_trace trace;
>> + char *buf;
>> + int ret, n = 0;
>> +
>> + if (*ppos < 0 || !count)
>> + return -EINVAL;
>> +
>> + buf = kmalloc(count, GFP_KERNEL);
>> + if (!buf)
>> + return -ENOMEM;
>
> Is count limited within proper size boundary for kmalloc()?
> If it can exceed page size, using vmalloc() is better than this.
>
> Thanks.
>

You are right. On my systems it is always much bigger than page size.
Thanks.

2015-02-18 13:12:41

by Safonov Dmitry

[permalink] [raw]
Subject: Re: [PATCH 2/4] mm: cma: add functions to get region pages counters

Hello,
On 02/14/2015 10:32 AM, Gioh Kim wrote:
> 2015-02-13 ???? 7:15?? Stefan Strogin ??(??) ?? ??:
>> From: Dmitry Safonov <[email protected]>
>>
>> Here are two functions that provide interface to compute/get used size
>> and size of biggest free chunk in cma region.
> I usually just try to allocate memory, not check free size before try,
> becuase free size can be changed after I check it.
>
> Could you tell me why biggest free chunk size is necessary?
>
It may have changed after checking - at beginning of allocation
this information is completely useless as you mentioned, but
it may be very helpful after failed allocation to detect fragmentation
problem: i.e, you failed to alloc 20 Mb from 100 Mb CMA region
with 60 Mb free space, so you will know the reason.

--
Best regards,
Safonov Dmitry.

2015-02-22 23:29:11

by Gioh Kim

[permalink] [raw]
Subject: Re: [PATCH 0/4] mm: cma: add some debug information for CMA



2015-02-17 오전 4:29에 Stefan Strogin 이(가) 쓴 글:
> Hello Gioh,
>
> Thank you for your answer.
>
> On 14/02/15 10:40, Gioh Kim wrote:
>>>
>>> If this tracer is justifiable, I think that making it conditional is
>>> better than just enabling always on CONFIG_CMA_DEBUGFS. Some users
>>> don't want to this feature although they enable CONFIG_CMA_DEBUGFS.
>>>
>>> Thanks.
>>>
>>
>> Hello,
>>
>> Thanks for your work. It must be helpful to me.
>>
>> What about add another option to activate stack-trace?
>> In my platform I know all devices using cma area, so I usually don't
>> need stack-trace.
>>
>
> So Joonsoo suggests to add an option for buffer list, and you suggest to
> add another in addition to the first one (and also add CONFIG_STACKTRACE
> to dependences) right?
>

Right. Another option for only stack-trace might be good.