linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] MM: CMA: add a simple kernel module as the helper to test CMA
@ 2012-03-07  3:14 Barry Song
  2012-03-07  4:14 ` Cong Wang
  2012-03-07 10:59 ` Michal Nazarewicz
  0 siblings, 2 replies; 16+ messages in thread
From: Barry Song @ 2012-03-07  3:14 UTC (permalink / raw)
  To: m.szyprowski, mina86
  Cc: linux-arm-kernel, linux-kernel, workgroup.linux, Barry Song

From: Barry Song <Baohua.Song@csr.com>

Any write request to /dev/cma_test will let the module to allocate memory from
CMA, for example:

1st time
$ echo 1024 > /dev/cma_test
will require cma_test to request 1MB(1024KB)
2nd time
$ echo 2048 > /dev/cma_test
will require cma_test to request 2MB(2048KB)

Any read request to /dev/cma_test will let the module to free the 1st valid
memory from CMA, for example:

1st time
$ cat /dev/cma_test
will require cma_test to free the 1MB allocated in the first write request
2nd time
$ cat /dev/cma_test
will require cma_test to free the 2MB allocated in the second write request

Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 -v2:
 some minor cleanups by Michal Nazarewicz

 tools/cma/Makefile   |   13 +++++
 tools/cma/cma_test.c |  140 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 153 insertions(+), 0 deletions(-)
 create mode 100644 tools/cma/Makefile
 create mode 100644 tools/cma/cma_test.c

diff --git a/tools/cma/Makefile b/tools/cma/Makefile
new file mode 100644
index 0000000..d15c2c0
--- /dev/null
+++ b/tools/cma/Makefile
@@ -0,0 +1,13 @@
+# Kernel modules
+#
+# To compile for ARM:
+# make ARCH=arm CC=arm-none-linux-gnueabi-gcc
+#
+obj-m	+= cma_test.o
+
+build: kernel_modules
+
+kernel_modules:
+	${MAKE} -C $(CURDIR)/../.. M=$(CURDIR)
+clean:
+	${MAKE} -C $(CURDIR)/../.. M=$(CURDIR) clean
diff --git a/tools/cma/cma_test.c b/tools/cma/cma_test.c
new file mode 100644
index 0000000..40219ad
--- /dev/null
+++ b/tools/cma/cma_test.c
@@ -0,0 +1,140 @@
+/*
+ * kernel module helper for testing CMA
+ *
+ * Copyright (c) 2012 Cambridge Silicon Radio Limited, a CSR plc group company.
+ *
+ * Licensed under GPLv2 or later.
+ */
+
+#include <linux/device.h>
+#include <linux/dma-mapping.h>
+#include <linux/fs.h>
+#include <linux/miscdevice.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+
+struct cma_allocation {
+	struct list_head list;
+	size_t size;
+	dma_addr_t dma;
+	void *virt;
+};
+
+static struct device *cma_dev;
+static LIST_HEAD(cma_allocations);
+static DEFINE_SPINLOCK(cma_lock);
+
+/*
+ * any read request will free the 1st allocated coherent memory, eg.
+ * cat /dev/cma_test
+ */
+static ssize_t
+cma_test_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
+{
+	struct cma_allocation *alloc = NULL;
+
+	spin_lock(&cma_lock);
+	if (!list_empty(&cma_allocations)) {
+		alloc = list_first_entry(&cma_allocations,
+			struct cma_allocation, list);
+		list_del(&alloc->list);
+	}
+	spin_unlock(&cma_lock);
+
+	if (!alloc)
+		return -EIDRM;
+
+	dma_free_coherent(cma_dev, alloc->size, alloc->virt,
+		alloc->dma);
+
+	_dev_info(cma_dev, "free: CM virt: %p dma: %p size:%uK\n",
+		alloc->virt, (void *)alloc->dma, alloc->size / SZ_1K);
+	kfree(alloc);
+
+	return 0;
+}
+
+/*
+ * any write request will alloc a new coherent memory, eg.
+ * echo 1024 > /dev/cma_test
+ * will request 1024KiB by CMA
+ */
+static ssize_t
+cma_test_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
+{
+	struct cma_allocation *alloc;
+	size_t size;
+	int ret;
+
+	ret = kstrtouint_from_user(buf, count, 0, &size);
+	if (ret)
+		return ret;
+
+	if (!size)
+		return -EINVAL;
+
+	if (size > ~(size_t)0 / SZ_1K)
+		return -EOVERFLOW;
+
+	alloc = kmalloc(sizeof *alloc, GFP_KERNEL);
+	if (!alloc)
+		return -ENOMEM;
+
+	alloc->size = size * SZ_1K;
+	alloc->virt = dma_alloc_coherent(cma_dev, alloc->size,
+		&alloc->dma, GFP_KERNEL);
+
+	if (alloc->virt) {
+		_dev_info(cma_dev, "alloc: virt: %p dma: %p size: %uK\n",
+			alloc->virt, (void *)alloc->dma, size);
+
+		spin_lock(&cma_lock);
+		list_add_tail(&alloc->list, &cma_allocations);
+		spin_unlock(&cma_lock);
+
+		return count;
+	} else {
+		dev_err(cma_dev, "no mem in CMA area\n");
+		kfree(alloc);
+		return -ENOSPC;
+	}
+}
+
+static const struct file_operations cma_test_fops = {
+	.owner =    THIS_MODULE,
+	.read  =    cma_test_read,
+	.write =    cma_test_write,
+};
+
+static struct miscdevice cma_test_misc = {
+	.name = "cma_test",
+	.fops = &cma_test_fops,
+};
+
+static int __init cma_test_init(void)
+{
+	int ret = misc_register(&cma_test_misc);
+
+	if (unlikely(ret)) {
+		pr_err("failed to register cma test misc device!\n");
+		return ret;
+	}
+	cma_dev = cma_test_misc.this_device;
+	cma_dev->coherent_dma_mask = ~0;
+	_dev_info(cma_dev, "registered.\n");
+
+	return 0;
+}
+module_init(cma_test_init);
+
+static void __exit cma_test_exit(void)
+{
+	misc_deregister(&cma_test_misc);
+}
+module_exit(cma_test_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Barry Song <Baohua.Song@csr.com>");
+MODULE_DESCRIPTION("kernel module to help the test of CMA");
+MODULE_ALIAS("cma_test");
-- 
1.7.1



Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Follow CSR on Twitter at http://twitter.com/CSR_PLC and read our blog at www.csr.com/blog

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v3] MM: CMA: add a simple kernel module as the helper to test CMA
  2012-03-07  3:14 [PATCH v3] MM: CMA: add a simple kernel module as the helper to test CMA Barry Song
@ 2012-03-07  4:14 ` Cong Wang
  2012-03-07  4:49   ` Barry Song
  2012-03-07 10:48   ` Michal Nazarewicz
  2012-03-07 10:59 ` Michal Nazarewicz
  1 sibling, 2 replies; 16+ messages in thread
From: Cong Wang @ 2012-03-07  4:14 UTC (permalink / raw)
  To: Barry Song
  Cc: m.szyprowski, mina86, linux-arm-kernel, linux-kernel,
	workgroup.linux, Barry Song

On 03/07/2012 11:14 AM, Barry Song wrote:
> From: Barry Song<Baohua.Song@csr.com>
>
> Any write request to /dev/cma_test will let the module to allocate memory from
> CMA, for example:
>
> 1st time
> $ echo 1024>  /dev/cma_test
> will require cma_test to request 1MB(1024KB)
> 2nd time
> $ echo 2048>  /dev/cma_test
> will require cma_test to request 2MB(2048KB)
>
> Any read request to /dev/cma_test will let the module to free the 1st valid
> memory from CMA, for example:
>
> 1st time
> $ cat /dev/cma_test
> will require cma_test to free the 1MB allocated in the first write request
> 2nd time
> $ cat /dev/cma_test
> will require cma_test to free the 2MB allocated in the second write request

Any reason why using /dev not /proc or /sys?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3] MM: CMA: add a simple kernel module as the helper to test CMA
  2012-03-07  4:14 ` Cong Wang
@ 2012-03-07  4:49   ` Barry Song
  2012-03-07  4:55     ` Cong Wang
  2012-03-07 10:48   ` Michal Nazarewicz
  1 sibling, 1 reply; 16+ messages in thread
From: Barry Song @ 2012-03-07  4:49 UTC (permalink / raw)
  To: Cong Wang
  Cc: Barry Song, linux-kernel, workgroup.linux, mina86, Barry Song,
	linux-arm-kernel, m.szyprowski

2012/3/7 Cong Wang <xiyou.wangcong@gmail.com>:
> On 03/07/2012 11:14 AM, Barry Song wrote:
>>
>> From: Barry Song<Baohua.Song@csr.com>
>>
>> Any write request to /dev/cma_test will let the module to allocate memory
>> from
>> CMA, for example:
>>
>> 1st time
>> $ echo 1024>  /dev/cma_test
>> will require cma_test to request 1MB(1024KB)
>> 2nd time
>> $ echo 2048>  /dev/cma_test
>> will require cma_test to request 2MB(2048KB)
>>
>> Any read request to /dev/cma_test will let the module to free the 1st
>> valid
>> memory from CMA, for example:
>>
>> 1st time
>> $ cat /dev/cma_test
>> will require cma_test to free the 1MB allocated in the first write request
>> 2nd time
>> $ cat /dev/cma_test
>> will require cma_test to free the 2MB allocated in the second write
>> request
>
>
> Any reason why using /dev not /proc or /sys?

pls note it is just a test module to verify CMA at runtime. it is not
important either it is /dev, it is /proc or it is /sys.

any way to make things easier, we take that way.

-barry

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3] MM: CMA: add a simple kernel module as the helper to test CMA
  2012-03-07  4:49   ` Barry Song
@ 2012-03-07  4:55     ` Cong Wang
  2012-03-07  4:57       ` Barry Song
  0 siblings, 1 reply; 16+ messages in thread
From: Cong Wang @ 2012-03-07  4:55 UTC (permalink / raw)
  To: Barry Song
  Cc: Barry Song, linux-kernel, workgroup.linux, mina86, Barry Song,
	linux-arm-kernel, m.szyprowski

On 03/07/2012 12:49 PM, Barry Song wrote:
> 2012/3/7 Cong Wang<xiyou.wangcong@gmail.com>:
>> On 03/07/2012 11:14 AM, Barry Song wrote:
>>>
>>> From: Barry Song<Baohua.Song@csr.com>
>>>
>>> Any write request to /dev/cma_test will let the module to allocate memory
>>> from
>>> CMA, for example:
>>>
>>> 1st time
>>> $ echo 1024>    /dev/cma_test
>>> will require cma_test to request 1MB(1024KB)
>>> 2nd time
>>> $ echo 2048>    /dev/cma_test
>>> will require cma_test to request 2MB(2048KB)
>>>
>>> Any read request to /dev/cma_test will let the module to free the 1st
>>> valid
>>> memory from CMA, for example:
>>>
>>> 1st time
>>> $ cat /dev/cma_test
>>> will require cma_test to free the 1MB allocated in the first write request
>>> 2nd time
>>> $ cat /dev/cma_test
>>> will require cma_test to free the 2MB allocated in the second write
>>> request
>>
>>
>> Any reason why using /dev not /proc or /sys?
>
> pls note it is just a test module to verify CMA at runtime. it is not
> important either it is /dev, it is /proc or it is /sys.

Why not important? You want it to be merged, don't you?

>
> any way to make things easier, we take that way.
>

Why /dev interface is easier than /proc and /sys? Confused, actually I 
think /proc or /sys should be easier...

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3] MM: CMA: add a simple kernel module as the helper to test CMA
  2012-03-07  4:55     ` Cong Wang
@ 2012-03-07  4:57       ` Barry Song
  2012-03-07  5:01         ` Cong Wang
  0 siblings, 1 reply; 16+ messages in thread
From: Barry Song @ 2012-03-07  4:57 UTC (permalink / raw)
  To: Cong Wang
  Cc: Barry Song, linux-kernel, workgroup.linux, mina86, Barry Song,
	linux-arm-kernel, m.szyprowski

2012/3/7 Cong Wang <xiyou.wangcong@gmail.com>:
> On 03/07/2012 12:49 PM, Barry Song wrote:
>>
>> 2012/3/7 Cong Wang<xiyou.wangcong@gmail.com>:
>>>
>>> On 03/07/2012 11:14 AM, Barry Song wrote:
>>>>
>>>>
>>>> From: Barry Song<Baohua.Song@csr.com>
>>>>
>>>> Any write request to /dev/cma_test will let the module to allocate
>>>> memory
>>>> from
>>>> CMA, for example:
>>>>
>>>> 1st time
>>>> $ echo 1024>    /dev/cma_test
>>>> will require cma_test to request 1MB(1024KB)
>>>> 2nd time
>>>> $ echo 2048>    /dev/cma_test
>>>> will require cma_test to request 2MB(2048KB)
>>>>
>>>> Any read request to /dev/cma_test will let the module to free the 1st
>>>> valid
>>>> memory from CMA, for example:
>>>>
>>>> 1st time
>>>> $ cat /dev/cma_test
>>>> will require cma_test to free the 1MB allocated in the first write
>>>> request
>>>> 2nd time
>>>> $ cat /dev/cma_test
>>>> will require cma_test to free the 2MB allocated in the second write
>>>> request
>>>
>>>
>>>
>>> Any reason why using /dev not /proc or /sys?
>>
>>
>> pls note it is just a test module to verify CMA at runtime. it is not
>> important either it is /dev, it is /proc or it is /sys.
>
>
> Why not important? You want it to be merged, don't you?
>
>
>>
>> any way to make things easier, we take that way.
>>
>
> Why /dev interface is easier than /proc and /sys? Confused, actually I think
> /proc or /sys should be easier...

if you think so, pls do that and send v4 with your s-o-b.

-barry

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3] MM: CMA: add a simple kernel module as the helper to test CMA
  2012-03-07  4:57       ` Barry Song
@ 2012-03-07  5:01         ` Cong Wang
  2012-03-07  5:05           ` Barry Song
  2012-03-07 10:38           ` Barry Song
  0 siblings, 2 replies; 16+ messages in thread
From: Cong Wang @ 2012-03-07  5:01 UTC (permalink / raw)
  To: Barry Song
  Cc: Barry Song, linux-kernel, workgroup.linux, mina86, Barry Song,
	linux-arm-kernel, m.szyprowski

On 03/07/2012 12:57 PM, Barry Song wrote:
>
> if you think so, pls do that and send v4 with your s-o-b.
>

This is not a good attitude, nor helps your patch to be merged...

Be collaborative.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3] MM: CMA: add a simple kernel module as the helper to test CMA
  2012-03-07  5:01         ` Cong Wang
@ 2012-03-07  5:05           ` Barry Song
  2012-03-07  5:09             ` Cong Wang
  2012-03-07 10:38           ` Barry Song
  1 sibling, 1 reply; 16+ messages in thread
From: Barry Song @ 2012-03-07  5:05 UTC (permalink / raw)
  To: Cong Wang
  Cc: Barry Song, linux-kernel, workgroup.linux, mina86, Barry Song,
	linux-arm-kernel, m.szyprowski

2012/3/7 Cong Wang <xiyou.wangcong@gmail.com>:
> On 03/07/2012 12:57 PM, Barry Song wrote:
>>
>>
>> if you think so, pls do that and send v4 with your s-o-b.
>>
>
> This is not a good attitude, nor helps your patch to be merged...
>
> Be collaborative.

some people like focusing on trivial things. i said i really didn't
care about the details whether it is /dev, /proc, /sys.
if you like a /proc, pls do it by yourself and i will agree. but i
will not waste my time on that.

-barry

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3] MM: CMA: add a simple kernel module as the helper to test CMA
  2012-03-07  5:05           ` Barry Song
@ 2012-03-07  5:09             ` Cong Wang
  2012-03-07  5:17               ` Barry Song
  0 siblings, 1 reply; 16+ messages in thread
From: Cong Wang @ 2012-03-07  5:09 UTC (permalink / raw)
  To: Barry Song
  Cc: Barry Song, linux-kernel, workgroup.linux, mina86, Barry Song,
	linux-arm-kernel, m.szyprowski

On 03/07/2012 01:05 PM, Barry Song wrote:
> 2012/3/7 Cong Wang<xiyou.wangcong@gmail.com>:
>> On 03/07/2012 12:57 PM, Barry Song wrote:
>>>
>>>
>>> if you think so, pls do that and send v4 with your s-o-b.
>>>
>>
>> This is not a good attitude, nor helps your patch to be merged...
>>
>> Be collaborative.
>
> some people like focusing on trivial things. i said i really didn't
> care about the details whether it is /dev, /proc, /sys.
> if you like a /proc, pls do it by yourself and i will agree. but i
> will not waste my time on that.

This is definitely not trivial... What's more, kernel developers *do* 
care about trivial things (trivial@kernel.org).

We are trying to make your patch perfect, you just refused, sigh...

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3] MM: CMA: add a simple kernel module as the helper to test CMA
  2012-03-07  5:09             ` Cong Wang
@ 2012-03-07  5:17               ` Barry Song
  2012-03-07  5:22                 ` Cong Wang
  0 siblings, 1 reply; 16+ messages in thread
From: Barry Song @ 2012-03-07  5:17 UTC (permalink / raw)
  To: Cong Wang
  Cc: Barry Song, linux-kernel, workgroup.linux, mina86, Barry Song,
	linux-arm-kernel, m.szyprowski

2012/3/7 Cong Wang <xiyou.wangcong@gmail.com>:
> On 03/07/2012 01:05 PM, Barry Song wrote:
>>
>> 2012/3/7 Cong Wang<xiyou.wangcong@gmail.com>:
>>>
>>> On 03/07/2012 12:57 PM, Barry Song wrote:
>>>>
>>>>
>>>>
>>>> if you think so, pls do that and send v4 with your s-o-b.
>>>>
>>>
>>> This is not a good attitude, nor helps your patch to be merged...
>>>
>>> Be collaborative.
>>
>>
>> some people like focusing on trivial things. i said i really didn't
>> care about the details whether it is /dev, /proc, /sys.
>> if you like a /proc, pls do it by yourself and i will agree. but i
>> will not waste my time on that.
>
>
> This is definitely not trivial... What's more, kernel developers *do* care
> about trivial things (trivial@kernel.org).

that is definitely trivial as we are talking about things in a test
module. and we are not talking about any bug in the test module as
well.

>
> We are trying to make your patch perfect, you just refused, sigh...

pls note i just care about people have an easy way to test the newest
contiguous memory allocator. pls be focusing on valuable things.

i said if you like /proc, pls do and i agree.

-barry

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3] MM: CMA: add a simple kernel module as the helper to test CMA
  2012-03-07  5:17               ` Barry Song
@ 2012-03-07  5:22                 ` Cong Wang
  2012-03-07  5:40                   ` Barry Song
  0 siblings, 1 reply; 16+ messages in thread
From: Cong Wang @ 2012-03-07  5:22 UTC (permalink / raw)
  To: Barry Song
  Cc: Barry Song, linux-kernel, workgroup.linux, mina86, Barry Song,
	linux-arm-kernel, m.szyprowski

On 03/07/2012 01:17 PM, Barry Song wrote:
> that is definitely trivial as we are talking about things in a test
> module. and we are not talking about any bug in the test module as
> well.

I am quite sure *any* patches have bugs will not be merged, IOW, you 
seem don't want your patch merged. :) Fine. That is all.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3] MM: CMA: add a simple kernel module as the helper to test CMA
  2012-03-07  5:22                 ` Cong Wang
@ 2012-03-07  5:40                   ` Barry Song
  0 siblings, 0 replies; 16+ messages in thread
From: Barry Song @ 2012-03-07  5:40 UTC (permalink / raw)
  To: Cong Wang
  Cc: Barry Song, linux-kernel, workgroup.linux, mina86, Barry Song,
	linux-arm-kernel, m.szyprowski

2012/3/7 Cong Wang <xiyou.wangcong@gmail.com>:
> On 03/07/2012 01:17 PM, Barry Song wrote:
>>
>> that is definitely trivial as we are talking about things in a test
>> module. and we are not talking about any bug in the test module as
>> well.
>
>
> I am quite sure *any* patches have bugs will not be merged, IOW, you seem
> don't want your patch merged. :) Fine. That is all.

Cong, i was not againest what you said as a techinical issue.
definitely, it can be a /proc, it is also able to be /sys if you like.
/proc is probably better than /dev from the original design thinking in kernel.

here i focused on people have this module to test CMA at runtime. i
also thought Michal also wants this module to show people the usage of
CMA as well.

i'll much appreciate if you have time to move it to /proc.

-barry

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3] MM: CMA: add a simple kernel module as the helper to test CMA
  2012-03-07  5:01         ` Cong Wang
  2012-03-07  5:05           ` Barry Song
@ 2012-03-07 10:38           ` Barry Song
  1 sibling, 0 replies; 16+ messages in thread
From: Barry Song @ 2012-03-07 10:38 UTC (permalink / raw)
  To: Cong Wang; +Cc: linux-kernel, mina86, linux-arm-kernel, m.szyprowski

2012/3/7 Cong Wang <xiyou.wangcong@gmail.com>:
> On 03/07/2012 12:57 PM, Barry Song wrote:
>>
>>
>> if you think so, pls do that and send v4 with your s-o-b.
>>
>
> This is not a good attitude, nor helps your patch to be merged...
>
> Be collaborative.

pls just focus on techinical discussion and note you are not the man
to decide whether the patch should be merged. Many people know how to
make patches merged well.
you said that just like you are the one maintaining MM. Be modest, man!

-barry

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3] MM: CMA: add a simple kernel module as the helper to test CMA
  2012-03-07  4:14 ` Cong Wang
  2012-03-07  4:49   ` Barry Song
@ 2012-03-07 10:48   ` Michal Nazarewicz
  2012-03-07 10:55     ` Barry Song
  1 sibling, 1 reply; 16+ messages in thread
From: Michal Nazarewicz @ 2012-03-07 10:48 UTC (permalink / raw)
  To: Barry Song, Cong Wang
  Cc: m.szyprowski, linux-arm-kernel, linux-kernel, workgroup.linux,
	Barry Song

> On 03/07/2012 11:14 AM, Barry Song wrote:
>> Any write request to /dev/cma_test will let the module to allocate memory from
>> CMA, for example:
>>
>> 1st time
>> $ echo 1024>  /dev/cma_test
>> will require cma_test to request 1MB(1024KB)
>> 2nd time
>> $ echo 2048>  /dev/cma_test
>> will require cma_test to request 2MB(2048KB)
>>
>> Any read request to /dev/cma_test will let the module to free the 1st valid
>> memory from CMA, for example:
>>
>> 1st time
>> $ cat /dev/cma_test
>> will require cma_test to free the 1MB allocated in the first write request
>> 2nd time
>> $ cat /dev/cma_test
>> will require cma_test to free the 2MB allocated in the second write request

On Wed, 07 Mar 2012 05:14:36 +0100, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> Any reason why using /dev not /proc or /sys?

Because /proc is for processes and /sys is for stable, documented APIs.

If anything, debugfs might make some sense, but since the module needs struct
device anyway, it can just register a misc device, which is simpler and does
not require debugfs to be compiled.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3] MM: CMA: add a simple kernel module as the helper to test CMA
  2012-03-07 10:48   ` Michal Nazarewicz
@ 2012-03-07 10:55     ` Barry Song
  0 siblings, 0 replies; 16+ messages in thread
From: Barry Song @ 2012-03-07 10:55 UTC (permalink / raw)
  To: Michal Nazarewicz
  Cc: Barry Song, Cong Wang, workgroup.linux, Barry Song, linux-kernel,
	linux-arm-kernel, m.szyprowski

2012/3/7 Michal Nazarewicz <mina86@mina86.com>:
>> On 03/07/2012 11:14 AM, Barry Song wrote:
>>>
>>> Any write request to /dev/cma_test will let the module to allocate memory
>>> from
>>> CMA, for example:
>>>
>>> 1st time
>>> $ echo 1024>  /dev/cma_test
>>> will require cma_test to request 1MB(1024KB)
>>> 2nd time
>>> $ echo 2048>  /dev/cma_test
>>> will require cma_test to request 2MB(2048KB)
>>>
>>> Any read request to /dev/cma_test will let the module to free the 1st
>>> valid
>>> memory from CMA, for example:
>>>
>>> 1st time
>>> $ cat /dev/cma_test
>>> will require cma_test to free the 1MB allocated in the first write
>>> request
>>> 2nd time
>>> $ cat /dev/cma_test
>>> will require cma_test to free the 2MB allocated in the second write
>>> request
>
>
> On Wed, 07 Mar 2012 05:14:36 +0100, Cong Wang <xiyou.wangcong@gmail.com>
> wrote:
>>
>> Any reason why using /dev not /proc or /sys?
>
>
> Because /proc is for processes and /sys is for stable, documented APIs.
>
> If anything, debugfs might make some sense, but since the module needs
> struct
> device anyway, it can just register a misc device, which is simpler and does
> not require debugfs to be compiled.

Michal, thanks for clarification. actually after i sent my mail to
agree Cong's /proc might be ok, then i had more thinking, i came back
to think the original /dev is better since anyway we need to device to
alloc CM.
then we let the device's read/write to handle alloc/free. it is much
more direct for a test module.

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

-barry

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3] MM: CMA: add a simple kernel module as the helper to test CMA
  2012-03-07  3:14 [PATCH v3] MM: CMA: add a simple kernel module as the helper to test CMA Barry Song
  2012-03-07  4:14 ` Cong Wang
@ 2012-03-07 10:59 ` Michal Nazarewicz
  2012-03-07 11:12   ` Barry Song
  1 sibling, 1 reply; 16+ messages in thread
From: Michal Nazarewicz @ 2012-03-07 10:59 UTC (permalink / raw)
  To: m.szyprowski, Barry Song
  Cc: linux-arm-kernel, linux-kernel, workgroup.linux, Barry Song

Thanks!  Some minor comments, hopefully last ones:

On Wed, 07 Mar 2012 04:14:58 +0100, Barry Song <Barry.Song@csr.com> wrote:
> +static ssize_t
> +cma_test_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
> +{

[...]

> +	_dev_info(cma_dev, "free: CM virt: %p dma: %p size:%uK\n",

This should read: “size: %zuK”.  “z” is the proper modifier to use for size_t type.

> +		alloc->virt, (void *)alloc->dma, alloc->size / SZ_1K);
> +	kfree(alloc);
> +
> +	return 0;
> +}

> +static ssize_t
> +cma_test_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
> +{
> +	struct cma_allocation *alloc;
> +	size_t size;
> +	int ret;
> +
> +	ret = kstrtouint_from_user(buf, count, 0, &size);

kstrtouint_from_user() expects pointer to unsigned int, size_t is not always
unsigned int.  It may be unsigned long.  As such, size should be of type
unsigned long and this line should use kstrtoulong_from_user().

[...]

> +	if (alloc->virt) {
> +		_dev_info(cma_dev, "alloc: virt: %p dma: %p size: %uK\n",
> +			alloc->virt, (void *)alloc->dma, size);

Like previously, this should be “size: %zuK”.

[...]

> +}

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3] MM: CMA: add a simple kernel module as the helper to test CMA
  2012-03-07 10:59 ` Michal Nazarewicz
@ 2012-03-07 11:12   ` Barry Song
  0 siblings, 0 replies; 16+ messages in thread
From: Barry Song @ 2012-03-07 11:12 UTC (permalink / raw)
  To: Michal Nazarewicz
  Cc: m.szyprowski, Barry Song, workgroup.linux, linux-kernel,
	linux-arm-kernel, Barry Song

2012/3/7 Michal Nazarewicz <mina86@mina86.com>:
> Thanks!  Some minor comments, hopefully last ones:
>
>
> On Wed, 07 Mar 2012 04:14:58 +0100, Barry Song <Barry.Song@csr.com> wrote:
>>
>> +static ssize_t
>> +cma_test_read(struct file *file, char __user *buf, size_t count, loff_t
>> *ppos)
>> +{
>
>
> [...]
>
>
>> +       _dev_info(cma_dev, "free: CM virt: %p dma: %p size:%uK\n",
>
>
> This should read: “size: %zuK”.  “z” is the proper modifier to use for
> size_t type.

ok. thanks.

>
>
>> +               alloc->virt, (void *)alloc->dma, alloc->size / SZ_1K);
>> +       kfree(alloc);
>> +
>> +       return 0;
>> +}
>
>
>> +static ssize_t
>> +cma_test_write(struct file *file, const char __user *buf, size_t count,
>> loff_t *ppos)
>> +{
>> +       struct cma_allocation *alloc;
>> +       size_t size;
>> +       int ret;
>> +
>> +       ret = kstrtouint_from_user(buf, count, 0, &size);
>
>
> kstrtouint_from_user() expects pointer to unsigned int, size_t is not always
> unsigned int.  It may be unsigned long.  As such, size should be of type
> unsigned long and this line should use kstrtoulong_from_user().

it comes to a compiling warning, so re-define "size" to unsigned long:

cma_test_write(struct file *file, const char __user *buf, size_t
count, loff_t *ppos)
 {
        struct cma_allocation *alloc;
-       size_t size;
+       unsigned long size;
        int ret;

-       ret = kstrtouint_from_user(buf, count, 0, &size);
+       ret = kstrtoul_from_user(buf, count, 0, &size);
        if (ret)
                return ret;

@@ -86,8 +86,8 @@ cma_test_write(struct file *file, const char __user
*buf, size_t count, loff_t *
                &alloc->dma, GFP_KERNEL);

        if (alloc->virt) {
-               _dev_info(cma_dev, "alloc: virt: %p dma: %p size: %uK\n",
-                       alloc->virt, (void *)alloc->dma, size);
+               _dev_info(cma_dev, "alloc: virt: %p dma: %p size: %zuK\n",
+                       alloc->virt, (void *)alloc->dma, alloc->size / SZ_1K);

                spin_lock(&cma_lock);
                list_add_tail(&alloc->list, &cma_allocations);

>
> [...]
>
>
>> +       if (alloc->virt) {
>> +               _dev_info(cma_dev, "alloc: virt: %p dma: %p size: %uK\n",
>> +                       alloc->virt, (void *)alloc->dma, size);
>
>
> Like previously, this should be “size: %zuK”.
>
> [...]
>
>> +}
>
>
> --
> Best regards,                                         _     _
> .o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
> ..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
> ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--

-barry

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2012-03-07 11:12 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-07  3:14 [PATCH v3] MM: CMA: add a simple kernel module as the helper to test CMA Barry Song
2012-03-07  4:14 ` Cong Wang
2012-03-07  4:49   ` Barry Song
2012-03-07  4:55     ` Cong Wang
2012-03-07  4:57       ` Barry Song
2012-03-07  5:01         ` Cong Wang
2012-03-07  5:05           ` Barry Song
2012-03-07  5:09             ` Cong Wang
2012-03-07  5:17               ` Barry Song
2012-03-07  5:22                 ` Cong Wang
2012-03-07  5:40                   ` Barry Song
2012-03-07 10:38           ` Barry Song
2012-03-07 10:48   ` Michal Nazarewicz
2012-03-07 10:55     ` Barry Song
2012-03-07 10:59 ` Michal Nazarewicz
2012-03-07 11:12   ` Barry Song

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).