Re: [PATCH v3] MM: CMA: add a simple kernel module as the helper totest CMA

[Barry Song]

2012/3/7 Michal Nazarewicz <mina86@xxxxxxxxxx>:
> Thanks! ÂSome minor comments, hopefully last ones:
>
>
> On Wed, 07 Mar 2012 04:14:58 +0100, Barry Song <Barry.Song@xxxxxxx> 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@xxxxxxxxxx>--------------ooO--(_)--Ooo--

-barry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/