From: Arnd Bergmann <arnd@arndb.de>
To: Maynard Johnson <maynardj@us.ibm.com>
Subject: Re: [PATCH] oprofile: vma_map: fix test on overlay_tbl_offset
Date: Tue, 1 Jul 2008 16:01:20 +0200
User-Agent: KMail/1.9.9
Cc: Roel Kluin <12o3l@tiscali.nl>, phil.el@wanadoo.fr,
       oprofile-list@lists.sourceforge.net,
       lkml <linux-kernel@vger.kernel.org>, Carl Love <cel@us.ibm.com>,
       Bob Nelson <rrnelson@linux.vnet.ibm.com>
References: <48074F18.7050802@tiscali.nl> <480F3BB0.9080507@us.ibm.com>
In-Reply-To: <480F3BB0.9080507@us.ibm.com>
MIME-Version: 1.0
Content-Type: text/plain;
  charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
Message-Id: <200807011601.21540.arnd@arndb.de>
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2008
Lines: 46

On Wednesday 23 April 2008, Maynard Johnson wrote:
> Roel Kluin wrote:
> > Offset is unsigned and when an address isn't found in the vma map
> > vma_map_lookup() returns the vma physical address + 0x10000000.
> >
> > Signed-off-by: Roel Kluin <12o3l@tiscali.nl>
> >   
> Patch looks correct.  vma_map_lookup used to return 0xffffffff on a 
> failed lookup, but a change was recently made to return the vma physical 
> address + 0x10000000 (as Roel notes above).  There are two callers of 
> vam_map_lookup: one of them correctly deals with this new return value, 
> but the other (below) did not.  Roel's patch fixes that hole.  Thanks!
> 
> Arnd, can you put this into the Cell tree for pushing upstream?  Thanks.

Sorry for not having looked at this earlier. I'm now queuing up patches
for 2.6.27, but this one looks incorrect:

> > diff --git a/arch/powerpc/oprofile/cell/vma_map.c b/arch/powerpc/oprofile/cell/vma_map.c
> > index 9a93217..1c28e2e 100644
> > --- a/arch/powerpc/oprofile/cell/vma_map.c
> > +++ b/arch/powerpc/oprofile/cell/vma_map.c
> > @@ -229,7 +229,7 @@ struct vma_to_fileoffset_map *create_vma_map(const struct spu *aSpu,
> >  	 */
> >  	overlay_tbl_offset = vma_map_lookup(map, ovly_table_sym,
> >  					    aSpu, &grd_val);
> > -	if (overlay_tbl_offset < 0) {
> > +	if (overlay_tbl_offset >= 0x10000000) {
> >  		printk(KERN_ERR "SPU_PROF: "
> >  		       "%s, line %d: Error finding SPU overlay table\n",
> >  		       __FUNCTION__, __LINE__);
> >

You mention that the kernel can handle the return code correctly, but
this code still prints an error message in that case, which does not
make sense then.

Also, where does the 0x10000000 number come from? Is that the maximum
size that all overlays can consume in one binary?

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