| Submitter | Artem Bityutskiy |
|---|---|
| Date | Jan. 17, 2010, 10:43 a.m. |
| Message ID | <1263724992.8276.140.camel@localhost.localdomain> |
| Download | mbox | patch |
| Permalink | /patch/43021/ |
| State | New |
| Headers | show |
Comments
Hello Artem, > On Sun, Jan 17, 2010 at 11:43 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote: >> On Sun, 2010-01-17 at 12:37 +0200, Artem Bityutskiy wrote: >>> Hi, >>> >>> On Wed, 2010-01-13 at 15:28 +0100, Marek Skuczynski wrote: >>> > Hello, >>> > I have prepare a simple volume update stress test (see test code below). >>> > This test has been run for kernel 2.6.23 with some updates from 2.6.28, >>> > and always after a few minutes the OOM killer was launched. >>> > >>> > What i found it that each an UBI volume truncate operation with >>> > ubiupdatevol tool >>> > causes memory leak. I think this happens because: >>> > >>> > - ubi_start_update() param "bytes" is equal 0 >>> > >>> > - vol->updating flag is re-set to 0 >>> > >>> > - vol->upd_buf is allocated regardless of vol->updating flag, >>> > but not released on device close by vol_cdev_release() >>> > >>> > I never run the test on a newer kernel version, so I cannot confirm >>> > that this problem still exists. >>> > >>> > Please confirm, whether my findings are correct or not, thanks. >>> >>> thanks for this finding. Looks like your analysis is right. Does this >>> simple patch help? >> >> Actually this even simpler patch should fix the issue. Could you please >> test it and let me know if it helps. >> >> diff --git a/drivers/mtd/ubi/upd.c b/drivers/mtd/ubi/upd.c >> index c1d7b88..425bf5a 100644 >> --- a/drivers/mtd/ubi/upd.c >> +++ b/drivers/mtd/ubi/upd.c >> @@ -155,6 +155,7 @@ int ubi_start_update(struct ubi_device *ubi, struct ubi_volume *vol, >> if (err) >> return err; >> vol->updating = 0; >> + return 0; >> } >> >> vol->upd_buf = vmalloc(ubi->leb_size); >> >> -- Thanks for you reply. It seems the fix solve the issue. Regards, Marek
On Mon, 2010-01-18 at 09:36 +0100, Marek Skuczynski wrote: > Hello Artem, > > On Sun, Jan 17, 2010 at 11:43 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote: > >> On Sun, 2010-01-17 at 12:37 +0200, Artem Bityutskiy wrote: > >>> Hi, > >>> > >>> On Wed, 2010-01-13 at 15:28 +0100, Marek Skuczynski wrote: > >>> > Hello, > >>> > I have prepare a simple volume update stress test (see test code below). > >>> > This test has been run for kernel 2.6.23 with some updates from 2.6.28, > >>> > and always after a few minutes the OOM killer was launched. > >>> > > >>> > What i found it that each an UBI volume truncate operation with > >>> > ubiupdatevol tool > >>> > causes memory leak. I think this happens because: > >>> > > >>> > - ubi_start_update() param "bytes" is equal 0 > >>> > > >>> > - vol->updating flag is re-set to 0 > >>> > > >>> > - vol->upd_buf is allocated regardless of vol->updating flag, > >>> > but not released on device close by vol_cdev_release() > >>> > > >>> > I never run the test on a newer kernel version, so I cannot confirm > >>> > that this problem still exists. > >>> > > >>> > Please confirm, whether my findings are correct or not, thanks. > >>> > >>> thanks for this finding. Looks like your analysis is right. Does this > >>> simple patch help? > >> > >> Actually this even simpler patch should fix the issue. Could you please > >> test it and let me know if it helps. > >> > >> diff --git a/drivers/mtd/ubi/upd.c b/drivers/mtd/ubi/upd.c > >> index c1d7b88..425bf5a 100644 > >> --- a/drivers/mtd/ubi/upd.c > >> +++ b/drivers/mtd/ubi/upd.c > >> @@ -155,6 +155,7 @@ int ubi_start_update(struct ubi_device *ubi, struct ubi_volume *vol, > >> if (err) > >> return err; > >> vol->updating = 0; > >> + return 0; > >> } > >> > >> vol->upd_buf = vmalloc(ubi->leb_size); > >> > >> -- > > Thanks for you reply. It seems the fix solve the issue. Cool. I'll push the fix and also send it to -stable a bit later. Thanks for catching this.
On Mon, 2010-01-18 at 09:36 +0100, Marek Skuczynski wrote: > >> diff --git a/drivers/mtd/ubi/upd.c b/drivers/mtd/ubi/upd.c > >> index c1d7b88..425bf5a 100644 > >> --- a/drivers/mtd/ubi/upd.c > >> +++ b/drivers/mtd/ubi/upd.c > >> @@ -155,6 +155,7 @@ int ubi_start_update(struct ubi_device *ubi, struct ubi_volume *vol, > >> if (err) > >> return err; > >> vol->updating = 0; > >> + return 0; > >> } > >> > >> vol->upd_buf = vmalloc(ubi->leb_size); > >> > >> -- > > Thanks for you reply. It seems the fix solve the issue. FYI, the fix is merged to the Linus's tree. Thanks.
Patch
diff --git a/drivers/mtd/ubi/upd.c b/drivers/mtd/ubi/upd.c index c1d7b88..425bf5a 100644 --- a/drivers/mtd/ubi/upd.c +++ b/drivers/mtd/ubi/upd.c @@ -155,6 +155,7 @@ int ubi_start_update(struct ubi_device *ubi, struct ubi_volume *vol, if (err) return err; vol->updating = 0; + return 0; } vol->upd_buf = vmalloc(ubi->leb_size);