Prev: Two small fixes for 2.6.34
Next: [PATCH 1/2] DMA ENGINE: Do not reset 'private' of channel
From: Steven Liu on 8 May 2010 05:00
[Linus WALLEIJ]

can this patch fix in?

2010/5/8 Nigel Cunningham <nigel(a)tuxonice.net>:
> Hi again.
>
> On 08/05/10 04:57, Linus WALLEIJ wrote:
>>
>> [Steven]
>>
>>> the code in arch/arm/mach-u300/dummyspichip.c is
>>>
>>> � � bigtxbuf_virtual = kmalloc(DMA_TEST_SIZE, GFP_KERNEL);
>>> � � if (bigtxbuf_virtual == NULL) {
>>> � � � � status = -ENOMEM;
>>> � � � � goto out;
>>> � � }
>>> � � bigrxbuf_virtual = kmalloc(DMA_TEST_SIZE, GFP_KERNEL);
>>>
>>>
>>> if kmalloc memory space for bigrxbuf_virtual is NULL, when it have
>>> kmalloc DMA_TEST_SIZE memory space for bigtxbuf_virtual,so ,if kmalloc
>>> memory for bigtxbuf_virtual success and kmalloc memory for
>>> bigrxbuf_virtual faild,i think we must kfree �bigtxbuf_virtual memory
>>
>> Ha, I also misread tx for rx, sorry.
>
> I've just looked again, and the original version did have rx in the test. We
> weren't seeing things :)
>
>> Acked-by: Linus Walleij<linus.walleij(a)stericsson.com>
>
> Acked-by: Nigel Cunningham <nigel(a)tuxonice.net>
>
> Regards,
>
> Nigel
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Andy Isaacson on 10 May 2010 18:00
On Fri, May 07, 2010 at 03:17:39PM +0800, Steven Liu wrote:
> bigrxbuf_virtual = kmalloc(DMA_TEST_SIZE, GFP_KERNEL);
> + if (bigrxbuf_virtual == NULL) {
> + status = -ENOMEM;
> + kfree(bigtxbuf_virtual);
> + goto out;
> + }

On Sat, May 08, 2010 at 11:27:09AM +1000, Nigel Cunningham wrote:
>> Acked-by: Linus Walleij<linus.walleij(a)stericsson.com>
>
> Acked-by: Nigel Cunningham <nigel(a)tuxonice.net>

NACK. Don't duplicate kfree(), instead do something like

--- a/arch/arm/mach-u300/dummyspichip.c
+++ b/arch/arm/mach-u300/dummyspichip.c
@@ -58,12 +58,13 @@ static ssize_t dummy_looptest(struct device *dev,
if (mutex_lock_interruptible(&p_dummy->lock))
return -ERESTARTSYS;

+ status = -ENOMEM;
bigtxbuf_virtual = kmalloc(DMA_TEST_SIZE, GFP_KERNEL);
- if (bigtxbuf_virtual == NULL) {
- status = -ENOMEM;
+ if (bigtxbuf_virtual == NULL)
goto out;
- }
bigrxbuf_virtual = kmalloc(DMA_TEST_SIZE, GFP_KERNEL);
+ if (bigrxbuf_virtual == NULL)
+ goto out_free_tx;

/* Fill TXBUF with some happy pattern */
memset(bigtxbuf_virtual, 0xAA, DMA_TEST_SIZE);
@@ -215,6 +216,7 @@ static ssize_t dummy_looptest(struct device *dev,

status = sprintf(buf, "loop test complete\n");
kfree(bigrxbuf_virtual);
+ out_free_tx:
kfree(bigtxbuf_virtual);
out:
mutex_unlock(&p_dummy->lock);

-andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Nigel Cunningham on 10 May 2010 18:10
Hi.

On 11/05/10 07:49, Andy Isaacson wrote:
> On Fri, May 07, 2010 at 03:17:39PM +0800, Steven Liu wrote:
>> bigrxbuf_virtual = kmalloc(DMA_TEST_SIZE, GFP_KERNEL);
>> + if (bigrxbuf_virtual == NULL) {
>> + status = -ENOMEM;
>> + kfree(bigtxbuf_virtual);
>> + goto out;
>> + }
>
> On Sat, May 08, 2010 at 11:27:09AM +1000, Nigel Cunningham wrote:
>>> Acked-by: Linus Walleij<linus.walleij(a)stericsson.com>
>>
>> Acked-by: Nigel Cunningham<nigel(a)tuxonice.net>
>
> NACK. Don't duplicate kfree(), instead do something like

Yeah... that is a better way of doing it.

Regards,

Nigel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
First  |  Prev  | 
Pages: 1 2 3
Prev: Two small fixes for 2.6.34
Next: [PATCH 1/2] DMA ENGINE: Do not reset 'private' of channel