答复: Re: ubi fastmap memory leak

[wang.bo116 at zte.com.cn]

Hi，

richard -rw- weinberger <richard.weinberger at gmail.com> 写于 2013-04-11 
22:53:15:

> Hi!
> 
> On Thu, Apr 11, 2013 at 11:23 AM,  <wang.bo116 at zte.com.cn> wrote:
> > Hello，
> >         When use ubi fastmap,i think there is a memory leak in
> > "ubi_attach" function.Think about this scene：
> >
> >         1. In "ubi_attach" function, "alloc_ai" function called to 
alloc a
> > slab cache named "ubi_aeb_slab_cache".
> >         Then use "ai->aeb_slab_cache" to record the cache.
> >
> >         2. ubi_attach -> scan_fast -> ubi_scan_fastmap ->
> > ubi_attach_fastmap,
> >         in "ubi_attach_fastmap" function, slab cache named
> > "ubi_ainf_peb_slab" will be alloced.
> >         Because of this slab cache also recorded in 
"ai->aeb_slab_cache",
> > so, in fact it overwrite the previously one.
> >
> >         3. When "destroy_ai" called , it only free 
"ubi_ainf_peb_slab".
> >
> >         I made a patch base on Linux 3.9-rc6, it fix this problem use 
a
> > temporary ai for fastmap scan.
> >         During my test, this patch like work good.
> 
> So, we call ubi_scan_fastmap() with an ai where ->aeb_slab_cache
> points already to a
> slab cache?
> I think it would be best to remove the kmem_cache_create() from
> ubi_scan_fastmap()
> completely.
> 
> What do you think?
> 
> --
> Thanks,
> //richard



Yes，i think ubi_attach_fastmap() no need to alloc slab cache, it already 
be alloced in "ubi_attach".
i think the fllowed code in "ubi_attach_fastmap" is needless.

        ai->aeb_slab_cache = kmem_cache_create("ubi_ainf_peb_slab",
                                               sizeof(struct 
ubi_ainf_peb),
                                               0, 0, NULL);
        if (!ai->aeb_slab_cache) {
                ret = -ENOMEM;
                goto fail;
        }

I notice that "scan_fast" function will scan PEB 0 to UBI_FM_MAX_START-1 
to find a fastmap.
During this scan, it will call "kmem_cache_alloc" to alloc "ubi_ainf_peb" 
form slab, and add it to a list in ai, 
but "ubi_attach_fastmap" expect the list is empty, so it will reinit 
ai->free list and so on.

If ubi_attach_fastmap() want to reuse the slab cache, the alloced slabs 
must be freed before the list(ai->free and so on) be inited.

Compare with free slab before ubi_attach_fastmap function, use a temporary 
ai in scan_fast can use "destroy_ai" function directly.