[PATCH v5 0/3] Retry Large Buffer Allocations

[PATCH v5 0/3] Retry Large Buffer Allocations

[Grant Erickson]

When handling user space read or write requests via mtd_{read,write}
or JFFS2 medium scan requests, the operations can fail because the
code backing these operations frequently requests large (128 KiB),
contiguous blocks of memory using kmalloc.

Under low-memory or highly-fragmented situations, particularly on
embedded systems with no swap store, these allocations will frequently
fail with -ENOMEM.

This patch series adds a common function to handle these allocation
requests in both the MTD driver and JFFS2 file system by exponentially
backing off on the size of the requested kernel transfer buffer until
it succeeds or until the requested transfer buffer size falls below
the page size.

This helps ensure the operation can succeed under low-memory,
highly-fragmented situations albeit somewhat more slowly.

  v2: Incorporated coding style and comment feedback from Artem.
  v3: Incorporated more feedback from Artem. Retargeted patch against
      l2-mtd-2.6.
  v4: Incorporated feedback from Artem about compilation on 64-bit
      architectures.
  v5: Split the patch into a series.

Signed-off-by: Grant Erickson <marathon96@gmail.com>
Tested-by: Ben Gardiner <bengardiner at nanometrics.ca>
---

[PATCH v5 1/3] MTD: Create Function to Perform Large Allocations

[Grant Erickson]

Introduce a common function to handle large, contiguous kmalloc buffer
allocations by exponentially backing off on the size of the requested
kernel transfer buffer until it succeeds or until the requested
transfer buffer size falls below the page size.

This helps ensure the operation can succeed under low-memory, highly-
fragmented situations albeit somewhat more slowly.

Signed-off-by: Grant Erickson <marathon96@gmail.com>
Tested-by: Ben Gardiner <bengardiner at nanometrics.ca>
---
 drivers/mtd/mtdcore.c   |   41 +++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/mtd.h |    2 ++
 2 files changed, 43 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index da69bc8..6f720cc 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -638,6 +638,46 @@ int default_mtd_writev(struct mtd_info *mtd, const struct kvec *vecs,
 	return ret;
 }
 
+/**
+ * mtd_kmalloc_up_to - allocate a contiguous buffer up to the specified size
+ * @size: A pointer to the ideal or maximum size of the allocation. Points
+ *        to the actual allocation size on success.
+ *
+ * This routine attempts to allocate a contiguous kernel buffer up to
+ * the specified size, backing off the size of the request exponentially
+ * until the request succeeds or until the allocation size falls below
+ * the system page size. This attempts to make sure it does not adversely
+ * impact system performance, so when allocating more than one page, we
+ * ask the memory allocator to avoid re-trying, swapping, writing back
+ * or performing I/O.
+ *
+ * This is called, for example by mtd_{read,write} and jffs2_scan_medium,
+ * to handle smaller (i.e. degraded) buffer allocations under low- or
+ * fragmented-memory situations where such reduced allocations, from a
+ * requested ideal, are allowed.
+ *
+ * Returns a pointer to the allocated buffer on success; otherwise, NULL.
+ */
+void *mtd_kmalloc_up_to(size_t *size)
+{
+	gfp_t flags = __GFP_NOWARN | __GFP_WAIT |
+		       __GFP_NORETRY | __GFP_NO_KSWAPD;
+	size_t try;
+	void *kbuf;
+
+	try = min_t(size_t, *size, KMALLOC_MAX_SIZE);
+
+	do {
+		if (try <= PAGE_SIZE)
+			flags = GFP_KERNEL;
+
+		kbuf = kmalloc(try, flags);
+	} while (!kbuf && ((try >>= 1) >= PAGE_SIZE));
+
+	*size = try;
+	return kbuf;
+}
+
 EXPORT_SYMBOL_GPL(add_mtd_device);
 EXPORT_SYMBOL_GPL(del_mtd_device);
 EXPORT_SYMBOL_GPL(get_mtd_device);
@@ -648,6 +688,7 @@ EXPORT_SYMBOL_GPL(__put_mtd_device);
 EXPORT_SYMBOL_GPL(register_mtd_user);
 EXPORT_SYMBOL_GPL(unregister_mtd_user);
 EXPORT_SYMBOL_GPL(default_mtd_writev);
+EXPORT_SYMBOL_GPL(mtd_kmalloc_up_to);
 
 #ifdef CONFIG_PROC_FS
 
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 9d5306b..a5d31ba 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -348,7 +348,8 @@ int default_mtd_writev(struct mtd_info *mtd, const struct kvec *vecs,
 int default_mtd_readv(struct mtd_info *mtd, struct kvec *vecs,
 		      unsigned long count, loff_t from, size_t *retlen);
 
+void *mtd_kmalloc_up_to(size_t *size);
+
 #ifdef CONFIG_MTD_PARTITIONS
 void mtd_erase_callback(struct erase_info *instr);
 #else
--
1.7.4.2

[PATCH v5 2/3] MTD: Retry Large Buffer Allocations

[Grant Erickson]

Replace direct call to kmalloc for a potentially large, contiguous
buffer allocation with one to mtd_kmalloc_up_to which helps ensure the
operation can succeed under low-memory, highly- fragmented situations
albeit somewhat more slowly.

Signed-off-by: Grant Erickson <marathon96@gmail.com>
Tested-by: Ben Gardiner <bengardiner at nanometrics.ca>
---
 drivers/mtd/mtdchar.c |   50 ++++++++++++++++++++++--------------------------
 1 files changed, 23 insertions(+), 27 deletions(-)

diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
index 145b3d0d..9301464 100644
--- a/drivers/mtd/mtdchar.c
+++ b/drivers/mtd/mtdchar.c
@@ -166,10 +166,23 @@ static int mtd_close(struct inode *inode, struct file *file)
 	return 0;
 } /* mtd_close */
 
-/* FIXME: This _really_ needs to die. In 2.5, we should lock the
-   userspace buffer down and use it directly with readv/writev.
-*/
-#define MAX_KMALLOC_SIZE 0x20000
+/* Back in April 2005, Linus wrote:
+ *
+ *   FIXME: This _really_ needs to die. In 2.5, we should lock the
+ *   userspace buffer down and use it directly with readv/writev.
+ *
+ * The implementation below, using mtd_kmalloc_up_to, mitigates
+ * allocation failures when the system is under low-memory situations
+ * or if memory is highly fragmented at the cost of reducing the
+ * performance of the requested transfer due to a smaller buffer size.
+ *
+ * A more complex but more memory-efficient implementation based on
+ * get_user_pages and iovecs to cover extents of those pages is a
+ * longer-term goal, as intimated by Linus above. However, for the
+ * write case, this requires yet more complex head and tail transfer
+ * handling when those head and tail offsets and sizes are such that
+ * alignment requirements are not met in the NAND subdriver.
+ */
 
 static ssize_t mtd_read(struct file *file, char __user *buf, size_t count,loff_t *ppos)
 {
@@ -179,6 +192,7 @@ static ssize_t mtd_read(struct file *file, char __user *buf, size_t count,loff_t
 	size_t total_retlen=0;
 	int ret=0;
 	int len;
+	size_t size = count;
 	char *kbuf;
 
 	DEBUG(MTD_DEBUG_LEVEL0,"MTD_read\n");
@@ -189,23 +203,12 @@ static ssize_t mtd_read(struct file *file, char __user *buf, size_t count,loff_t
 	if (!count)
 		return 0;
 
-	/* FIXME: Use kiovec in 2.5 to lock down the user's buffers
-	   and pass them directly to the MTD functions */
-
-	if (count > MAX_KMALLOC_SIZE)
-		kbuf=kmalloc(MAX_KMALLOC_SIZE, GFP_KERNEL);
-	else
-		kbuf=kmalloc(count, GFP_KERNEL);
-
+	kbuf = mtd_kmalloc_up_to(&size);
 	if (!kbuf)
 		return -ENOMEM;
 
 	while (count) {
-
-		if (count > MAX_KMALLOC_SIZE)
-			len = MAX_KMALLOC_SIZE;
-		else
-			len = count;
+		len = min_t(size_t, count, size);
 
 		switch (mfi->mode) {
 		case MTD_MODE_OTP_FACTORY:
@@ -268,6 +271,7 @@ static ssize_t mtd_write(struct file *file, const char __user *buf, size_t count
 {
 	struct mtd_file_info *mfi = file->private_data;
 	struct mtd_info *mtd = mfi->mtd;
+	size_t size = count;
 	char *kbuf;
 	size_t retlen;
 	size_t total_retlen=0;
@@ -285,21 +289,12 @@ static ssize_t mtd_write(struct file *file, const char __user *buf, size_t count
 	if (!count)
 		return 0;
 
-	if (count > MAX_KMALLOC_SIZE)
-		kbuf=kmalloc(MAX_KMALLOC_SIZE, GFP_KERNEL);
-	else
-		kbuf=kmalloc(count, GFP_KERNEL);
-
+	kbuf = mtd_kmalloc_up_to(&size);
 	if (!kbuf)
 		return -ENOMEM;
 
 	while (count) {
-
-		if (count > MAX_KMALLOC_SIZE)
-			len = MAX_KMALLOC_SIZE;
-		else
-			len = count;
+		len = min_t(size_t, count, size);
 
 		if (copy_from_user(kbuf, buf, len)) {
 			kfree(kbuf);
--
1.7.4.2

[PATCH v5 3/3] JFFS2: Retry Large Buffer Allocations

[Grant Erickson]

Replace direct call to kmalloc for a potentially large, contiguous
buffer allocation with one to mtd_kmalloc_up_to which helps ensure the
operation can succeed under low-memory, highly- fragmented situations
albeit somewhat more slowly.

Signed-off-by: Grant Erickson <marathon96@gmail.com>
Tested-by: Ben Gardiner <bengardiner at nanometrics.ca>
---
 fs/jffs2/scan.c |   19 +++++++++++--------
 1 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/fs/jffs2/scan.c b/fs/jffs2/scan.c
index b632ddd..e393213 100644
--- a/fs/jffs2/scan.c
+++ b/fs/jffs2/scan.c
@@ -94,7 +94,7 @@ int jffs2_scan_medium(struct jffs2_sb_info *c)
 	uint32_t buf_size = 0;
 	struct jffs2_summary *s = NULL; /* summary info collected by the scan process */
 #ifndef __ECOS
-	size_t pointlen;
+	size_t pointlen, try_size;
 
 	if (c->mtd->point) {
 		ret = c->mtd->point(c->mtd, 0, c->mtd->size, &pointlen,
@@ -113,19 +113,21 @@ int jffs2_scan_medium(struct jffs2_sb_info *c)
 		/* For NAND it's quicker to read a whole eraseblock at a time,
 		   apparently */
 		if (jffs2_cleanmarker_oob(c))
-			buf_size = c->sector_size;
+			try_size = c->sector_size;
 		else
-			buf_size = PAGE_SIZE;
+			try_size = PAGE_SIZE;
 
-		/* Respect kmalloc limitations */
-		if (buf_size > 128*1024)
-			buf_size = 128*1024;
+		D1(printk(KERN_DEBUG "Trying to allocate readbuf of %zu "
+			"bytes\n", try_size));
 
-		D1(printk(KERN_DEBUG "Allocating readbuf of %d bytes\n", buf_size));
-		flashbuf = kmalloc(buf_size, GFP_KERNEL);
+		flashbuf = mtd_kmalloc_up_to(&try_size);
 		if (!flashbuf)
 			return -ENOMEM;
+
+		D1(printk(KERN_DEBUG "Allocated readbuf of %zu bytes\n",
+			try_size));
+
+		buf_size = (uint32_t)try_size;
 	}
 
 	if (jffs2_sum_active()) {
--
1.7.4.2

Re: [PATCH v5 3/3] JFFS2: Retry Large Buffer Allocations

[Artem Bityutskiy]

On Fri, 2011-04-08 at 08:51 -0700, Grant Erickson wrote:
> Replace direct call to kmalloc for a potentially large, contiguous
> buffer allocation with one to mtd_kmalloc_up_to which helps ensure the
> operation can succeed under low-memory, highly- fragmented situations
> albeit somewhat more slowly.
> 
> Signed-off-by: Grant Erickson <marathon96@gmail.com>
> Tested-by: Ben Gardiner <bengardiner at nanometrics.ca>

Note, I killed this Tested-by because Ben did not test the JFFS2 part.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: [PATCH v5 0/3] Retry Large Buffer Allocations

[Artem Bityutskiy]

On Fri, 2011-04-08 at 08:51 -0700, Grant Erickson wrote:
> When handling user space read or write requests via mtd_{read,write}
> or JFFS2 medium scan requests, the operations can fail because the
> code backing these operations frequently requests large (128 KiB),
> contiguous blocks of memory using kmalloc.

Thanks, pushed the series to l2-mtd-2.6.git.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: [PATCH v5 0/3] Retry Large Buffer Allocations

[Ben Gardiner]

Hi Grant, Artem,

Thanks for this fix (again) :)

On Fri, Apr 8, 2011 at 11:51 AM, Grant Erickson <marathon96@gmail.com> wrote:
> When handling user space read or write requests via mtd_{read,write}
> or JFFS2 medium scan requests, the operations can fail because the
> code backing these operations frequently requests large (128 KiB),
> contiguous blocks of memory using kmalloc.
>
> Under low-memory or highly-fragmented situations, particularly on
> embedded systems with no swap store, these allocations will frequently
> fail with -ENOMEM.
>
> This patch series adds a common function to handle these allocation
> requests in both the MTD driver and JFFS2 file system by exponentially
> backing off on the size of the requested kernel transfer buffer until
> it succeeds or until the requested transfer buffer size falls below
> the page size.
>
> This helps ensure the operation can succeed under low-memory,
> highly-fragmented situations albeit somewhat more slowly.
>
>  v2: Incorporated coding style and comment feedback from Artem.
>  v3: Incorporated more feedback from Artem. Retargeted patch against
>      l2-mtd-2.6.
>  v4: Incorporated feedback from Artem about compilation on 64-bit
>      architectures.
>  v5: Split the patch into a series.
>
> Signed-off-by: Grant Erickson <marathon96@gmail.com>

I tested this on da850evm by cherry-picking commits:

8fa3c20 JFFS2: retry large buffer allocations
88ab142 mtd: mtdchar: retry large buffer allocations
99815a5 mtd: create function to perform large allocations

from
git://git.infradead.org/users/dedekind/l2-mtd-2.6.git

I have a memory hole @32M (bootargs: mem=32M@0xc0000000
mem=64M@0xc4000000) and I was also (like Bastian) getting "ubiformat:
page allocation failure" errors.  Cherry-picking these commits fixed the
problem for me; I was able to do 3x repeated ubiformats without error.

Tested-by: Ben Gardiner <bengardiner@nanometrics.ca>

Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca

Re: [PATCH v5 0/3] Retry Large Buffer Allocations

[Artem Bityutskiy]

On Mon, 2011-04-11 at 08:40 -0400, Ben Gardiner wrote:
> Tested-by: Ben Gardiner <bengardiner@nanometrics.ca>

Thanks!

-- 
Best Regards,
Artem Bityutskiy (Битюцкий Артём)