* [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: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". =C2=A0The a= pi 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_storag= e_device *dev, >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 memcpy(buf, dev->bounce_buf+offset, size); >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 offset +=3D size; >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 flush_kernel_dcache_pag= e(bvec->bv_page); >> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 bvec_kunmap_irq(bvec, &flags= ); >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 bvec_kunmap_irq(buf, &flags)= ; >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 i++; >> =C2=A0 =C2=A0 =C2=A0 } >> =C2=A0} > > 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, =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 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. Bu= t when I'm talking to journalists I just say "programmer" or something like t= hat. =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0=C2=A0 =C2=A0=C2=A0 -- 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". =C2=A0The= 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_stor= age_device *dev, >>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 memcpy(buf, dev->bounce_buf+offset, size); >>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 offset +=3D size; >>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 flush_kernel_dcache_p= age(bvec->bv_page); >>>> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 bvec_kunmap_irq(bvec, &fla= gs); >>>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 bvec_kunmap_irq(buf, &flag= s); >>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 i++; >>>> =C2=A0 =C2=A0 =C2=A0 } >>>> =C2=A0} >>> >>> Thanks applied, that bug is all too common. >> >> Because =C2=A0bvec_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, =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 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. Bu= t when I'm talking to journalists I just say "programmer" or something like t= hat. =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0=C2=A0 =C2=A0=C2=A0 -- Linus Torvalds ^ 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
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; as well as URLs for NNTP newsgroup(s).