| Message ID | 1386377017-909-1-git-send-email-imirkin@alum.mit.edu (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Commit | 1b429835be7ce514b36b551e785d425fd56cd1f2 |
| Headers | show |
Hi Ilia Mirkin, Thanks for your info. I did investigated why our test didn't detect it and found out that the struct ocm_block is only used on ocm_debugfs_show function when we want to know information about ocm and it's available when we enable debugfs. But our test only tried to use the OCM block functions and didn't care about the OCM information. So I think we should apply your patch to solve this issue instead of removing ocm part. On Sat, Dec 7, 2013 at 7:43 AM, Ilia Mirkin <imirkin@alum.mit.edu> wrote: > Allocate enough memory for the ocm_block structure, not just a pointer > to it. > > Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu> > --- > > I have neither the hardware to test nor the toolchain to even build-test > this. However this seems like a fairly obvious fix (and I have to wonder > how > this ever worked at all). Found with spatch. > > Actually further investigation makes it seem like this function is never > called, perhaps it should just be removed? If it is kept around though, > would > be nice to apply this patch so that tools don't trip over this wrong code. > > arch/powerpc/sysdev/ppc4xx_ocm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/powerpc/sysdev/ppc4xx_ocm.c > b/arch/powerpc/sysdev/ppc4xx_ocm.c > index b7c4345..85d9e37 100644 > --- a/arch/powerpc/sysdev/ppc4xx_ocm.c > +++ b/arch/powerpc/sysdev/ppc4xx_ocm.c > @@ -339,7 +339,7 @@ void *ppc4xx_ocm_alloc(phys_addr_t *phys, int size, > int align, > if (IS_ERR_VALUE(offset)) > continue; > > - ocm_blk = kzalloc(sizeof(struct ocm_block *), GFP_KERNEL); > + ocm_blk = kzalloc(sizeof(struct ocm_block), GFP_KERNEL); > if (!ocm_blk) { > printk(KERN_ERR "PPC4XX OCM: could not allocate > ocm block"); > rh_free(ocm_reg->rh, offset); > -- > 1.8.3.2 > >
On Mon, Dec 9, 2013 at 3:38 AM, Vinh Huu Tuong Nguyen <vhtnguyen@apm.com> wrote: > > Hi Ilia Mirkin, > Thanks for your info. I did investigated why our test didn't detect it and found out that > the struct ocm_block is only used on ocm_debugfs_show function when we want to > know information about ocm and it's available when we enable debugfs. But our test > only tried to use the OCM block functions and didn't care about the OCM information. > So I think we should apply your patch to solve this issue instead of removing ocm part. > OK, perhaps there's something clever gong on. However on my git tree (updated as of a few days ago): $ git grep ppc4xx_ocm_alloc arch/powerpc/include/asm/ppc4xx_ocm.h:void *ppc4xx_ocm_alloc(phys_addr_t *phys, int size, int align, arch/powerpc/include/asm/ppc4xx_ocm.h:#define ppc4xx_ocm_alloc(phys, size, align, flags, owner) NULL arch/powerpc/sysdev/ppc4xx_ocm.c:void *ppc4xx_ocm_alloc(phys_addr_t *phys, int size, int align, So... no users. Unless there's macro-related cleverness going on (I'll freely admit to not having read/understood the full code, so could well be.) Perhaps it was meant to be used but the call got lost? -ilia > > > > On Sat, Dec 7, 2013 at 7:43 AM, Ilia Mirkin <imirkin@alum.mit.edu> wrote: >> >> Allocate enough memory for the ocm_block structure, not just a pointer >> to it. >> >> Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu> >> --- >> >> I have neither the hardware to test nor the toolchain to even build-test >> this. However this seems like a fairly obvious fix (and I have to wonder how >> this ever worked at all). Found with spatch. >> >> Actually further investigation makes it seem like this function is never >> called, perhaps it should just be removed? If it is kept around though, would >> be nice to apply this patch so that tools don't trip over this wrong code. >> >> arch/powerpc/sysdev/ppc4xx_ocm.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/powerpc/sysdev/ppc4xx_ocm.c b/arch/powerpc/sysdev/ppc4xx_ocm.c >> index b7c4345..85d9e37 100644 >> --- a/arch/powerpc/sysdev/ppc4xx_ocm.c >> +++ b/arch/powerpc/sysdev/ppc4xx_ocm.c >> @@ -339,7 +339,7 @@ void *ppc4xx_ocm_alloc(phys_addr_t *phys, int size, int align, >> if (IS_ERR_VALUE(offset)) >> continue; >> >> - ocm_blk = kzalloc(sizeof(struct ocm_block *), GFP_KERNEL); >> + ocm_blk = kzalloc(sizeof(struct ocm_block), GFP_KERNEL); >> if (!ocm_blk) { >> printk(KERN_ERR "PPC4XX OCM: could not allocate ocm block"); >> rh_free(ocm_reg->rh, offset); >> -- >> 1.8.3.2 >> > > > > -- > > Vinh Nguyen Huu Tuong | Staff SW Engineer > > C: 090.335.7841 | O: 083.770.0640 ext: 3719 > > F: 083.770.0641 | vhtnguyen@apm.com > > > > > > > >
diff --git a/arch/powerpc/sysdev/ppc4xx_ocm.c b/arch/powerpc/sysdev/ppc4xx_ocm.c index b7c4345..85d9e37 100644 --- a/arch/powerpc/sysdev/ppc4xx_ocm.c +++ b/arch/powerpc/sysdev/ppc4xx_ocm.c @@ -339,7 +339,7 @@ void *ppc4xx_ocm_alloc(phys_addr_t *phys, int size, int align, if (IS_ERR_VALUE(offset)) continue; - ocm_blk = kzalloc(sizeof(struct ocm_block *), GFP_KERNEL); + ocm_blk = kzalloc(sizeof(struct ocm_block), GFP_KERNEL); if (!ocm_blk) { printk(KERN_ERR "PPC4XX OCM: could not allocate ocm block"); rh_free(ocm_reg->rh, offset);
Allocate enough memory for the ocm_block structure, not just a pointer to it. Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu> --- I have neither the hardware to test nor the toolchain to even build-test this. However this seems like a fairly obvious fix (and I have to wonder how this ever worked at all). Found with spatch. Actually further investigation makes it seem like this function is never called, perhaps it should just be removed? If it is kept around though, would be nice to apply this patch so that tools don't trip over this wrong code. arch/powerpc/sysdev/ppc4xx_ocm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)