[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-id: <54F48560.1090800@partner.samsung.com>
Date: Mon, 02 Mar 2015 18:44:32 +0300
From: Stefan Strogin <s.strogin@...tner.samsung.com>
To: Michal Nazarewicz <mina86@...a86.com>, linux-mm@...ck.org,
linux-kernel@...r.kernel.org
Cc: Joonsoo Kim <iamjoonsoo.kim@....com>,
Andrew Morton <akpm@...ux-foundation.org>,
Marek Szyprowski <m.szyprowski@...sung.com>,
aneesh.kumar@...ux.vnet.ibm.com,
Laurent Pinchart <laurent.pinchart@...asonboard.com>,
Dmitry Safonov <d.safonov@...tner.samsung.com>,
Pintu Kumar <pintu.k@...sung.com>,
Weijie Yang <weijie.yang@...sung.com>,
Laura Abbott <lauraa@...eaurora.org>,
SeongJae Park <sj38.park@...il.com>,
Hui Zhu <zhuhui@...omi.com>, Minchan Kim <minchan@...nel.org>,
Dyasly Sergey <s.dyasly@...sung.com>,
Vyacheslav Tyrtov <v.tyrtov@...sung.com>,
Aleksei Mateosian <a.mateosian@...sung.com>,
gregory.0xf0@...il.com, sasha.levin@...cle.com, gioh.kim@....com,
pavel@....cz, stefan.strogin@...il.com
Subject: Re: [PATCH v3 3/4] mm: cma: add list of currently allocated CMA
buffers to debugfs
Hi MichaĆ,
Thank you for the answer.
On 25/02/15 00:32, Michal Nazarewicz wrote:
> On Tue, Feb 24 2015, Stefan Strogin <s.strogin@...tner.samsung.com> wrote:
>> --- a/mm/cma.h
>> +++ b/mm/cma.h
>> @@ -11,8 +13,32 @@ 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
>> +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;
>> +};
>
> This structure is only ever used in cma_debug.c so is there a reason
> to define it in the header file?
>
No, there isn't. Thanks. I'll move it to cma_debug.c
>> +
>> +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 /* CONFIG_CMA_BUFFER_LIST */
>> +
>> extern struct cma cma_areas[MAX_CMA_AREAS];
>> extern unsigned cma_area_count;
>
>
>> +#ifdef CONFIG_CMA_BUFFER_LIST
>> +static ssize_t cma_buffer_list_read(struct file *file, char __user *userbuf,
>> + size_t count, loff_t *ppos)
>> +{
>> + struct cma *cma = file->private_data;
>> + struct cma_buffer *cmabuf;
>> + char *buf;
>> + int ret, n = 0;
>> +#ifdef CONFIG_CMA_ALLOC_STACKTRACE
>> + struct stack_trace trace;
>> +#endif
>> +
>> + if (*ppos < 0 || !count)
>> + return -EINVAL;
>> +
>> + buf = vmalloc(count);
>> + if (!buf)
>> + return -ENOMEM;
>> +
>> + mutex_lock(&cma->list_lock);
>> + list_for_each_entry(cmabuf, &cma->buffer_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);
>> +
>> +#ifdef CONFIG_CMA_ALLOC_STACKTRACE
>> + 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");
>> +#endif
>> + }
>> + mutex_unlock(&cma->list_lock);
>> +
>> + ret = simple_read_from_buffer(userbuf, count, ppos, buf, n);
>> + vfree(buf);
>> +
>> + return ret;
>> +}
>
> So in practice user space must allocate buffer big enough to read the
> whole file into memory. Calling read(2) with some count will never read
> anything past the first count bytes of the file.
>
My fault. You are right.
I'm not sure how to do the output nice... I could use *ppos to point the
number of next list entry to read (like that is used in
read_page_owner()). But in this case the list could be changed before we
finish reading, it's bad.
Or we could use seq_files like in v1, iterating over buffer_list
entries. But seq_print_stack_trace() has to be added.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists