From patchwork Tue Apr 26 08:37:23 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Artem Bityutskiy X-Patchwork-Id: 92892 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from bombadil.infradead.org (bombadil.infradead.org [18.85.46.34]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 64956B6F14 for ; Tue, 26 Apr 2011 18:42:54 +1000 (EST) Received: from canuck.infradead.org ([2001:4978:20e::1]) by bombadil.infradead.org with esmtps (Exim 4.72 #1 (Red Hat Linux)) id 1QEdpR-0001eL-JB; Tue, 26 Apr 2011 08:41:05 +0000 Received: from localhost ([127.0.0.1] helo=canuck.infradead.org) by canuck.infradead.org with esmtp (Exim 4.72 #1 (Red Hat Linux)) id 1QEdpP-0007c4-Pl; Tue, 26 Apr 2011 08:41:03 +0000 Received: from mail-ey0-f177.google.com ([209.85.215.177]) by canuck.infradead.org with esmtps (Exim 4.72 #1 (Red Hat Linux)) id 1QEdpL-0007bl-Dj for linux-mtd@lists.infradead.org; Tue, 26 Apr 2011 08:41:00 +0000 Received: by eyh6 with SMTP id 6so130684eyh.36 for ; Tue, 26 Apr 2011 01:40:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:subject:from:reply-to:to:cc:in-reply-to :references:content-type:date:message-id:mime-version:x-mailer :content-transfer-encoding; bh=tERauotAiLPX1JE4dZpAPzbQvrdhvHCqvmD8WVICcP0=; b=Oyo3Z7oD9Yme4eDcDRnG6vzFNdVV5ppuQmhZuAYHYB+YWTOgbT8YR6NSMe0+oxRH/G MbfuIdm8GK9tNvZWPEvXwRHCWHAqgmt8ya7mW0KsN5e9En6VNi9GdNYFw/BqqFiPLpR9 qgdnjjq+SfbbbF8iUpJrdzKmoNpxwCKgcwYZA= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=subject:from:reply-to:to:cc:in-reply-to:references:content-type :date:message-id:mime-version:x-mailer:content-transfer-encoding; b=T/8ziLrGIHlJN4qxc9WmhXdoMSruDL/Dok1dRDFy1y+v2klE5XXWDr2WdQwiRY6yyd fS0nQE3ooURxcz/ltu6eB1Re0Hsw1WwqtHqhuwkZnrdJ7sGcYwgBMQxJ3st1TK5113mh IpNPiHdKLzhcx8IUKGPco9QH2gU2n2sA+YPTc= Received: by 10.14.122.205 with SMTP id t53mr211894eeh.106.1303807257506; Tue, 26 Apr 2011 01:40:57 -0700 (PDT) Received: from ?IPv6:::1? (shutemov.name [188.40.19.243]) by mx.google.com with ESMTPS id u1sm4630962eeh.13.2011.04.26.01.40.56 (version=SSLv3 cipher=OTHER); Tue, 26 Apr 2011 01:40:57 -0700 (PDT) Subject: Re: ubiformat: libmt error (side effect from last Large Buffer Allocations patch) From: Artem Bityutskiy To: Stefano Babic In-Reply-To: <4DB66CA3.2040906@denx.de> References: <4DB66CA3.2040906@denx.de> Date: Tue, 26 Apr 2011 11:37:23 +0300 Message-ID: <1303807043.2778.45.camel@localhost> Mime-Version: 1.0 X-Mailer: Evolution 2.32.2 (2.32.2-1.fc14) X-CRM114-Version: 20090807-BlameThorstenAndJenny ( TRE 0.7.6 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20110426_044059_792215_4F1B7787 X-CRM114-Status: GOOD ( 28.68 ) X-Spam-Score: 1.4 (+) X-Spam-Report: SpamAssassin version 3.3.1 on canuck.infradead.org summary: Content analysis details: (1.4 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/, low trust [209.85.215.177 listed in list.dnswl.org] 0.0 FREEMAIL_FROM Sender email is freemail (dedekind1[at]gmail.com) 2.2 FREEMAIL_ENVFROM_END_DIGIT Envelope-from freemail username ends in digit (dedekind1[at]gmail.com) -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature 0.0 RFC_ABUSE_POST Both abuse and postmaster missing on sender domain Cc: Grant Erickson , linux-mtd@lists.infradead.org, Bastian Ruppert X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.12 Precedence: list Reply-To: dedekind1@gmail.com List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-mtd-bounces@lists.infradead.org Errors-To: linux-mtd-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org Hi, On Tue, 2011-04-26 at 08:56 +0200, Stefano Babic wrote: > Hi, > > 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 Date: Tue, 26 Apr 2011 11:42:10 +0300 Subject: [PATCH] mtd: a fix candidate for mtd_kmalloc_up_to This is a fix candidate for the problem reported here: http://lists.infradead.org/pipermail/linux-mtd/2011-April/035190.html If it is OK, we'll need to fold it to Grant's patches and add credits to Stefano Babic there, probably in form of a "Tested-by:" tag. Signed-off-by: Artem Bityutskiy Tested-by: Stefano Babic --- drivers/mtd/mtdchar.c | 4 ++-- drivers/mtd/mtdcore.c | 28 ++++++++++++++++++---------- fs/jffs2/scan.c | 2 +- include/linux/mtd/mtd.h | 2 +- 4 files changed, 22 insertions(+), 14 deletions(-) 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);