* RE: Re: [PATCH] UBI: fix memory leak when use fastmap
@ 2013-05-02 8:30 wang.bo116
2013-05-02 19:21 ` richard -rw- weinberger
0 siblings, 1 reply; 12+ messages in thread
From: wang.bo116 @ 2013-05-02 8:30 UTC (permalink / raw)
To: richard -rw- weinberger
Cc: artem.bityutskiy, cui.yunfeng, LKML, linux-mtd, liu.dong3
> Hi!
>
> On Tue, Apr 16, 2013 at 10:53 AM, <wang.bo116@zte.com.cn> wrote:
> > Hello,
> > Sorry, there is still something wrong with the previous
patch's
> > format, try to submit it again. When use ubi fastmap, there is a
memory
> > leak which will make destroy_ai() fail to free the slab alloced in
> > scan_fast(). The following patch fix this problem by use a temporary
> > "ubi_attach_info" variable in scan_fast().
>
> Thanks a lot for your patch!
>
> Did you test it well?
> We need to make sure that it does the right thing for the following
cases:
> 1. fastmap disabled, attaching a non-fastmap volume
> 1. fastmap disabled, attaching a fastmap volume
> 3. fastmap enabled, attaching a non-fastmap volume
> 4. fastmap enablled, attaching a fastmap volume
>
> --
> Thanks,
> //richard
Thanks for your advice. I consider this problem like this:
1: if fastmap not config, ubi_attach() just call scan_all(ubi, ai, 0),
there is nothing changed, so there is all right.
2: if fastmap config, and force_scan is 0, ubi_attach() will call
scan_fast(), when scan_fast() return, temp ai used in scan_fast() has been
free, and there will be two conditions:
A: scan_fast() return UBI_NO_FASTMAP(may be flash is empty or attaching a
non-fastmap volume), all ubi_attach() need is to call scan_all(ubi, ai, 0)
to scan all blocks.
B: scan_fast() return UBI_BAD_FASTMAP, ubi_attach() first free ai used in
ubi_scan_fastmap(), then alloc a clean ai, at last call scan_all(ubi, ai,
0) to scan all blocks.
3: if fastmap config, and force_scan is 1, just call scan_all(ubi, ai, 0).
This patch pass the following cases(include attach and detach):
1. fastmap config, flash is empty,fm_autoconvert is 1.
2. fastmap config, flash is empty,fm_autoconvert is 0.
3. fastmap config, attaching a fastmap volume
4. fastmap config, attaching a bad fastmap volume
5. fastmap config, attaching a non-fastmap volume
6. fastmap not config, attaching a fastmap volume
7. fastmap not config, attaching a non-fastmap volume
8. fastmap not config, flash is empty.
By the way, the problem may cause ubi_attach() fail after ubi detach when
CONFIG_DEBUG_VM config.
The reason is that ubi_attach() will alloc a slab cache, but the
kmem_cache_sanity_check() will find the slab cache is already exist(not
been free after last detach), so slab cache alloc fail, and ubi_attach()
fail, too.
Thanks
--wangbo
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Re: [PATCH] UBI: fix memory leak when use fastmap
2013-05-02 8:30 Re: [PATCH] UBI: fix memory leak when use fastmap wang.bo116
@ 2013-05-02 19:21 ` richard -rw- weinberger
2013-05-03 3:31 ` wang.bo116
0 siblings, 1 reply; 12+ messages in thread
From: richard -rw- weinberger @ 2013-05-02 19:21 UTC (permalink / raw)
To: wang.bo116; +Cc: artem.bityutskiy, cui.yunfeng, LKML, linux-mtd, liu.dong3
On Thu, May 2, 2013 at 10:30 AM, <wang.bo116@zte.com.cn> wrote:
>> Hi!
>>
>> On Tue, Apr 16, 2013 at 10:53 AM, <wang.bo116@zte.com.cn> wrote:
>> > Hello,
>> > Sorry, there is still something wrong with the previous
> patch's
>> > format, try to submit it again. When use ubi fastmap, there is a
> memory
>> > leak which will make destroy_ai() fail to free the slab alloced in
>> > scan_fast(). The following patch fix this problem by use a temporary
>> > "ubi_attach_info" variable in scan_fast().
>>
>> Thanks a lot for your patch!
>>
>> Did you test it well?
>> We need to make sure that it does the right thing for the following
> cases:
>> 1. fastmap disabled, attaching a non-fastmap volume
>> 1. fastmap disabled, attaching a fastmap volume
>> 3. fastmap enabled, attaching a non-fastmap volume
>> 4. fastmap enablled, attaching a fastmap volume
>>
>> --
>> Thanks,
>> //richard
>
>
> Thanks for your advice. I consider this problem like this:
> 1: if fastmap not config, ubi_attach() just call scan_all(ubi, ai, 0),
> there is nothing changed, so there is all right.
>
> 2: if fastmap config, and force_scan is 0, ubi_attach() will call
> scan_fast(), when scan_fast() return, temp ai used in scan_fast() has been
> free, and there will be two conditions:
> A: scan_fast() return UBI_NO_FASTMAP(may be flash is empty or attaching a
> non-fastmap volume), all ubi_attach() need is to call scan_all(ubi, ai, 0)
> to scan all blocks.
> B: scan_fast() return UBI_BAD_FASTMAP, ubi_attach() first free ai used in
> ubi_scan_fastmap(), then alloc a clean ai, at last call scan_all(ubi, ai,
> 0) to scan all blocks.
>
> 3: if fastmap config, and force_scan is 1, just call scan_all(ubi, ai, 0).
>
> This patch pass the following cases(include attach and detach):
> 1. fastmap config, flash is empty,fm_autoconvert is 1.
> 2. fastmap config, flash is empty,fm_autoconvert is 0.
> 3. fastmap config, attaching a fastmap volume
> 4. fastmap config, attaching a bad fastmap volume
> 5. fastmap config, attaching a non-fastmap volume
> 6. fastmap not config, attaching a fastmap volume
> 7. fastmap not config, attaching a non-fastmap volume
> 8. fastmap not config, flash is empty.
Good. :)
> By the way, the problem may cause ubi_attach() fail after ubi detach when
> CONFIG_DEBUG_VM config.
> The reason is that ubi_attach() will alloc a slab cache, but the
> kmem_cache_sanity_check() will find the slab cache is already exist(not
> been free after last detach), so slab cache alloc fail, and ubi_attach()
> fail, too.
Doesn't your patch address this issue too?
If not, we should fix this.
--
Thanks,
//richard
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Re: Re: [PATCH] UBI: fix memory leak when use fastmap
2013-05-02 19:21 ` richard -rw- weinberger
@ 2013-05-03 3:31 ` wang.bo116
2013-05-13 8:08 ` richard -rw- weinberger
0 siblings, 1 reply; 12+ messages in thread
From: wang.bo116 @ 2013-05-03 3:31 UTC (permalink / raw)
To: richard -rw- weinberger
Cc: artem.bityutskiy, cui.yunfeng, LKML, linux-mtd, liu.dong3
richard -rw- weinberger <richard.weinberger@gmail.com> write 2013-05-03
03:21:33:
> On Thu, May 2, 2013 at 10:30 AM, <wang.bo116@zte.com.cn> wrote:
> >> Hi!
> >>
> >> On Tue, Apr 16, 2013 at 10:53 AM, <wang.bo116@zte.com.cn> wrote:
> >> > Hello,
> >> > Sorry, there is still something wrong with the previous
> > patch's
> >> > format, try to submit it again. When use ubi fastmap, there is a
> > memory
> >> > leak which will make destroy_ai() fail to free the slab alloced in
> >> > scan_fast(). The following patch fix this problem by use a
temporary
> >> > "ubi_attach_info" variable in scan_fast().
> >>
> >> Thanks a lot for your patch!
> >>
> >> Did you test it well?
> >> We need to make sure that it does the right thing for the following
> > cases:
> >> 1. fastmap disabled, attaching a non-fastmap volume
> >> 1. fastmap disabled, attaching a fastmap volume
> >> 3. fastmap enabled, attaching a non-fastmap volume
> >> 4. fastmap enablled, attaching a fastmap volume
> >>
> >> --
> >> Thanks,
> >> //richard
> >
> >
> > Thanks for your advice. I consider this problem like this:
> > 1: if fastmap not config, ubi_attach() just call scan_all(ubi, ai, 0),
> > there is nothing changed, so there is all right.
> >
> > 2: if fastmap config, and force_scan is 0, ubi_attach() will call
> > scan_fast(), when scan_fast() return, temp ai used in scan_fast() has
been
> > free, and there will be two conditions:
> > A: scan_fast() return UBI_NO_FASTMAP(may be flash is empty or
attaching a
> > non-fastmap volume), all ubi_attach() need is to call scan_all(ubi,
ai, 0)
> > to scan all blocks.
> > B: scan_fast() return UBI_BAD_FASTMAP, ubi_attach() first free ai used
in
> > ubi_scan_fastmap(), then alloc a clean ai, at last call scan_all(ubi,
ai,
> > 0) to scan all blocks.
> >
> > 3: if fastmap config, and force_scan is 1, just call scan_all(ubi, ai,
0).
> >
> > This patch pass the following cases(include attach and detach):
> > 1. fastmap config, flash is empty,fm_autoconvert is 1.
> > 2. fastmap config, flash is empty,fm_autoconvert is 0.
> > 3. fastmap config, attaching a fastmap volume
> > 4. fastmap config, attaching a bad fastmap volume
> > 5. fastmap config, attaching a non-fastmap volume
> > 6. fastmap not config, attaching a fastmap volume
> > 7. fastmap not config, attaching a non-fastmap volume
> > 8. fastmap not config, flash is empty.
>
> Good. :)
>
> > By the way, the problem may cause ubi_attach() fail after ubi detach
when
> > CONFIG_DEBUG_VM config.
> > The reason is that ubi_attach() will alloc a slab cache, but the
> > kmem_cache_sanity_check() will find the slab cache is already
exist(not
> > been free after last detach), so slab cache alloc fail, and
ubi_attach()
> > fail, too.
>
> Doesn't your patch address this issue too?
> If not, we should fix this.Th
The patch had already fix the above problem mentioned.
Now, all the slabs belong to the slab-cache named "ubi_aeb_slab_cache" can
be rightly freed, so the slab-cache can be rightly freed when ubi_attach()
return.
When ubi attach again after detach, ubi can alloc a slab-cache named "
ubi_aeb_slab_cache" again.
Thanks
---wangbo
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Re: Re: [PATCH] UBI: fix memory leak when use fastmap
2013-05-03 3:31 ` wang.bo116
@ 2013-05-13 8:08 ` richard -rw- weinberger
2013-05-23 12:00 ` wang.bo116
0 siblings, 1 reply; 12+ messages in thread
From: richard -rw- weinberger @ 2013-05-13 8:08 UTC (permalink / raw)
To: wang.bo116; +Cc: artem.bityutskiy, cui.yunfeng, LKML, linux-mtd, liu.dong3
On Fri, May 3, 2013 at 5:31 AM, <wang.bo116@zte.com.cn> wrote:
> richard -rw- weinberger <richard.weinberger@gmail.com> write 2013-05-03
> 03:21:33:
>
>> On Thu, May 2, 2013 at 10:30 AM, <wang.bo116@zte.com.cn> wrote:
>> >> Hi!
>> >>
>> >> On Tue, Apr 16, 2013 at 10:53 AM, <wang.bo116@zte.com.cn> wrote:
>> >> > Hello,
>> >> > Sorry, there is still something wrong with the previous
>> > patch's
>> >> > format, try to submit it again. When use ubi fastmap, there is a
>> > memory
>> >> > leak which will make destroy_ai() fail to free the slab alloced in
>> >> > scan_fast(). The following patch fix this problem by use a
> temporary
>> >> > "ubi_attach_info" variable in scan_fast().
>> >>
>> >> Thanks a lot for your patch!
>> >>
>> >> Did you test it well?
>> >> We need to make sure that it does the right thing for the following
>> > cases:
>> >> 1. fastmap disabled, attaching a non-fastmap volume
>> >> 1. fastmap disabled, attaching a fastmap volume
>> >> 3. fastmap enabled, attaching a non-fastmap volume
>> >> 4. fastmap enablled, attaching a fastmap volume
>> >>
>> >> --
>> >> Thanks,
>> >> //richard
>> >
>> >
>> > Thanks for your advice. I consider this problem like this:
>> > 1: if fastmap not config, ubi_attach() just call scan_all(ubi, ai, 0),
>> > there is nothing changed, so there is all right.
>> >
>> > 2: if fastmap config, and force_scan is 0, ubi_attach() will call
>> > scan_fast(), when scan_fast() return, temp ai used in scan_fast() has
> been
>> > free, and there will be two conditions:
>> > A: scan_fast() return UBI_NO_FASTMAP(may be flash is empty or
> attaching a
>> > non-fastmap volume), all ubi_attach() need is to call scan_all(ubi,
> ai, 0)
>> > to scan all blocks.
>> > B: scan_fast() return UBI_BAD_FASTMAP, ubi_attach() first free ai used
> in
>> > ubi_scan_fastmap(), then alloc a clean ai, at last call scan_all(ubi,
> ai,
>> > 0) to scan all blocks.
>> >
>> > 3: if fastmap config, and force_scan is 1, just call scan_all(ubi, ai,
> 0).
>> >
>> > This patch pass the following cases(include attach and detach):
>> > 1. fastmap config, flash is empty,fm_autoconvert is 1.
>> > 2. fastmap config, flash is empty,fm_autoconvert is 0.
>> > 3. fastmap config, attaching a fastmap volume
>> > 4. fastmap config, attaching a bad fastmap volume
>> > 5. fastmap config, attaching a non-fastmap volume
>> > 6. fastmap not config, attaching a fastmap volume
>> > 7. fastmap not config, attaching a non-fastmap volume
>> > 8. fastmap not config, flash is empty.
>>
>> Good. :)
>>
>> > By the way, the problem may cause ubi_attach() fail after ubi detach
> when
>> > CONFIG_DEBUG_VM config.
>> > The reason is that ubi_attach() will alloc a slab cache, but the
>> > kmem_cache_sanity_check() will find the slab cache is already
> exist(not
>> > been free after last detach), so slab cache alloc fail, and
> ubi_attach()
>> > fail, too.
>>
>> Doesn't your patch address this issue too?
>> If not, we should fix this.Th
>
> The patch had already fix the above problem mentioned.
> Now, all the slabs belong to the slab-cache named "ubi_aeb_slab_cache" can
> be rightly freed, so the slab-cache can be rightly freed when ubi_attach()
> return.
> When ubi attach again after detach, ubi can alloc a slab-cache named "
> ubi_aeb_slab_cache" again.
I tried to apply/test your patch.
It has lots of white spaces damages.
Can you please resend it using git send-email?
And also run checkpatch.pl before sending.
--
Thanks,
//richard
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Re: Re: Re: [PATCH] UBI: fix memory leak when use fastmap
2013-05-13 8:08 ` richard -rw- weinberger
@ 2013-05-23 12:00 ` wang.bo116
2013-05-23 12:13 ` richard -rw- weinberger
0 siblings, 1 reply; 12+ messages in thread
From: wang.bo116 @ 2013-05-23 12:00 UTC (permalink / raw)
To: richard -rw- weinberger
Cc: cui.yunfeng, wang.bo116, LKML, linux-mtd, liu.dong3
richard -rw- weinberger <richard.weinberger@gmail.com> 2013-05-13
16:08:35:
>
> I tried to apply/test your patch.
> It has lots of white spaces damages.
> Can you please resend it using git send-email?
> And also run checkpatch.pl before sending.
>
> --
> Thanks,
> //richard
hello:
I use git to remake a patch, and pass the checkpatch.pl's test.
But my email client is lotus notes, and the patch's format may be damage
by the client.
Subject: [PATCH] Fix ubi fastmap memory leak
Signed-off-by: wang bo <wang.bo116@zte.com.cn>
---
linux-3.9-rc6/drivers/mtd/ubi/attach.c | 58
+++++++++++++++++-------------
linux-3.9-rc6/drivers/mtd/ubi/fastmap.c | 13 -------
2 files changed, 33 insertions(+), 38 deletions(-)
diff --git a/linux-3.9-rc6/drivers/mtd/ubi/attach.c
b/linux-3.9-rc6/drivers/mtd/ubi/attach.c
index c071d41..e9f64bc
--- a/linux-3.9-rc6/drivers/mtd/ubi/attach.c
+++ b/linux-3.9-rc6/drivers/mtd/ubi/attach.c
@@ -1212,6 +1212,30 @@ static void destroy_ai(struct ubi_attach_info *ai)
kfree(ai);
}
+static struct ubi_attach_info *alloc_ai(const char *slab_name)
+{
+ struct ubi_attach_info *ai;
+
+ ai = kzalloc(sizeof(struct ubi_attach_info), GFP_KERNEL);
+ if (!ai)
+ return ai;
+
+ INIT_LIST_HEAD(&ai->corr);
+ INIT_LIST_HEAD(&ai->free);
+ INIT_LIST_HEAD(&ai->erase);
+ INIT_LIST_HEAD(&ai->alien);
+ ai->volumes = RB_ROOT;
+ ai->aeb_slab_cache = kmem_cache_create(slab_name,
+ sizeof(struct
ubi_ainf_peb),
+ 0, 0, NULL);
+ if (!ai->aeb_slab_cache) {
+ kfree(ai);
+ ai = NULL;
+ }
+
+ return ai;
+}
+
/**
* scan_all - scan entire MTD device.
* @ubi: UBI device description object
@@ -1315,8 +1339,13 @@ static int scan_fast(struct ubi_device *ubi, struct
ubi_attach_info *ai)
int err, pnum, fm_anchor = -1;
unsigned long long max_sqnum = 0;
+ struct ubi_attach_info *fm_temp_ai = NULL;
err = -ENOMEM;
+ fm_temp_ai = alloc_ai("ubi_scan_fastmap_slab_cache");
+ if (!fm_temp_ai)
+ goto out;
+
ech = kzalloc(ubi->ec_hdr_alsize, GFP_KERNEL);
if (!ech)
goto out;
@@ -1331,7 +1360,7 @@ static int scan_fast(struct ubi_device *ubi, struct
ubi_attach_info *ai)
cond_resched();
dbg_gen("process PEB %d", pnum);
- err = scan_peb(ubi, ai, pnum, &vol_id, &sqnum);
+ err = scan_peb(ubi, fm_temp_ai, pnum, &vol_id, &sqnum);
if (err < 0)
goto out_vidh;
@@ -1343,6 +1372,7 @@ static int scan_fast(struct ubi_device *ubi, struct
ubi_attach_info *ai)
ubi_free_vid_hdr(ubi, vidh);
kfree(ech);
+ destroy_ai(fm_temp_ai);
if (fm_anchor < 0)
return UBI_NO_FASTMAP;
@@ -1351,6 +1381,7 @@ static int scan_fast(struct ubi_device *ubi, struct
ubi_attach_info *ai)
out_vidh:
ubi_free_vid_hdr(ubi, vidh);
+ destroy_ai(fm_temp_ai);
out_ech:
kfree(ech);
out:
@@ -1359,29 +1390,6 @@ out:
#endif
-static struct ubi_attach_info *alloc_ai(const char *slab_name)
-{
- struct ubi_attach_info *ai;
-
- ai = kzalloc(sizeof(struct ubi_attach_info), GFP_KERNEL);
- if (!ai)
- return ai;
-
- INIT_LIST_HEAD(&ai->corr);
- INIT_LIST_HEAD(&ai->free);
- INIT_LIST_HEAD(&ai->erase);
- INIT_LIST_HEAD(&ai->alien);
- ai->volumes = RB_ROOT;
- ai->aeb_slab_cache = kmem_cache_create(slab_name,
- sizeof(struct
ubi_ainf_peb),
- 0, 0, NULL);
- if (!ai->aeb_slab_cache) {
- kfree(ai);
- ai = NULL;
- }
-
- return ai;
-}
/**
* ubi_attach - attach an MTD device.
@@ -1419,7 +1427,7 @@ int ubi_attach(struct ubi_device *ubi, int
force_scan)
return -ENOMEM;
}
- err = scan_all(ubi, ai, UBI_FM_MAX_START);
+ err = scan_all(ubi, ai, 0);
}
}
#else
diff --git a/linux-3.9-rc6/drivers/mtd/ubi/fastmap.c
b/linux-3.9-rc6/drivers/mtd/ubi/fastmap.c
index 0648c69..7b73d23
--- a/linux-3.9-rc6/drivers/mtd/ubi/fastmap.c
+++ b/linux-3.9-rc6/drivers/mtd/ubi/fastmap.c
@@ -552,21 +552,8 @@ static int ubi_attach_fastmap(struct ubi_device *ubi,
INIT_LIST_HEAD(&used);
INIT_LIST_HEAD(&free);
INIT_LIST_HEAD(&eba_orphans);
- INIT_LIST_HEAD(&ai->corr);
- INIT_LIST_HEAD(&ai->free);
- INIT_LIST_HEAD(&ai->erase);
- INIT_LIST_HEAD(&ai->alien);
- ai->volumes = RB_ROOT;
ai->min_ec = UBI_MAX_ERASECOUNTER;
- 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;
- }
-
fmsb = (struct ubi_fm_sb *)(fm_raw);
ai->max_sqnum = fmsb->sqnum;
fm_pos += sizeof(struct ubi_fm_sb);
--
1.7.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: Re: Re: Re: [PATCH] UBI: fix memory leak when use fastmap
2013-05-23 12:00 ` wang.bo116
@ 2013-05-23 12:13 ` richard -rw- weinberger
2013-05-27 8:02 ` [PATCH] UBI: Fastmap: Fix memory leak Thomas Weber
0 siblings, 1 reply; 12+ messages in thread
From: richard -rw- weinberger @ 2013-05-23 12:13 UTC (permalink / raw)
To: wang.bo116; +Cc: cui.yunfeng, LKML, linux-mtd, liu.dong3
On Thu, May 23, 2013 at 2:00 PM, <wang.bo116@zte.com.cn> wrote:
> richard -rw- weinberger <richard.weinberger@gmail.com> 2013-05-13
> 16:08:35:
>
>>
>> I tried to apply/test your patch.
>> It has lots of white spaces damages.
>> Can you please resend it using git send-email?
>> And also run checkpatch.pl before sending.
>>
>> --
>> Thanks,
>> //richard
>
> hello:
> I use git to remake a patch, and pass the checkpatch.pl's test.
> But my email client is lotus notes, and the patch's format may be damage
> by the client.
This explains the broken patch.
You can also send me the patch as attachment. :)
--
Thanks,
//richard
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] UBI: Fastmap: Fix memory leak
2013-05-23 12:13 ` richard -rw- weinberger
@ 2013-05-27 8:02 ` Thomas Weber
2013-05-27 8:10 ` Richard Weinberger
0 siblings, 1 reply; 12+ messages in thread
From: Thomas Weber @ 2013-05-27 8:02 UTC (permalink / raw)
Cc: Thomas Weber, Richard Weinberger, Artem Bityutskiy,
David Woodhouse, linux-mtd, linux-kernel, wang bo
Signed-off-by: wang bo <wang.bo116@zte.com.cn>
[fix whitespace errors]
Tested with linux-v3.10-rc3 on Devkit8000.
Signed-off-by: Thomas Weber <thomas@tomweber.eu>
The discussion about this patch can be found:
https://lkml.org/lkml/2013/5/2/118
This patches fixes the following bug for me:
When CONFIG_DEBUG_VM is active and making
ubiattach ... ; ubidetach...; ubiattach ...
the second ubiattach stops with:
[ 37.918304] UBI: default fastmap pool size: 95
[ 37.923004] UBI: default fastmap WL pool size: 25
[ 37.928741] UBI: attaching mtd4 to ubi0
[ 37.933197] kmem_cache_sanity_check (ubi_aeb_slab_cache): Cache name already exists.
[ 37.941864] CPU: 0 PID: 757 Comm: ubiattach Not tainted 3.10.0-rc3 #66
[ 37.949066] [<c0013d90>] (unwind_backtrace+0x0/0xec) from [<c0011660>] (show_stack+0x20/0x24)
[ 37.958343] [<c0011660>] (show_stack+0x20/0x24) from [<c041527c>] (dump_stack+0x20/0x28)
[ 37.967163] [<c041527c>] (dump_stack+0x20/0x28) from [<c00d70b4>] (kmem_cache_create_memcg+0x120/0x2a4)
[ 37.977478] [<c00d70b4>] (kmem_cache_create_memcg+0x120/0x2a4) from [<c00d7280>] (kmem_cache_create+0x48/0x50)
[ 37.988281] [<c00d7280>] (kmem_cache_create+0x48/0x50) from [<c02fa090>] (alloc_ai+0x84/0xb0)
[ 37.997528] [<c02fa090>] (alloc_ai+0x84/0xb0) from [<c02fbde4>] (ubi_attach+0x28/0x34c)
[ 38.006225] [<c02fbde4>] (ubi_attach+0x28/0x34c) from [<c02f07c8>] (ubi_attach_mtd_dev+0x69c/0xca4)
[ 38.016021] [<c02f07c8>] (ubi_attach_mtd_dev+0x69c/0xca4) from [<c02f1078>] (ctrl_cdev_ioctl+0xe4/0x190)
[ 38.026275] [<c02f1078>] (ctrl_cdev_ioctl+0xe4/0x190) from [<c00fee00>] (do_vfs_ioctl+0x554/0x5c4)
[ 38.035980] [<c00fee00>] (do_vfs_ioctl+0x554/0x5c4) from [<c00feebc>] (SyS_ioctl+0x4c/0x6c)
[ 38.045043] [<c00feebc>] (SyS_ioctl+0x4c/0x6c) from [<c000dca0>] (ret_fast_syscall+0x0/0x48)
[ 38.054168] UBI error: ubi_attach_mtd_dev: failed to attach mtd4, error -12
ubiattach: error!: cannot attach "/dev/mtd4"
error 12 (Cannot allocate memory)
---
drivers/mtd/ubi/attach.c | 58 +++++++++++++++++++++++++++--------------------
drivers/mtd/ubi/fastmap.c | 13 -----------
2 files changed, 33 insertions(+), 38 deletions(-)
diff --git a/drivers/mtd/ubi/attach.c b/drivers/mtd/ubi/attach.c
index c071d41..e9f64bc 100644
--- a/drivers/mtd/ubi/attach.c
+++ b/drivers/mtd/ubi/attach.c
@@ -1212,6 +1212,30 @@ static void destroy_ai(struct ubi_attach_info *ai)
kfree(ai);
}
+static struct ubi_attach_info *alloc_ai(const char *slab_name)
+{
+ struct ubi_attach_info *ai;
+
+ ai = kzalloc(sizeof(struct ubi_attach_info), GFP_KERNEL);
+ if (!ai)
+ return ai;
+
+ INIT_LIST_HEAD(&ai->corr);
+ INIT_LIST_HEAD(&ai->free);
+ INIT_LIST_HEAD(&ai->erase);
+ INIT_LIST_HEAD(&ai->alien);
+ ai->volumes = RB_ROOT;
+ ai->aeb_slab_cache = kmem_cache_create(slab_name,
+ sizeof(struct ubi_ainf_peb),
+ 0, 0, NULL);
+ if (!ai->aeb_slab_cache) {
+ kfree(ai);
+ ai = NULL;
+ }
+
+ return ai;
+}
+
/**
* scan_all - scan entire MTD device.
* @ubi: UBI device description object
@@ -1315,8 +1339,13 @@ static int scan_fast(struct ubi_device *ubi, struct ubi_attach_info *ai)
int err, pnum, fm_anchor = -1;
unsigned long long max_sqnum = 0;
+ struct ubi_attach_info *fm_temp_ai = NULL;
err = -ENOMEM;
+ fm_temp_ai = alloc_ai("ubi_scan_fastmap_slab_cache");
+ if (!fm_temp_ai)
+ goto out;
+
ech = kzalloc(ubi->ec_hdr_alsize, GFP_KERNEL);
if (!ech)
goto out;
@@ -1331,7 +1360,7 @@ static int scan_fast(struct ubi_device *ubi, struct ubi_attach_info *ai)
cond_resched();
dbg_gen("process PEB %d", pnum);
- err = scan_peb(ubi, ai, pnum, &vol_id, &sqnum);
+ err = scan_peb(ubi, fm_temp_ai, pnum, &vol_id, &sqnum);
if (err < 0)
goto out_vidh;
@@ -1343,6 +1372,7 @@ static int scan_fast(struct ubi_device *ubi, struct ubi_attach_info *ai)
ubi_free_vid_hdr(ubi, vidh);
kfree(ech);
+ destroy_ai(fm_temp_ai);
if (fm_anchor < 0)
return UBI_NO_FASTMAP;
@@ -1351,6 +1381,7 @@ static int scan_fast(struct ubi_device *ubi, struct ubi_attach_info *ai)
out_vidh:
ubi_free_vid_hdr(ubi, vidh);
+ destroy_ai(fm_temp_ai);
out_ech:
kfree(ech);
out:
@@ -1359,29 +1390,6 @@ out:
#endif
-static struct ubi_attach_info *alloc_ai(const char *slab_name)
-{
- struct ubi_attach_info *ai;
-
- ai = kzalloc(sizeof(struct ubi_attach_info), GFP_KERNEL);
- if (!ai)
- return ai;
-
- INIT_LIST_HEAD(&ai->corr);
- INIT_LIST_HEAD(&ai->free);
- INIT_LIST_HEAD(&ai->erase);
- INIT_LIST_HEAD(&ai->alien);
- ai->volumes = RB_ROOT;
- ai->aeb_slab_cache = kmem_cache_create(slab_name,
- sizeof(struct ubi_ainf_peb),
- 0, 0, NULL);
- if (!ai->aeb_slab_cache) {
- kfree(ai);
- ai = NULL;
- }
-
- return ai;
-}
/**
* ubi_attach - attach an MTD device.
@@ -1419,7 +1427,7 @@ int ubi_attach(struct ubi_device *ubi, int force_scan)
return -ENOMEM;
}
- err = scan_all(ubi, ai, UBI_FM_MAX_START);
+ err = scan_all(ubi, ai, 0);
}
}
#else
diff --git a/drivers/mtd/ubi/fastmap.c b/drivers/mtd/ubi/fastmap.c
index 0648c69..7b73d23 100644
--- a/drivers/mtd/ubi/fastmap.c
+++ b/drivers/mtd/ubi/fastmap.c
@@ -552,21 +552,8 @@ static int ubi_attach_fastmap(struct ubi_device *ubi,
INIT_LIST_HEAD(&used);
INIT_LIST_HEAD(&free);
INIT_LIST_HEAD(&eba_orphans);
- INIT_LIST_HEAD(&ai->corr);
- INIT_LIST_HEAD(&ai->free);
- INIT_LIST_HEAD(&ai->erase);
- INIT_LIST_HEAD(&ai->alien);
- ai->volumes = RB_ROOT;
ai->min_ec = UBI_MAX_ERASECOUNTER;
- 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;
- }
-
fmsb = (struct ubi_fm_sb *)(fm_raw);
ai->max_sqnum = fmsb->sqnum;
fm_pos += sizeof(struct ubi_fm_sb);
--
1.8.2.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH] UBI: Fastmap: Fix memory leak
2013-05-27 8:02 ` [PATCH] UBI: Fastmap: Fix memory leak Thomas Weber
@ 2013-05-27 8:10 ` Richard Weinberger
2013-05-27 8:22 ` Thomas Weber
2013-05-29 12:27 ` Artem Bityutskiy
0 siblings, 2 replies; 12+ messages in thread
From: Richard Weinberger @ 2013-05-27 8:10 UTC (permalink / raw)
To: Thomas Weber
Cc: Artem Bityutskiy, David Woodhouse, linux-mtd, linux-kernel, wang bo
Hi!
Am 27.05.2013 10:02, schrieb Thomas Weber:
> Signed-off-by: wang bo <wang.bo116@zte.com.cn>
>
> [fix whitespace errors]
>
> Tested with linux-v3.10-rc3 on Devkit8000.
> Signed-off-by: Thomas Weber <thomas@tomweber.eu>
>
> The discussion about this patch can be found:
> https://lkml.org/lkml/2013/5/2/118
Thanks a lot for fixing the whitespace issues!
Am I allowed to add a Tested-by: Thomas Weber <thomas@tomweber.eu>?
IOW did you test the patch? :D
Thanks,
//richard
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] UBI: Fastmap: Fix memory leak
2013-05-27 8:10 ` Richard Weinberger
@ 2013-05-27 8:22 ` Thomas Weber
2013-05-29 12:27 ` Artem Bityutskiy
1 sibling, 0 replies; 12+ messages in thread
From: Thomas Weber @ 2013-05-27 8:22 UTC (permalink / raw)
To: Richard Weinberger
Cc: Artem Bityutskiy, David Woodhouse, linux-mtd, linux-kernel, wang bo
Hello,
On 2013-05-27 10:10, Richard Weinberger wrote:
> Hi!
>
> Am 27.05.2013 10:02, schrieb Thomas Weber:
>> Signed-off-by: wang bo <wang.bo116@zte.com.cn>
>>
>> [fix whitespace errors]
>>
>> Tested with linux-v3.10-rc3 on Devkit8000.
>> Signed-off-by: Thomas Weber <thomas@tomweber.eu>
>>
>> The discussion about this patch can be found:
>> https://lkml.org/lkml/2013/5/2/118
>
> Thanks a lot for fixing the whitespace issues!
> Am I allowed to add a Tested-by: Thomas Weber <thomas@tomweber.eu>?
> IOW did you test the patch? :D
You can add my Tested-by.
Thomas
>
> Thanks,
> //richard
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] UBI: Fastmap: Fix memory leak
2013-05-27 8:10 ` Richard Weinberger
2013-05-27 8:22 ` Thomas Weber
@ 2013-05-29 12:27 ` Artem Bityutskiy
2013-05-29 12:28 ` Richard Weinberger
1 sibling, 1 reply; 12+ messages in thread
From: Artem Bityutskiy @ 2013-05-29 12:27 UTC (permalink / raw)
To: Richard Weinberger
Cc: Thomas Weber, David Woodhouse, linux-mtd, linux-kernel, wang bo
On Mon, 2013-05-27 at 10:10 +0200, Richard Weinberger wrote:
> Hi!
>
> Am 27.05.2013 10:02, schrieb Thomas Weber:
> > Signed-off-by: wang bo <wang.bo116@zte.com.cn>
> >
> > [fix whitespace errors]
> >
> > Tested with linux-v3.10-rc3 on Devkit8000.
> > Signed-off-by: Thomas Weber <thomas@tomweber.eu>
> >
> > The discussion about this patch can be found:
> > https://lkml.org/lkml/2013/5/2/118
>
> Thanks a lot for fixing the whitespace issues!
> Am I allowed to add a Tested-by: Thomas Weber <thomas@tomweber.eu>?
> IOW did you test the patch? :D
Did you intend to re-send the patch with all the tags at proper places?
--
Best Regards,
Artem Bityutskiy
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] UBI: Fastmap: Fix memory leak
2013-05-29 12:27 ` Artem Bityutskiy
@ 2013-05-29 12:28 ` Richard Weinberger
0 siblings, 0 replies; 12+ messages in thread
From: Richard Weinberger @ 2013-05-29 12:28 UTC (permalink / raw)
To: dedekind1; +Cc: Thomas Weber, David Woodhouse, linux-mtd, linux-kernel, wang bo
Am 29.05.2013 14:27, schrieb Artem Bityutskiy:
> On Mon, 2013-05-27 at 10:10 +0200, Richard Weinberger wrote:
>> Hi!
>>
>> Am 27.05.2013 10:02, schrieb Thomas Weber:
>>> Signed-off-by: wang bo <wang.bo116@zte.com.cn>
>>>
>>> [fix whitespace errors]
>>>
>>> Tested with linux-v3.10-rc3 on Devkit8000.
>>> Signed-off-by: Thomas Weber <thomas@tomweber.eu>
>>>
>>> The discussion about this patch can be found:
>>> https://lkml.org/lkml/2013/5/2/118
>>
>> Thanks a lot for fixing the whitespace issues!
>> Am I allowed to add a Tested-by: Thomas Weber <thomas@tomweber.eu>?
>> IOW did you test the patch? :D
>
> Did you intend to re-send the patch with all the tags at proper places?
Yes. But after I've tested it.
Currently I'm very busy. But starting with next week I'll have more time.
Thanks,
//richard
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] UBI: Fastmap: Fix memory leak
@ 2013-08-06 6:26 Richard Weinberger
0 siblings, 0 replies; 12+ messages in thread
From: Richard Weinberger @ 2013-08-06 6:26 UTC (permalink / raw)
To: linux-mtd
Cc: dedekind1, dwmw2, linux-kernel, Thomas Weber, wang bo,
Richard Weinberger
From: Thomas Weber <thomas@tomweber.eu>
Tested with linux-v3.10-rc3 on Devkit8000.
The discussion about this patch can be found:
https://lkml.org/lkml/2013/5/2/118
This patches fixes the following bug for me:
When CONFIG_DEBUG_VM is active and making
ubiattach ... ; ubidetach...; ubiattach ...
the second ubiattach stops with:
[ 37.918304] UBI: default fastmap pool size: 95
[ 37.923004] UBI: default fastmap WL pool size: 25
[ 37.928741] UBI: attaching mtd4 to ubi0
[ 37.933197] kmem_cache_sanity_check (ubi_aeb_slab_cache): Cache name already exists.
[ 37.941864] CPU: 0 PID: 757 Comm: ubiattach Not tainted 3.10.0-rc3 #66
[ 37.949066] [<c0013d90>] (unwind_backtrace+0x0/0xec) from [<c0011660>] (show_stack+0x20/0x24)
[ 37.958343] [<c0011660>] (show_stack+0x20/0x24) from [<c041527c>] (dump_stack+0x20/0x28)
[ 37.967163] [<c041527c>] (dump_stack+0x20/0x28) from [<c00d70b4>] (kmem_cache_create_memcg+0x120/0x2a4)
[ 37.977478] [<c00d70b4>] (kmem_cache_create_memcg+0x120/0x2a4) from [<c00d7280>] (kmem_cache_create+0x48/0x50)
[ 37.988281] [<c00d7280>] (kmem_cache_create+0x48/0x50) from [<c02fa090>] (alloc_ai+0x84/0xb0)
[ 37.997528] [<c02fa090>] (alloc_ai+0x84/0xb0) from [<c02fbde4>] (ubi_attach+0x28/0x34c)
[ 38.006225] [<c02fbde4>] (ubi_attach+0x28/0x34c) from [<c02f07c8>] (ubi_attach_mtd_dev+0x69c/0xca4)
[ 38.016021] [<c02f07c8>] (ubi_attach_mtd_dev+0x69c/0xca4) from [<c02f1078>] (ctrl_cdev_ioctl+0xe4/0x190)
[ 38.026275] [<c02f1078>] (ctrl_cdev_ioctl+0xe4/0x190) from [<c00fee00>] (do_vfs_ioctl+0x554/0x5c4)
[ 38.035980] [<c00fee00>] (do_vfs_ioctl+0x554/0x5c4) from [<c00feebc>] (SyS_ioctl+0x4c/0x6c)
[ 38.045043] [<c00feebc>] (SyS_ioctl+0x4c/0x6c) from [<c000dca0>] (ret_fast_syscall+0x0/0x48)
[ 38.054168] UBI error: ubi_attach_mtd_dev: failed to attach mtd4, error -12
ubiattach: error!: cannot attach "/dev/mtd4"
error 12 (Cannot allocate memory)
Signed-off-by: wang bo <wang.bo116@zte.com.cn>
Signed-off-by: Thomas Weber <thomas@tomweber.eu>
[Thomas: fix whitespace errors]
Signed-off-by: Richard Weinberger <richard@nod.at>
diff --git a/drivers/mtd/ubi/attach.c b/drivers/mtd/ubi/attach.c
index c071d41..e9f64bc 100644
--- a/drivers/mtd/ubi/attach.c
+++ b/drivers/mtd/ubi/attach.c
@@ -1212,6 +1212,30 @@ static void destroy_ai(struct ubi_attach_info *ai)
kfree(ai);
}
+static struct ubi_attach_info *alloc_ai(const char *slab_name)
+{
+ struct ubi_attach_info *ai;
+
+ ai = kzalloc(sizeof(struct ubi_attach_info), GFP_KERNEL);
+ if (!ai)
+ return ai;
+
+ INIT_LIST_HEAD(&ai->corr);
+ INIT_LIST_HEAD(&ai->free);
+ INIT_LIST_HEAD(&ai->erase);
+ INIT_LIST_HEAD(&ai->alien);
+ ai->volumes = RB_ROOT;
+ ai->aeb_slab_cache = kmem_cache_create(slab_name,
+ sizeof(struct ubi_ainf_peb),
+ 0, 0, NULL);
+ if (!ai->aeb_slab_cache) {
+ kfree(ai);
+ ai = NULL;
+ }
+
+ return ai;
+}
+
/**
* scan_all - scan entire MTD device.
* @ubi: UBI device description object
@@ -1315,8 +1339,13 @@ static int scan_fast(struct ubi_device *ubi, struct ubi_attach_info *ai)
int err, pnum, fm_anchor = -1;
unsigned long long max_sqnum = 0;
+ struct ubi_attach_info *fm_temp_ai = NULL;
err = -ENOMEM;
+ fm_temp_ai = alloc_ai("ubi_scan_fastmap_slab_cache");
+ if (!fm_temp_ai)
+ goto out;
+
ech = kzalloc(ubi->ec_hdr_alsize, GFP_KERNEL);
if (!ech)
goto out;
@@ -1331,7 +1360,7 @@ static int scan_fast(struct ubi_device *ubi, struct ubi_attach_info *ai)
cond_resched();
dbg_gen("process PEB %d", pnum);
- err = scan_peb(ubi, ai, pnum, &vol_id, &sqnum);
+ err = scan_peb(ubi, fm_temp_ai, pnum, &vol_id, &sqnum);
if (err < 0)
goto out_vidh;
@@ -1343,6 +1372,7 @@ static int scan_fast(struct ubi_device *ubi, struct ubi_attach_info *ai)
ubi_free_vid_hdr(ubi, vidh);
kfree(ech);
+ destroy_ai(fm_temp_ai);
if (fm_anchor < 0)
return UBI_NO_FASTMAP;
@@ -1351,6 +1381,7 @@ static int scan_fast(struct ubi_device *ubi, struct ubi_attach_info *ai)
out_vidh:
ubi_free_vid_hdr(ubi, vidh);
+ destroy_ai(fm_temp_ai);
out_ech:
kfree(ech);
out:
@@ -1359,29 +1390,6 @@ out:
#endif
-static struct ubi_attach_info *alloc_ai(const char *slab_name)
-{
- struct ubi_attach_info *ai;
-
- ai = kzalloc(sizeof(struct ubi_attach_info), GFP_KERNEL);
- if (!ai)
- return ai;
-
- INIT_LIST_HEAD(&ai->corr);
- INIT_LIST_HEAD(&ai->free);
- INIT_LIST_HEAD(&ai->erase);
- INIT_LIST_HEAD(&ai->alien);
- ai->volumes = RB_ROOT;
- ai->aeb_slab_cache = kmem_cache_create(slab_name,
- sizeof(struct ubi_ainf_peb),
- 0, 0, NULL);
- if (!ai->aeb_slab_cache) {
- kfree(ai);
- ai = NULL;
- }
-
- return ai;
-}
/**
* ubi_attach - attach an MTD device.
@@ -1419,7 +1427,7 @@ int ubi_attach(struct ubi_device *ubi, int force_scan)
return -ENOMEM;
}
- err = scan_all(ubi, ai, UBI_FM_MAX_START);
+ err = scan_all(ubi, ai, 0);
}
}
#else
diff --git a/drivers/mtd/ubi/fastmap.c b/drivers/mtd/ubi/fastmap.c
index 1542751..6935f0c 100644
--- a/drivers/mtd/ubi/fastmap.c
+++ b/drivers/mtd/ubi/fastmap.c
@@ -552,21 +552,8 @@ static int ubi_attach_fastmap(struct ubi_device *ubi,
INIT_LIST_HEAD(&used);
INIT_LIST_HEAD(&free);
INIT_LIST_HEAD(&eba_orphans);
- INIT_LIST_HEAD(&ai->corr);
- INIT_LIST_HEAD(&ai->free);
- INIT_LIST_HEAD(&ai->erase);
- INIT_LIST_HEAD(&ai->alien);
- ai->volumes = RB_ROOT;
ai->min_ec = UBI_MAX_ERASECOUNTER;
- 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;
- }
-
fmsb = (struct ubi_fm_sb *)(fm_raw);
ai->max_sqnum = fmsb->sqnum;
fm_pos += sizeof(struct ubi_fm_sb);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2013-08-06 6:26 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-02 8:30 Re: [PATCH] UBI: fix memory leak when use fastmap wang.bo116
2013-05-02 19:21 ` richard -rw- weinberger
2013-05-03 3:31 ` wang.bo116
2013-05-13 8:08 ` richard -rw- weinberger
2013-05-23 12:00 ` wang.bo116
2013-05-23 12:13 ` richard -rw- weinberger
2013-05-27 8:02 ` [PATCH] UBI: Fastmap: Fix memory leak Thomas Weber
2013-05-27 8:10 ` Richard Weinberger
2013-05-27 8:22 ` Thomas Weber
2013-05-29 12:27 ` Artem Bityutskiy
2013-05-29 12:28 ` Richard Weinberger
2013-08-06 6:26 Richard Weinberger
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).