[PATCH v4 0/5] mm: cma: add some debug information for CMA

[PATCH v4 0/5] mm: cma: add some debug information for CMA

[Stefan Strogin]

Hi all.

Here is the fourth version of a patch set that adds some debugging facility for
CMA.

This patch set is based on next-20150316.
It is also available on git:
git://github.com/stefanstrogin/linux -b cmainfo-v4

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).

For example. 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
could fail. If it happened then with such an interface we could find who used
CMA at the moment of failure, who caused fragmentation and so on. Ftrace also
would be helpful here, but with ftrace we can see the whole history of
allocations and releases, whereas with this patch set we can see a snapshot of
CMA region with actual information about its allocations.

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

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

When CONFIG_CMA_BUFFER_LIST is enabled (depended on CONFIG_CMA_DEBUGFS)
/sys/kernel/debug/cma/cma-<N>/buffers is added. It contains a list of
currently allocated contiguous buffers for each CMA region (N stands for
region number). Format is:
<base_phys_addr> - <end_phys_addr> (<size> kB), allocated by <PID> (<comm>)

Another added option is CONFIG_CMA_ALLOC_STACKTRACE. When it's enabled then
stack traces are also saved when the allocations are made. The stack traces
are added to each entry in cma/cma-<N>/buffers (see an example [3]). This
can be used to see who and whence allocated each buffer.

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

Changes from v3 (https://lkml.org/lkml/2015/2/24/555):
- Rebased on next-20150316 (includes Sasha Levin's patch set).
- Use seq_file for cma/cma-<N>/buffers.
- Trace allocation failures in cma_alloc() as well as successful allocations.
- Added acks by Michal Nazarewicz and a review by SeongJae Park.

Changes from v2 (https://lkml.org/lkml/2015/2/12/647):
- Rebased on v4.0-rc1 and Sasha Levin's patch set v5 [1].
- Changed kmalloc() to vmalloc() in cma_buffer_list_read().
- Added CONFIG_CMA_BUFFER_LIST and CONFIG_CMA_ALLOC_STACKTRACE.
- Considered order_per_bit for returning page number in cma_get_used() and
  cma_get_maxchunk().
- Reordered the patches to make the one with trace events indepentent of
  others.
- Moved the prototypes of cma_buffer_list_add() and cma_buffer_list_del()
  from include/linux/cma.h to mm/cma.h.
- Various small changes.

Changes from v1 (aka "mm: cma: add /proc/cmainfo")
(https://lkml.org/lkml/2014/12/26/95):
- Rebased on v3.19 and Sasha Levin's patch set v4 [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/2015/2/12/657

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

[3] 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 (4):
  mm: cma: add trace events to debug physically-contiguous memory
    allocations
  mm: cma: add number of pages to debug message in cma_release()
  stacktrace: add seq_print_stack_trace()
  mm: cma: add list of currently allocated CMA buffers to debugfs

 include/linux/cma.h        |   2 +
 include/linux/stacktrace.h |   4 +
 include/trace/events/cma.h |  57 +++++++++++
 kernel/stacktrace.c        |  17 ++++
 mm/Kconfig                 |  17 ++++
 mm/cma.c                   |  44 ++++++++-
 mm/cma.h                   |  14 +++
 mm/cma_debug.c             | 230 ++++++++++++++++++++++++++++++++++++++++++++-
 8 files changed, 383 insertions(+), 2 deletions(-)
 create mode 100644 include/trace/events/cma.h

-- 
2.1.0

[PATCH v4 1/5] mm: cma: add trace events to debug physically-contiguous memory allocations

[Stefan Strogin]

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

Signed-off-by: Stefan Strogin <stefan.strogin@gmail.com>
---
 include/trace/events/cma.h | 57 ++++++++++++++++++++++++++++++++++++++++++++++
 mm/cma.c                   |  5 ++++
 2 files changed, 62 insertions(+)
 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..d88881b
--- /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, struct page *page, int count),
+
+	TP_ARGS(cma, page, count),
+
+	TP_STRUCT__entry(
+		__field(struct page *, page)
+		__field(unsigned long, count)
+	),
+
+	TP_fast_assign(
+		__entry->page = page;
+		__entry->count = count;
+	),
+
+	TP_printk("page=%p pfn=%lu count=%lu",
+		  __entry->page,
+		  __entry->page ? page_to_pfn(__entry->page) : 0,
+		  __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",
+		  __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 47203fa..63dfc0e 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>
@@ -34,6 +35,7 @@
 #include <linux/cma.h>
 #include <linux/highmem.h>
 #include <linux/io.h>
+#include <trace/events/cma.h>
 
 #include "cma.h"
 
@@ -414,6 +416,8 @@ struct page *cma_alloc(struct cma *cma, unsigned int count, unsigned int align)
 		start = bitmap_no + mask + 1;
 	}
 
+	trace_cma_alloc(cma, page, count);
+
 	pr_debug("%s(): returned %p\n", __func__, page);
 	return page;
 }
@@ -446,6 +450,7 @@ bool cma_release(struct cma *cma, const struct page *pages, unsigned int count)
 
 	free_contig_range(pfn, count);
 	cma_clear_bitmap(cma, pfn, count);
+	trace_cma_release(cma, pfn, count);
 
 	return true;
 }
-- 
2.1.0

[PATCH v4 2/5] mm: cma: add number of pages to debug message in cma_release()

[Stefan Strogin]

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

Signed-off-by: Stefan Strogin <stefan.strogin@gmail.com>
Acked-by: Michal Nazarewicz <mina86@mina86.com>
---
 mm/cma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/cma.c b/mm/cma.c
index 63dfc0e..77960af 100644
--- a/mm/cma.c
+++ b/mm/cma.c
@@ -439,7 +439,7 @@ bool cma_release(struct cma *cma, const struct page *pages, unsigned 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

[PATCH v4 3/5] stacktrace: add seq_print_stack_trace()

[Stefan Strogin]

Add a function seq_print_stack_trace() which prints stacktraces to seq_files.

Signed-off-by: Stefan Strogin <stefan.strogin@gmail.com>
Reviewed-by: SeongJae Park <sj38.park@gmail.com>
---
 include/linux/stacktrace.h |  4 ++++
 kernel/stacktrace.c        | 17 +++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/include/linux/stacktrace.h b/include/linux/stacktrace.h
index 0a34489..d80f2e9 100644
--- a/include/linux/stacktrace.h
+++ b/include/linux/stacktrace.h
@@ -2,6 +2,7 @@
 #define __LINUX_STACKTRACE_H
 
 #include <linux/types.h>
+#include <linux/seq_file.h>
 
 struct task_struct;
 struct pt_regs;
@@ -22,6 +23,8 @@ extern void save_stack_trace_tsk(struct task_struct *tsk,
 extern void print_stack_trace(struct stack_trace *trace, int spaces);
 extern int snprint_stack_trace(char *buf, size_t size,
 			struct stack_trace *trace, int spaces);
+extern void seq_print_stack_trace(struct seq_file *m,
+			struct stack_trace *trace, int spaces);
 
 #ifdef CONFIG_USER_STACKTRACE_SUPPORT
 extern void save_stack_trace_user(struct stack_trace *trace);
@@ -35,6 +38,7 @@ extern void save_stack_trace_user(struct stack_trace *trace);
 # define save_stack_trace_user(trace)			do { } while (0)
 # define print_stack_trace(trace, spaces)		do { } while (0)
 # define snprint_stack_trace(buf, size, trace, spaces)	do { } while (0)
+# define seq_print_stack_trace(m, trace, spaces)	do { } while (0)
 #endif
 
 #endif
diff --git a/kernel/stacktrace.c b/kernel/stacktrace.c
index b6e4c16..66ef6f4 100644
--- a/kernel/stacktrace.c
+++ b/kernel/stacktrace.c
@@ -57,6 +57,23 @@ int snprint_stack_trace(char *buf, size_t size,
 }
 EXPORT_SYMBOL_GPL(snprint_stack_trace);
 
+void seq_print_stack_trace(struct seq_file *m, struct stack_trace *trace,
+			int spaces)
+{
+	int i;
+
+	if (WARN_ON(!trace->entries))
+		return;
+
+	for (i = 0; i < trace->nr_entries; i++) {
+		unsigned long ip = trace->entries[i];
+
+		seq_printf(m, "%*c[<%p>] %pS\n", 1 + spaces, ' ',
+				(void *) ip, (void *) ip);
+	}
+}
+EXPORT_SYMBOL_GPL(seq_print_stack_trace);
+
 /*
  * Architectures that do not implement save_stack_trace_tsk or
  * save_stack_trace_regs get this weak alias and a once-per-bootup warning
-- 
2.1.0

[PATCH v4 4/5] mm: cma: add list of currently allocated CMA buffers to debugfs

[Stefan Strogin]

When CONFIG_CMA_BUFFER_LIST is configured a file is added to debugfs:
/sys/kernel/debug/cma/cma-<N>/buffers contains a list of currently allocated
CMA buffers for each CMA region (N stands for number of CMA region).

Format is:
<base_phys_addr> - <end_phys_addr> (<size> kB), allocated by <PID> (<comm>)

When CONFIG_CMA_ALLOC_STACKTRACE is configured then stack traces are saved when
the allocations are made. The stack traces are added to cma/cma-<N>/buffers
for each buffer list entry.

Example:

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
<...>

Signed-off-by: Stefan Strogin <stefan.strogin@gmail.com>
---
 mm/Kconfig     |  17 +++++
 mm/cma.c       |   7 ++
 mm/cma.h       |  14 ++++
 mm/cma_debug.c | 206 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 243 insertions(+), 1 deletion(-)

diff --git a/mm/Kconfig b/mm/Kconfig
index 390214d..5ee2388 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -523,6 +523,23 @@ config CMA_DEBUGFS
 	help
 	  Turns on the DebugFS interface for CMA.
 
+config CMA_BUFFER_LIST
+	bool "List of currently allocated CMA buffers in debugfs"
+	depends on CMA_DEBUGFS
+	help
+	  /sys/kernel/debug/cma/cma-<N>/buffers contains a list of currently
+	  allocated CMA buffers for each CMA region (N stands for number of
+	  CMA region).
+	  Format is:
+	  <base_phys_addr> - <end_phys_addr> (<size> kB), allocated by <PID> (<comm>)
+
+config CMA_ALLOC_STACKTRACE
+	bool "Add stack trace to CMA buffer list"
+	depends on CMA_BUFFER_LIST && STACKTRACE
+	help
+	  Add stack traces saved at the moment of allocation for each buffer
+	  listed in /sys/kernel/debug/cma/cma-<N>/buffers.
+
 config CMA_AREAS
 	int "Maximum count of the CMA areas"
 	depends on CMA
diff --git a/mm/cma.c b/mm/cma.c
index 77960af..faf8eac 100644
--- a/mm/cma.c
+++ b/mm/cma.c
@@ -133,6 +133,10 @@ static int __init cma_activate_area(struct cma *cma)
 	INIT_HLIST_HEAD(&cma->mem_head);
 	spin_lock_init(&cma->mem_head_lock);
 #endif
+#ifdef CONFIG_CMA_BUFFER_LIST
+	INIT_LIST_HEAD(&cma->buffer_list);
+	mutex_init(&cma->list_lock);
+#endif
 
 	return 0;
 
@@ -416,6 +420,8 @@ struct page *cma_alloc(struct cma *cma, unsigned int count, unsigned int align)
 		start = bitmap_no + mask + 1;
 	}
 
+	if (page)
+		cma_buffer_list_add(cma, pfn, count);
 	trace_cma_alloc(cma, page, count);
 
 	pr_debug("%s(): returned %p\n", __func__, page);
@@ -451,6 +457,7 @@ bool cma_release(struct cma *cma, const struct page *pages, unsigned 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;
 }
diff --git a/mm/cma.h b/mm/cma.h
index 1132d73..e174272 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;
@@ -11,8 +13,20 @@ struct cma {
 	struct hlist_head mem_head;
 	spinlock_t mem_head_lock;
 #endif
+#ifdef CONFIG_CMA_BUFFER_LIST
+	struct list_head buffer_list;
+	struct mutex	list_lock;
+#endif
 };
 
+#ifdef CONFIG_CMA_BUFFER_LIST
+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
+
 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 ec915e6..20bad2d 100644
--- a/mm/cma_debug.c
+++ b/mm/cma_debug.c
@@ -2,6 +2,7 @@
  * CMA DebugFS Interface
  *
  * Copyright (c) 2015 Sasha Levin <sasha.levin@oracle.com>
+ * Copyright (c) 2015 Stefan Strogin <stefan.strogin@gmail.com>
  */
 
 
@@ -10,7 +11,8 @@
 #include <linux/list.h>
 #include <linux/kernel.h>
 #include <linux/slab.h>
-#include <linux/mm_types.h>
+#include <linux/mm.h>
+#include <linux/stacktrace.h>
 
 #include "cma.h"
 
@@ -20,8 +22,119 @@ struct cma_mem {
 	unsigned long n;
 };
 
+#ifdef CONFIG_CMA_BUFFER_LIST
+struct cma_buffer {
+	unsigned long pfn;
+	unsigned long count;
+	pid_t pid;
+	char comm[TASK_COMM_LEN];
+#ifdef CONFIG_CMA_ALLOC_STACKTRACE
+	unsigned long trace_entries[16];
+	unsigned int nr_entries;
+#endif
+	struct list_head list;
+};
+#endif /* CONFIG_CMA_BUFFER_LIST */
+
 static struct dentry *cma_debugfs_root;
 
+#ifdef CONFIG_CMA_BUFFER_LIST
+/* 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;
+#ifdef CONFIG_CMA_ALLOC_STACKTRACE
+	struct stack_trace trace;
+#endif
+
+	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;
+	}
+
+#ifdef CONFIG_CMA_ALLOC_STACKTRACE
+	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->nr_entries = trace.nr_entries;
+#endif
+	cmabuf->pfn = pfn;
+	cmabuf->count = count;
+	cmabuf->pid = task_pid_nr(current);
+	get_task_comm(cmabuf->comm, current);
+
+	list_add_tail(&cmabuf->list, &cma->buffer_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 region.
+ */
+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->buffer_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);
+
+}
+#endif /* CONFIG_CMA_BUFFER_LIST */
+
 static int cma_debugfs_get(void *data, u64 *val)
 {
 	unsigned long *p = data;
@@ -127,6 +240,93 @@ static int cma_alloc_write(void *data, u64 val)
 
 DEFINE_SIMPLE_ATTRIBUTE(cma_alloc_fops, NULL, cma_alloc_write, "%llu\n");
 
+#ifdef CONFIG_CMA_BUFFER_LIST
+static void *s_start(struct seq_file *seq, loff_t *ppos)
+{
+	struct cma *cma = seq->private;
+	struct cma_buffer *cmabuf;
+	loff_t n = *ppos;
+
+	mutex_lock(&cma->list_lock);
+	cmabuf = list_first_entry(&cma->buffer_list, typeof(*cmabuf), list);
+	list_for_each_entry(cmabuf, &cma->buffer_list, list)
+		if (n-- == 0)
+			return cmabuf;
+
+	return 0;
+}
+
+static int s_show(struct seq_file *seq, void *p)
+{
+	struct cma_buffer *cmabuf = p;
+#ifdef CONFIG_CMA_ALLOC_STACKTRACE
+	struct stack_trace trace;
+#endif
+
+	seq_printf(seq, "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);
+
+#ifdef CONFIG_CMA_ALLOC_STACKTRACE
+	trace.nr_entries = cmabuf->nr_entries;
+	trace.entries = &cmabuf->trace_entries[0];
+	seq_print_stack_trace(seq, &trace, 0);
+	seq_putc(seq, '\n');
+#endif
+
+	return 0;
+}
+
+static void *s_next(struct seq_file *seq, void *p, loff_t *ppos)
+{
+	struct cma *cma = seq->private;
+	struct cma_buffer *cmabuf = (struct cma_buffer *)p, *next;
+
+	++*ppos;
+	next = list_next_entry(cmabuf, list);
+	if (&next->list != &cma->buffer_list)
+		return next;
+
+	return 0;
+}
+
+static void s_stop(struct seq_file *seq, void *p)
+{
+	struct cma *cma = seq->private;
+
+	mutex_unlock(&cma->list_lock);
+}
+
+static const struct seq_operations cma_buffers_sops = {
+	.start = s_start,
+	.show = s_show,
+	.next = s_next,
+	.stop = s_stop,
+};
+
+static int cma_buffers_open(struct inode *inode, struct file *file)
+{
+	int ret = seq_open(file, &cma_buffers_sops);
+
+	if (ret == 0) {
+		struct seq_file *seq = file->private_data;
+
+		seq->private = inode->i_private;
+	}
+	return ret;
+}
+
+static const struct file_operations cma_buffers_fops = {
+	.open = cma_buffers_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = seq_release,
+};
+#endif /* CONFIG_CMA_BUFFER_LIST */
+
 static void cma_debugfs_add_one(struct cma *cma, int idx)
 {
 	struct dentry *tmp;
@@ -149,6 +349,10 @@ 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);
+#ifdef CONFIG_CMA_BUFFER_LIST
+	debugfs_create_file("buffers", S_IRUGO, tmp, cma,
+				&cma_buffers_fops);
+#endif
 
 	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);
-- 
2.1.0

[PATCH v4 5/5] mm: cma: add functions to get region pages counters

[Stefan Strogin]

From: Dmitry Safonov <d.safonov@partner.samsung.com>

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 <d.safonov@partner.samsung.com>
Signed-off-by: Stefan Strogin <stefan.strogin@gmail.com>
Acked-by: Michal Nazarewicz <mina86@mina86.com>
---
 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 f7ef093..1231f50 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(const struct cma *cma);
 extern unsigned long cma_get_size(const 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 faf8eac..78b262a 100644
--- a/mm/cma.c
+++ b/mm/cma.c
@@ -53,6 +53,36 @@ unsigned long cma_get_size(const 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 << cma->order_per_bit;
+}
+
+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 << cma->order_per_bit;
+}
+
 static unsigned long cma_bitmap_aligned_mask(const struct cma *cma,
 					     int align_order)
 {
diff --git a/mm/cma_debug.c b/mm/cma_debug.c
index 20bad2d..e1ba160 100644
--- a/mm/cma_debug.c
+++ b/mm/cma_debug.c
@@ -146,6 +146,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);
@@ -349,6 +371,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);
 #ifdef CONFIG_CMA_BUFFER_LIST
 	debugfs_create_file("buffers", S_IRUGO, tmp, cma,
 				&cma_buffers_fops);
-- 
2.1.0

Re: [PATCH v4 3/5] stacktrace: add seq_print_stack_trace()

[Michal Nazarewicz]

On Mon, Mar 16 2015, Stefan Strogin wrote:
> Add a function seq_print_stack_trace() which prints stacktraces to seq_files.
>
> Signed-off-by: Stefan Strogin <stefan.strogin@gmail.com>
> Reviewed-by: SeongJae Park <sj38.park@gmail.com>

Acked-by: Michal Nazarewicz <mina86@mina86.com>

> ---
>  include/linux/stacktrace.h |  4 ++++
>  kernel/stacktrace.c        | 17 +++++++++++++++++
>  2 files changed, 21 insertions(+)
>
> diff --git a/include/linux/stacktrace.h b/include/linux/stacktrace.h
> index 0a34489..d80f2e9 100644
> --- a/include/linux/stacktrace.h
> +++ b/include/linux/stacktrace.h
> @@ -2,6 +2,7 @@
>  #define __LINUX_STACKTRACE_H
>  
>  #include <linux/types.h>
> +#include <linux/seq_file.h>
>  
>  struct task_struct;
>  struct pt_regs;
> @@ -22,6 +23,8 @@ extern void save_stack_trace_tsk(struct task_struct *tsk,
>  extern void print_stack_trace(struct stack_trace *trace, int spaces);
>  extern int snprint_stack_trace(char *buf, size_t size,
>  			struct stack_trace *trace, int spaces);
> +extern void seq_print_stack_trace(struct seq_file *m,
> +			struct stack_trace *trace, int spaces);
>  
>  #ifdef CONFIG_USER_STACKTRACE_SUPPORT
>  extern void save_stack_trace_user(struct stack_trace *trace);
> @@ -35,6 +38,7 @@ extern void save_stack_trace_user(struct stack_trace *trace);
>  # define save_stack_trace_user(trace)			do { } while (0)
>  # define print_stack_trace(trace, spaces)		do { } while (0)
>  # define snprint_stack_trace(buf, size, trace, spaces)	do { } while (0)
> +# define seq_print_stack_trace(m, trace, spaces)	do { } while (0)
>  #endif
>  
>  #endif
> diff --git a/kernel/stacktrace.c b/kernel/stacktrace.c
> index b6e4c16..66ef6f4 100644
> --- a/kernel/stacktrace.c
> +++ b/kernel/stacktrace.c
> @@ -57,6 +57,23 @@ int snprint_stack_trace(char *buf, size_t size,
>  }
>  EXPORT_SYMBOL_GPL(snprint_stack_trace);
>  
> +void seq_print_stack_trace(struct seq_file *m, struct stack_trace *trace,
> +			int spaces)
> +{
> +	int i;
> +
> +	if (WARN_ON(!trace->entries))
> +		return;
> +
> +	for (i = 0; i < trace->nr_entries; i++) {
> +		unsigned long ip = trace->entries[i];
> +
> +		seq_printf(m, "%*c[<%p>] %pS\n", 1 + spaces, ' ',
> +				(void *) ip, (void *) ip);
> +	}
> +}
> +EXPORT_SYMBOL_GPL(seq_print_stack_trace);
> +
>  /*
>   * Architectures that do not implement save_stack_trace_tsk or
>   * save_stack_trace_regs get this weak alias and a once-per-bootup warning
> -- 
> 2.1.0
>

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--

Re: [PATCH v4 4/5] mm: cma: add list of currently allocated CMA buffers to debugfs

[Michal Nazarewicz]

On Mon, Mar 16 2015, Stefan Strogin wrote:
> When CONFIG_CMA_BUFFER_LIST is configured a file is added to debugfs:
> /sys/kernel/debug/cma/cma-<N>/buffers contains a list of currently allocated
> CMA buffers for each CMA region (N stands for number of CMA region).
>
> Format is:
> <base_phys_addr> - <end_phys_addr> (<size> kB), allocated by <PID> (<comm>)
>
> When CONFIG_CMA_ALLOC_STACKTRACE is configured then stack traces are saved when
> the allocations are made. The stack traces are added to cma/cma-<N>/buffers
> for each buffer list entry.
>
> Example:
>
> 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
> <...>
>
> Signed-off-by: Stefan Strogin <stefan.strogin@gmail.com>

Acked-by: Michal Nazarewicz <mina86@mina86.com>

> @@ -127,6 +240,93 @@ static int cma_alloc_write(void *data, u64 val)
>  
>  DEFINE_SIMPLE_ATTRIBUTE(cma_alloc_fops, NULL, cma_alloc_write, "%llu\n");
>  
> +#ifdef CONFIG_CMA_BUFFER_LIST
> +static void *s_start(struct seq_file *seq, loff_t *ppos)
> +{
> +	struct cma *cma = seq->private;
> +	struct cma_buffer *cmabuf;
> +	loff_t n = *ppos;
> +
> +	mutex_lock(&cma->list_lock);
> +	cmabuf = list_first_entry(&cma->buffer_list, typeof(*cmabuf), list);
> +	list_for_each_entry(cmabuf, &cma->buffer_list, list)
> +		if (n-- == 0)
> +			return cmabuf;
> +
> +	return 0;

	return NULL;

> +}

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--

Re: [PATCH v4 1/5] mm: cma: add trace events to debug physically-contiguous memory allocations

[Stefan Strogin]

Oops... forgot to cc tracing maintainers. Sorry!

On 16/03/15 19:06, Stefan Strogin wrote:
> Add trace events for cma_alloc() and cma_release().
> 
> Signed-off-by: Stefan Strogin <stefan.strogin@gmail.com>
> ---
>  include/trace/events/cma.h | 57 ++++++++++++++++++++++++++++++++++++++++++++++
>  mm/cma.c                   |  5 ++++
>  2 files changed, 62 insertions(+)
>  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..d88881b
> --- /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, struct page *page, int count),
> +
> +	TP_ARGS(cma, page, count),
> +
> +	TP_STRUCT__entry(
> +		__field(struct page *, page)
> +		__field(unsigned long, count)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->page = page;
> +		__entry->count = count;
> +	),
> +
> +	TP_printk("page=%p pfn=%lu count=%lu",
> +		  __entry->page,
> +		  __entry->page ? page_to_pfn(__entry->page) : 0,
> +		  __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",
> +		  __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 47203fa..63dfc0e 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>
> @@ -34,6 +35,7 @@
>  #include <linux/cma.h>
>  #include <linux/highmem.h>
>  #include <linux/io.h>
> +#include <trace/events/cma.h>
>  
>  #include "cma.h"
>  
> @@ -414,6 +416,8 @@ struct page *cma_alloc(struct cma *cma, unsigned int count, unsigned int align)
>  		start = bitmap_no + mask + 1;
>  	}
>  
> +	trace_cma_alloc(cma, page, count);
> +
>  	pr_debug("%s(): returned %p\n", __func__, page);
>  	return page;
>  }
> @@ -446,6 +450,7 @@ bool cma_release(struct cma *cma, const struct page *pages, unsigned int count)
>  
>  	free_contig_range(pfn, count);
>  	cma_clear_bitmap(cma, pfn, count);
> +	trace_cma_release(cma, pfn, count);
>  
>  	return true;
>  }
> 

Re: [PATCH v4 1/5] mm: cma: add trace events to debug physically-contiguous memory allocations

[Steven Rostedt]

On Mon, 16 Mar 2015 23:49:01 +0300
Stefan Strogin <s.strogin@partner.samsung.com> wrote:

> Oops... forgot to cc tracing maintainers. Sorry!

Thanks,

> 
> On 16/03/15 19:06, Stefan Strogin wrote:
> > Add trace events for cma_alloc() and cma_release().
> > 
> > Signed-off-by: Stefan Strogin <stefan.strogin@gmail.com>
> > ---
> >  include/trace/events/cma.h | 57 ++++++++++++++++++++++++++++++++++++++++++++++
> >  mm/cma.c                   |  5 ++++
> >  2 files changed, 62 insertions(+)
> >  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..d88881b
> > --- /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, struct page *page, int count),
> > +
> > +	TP_ARGS(cma, page, count),
> > +
> > +	TP_STRUCT__entry(
> > +		__field(struct page *, page)
> > +		__field(unsigned long, count)
> > +	),
> > +
> > +	TP_fast_assign(
> > +		__entry->page = page;
> > +		__entry->count = count;
> > +	),
> > +
> > +	TP_printk("page=%p pfn=%lu count=%lu",
> > +		  __entry->page,
> > +		  __entry->page ? page_to_pfn(__entry->page) : 0,

Can page_to_pfn(value) ever be different throughout the life of the
boot? That is, can it return a different result given the same value
(vmalloc area comes to mind).

> > +		  __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",
> > +		  __entry->pfn,
> > +		  pfn_to_page(__entry->pfn),

Same here. Can pfn_to_page(value) ever return a different result with
the same value in a single boot?

-- Steve

> > +		  __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 47203fa..63dfc0e 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>
> > @@ -34,6 +35,7 @@
> >  #include <linux/cma.h>
> >  #include <linux/highmem.h>
> >  #include <linux/io.h>
> > +#include <trace/events/cma.h>
> >  
> >  #include "cma.h"
> >  
> > @@ -414,6 +416,8 @@ struct page *cma_alloc(struct cma *cma, unsigned int count, unsigned int align)
> >  		start = bitmap_no + mask + 1;
> >  	}
> >  
> > +	trace_cma_alloc(cma, page, count);
> > +
> >  	pr_debug("%s(): returned %p\n", __func__, page);
> >  	return page;
> >  }
> > @@ -446,6 +450,7 @@ bool cma_release(struct cma *cma, const struct page *pages, unsigned int count)
> >  
> >  	free_contig_range(pfn, count);
> >  	cma_clear_bitmap(cma, pfn, count);
> > +	trace_cma_release(cma, pfn, count);
> >  
> >  	return true;
> >  }
> > 

Re: [PATCH v4 0/5] mm: cma: add some debug information for CMA

[Joonsoo Kim]

On Mon, Mar 16, 2015 at 07:06:55PM +0300, Stefan Strogin wrote:
> Hi all.
> 
> Here is the fourth version of a patch set that adds some debugging facility for
> CMA.
> 
> This patch set is based on next-20150316.
> It is also available on git:
> git://github.com/stefanstrogin/linux -b cmainfo-v4
> 
> 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).
> 
> For example. 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
> could fail. If it happened then with such an interface we could find who used
> CMA at the moment of failure, who caused fragmentation and so on. Ftrace also
> would be helpful here, but with ftrace we can see the whole history of
> allocations and releases, whereas with this patch set we can see a snapshot of
> CMA region with actual information about its allocations.

Hello,

Hmm... I still don't think that this is really helpful to find root
cause of fragmentation. Think about following example.

Assume 1024 MB CMA region.

128 MB allocation * 4
1 MB allocation
128 MB allocation
128 MB release * 4 (first 4)
try 512 MB allocation

With above sequences, fragmentation happens and 512 MB allocation would
be failed. We can get information about 1 MB allocation and 128 MB one
from the buffer list as you suggested, but, fragmentation are related
to whole sequence of allocation/free history, not snapshot of allocation.

Thanks.

Re: [PATCH v4 0/5] mm: cma: add some debug information for CMA

[Sasha Levin]

On 03/16/2015 09:43 PM, Joonsoo Kim wrote:
> On Mon, Mar 16, 2015 at 07:06:55PM +0300, Stefan Strogin wrote:
>> > Hi all.
>> > 
>> > Here is the fourth version of a patch set that adds some debugging facility for
>> > CMA.
>> > 
>> > This patch set is based on next-20150316.
>> > It is also available on git:
>> > git://github.com/stefanstrogin/linux -b cmainfo-v4
>> > 
>> > 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).
>> > 
>> > For example. 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
>> > could fail. If it happened then with such an interface we could find who used
>> > CMA at the moment of failure, who caused fragmentation and so on. Ftrace also
>> > would be helpful here, but with ftrace we can see the whole history of
>> > allocations and releases, whereas with this patch set we can see a snapshot of
>> > CMA region with actual information about its allocations.
> Hello,
> 
> Hmm... I still don't think that this is really helpful to find root
> cause of fragmentation. Think about following example.
> 
> Assume 1024 MB CMA region.
> 
> 128 MB allocation * 4
> 1 MB allocation
> 128 MB allocation
> 128 MB release * 4 (first 4)
> try 512 MB allocation
> 
> With above sequences, fragmentation happens and 512 MB allocation would
> be failed. We can get information about 1 MB allocation and 128 MB one
> from the buffer list as you suggested, but, fragmentation are related
> to whole sequence of allocation/free history, not snapshot of allocation.

This is solvable by dumping task->comm in the tracepoint patch (1/5), right?


Thanks,
Sasha

Re: [PATCH v4 0/5] mm: cma: add some debug information for CMA

[Joonsoo Kim]

On Mon, Mar 16, 2015 at 09:54:18PM -0400, Sasha Levin wrote:
> On 03/16/2015 09:43 PM, Joonsoo Kim wrote:
> > On Mon, Mar 16, 2015 at 07:06:55PM +0300, Stefan Strogin wrote:
> >> > Hi all.
> >> > 
> >> > Here is the fourth version of a patch set that adds some debugging facility for
> >> > CMA.
> >> > 
> >> > This patch set is based on next-20150316.
> >> > It is also available on git:
> >> > git://github.com/stefanstrogin/linux -b cmainfo-v4
> >> > 
> >> > 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).
> >> > 
> >> > For example. 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
> >> > could fail. If it happened then with such an interface we could find who used
> >> > CMA at the moment of failure, who caused fragmentation and so on. Ftrace also
> >> > would be helpful here, but with ftrace we can see the whole history of
> >> > allocations and releases, whereas with this patch set we can see a snapshot of
> >> > CMA region with actual information about its allocations.
> > Hello,
> > 
> > Hmm... I still don't think that this is really helpful to find root
> > cause of fragmentation. Think about following example.
> > 
> > Assume 1024 MB CMA region.
> > 
> > 128 MB allocation * 4
> > 1 MB allocation
> > 128 MB allocation
> > 128 MB release * 4 (first 4)
> > try 512 MB allocation
> > 
> > With above sequences, fragmentation happens and 512 MB allocation would
> > be failed. We can get information about 1 MB allocation and 128 MB one
> > from the buffer list as you suggested, but, fragmentation are related
> > to whole sequence of allocation/free history, not snapshot of allocation.
> 
> This is solvable by dumping task->comm in the tracepoint patch (1/5), right?

Yes, it can be solved by 1/5.
I mean that I'm not sure patch 4/5 is really needed or not.

Thanks.

Re: [PATCH v4 1/5] mm: cma: add trace events to debug physically-contiguous memory allocations

[Ingo Molnar]

* Stefan Strogin <s.strogin@partner.samsung.com> wrote:

> > +TRACE_EVENT(cma_alloc,
> > +
> > +	TP_PROTO(struct cma *cma, struct page *page, int count),
> > +
> > +	TP_ARGS(cma, page, count),
> > +
> > +	TP_STRUCT__entry(
> > +		__field(struct page *, page)
> > +		__field(unsigned long, count)
> > +	),
> > +
> > +	TP_fast_assign(
> > +		__entry->page = page;
> > +		__entry->count = count;
> > +	),
> > +
> > +	TP_printk("page=%p pfn=%lu count=%lu",
> > +		  __entry->page,
> > +		  __entry->page ? page_to_pfn(__entry->page) : 0,
> > +		  __entry->count)

So I'm wondering, the fast-assign side is not equivalent to the 
TP_printk() side:

> > +		__entry->page = page;
> > +		  __entry->page ? page_to_pfn(__entry->page) : 0,

to me it seems it would be useful if MM tracing standardized on pfn 
printing. Just like you did for trace_cma_release().

Also:

> > +		  __entry->page ? page_to_pfn(__entry->page) : 0,

pfn 0 should probably be reserved for the true 0th pfn - those exist 
in some machines. Returning -1ll could be the 'no such pfn' condition?

> > +	TP_STRUCT__entry(
> > +		__field(unsigned long, pfn)

Btw., does pfn always fit into 32 bits on 32-bit platforms?

> > +		__field(unsigned long, count)

Does this have to be 64-bit on 64-bit platforms?

> > +	),
> > +
> > +	TP_fast_assign(
> > +		__entry->pfn = pfn;
> > +		__entry->count = count;
> > +	),
> > +
> > +	TP_printk("pfn=%lu page=%p count=%lu",
> > +		  __entry->pfn,
> > +		  pfn_to_page(__entry->pfn),
> > +		  __entry->count)

So here you print more in the TP_printk() line than in the fast-assign 
side.

Again I'd double check the various boundary conditions.

Thanks,

	Ingo

Re: [PATCH v4 1/5] mm: cma: add trace events to debug physically-contiguous memory allocations

[Stefan Strogin]

On 17/03/15 02:47, Steven Rostedt wrote:
>>> +TRACE_EVENT(cma_alloc,
>>> +
>>> +	TP_PROTO(struct cma *cma, struct page *page, int count),
>>> +
>>> +	TP_ARGS(cma, page, count),
>>> +
>>> +	TP_STRUCT__entry(
>>> +		__field(struct page *, page)
>>> +		__field(unsigned long, count)
>>> +	),
>>> +
>>> +	TP_fast_assign(
>>> +		__entry->page = page;
>>> +		__entry->count = count;
>>> +	),
>>> +
>>> +	TP_printk("page=%p pfn=%lu count=%lu",
>>> +		  __entry->page,
>>> +		  __entry->page ? page_to_pfn(__entry->page) : 0,
> 
> Can page_to_pfn(value) ever be different throughout the life of the
> boot? That is, can it return a different result given the same value
> (vmalloc area comes to mind).
> 
>>> +	TP_printk("pfn=%lu page=%p count=%lu",
>>> +		  __entry->pfn,
>>> +		  pfn_to_page(__entry->pfn),
> 
> Same here. Can pfn_to_page(value) ever return a different result with
> the same value in a single boot?
> 

Thank you for the reply, Steven.
I supposed that page_to_pfn() cannot change after mem_map
initialization, can it? I'm not sure about such things as memory hotplug
though...
Also cma_alloc() calls alloc_contig_range() which returns pfn, then it's
converted to struct page * and cma_alloc() returns struct page *, and
vice versa in cma_release() (receives struct page * and passes pfn to
free_contig_rage()).
Do you mean that printing pfn (or struct page *) in trace event is
redundant?

Re: [PATCH v4 1/5] mm: cma: add trace events to debug physically-contiguous memory allocations

[Stefan Strogin]

On 17/03/15 10:40, Ingo Molnar wrote:
> 
> * Stefan Strogin <s.strogin@partner.samsung.com> wrote:
> 
>>> +TRACE_EVENT(cma_alloc,
>>> +
>>> +	TP_PROTO(struct cma *cma, struct page *page, int count),
>>> +
>>> +	TP_ARGS(cma, page, count),
>>> +
>>> +	TP_STRUCT__entry(
>>> +		__field(struct page *, page)
>>> +		__field(unsigned long, count)
>>> +	),
>>> +
>>> +	TP_fast_assign(
>>> +		__entry->page = page;
>>> +		__entry->count = count;
>>> +	),
>>> +
>>> +	TP_printk("page=%p pfn=%lu count=%lu",
>>> +		  __entry->page,
>>> +		  __entry->page ? page_to_pfn(__entry->page) : 0,
>>> +		  __entry->count)
> 
> So I'm wondering, the fast-assign side is not equivalent to the 
> TP_printk() side:
> 
>>> +		__entry->page = page;
>>> +		  __entry->page ? page_to_pfn(__entry->page) : 0,
> 
> to me it seems it would be useful if MM tracing standardized on pfn 
> printing. Just like you did for trace_cma_release().
> 

Hello Ingo, thank you for the reply.
I afraid there is no special sense in printing both struct page * and
pfn. But cma_alloc() returns struct page *, cma_release receives struct
page *, and pr_debugs in these functions print struct page *. Maybe it
would be better to print the same here too?

> Also:
> 
>>> +		  __entry->page ? page_to_pfn(__entry->page) : 0,
> 
> pfn 0 should probably be reserved for the true 0th pfn - those exist 
> in some machines. Returning -1ll could be the 'no such pfn' condition?
> 

I took this from trace_mm_page_alloc() and other trace events from
trace/events/kmem.h. If we return -1 here to indicate "no such pfn",
should we change do this in kmem.h too?

>>> +	TP_STRUCT__entry(
>>> +		__field(unsigned long, pfn)
> 
> Btw., does pfn always fit into 32 bits on 32-bit platforms?
> 

Well, I think it does. cma_release() uses 'unsigned long' on all platforms.

>>> +		__field(unsigned long, count)
> 
> Does this have to be 64-bit on 64-bit platforms?
> 

Oops! I'm terribly wrong.
+		__field(unsigned int, count)

I guess it shouldn't be 64-bit on 64-bit platforms. It's the number of
pages being freed, and in cma_release() 'unsigned int' is used for it.

>>> +	),
>>> +
>>> +	TP_fast_assign(
>>> +		__entry->pfn = pfn;
>>> +		__entry->count = count;
>>> +	),
>>> +
>>> +	TP_printk("pfn=%lu page=%p count=%lu",
>>> +		  __entry->pfn,
>>> +		  pfn_to_page(__entry->pfn),
>>> +		  __entry->count)
> 
> So here you print more in the TP_printk() line than in the fast-assign 
> side.
> 

See above, I think it's the same case as in trace_cma_alloc() TP_printk().

> Again I'd double check the various boundary conditions.
> 

Sorry, I don't quite understand. Boundary conditions are already [should
be] checked in cma_alloc()/cma_release, we should only pass to a trace
event the information we want to be known, isn't it so?

I again terribly sorry, I also completely forgot about struct cma *
being passed to trace event. I think either it should be used somehow
(e.g. to print the number of CMA region) or shouldn't be passed...

Re: [PATCH v4 1/5] mm: cma: add trace events to debug physically-contiguous memory allocations

[Steven Rostedt]

On Thu, 19 Mar 2015 23:18:18 +0300
Stefan Strogin <s.strogin@partner.samsung.com> wrote:

> Thank you for the reply, Steven.
> I supposed that page_to_pfn() cannot change after mem_map
> initialization, can it? I'm not sure about such things as memory hotplug
> though...
> Also cma_alloc() calls alloc_contig_range() which returns pfn, then it's
> converted to struct page * and cma_alloc() returns struct page *, and
> vice versa in cma_release() (receives struct page * and passes pfn to
> free_contig_rage()).
> Do you mean that printing pfn (or struct page *) in trace event is
> redundant?

I'm concerned about the time TP_printk() is executed and when
TP_fast_assign() is. That is, when the tracepoint is called,
TP_fast_assign() is executed, but TP_printk() is called when someone
reads the trace files, which can happen seconds, hours, days, weeks,
months later.

As long as the result of page_to_pfn() and pfn_to_page() stay the same
throughout the life of the boot, things will be fine. But if they could
ever change, due to hotplug memory or whatever. The data in the trace
buffer will become stale, and report the wrong information.

-- Steve

Re: [PATCH v4 1/5] mm: cma: add trace events to debug physically-contiguous memory allocations

[Stefan Strogin]

On 19/03/15 23:34, Steven Rostedt wrote:
> On Thu, 19 Mar 2015 23:18:18 +0300
> Stefan Strogin <s.strogin@partner.samsung.com> wrote:
> 
>> Thank you for the reply, Steven.
>> I supposed that page_to_pfn() cannot change after mem_map
>> initialization, can it? I'm not sure about such things as memory hotplug
>> though...
>> Also cma_alloc() calls alloc_contig_range() which returns pfn, then it's
>> converted to struct page * and cma_alloc() returns struct page *, and
>> vice versa in cma_release() (receives struct page * and passes pfn to
>> free_contig_rage()).
>> Do you mean that printing pfn (or struct page *) in trace event is
>> redundant?
> 
> I'm concerned about the time TP_printk() is executed and when
> TP_fast_assign() is. That is, when the tracepoint is called,
> TP_fast_assign() is executed, but TP_printk() is called when someone
> reads the trace files, which can happen seconds, hours, days, weeks,
> months later.
> 
> As long as the result of page_to_pfn() and pfn_to_page() stay the same
> throughout the life of the boot, things will be fine. But if they could
> ever change, due to hotplug memory or whatever. The data in the trace
> buffer will become stale, and report the wrong information.
> 
> -- Steve
> 

Ah, thanks, I see. So will this solve the described issue?
+	TP_fast_assign(
+		__entry->page = page;
+		__entry->pfn = page_to_pfn(__entry->page) : 0;
/* or -1 as Ingo suggested */
+		__entry->count = count;
+	),
+
+	TP_printk("page=%p pfn=%lu count=%u",
+		  __entry->page,
+		  __entry->pfn,
+		  __entry->count)

Should we do the same in trace/events/kmem.h then?

But really I'm not sure why page_to_pfn()/pfn_to_page() can return
different results... I thought that there can appear new 'struct page'
entries arrays throughout one boot due to memory hotplug or smth. But
how can existing 'struct page' entries associated with the same physical
pages change their physical addresses? Or how can one physical address
correspond to different physical page throughout one boot?

Re: [PATCH v4 1/5] mm: cma: add trace events to debug physically-contiguous memory allocations

[Steven Rostedt]

On Fri, 20 Mar 2015 13:46:39 +0300
Stefan Strogin <s.strogin@partner.samsung.com> wrote:
 
> Ah, thanks, I see. So will this solve the described issue?
> +	TP_fast_assign(
> +		__entry->page = page;
> +		__entry->pfn = page_to_pfn(__entry->page) : 0;
> /* or -1 as Ingo suggested */
> +		__entry->count = count;
> +	),
> +
> +	TP_printk("page=%p pfn=%lu count=%u",
> +		  __entry->page,
> +		  __entry->pfn,
> +		  __entry->count)
> 
> Should we do the same in trace/events/kmem.h then?
> 
> But really I'm not sure why page_to_pfn()/pfn_to_page() can return
> different results... I thought that there can appear new 'struct page'
> entries arrays throughout one boot due to memory hotplug or smth. But
> how can existing 'struct page' entries associated with the same physical
> pages change their physical addresses? Or how can one physical address
> correspond to different physical page throughout one boot?

I don't know if those mappings can change. I'm just warning you that if
they can, then you can have an issue with it. If that's the case, then
it would be best to do the work in the tracepoint instead of the print.

One benefit for making this change is that it will let userspace tools
such as perf and trace-cmd parse it better.

-- Steve

Re: [PATCH v4 1/5] mm: cma: add trace events to debug physically-contiguous memory allocations

[Ingo Molnar]

* Stefan Strogin <s.strogin@partner.samsung.com> wrote:

> 
> On 17/03/15 10:40, Ingo Molnar wrote:
> > 
> > * Stefan Strogin <s.strogin@partner.samsung.com> wrote:
> > 
> >>> +TRACE_EVENT(cma_alloc,
> >>> +
> >>> +	TP_PROTO(struct cma *cma, struct page *page, int count),
> >>> +
> >>> +	TP_ARGS(cma, page, count),
> >>> +
> >>> +	TP_STRUCT__entry(
> >>> +		__field(struct page *, page)
> >>> +		__field(unsigned long, count)
> >>> +	),
> >>> +
> >>> +	TP_fast_assign(
> >>> +		__entry->page = page;
> >>> +		__entry->count = count;
> >>> +	),
> >>> +
> >>> +	TP_printk("page=%p pfn=%lu count=%lu",
> >>> +		  __entry->page,
> >>> +		  __entry->page ? page_to_pfn(__entry->page) : 0,
> >>> +		  __entry->count)
> > 
> > So I'm wondering, the fast-assign side is not equivalent to the 
> > TP_printk() side:
> > 
> >>> +		__entry->page = page;
> >>> +		  __entry->page ? page_to_pfn(__entry->page) : 0,
> > 
> > to me it seems it would be useful if MM tracing standardized on pfn 
> > printing. Just like you did for trace_cma_release().
> > 
> 
> Hello Ingo, thank you for the reply.
> I afraid there is no special sense in printing both struct page * and
> pfn. But cma_alloc() returns struct page *, cma_release receives struct
> page *, and pr_debugs in these functions print struct page *. Maybe it
> would be better to print the same here too?

So will the tracepoints primarily log 'struct page *'?

If yes, my question is: why not log pfn? pfn is much more informative 
(it's a hardware property of the page, not a kernel-internal 
descriptor like 'struct page *') , and it tells us (without knowing 
the layout of the kernel) which NUMA node a given area lies on, etc.

Or do other mm tracepoints already (mistakenly) use 'struct page *'?

> > Again I'd double check the various boundary conditions.
> > 
> 
> Sorry, I don't quite understand. Boundary conditions are already 
> [should be] checked in cma_alloc()/cma_release, we should only pass 
> to a trace event the information we want to be known, isn't it so?

No, I mean tracing info boundary conditions: what is returned when no 
such page is allocated, what is returned when pfn #0 is allocated, 
etc.

Thanks,

	Ingo