| Message ID | 1303807043.2778.45.camel@localhost |
|---|---|
| State | New, archived |
| Headers | show |
On 04/26/2011 10:37 AM, Artem Bityutskiy wrote: > Hi, Hi Artem, > Yes, I think this is the right solution. I've cooked the following > patch, compile-tested only - please review and test. I have applied and tested your patch and the problem is solved. Tested-by: Stefano Babic <sbabic@denx.de> Best regards, Stefano Babic
On 4/26/11 1:37 AM, Artem Bityutskiy wrote: > On Tue, 2011-04-26 at 08:56 +0200, Stefano Babic wrote: >> I have seen a side effect introduced with Grant's patch. I have applied >> the patchset and this fix the allocation problem in kernel. > > I think it would be great to actually CC grant :-) So, this e-mail is > about this patch: > > http://lists.infradead.org/pipermail/linux-mtd/2011-April/034818.html > > Grant, the original e-mail from Stefano: > http://lists.infradead.org/pipermail/linux-mtd/2011-April/035190.html > >> However, the result lenght can be an odd multiple of the minimum I/O >> size, and when there is not enough memory available, the introduced >> mtd_malloc_up_to_size() tries to allocate half the amount of requested >> size, getting a buffer not aligned with the minimum I/O size. The result >> is that the test in nand_base.c for the alignment fails: > > Good catch! Shame on me to not foresee this issue. Thanks for reporting! > >> IMHO mtd_malloc_up_to_size() must allocate a buffer aligned with the >> minimum I/O size. What about adding the I/O size as parameter to >> mtd_malloc_up_to_size, so the function will return always an aligned >> buffer ? > > Yes, I think this is the right solution. I've cooked the following > patch, compile-tested only - please review and test. > > > From 18d93dac176025defd5711b429acd91ef8563a9a Mon Sep 17 00:00:00 2001 > From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com> > Date: Tue, 26 Apr 2011 11:42:10 +0300 > Subject: [PATCH] mtd: a fix candidate for mtd_kmalloc_up_to Artem: Thanks for adding me to the thread. Agree that the proposed patch seems like a good solution. Best, Grant
On Tue, 2011-04-26 at 12:45 +0200, Stefano Babic wrote:
> Tested-by: Stefano Babic <sbabic@denx.de>
Thanks, I've tweaked Grant's patches and folded my patch into them, and
added your Tested-by tag, and pushed to l2-mtd-2.6.git.
diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c index 9301464..7b7f992 100644 --- a/drivers/mtd/mtdchar.c +++ b/drivers/mtd/mtdchar.c @@ -203,7 +203,7 @@ static ssize_t mtd_read(struct file *file, char __user *buf, size_t count,loff_t if (!count) return 0; - kbuf = mtd_kmalloc_up_to(&size); + kbuf = mtd_kmalloc_up_to(mtd, &size); if (!kbuf) return -ENOMEM; @@ -289,7 +289,7 @@ static ssize_t mtd_write(struct file *file, const char __user *buf, size_t count if (!count) return 0; - kbuf = mtd_kmalloc_up_to(&size); + kbuf = mtd_kmalloc_up_to(mtd, &size); if (!kbuf) return -ENOMEM; diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c index 6f720cc..a50348b 100644 --- a/drivers/mtd/mtdcore.c +++ b/drivers/mtd/mtdcore.c @@ -651,6 +651,9 @@ int default_mtd_writev(struct mtd_info *mtd, const struct kvec *vecs, * ask the memory allocator to avoid re-trying, swapping, writing back * or performing I/O. * + * Note, this function also makes sure that the allocated buffer is aligned to + * the MTD device's min. I/O unit, i.e. the "mtd->writesize" value. + * * 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 @@ -658,24 +661,29 @@ int default_mtd_writev(struct mtd_info *mtd, const struct kvec *vecs, * * Returns a pointer to the allocated buffer on success; otherwise, NULL. */ -void *mtd_kmalloc_up_to(size_t *size) +void *mtd_kmalloc_up_to(const struct mtd_info *mtd, size_t *size) { gfp_t flags = __GFP_NOWARN | __GFP_WAIT | __GFP_NORETRY | __GFP_NO_KSWAPD; - size_t try; + size_t min_alloc = max_t(size_t, mtd->writesize, PAGE_SIZE); void *kbuf; - try = min_t(size_t, *size, KMALLOC_MAX_SIZE); + *size = min_t(size_t, *size, KMALLOC_MAX_SIZE); - do { - if (try <= PAGE_SIZE) - flags = GFP_KERNEL; + while (*size > min_alloc) { + kbuf = kmalloc(*size, flags); + if (kbuf) + return kbuf; - kbuf = kmalloc(try, flags); - } while (!kbuf && ((try >>= 1) >= PAGE_SIZE)); + *size >>= 1; + *size = ALIGN(*size, mtd->writesize); + } - *size = try; - return kbuf; + /* + * For the last resort allocation allow 'kmalloc()' to do all sorts of + * things (write-back, dropping caches, etc) by using GFP_KERNEL. + */ + return kmalloc(*size, GFP_KERNEL); } EXPORT_SYMBOL_GPL(add_mtd_device); diff --git a/fs/jffs2/scan.c b/fs/jffs2/scan.c index e393213..8d8cd34 100644 --- a/fs/jffs2/scan.c +++ b/fs/jffs2/scan.c @@ -120,7 +120,7 @@ int jffs2_scan_medium(struct jffs2_sb_info *c) D1(printk(KERN_DEBUG "Trying to allocate readbuf of %zu " "bytes\n", try_size)); - flashbuf = mtd_kmalloc_up_to(&try_size); + flashbuf = mtd_kmalloc_up_to(c->mtd, &try_size); if (!flashbuf) return -ENOMEM; diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h index a5d31ba..06b489a 100644 --- a/include/linux/mtd/mtd.h +++ b/include/linux/mtd/mtd.h @@ -348,7 +348,7 @@ 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); +void *mtd_kmalloc_up_to(const struct mtd_info *mtd, size_t *size); #ifdef CONFIG_MTD_PARTITIONS void mtd_erase_callback(struct erase_info *instr);