Patchwork [(mtd-utils)] mtd_write: fall back to old method on ENOMEM

login
register
mail settings
Submitter Voss, Nikolaus
Date April 23, 2012, 5:45 a.m.
Message ID <EF2E73589CA71846A15D0B2CDF79505D0905DB465E@wm021.weinmann.com>
Download mbox | patch
Permalink /patch/154321/
State New
Headers show

Comments

Voss, Nikolaus - April 23, 2012, 5:45 a.m.
MEMWRITE ioctl tries to kmalloc the whole data. With usual
eraseblock sizes of 128 KiB, on memory constrained systems
this can easily lead to ENOMEM due to memory fragmentation
even if there are plenty of free pages left e.g.:

--------------------------------------------------------------

libmtd: error!: MEMWRITE ioctl failed for eraseblock 55 (mtd6)
        error 12 (Cannot allocate memory)
ubiformat: error!: cannot write eraseblock 55
           error 12 (Cannot allocate memory)

Kernel log:
[   67.870000] Normal: 85*4kB 37*8kB 15*16kB 4*32kB 8*64kB 0*128kB 0*256kB
0*512kB 0*1024kB 0*2048kB 0*4096kB = 1516kB
[   67.870000] 11214 total pagecache pages
[   67.873000] 16384 pages of RAM
[   67.873000] 493 free pages
[   67.873000] 1067 reserved pages
[   67.873000] 887 slab pages
[   67.873000] 6266 pages shared 
[   67.873000] 0 pages swap cached

--------------------------------------------------------------

In such a case, it is better to write slower than not at all.

Signed-off-by: Nikolaus Voss <n.voss@weinmann.de>
---
 lib/libmtd.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Brian Norris - April 23, 2012, 6:31 a.m.
On Sun, Apr 22, 2012 at 10:45 PM, Voss, Nikolaus <N.Voss@weinmann.de> wrote:
> MEMWRITE ioctl tries to kmalloc the whole data. With usual
> eraseblock sizes of 128 KiB, on memory constrained systems
> this can easily lead to ENOMEM due to memory fragmentation
> even if there are plenty of free pages left e.g.:
>
> --------------------------------------------------------------
>
> libmtd: error!: MEMWRITE ioctl failed for eraseblock 55 (mtd6)
>        error 12 (Cannot allocate memory)
> ubiformat: error!: cannot write eraseblock 55
>           error 12 (Cannot allocate memory)

What version of mtd-utils are using? I don't think ubiformat should be
utilizing the MEMWRITE codepath anymore, since commit 71c76e7.

> ---
>  lib/libmtd.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/lib/libmtd.c b/lib/libmtd.c
> index 9b247ae..9d24ef0 100644
> --- a/lib/libmtd.c
> +++ b/lib/libmtd.c
> @@ -1151,7 +1151,7 @@ int mtd_write(libmtd_t desc, const struct mtd_dev_info *mtd, int fd, int eb,
>        ret = ioctl(fd, MEMWRITE, &ops);
>        if (ret == 0)
>                return 0;
> -       else if (errno != ENOTTY && errno != EOPNOTSUPP)
> +       else if (errno != ENOTTY && errno != EOPNOTSUPP && errno != ENOMEM)
>                return mtd_ioctl_error(mtd, eb, "MEMWRITE");
>
>        /* Fall back to old methods if necessary */

I'm not sure we want to go down the path of special cases for every
possible error; the "fall back" is because this ioctl doesn't exist on
older kernels. If MEMWRITE is causing real problems for the intended
use case (writing page data + OOB), then userspace or kernel-space
should be tweaked. But it's really not a problem AFAIK, since it's
real use is for page-at-a-time (data+OOB) writes from tools like
nandwrite.

Brian
Voss, Nikolaus - April 23, 2012, 7:43 a.m.
Brian Norris wrote on 2012-04-23:
> On Sun, Apr 22, 2012 at 10:45 PM, Voss, Nikolaus <N.Voss@weinmann.de> wrote:
>> MEMWRITE ioctl tries to kmalloc the whole data. With usual
>> eraseblock sizes of 128 KiB, on memory constrained systems
>> this can easily lead to ENOMEM due to memory fragmentation
>> even if there are plenty of free pages left e.g.:
>> 
>> --------------------------------------------------------------
>> 
>> libmtd: error!: MEMWRITE ioctl failed for eraseblock 55 (mtd6)
>>        error 12 (Cannot allocate memory)
>> ubiformat: error!: cannot write eraseblock 55
>>           error 12 (Cannot allocate memory)
> 
> What version of mtd-utils are using? I don't think ubiformat should be
> utilizing the MEMWRITE codepath anymore, since commit 71c76e7.

I used the latest release, 1.4.9. Commit 71c76e7 fixes my ubiformat
issue, thanks for hint.

[...]
> the "fall back" is because this ioctl doesn't exist on
> older kernels. If MEMWRITE is causing real problems for the intended
> use case (writing page data + OOB), then userspace or kernel-space
> should be tweaked. But it's really not a problem AFAIK, since it's
> real use is for page-at-a-time (data+OOB) writes from tools like
> nandwrite.

Ok, I see your point, thanks for the explanation. I agree completely.

Niko

Patch

diff --git a/lib/libmtd.c b/lib/libmtd.c
index 9b247ae..9d24ef0 100644
--- a/lib/libmtd.c
+++ b/lib/libmtd.c
@@ -1151,7 +1151,7 @@  int mtd_write(libmtd_t desc, const struct mtd_dev_info *mtd, int fd, int eb,
 	ret = ioctl(fd, MEMWRITE, &ops);
 	if (ret == 0)
 		return 0;
-	else if (errno != ENOTTY && errno != EOPNOTSUPP)
+	else if (errno != ENOTTY && errno != EOPNOTSUPP && errno != ENOMEM)
 		return mtd_ioctl_error(mtd, eb, "MEMWRITE");
 
 	/* Fall back to old methods if necessary */