* [patch] ps3disk: passing wrong variable to bvec_kunmap_irq()
@ 2010-10-11 19:13 Dan Carpenter
2010-10-11 19:42 ` Jens Axboe
2010-10-12 16:01 ` Geoff Levand
0 siblings, 2 replies; 6+ messages in thread
From: Dan Carpenter @ 2010-10-11 19:13 UTC (permalink / raw)
To: Geoff Levand
Cc: cbe-oss-dev, Martin K. Petersen, Jens Axboe, kernel-janitors,
FUJITA Tomonori, linuxppc-dev
This should pass "buf" to bvec_kunmap_irq() instead of "bv". The api is
like kmap_atomic() instead of kmap().
Signed-off-by: Dan Carpenter <error27@gmail.com>
diff --git a/drivers/block/ps3disk.c b/drivers/block/ps3disk.c
index e9da874..03688c2 100644
--- a/drivers/block/ps3disk.c
+++ b/drivers/block/ps3disk.c
@@ -113,7 +113,7 @@ static void ps3disk_scatter_gather(struct ps3_storage_device *dev,
memcpy(buf, dev->bounce_buf+offset, size);
offset += size;
flush_kernel_dcache_page(bvec->bv_page);
- bvec_kunmap_irq(bvec, &flags);
+ bvec_kunmap_irq(buf, &flags);
i++;
}
}
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [patch] ps3disk: passing wrong variable to bvec_kunmap_irq()
2010-10-11 19:13 [patch] ps3disk: passing wrong variable to bvec_kunmap_irq() Dan Carpenter
@ 2010-10-11 19:42 ` Jens Axboe
2010-10-16 19:39 ` Geert Uytterhoeven
2010-10-12 16:01 ` Geoff Levand
1 sibling, 1 reply; 6+ messages in thread
From: Jens Axboe @ 2010-10-11 19:42 UTC (permalink / raw)
To: Dan Carpenter
Cc: cbe-oss-dev@lists.ozlabs.org, Martin K. Petersen, Geoff Levand,
kernel-janitors@vger.kernel.org, FUJITA Tomonori,
linuxppc-dev@lists.ozlabs.org
On 2010-10-11 21:13, Dan Carpenter wrote:
> This should pass "buf" to bvec_kunmap_irq() instead of "bv". The api is
> like kmap_atomic() instead of kmap().
>
> Signed-off-by: Dan Carpenter <error27@gmail.com>
>
> diff --git a/drivers/block/ps3disk.c b/drivers/block/ps3disk.c
> index e9da874..03688c2 100644
> --- a/drivers/block/ps3disk.c
> +++ b/drivers/block/ps3disk.c
> @@ -113,7 +113,7 @@ static void ps3disk_scatter_gather(struct ps3_storage_device *dev,
> memcpy(buf, dev->bounce_buf+offset, size);
> offset += size;
> flush_kernel_dcache_page(bvec->bv_page);
> - bvec_kunmap_irq(bvec, &flags);
> + bvec_kunmap_irq(buf, &flags);
> i++;
> }
> }
Thanks applied, that bug is all too common.
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] ps3disk: passing wrong variable to bvec_kunmap_irq()
2010-10-11 19:13 [patch] ps3disk: passing wrong variable to bvec_kunmap_irq() Dan Carpenter
2010-10-11 19:42 ` Jens Axboe
@ 2010-10-12 16:01 ` Geoff Levand
1 sibling, 0 replies; 6+ messages in thread
From: Geoff Levand @ 2010-10-12 16:01 UTC (permalink / raw)
To: Dan Carpenter
Cc: cbe-oss-dev@lists.ozlabs.org, Martin K. Petersen, Geoff Levand,
Jens Axboe, kernel-janitors@vger.kernel.org, FUJITA Tomonori,
linuxppc-dev@lists.ozlabs.org
On 10/11/2010 12:13 PM, Dan Carpenter wrote:
> This should pass "buf" to bvec_kunmap_irq() instead of "bv". The api is
> like kmap_atomic() instead of kmap().
>
> Signed-off-by: Dan Carpenter <error27@gmail.com>
Looks good. Thanks.
Acked-by: Geoff Levand <geoff@infradead.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] ps3disk: passing wrong variable to bvec_kunmap_irq()
2010-10-11 19:42 ` Jens Axboe
@ 2010-10-16 19:39 ` Geert Uytterhoeven
2010-10-17 6:49 ` Jens Axboe
0 siblings, 1 reply; 6+ messages in thread
From: Geert Uytterhoeven @ 2010-10-16 19:39 UTC (permalink / raw)
To: Jens Axboe
Cc: cbe-oss-dev@lists.ozlabs.org, Dan Carpenter, Martin K. Petersen,
Geoff Levand, kernel-janitors@vger.kernel.org, FUJITA Tomonori,
linuxppc-dev@lists.ozlabs.org
On Mon, Oct 11, 2010 at 21:42, Jens Axboe <jaxboe@fusionio.com> wrote:
> On 2010-10-11 21:13, Dan Carpenter wrote:
>> This should pass "buf" to bvec_kunmap_irq() instead of "bv". The api is
>> like kmap_atomic() instead of kmap().
>>
>> Signed-off-by: Dan Carpenter <error27@gmail.com>
>>
>> diff --git a/drivers/block/ps3disk.c b/drivers/block/ps3disk.c
>> index e9da874..03688c2 100644
>> --- a/drivers/block/ps3disk.c
>> +++ b/drivers/block/ps3disk.c
>> @@ -113,7 +113,7 @@ static void ps3disk_scatter_gather(struct ps3_storage_device *dev,
>> memcpy(buf, dev->bounce_buf+offset, size);
>> offset += size;
>> flush_kernel_dcache_page(bvec->bv_page);
>> - bvec_kunmap_irq(bvec, &flags);
>> + bvec_kunmap_irq(buf, &flags);
>> i++;
>> }
>> }
>
> Thanks applied, that bug is all too common.
Because bvec_kunmap_irq() is a macro if !CONFIG_HIGHMEM, and thus there's no
argument type checking on e.g. pp64, which doesn't support HIGHMEM?
What about turning it into an inline function?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] ps3disk: passing wrong variable to bvec_kunmap_irq()
2010-10-16 19:39 ` Geert Uytterhoeven
@ 2010-10-17 6:49 ` Jens Axboe
2010-10-17 11:36 ` Geert Uytterhoeven
0 siblings, 1 reply; 6+ messages in thread
From: Jens Axboe @ 2010-10-17 6:49 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: cbe-oss-dev@lists.ozlabs.org, Dan Carpenter, Martin K. Petersen,
Geoff Levand, kernel-janitors@vger.kernel.org, FUJITA Tomonori,
linuxppc-dev@lists.ozlabs.org
On 2010-10-16 21:39, Geert Uytterhoeven wrote:
> On Mon, Oct 11, 2010 at 21:42, Jens Axboe <jaxboe@fusionio.com> wrote:
>> On 2010-10-11 21:13, Dan Carpenter wrote:
>>> This should pass "buf" to bvec_kunmap_irq() instead of "bv". The api is
>>> like kmap_atomic() instead of kmap().
>>>
>>> Signed-off-by: Dan Carpenter <error27@gmail.com>
>>>
>>> diff --git a/drivers/block/ps3disk.c b/drivers/block/ps3disk.c
>>> index e9da874..03688c2 100644
>>> --- a/drivers/block/ps3disk.c
>>> +++ b/drivers/block/ps3disk.c
>>> @@ -113,7 +113,7 @@ static void ps3disk_scatter_gather(struct ps3_storage_device *dev,
>>> memcpy(buf, dev->bounce_buf+offset, size);
>>> offset += size;
>>> flush_kernel_dcache_page(bvec->bv_page);
>>> - bvec_kunmap_irq(bvec, &flags);
>>> + bvec_kunmap_irq(buf, &flags);
>>> i++;
>>> }
>>> }
>>
>> Thanks applied, that bug is all too common.
>
> Because bvec_kunmap_irq() is a macro if !CONFIG_HIGHMEM, and thus there's no
> argument type checking on e.g. pp64, which doesn't support HIGHMEM?
It's a generic problem, not isolated to this case. The problem is that
the API isn't symmetric, and the unmap parts take a void * pointer.
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] ps3disk: passing wrong variable to bvec_kunmap_irq()
2010-10-17 6:49 ` Jens Axboe
@ 2010-10-17 11:36 ` Geert Uytterhoeven
0 siblings, 0 replies; 6+ messages in thread
From: Geert Uytterhoeven @ 2010-10-17 11:36 UTC (permalink / raw)
To: Jens Axboe
Cc: cbe-oss-dev@lists.ozlabs.org, Dan Carpenter, Martin K. Petersen,
Geoff Levand, kernel-janitors@vger.kernel.org, FUJITA Tomonori,
linuxppc-dev@lists.ozlabs.org
On Sun, Oct 17, 2010 at 08:49, Jens Axboe <jaxboe@fusionio.com> wrote:
> On 2010-10-16 21:39, Geert Uytterhoeven wrote:
>> On Mon, Oct 11, 2010 at 21:42, Jens Axboe <jaxboe@fusionio.com> wrote:
>>> On 2010-10-11 21:13, Dan Carpenter wrote:
>>>> This should pass "buf" to bvec_kunmap_irq() instead of "bv". The api is
>>>> like kmap_atomic() instead of kmap().
>>>>
>>>> Signed-off-by: Dan Carpenter <error27@gmail.com>
>>>>
>>>> diff --git a/drivers/block/ps3disk.c b/drivers/block/ps3disk.c
>>>> index e9da874..03688c2 100644
>>>> --- a/drivers/block/ps3disk.c
>>>> +++ b/drivers/block/ps3disk.c
>>>> @@ -113,7 +113,7 @@ static void ps3disk_scatter_gather(struct ps3_storage_device *dev,
>>>> memcpy(buf, dev->bounce_buf+offset, size);
>>>> offset += size;
>>>> flush_kernel_dcache_page(bvec->bv_page);
>>>> - bvec_kunmap_irq(bvec, &flags);
>>>> + bvec_kunmap_irq(buf, &flags);
>>>> i++;
>>>> }
>>>> }
>>>
>>> Thanks applied, that bug is all too common.
>>
>> Because bvec_kunmap_irq() is a macro if !CONFIG_HIGHMEM, and thus there's no
>> argument type checking on e.g. pp64, which doesn't support HIGHMEM?
>
> It's a generic problem, not isolated to this case. The problem is that
> the API isn't symmetric, and the unmap parts take a void * pointer.
No, the unmap takes a char *:
static inline void bvec_kunmap_irq(char *buffer, unsigned long *flags)
So it could be detected. Patch will follow.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-10-17 11:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-11 19:13 [patch] ps3disk: passing wrong variable to bvec_kunmap_irq() Dan Carpenter
2010-10-11 19:42 ` Jens Axboe
2010-10-16 19:39 ` Geert Uytterhoeven
2010-10-17 6:49 ` Jens Axboe
2010-10-17 11:36 ` Geert Uytterhoeven
2010-10-12 16:01 ` Geoff Levand
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox