Patchwork powerpc/cell: fix the prototype of create_vma_map

login
register
mail settings
Submitter Stephen Rothwell
Date Jan. 6, 2009, 11:58 p.m.
Message ID <20090107105822.221c4395.sfr@canb.auug.org.au>
Download mbox | patch
Permalink /patch/16973/
State Accepted, archived
Commit 9b635642bce0500bdc7331ce8eeca97907b77117
Headers show

Comments

Stephen Rothwell - Jan. 6, 2009, 11:58 p.m.
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 arch/powerpc/oprofile/cell/pr_util.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Benjamin Herrenschmidt - Jan. 7, 2009, 3:34 a.m.
On Wed, 2009-01-07 at 10:58 +1100, Stephen Rothwell wrote:
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>  arch/powerpc/oprofile/cell/pr_util.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

Arnd, I'm not too sure about this one, what do you think ? Shouldn't the
objectid be a u64 and the callers be fixed instead ?

Cheers,
Ben.

> diff --git a/arch/powerpc/oprofile/cell/pr_util.h b/arch/powerpc/oprofile/cell/pr_util.h
> index 628009c..dfdbffa 100644
> --- a/arch/powerpc/oprofile/cell/pr_util.h
> +++ b/arch/powerpc/oprofile/cell/pr_util.h
> @@ -79,7 +79,7 @@ struct spu_buffer {
>   * the vma-to-fileoffset map.
>   */
>  struct vma_to_fileoffset_map *create_vma_map(const struct spu *spu,
> -					     u64 objectid);
> +					     unsigned long objectid);
>  unsigned int vma_map_lookup(struct vma_to_fileoffset_map *map,
>  			    unsigned int vma, const struct spu *aSpu,
>  			    int *grd_val);
> -- 
> 1.6.0.5
>
Stephen Rothwell - Jan. 7, 2009, 5:36 a.m.
On Wed, 07 Jan 2009 14:34:59 +1100 Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>
> On Wed, 2009-01-07 at 10:58 +1100, Stephen Rothwell wrote:
> > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > ---
> >  arch/powerpc/oprofile/cell/pr_util.h |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> Arnd, I'm not too sure about this one, what do you think ? Shouldn't the
> objectid be a u64 and the callers be fixed instead ?

The *one* caller passes an "unsigned long" and in the function, objectid
(which is called __spu_elf_start there) is only cast to a "void __user *".
Don't we assume that all pointers (kernel and user) can fit into an
"unsigned long"?

Also the value originally comes from a notifier callback
(spu_active_notify) which is passed this value as an "unsigned long".
Arnd Bergmann - Jan. 7, 2009, 7:01 p.m.
On Wednesday 07 January 2009, Stephen Rothwell wrote:

> Also the value originally comes from a notifier callback
> (spu_active_notify) which is passed this value as an "unsigned long".

The data that gets passed down here is originally the u64 object_id
from struct spu_context, but it gets passed as an unsigned long
through the notifier chain. I think either way is fine, as it's
not possible to use spu notifiers on a 32 bit kernel and I wouldn't
want to change it more fundamentally.
If you like, you can change the whole call chain from spu_active_notify
to pass a u64, but it woulnd't make the code more correct.

Original patch
Acked-by: Arnd Bergmann <arnd@arndb.de>

Patch

diff --git a/arch/powerpc/oprofile/cell/pr_util.h b/arch/powerpc/oprofile/cell/pr_util.h
index 628009c..dfdbffa 100644
--- a/arch/powerpc/oprofile/cell/pr_util.h
+++ b/arch/powerpc/oprofile/cell/pr_util.h
@@ -79,7 +79,7 @@  struct spu_buffer {
  * the vma-to-fileoffset map.
  */
 struct vma_to_fileoffset_map *create_vma_map(const struct spu *spu,
-					     u64 objectid);
+					     unsigned long objectid);
 unsigned int vma_map_lookup(struct vma_to_fileoffset_map *map,
 			    unsigned int vma, const struct spu *aSpu,
 			    int *grd_val);