public inbox for linux-mtd@lists.infradead.org
 help / color / mirror / Atom feed
From: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
To: Artem Bityutskiy <dedekind1@gmail.com>
Cc: Richard Weinberger <richard@nod.at>, linux-mtd@lists.infradead.org
Subject: Re: [PATCH v2 for v3.15 0/3] UBI: block: Support very large volumes
Date: Fri, 25 Jul 2014 20:10:01 -0300	[thread overview]
Message-ID: <20140725231001.GA29798@arch.cereza> (raw)
In-Reply-To: <20140527123017.GA1655@arch.cereza>

Hi Artem,

On 27 May 09:30 AM, Ezequiel Garcia wrote:
> On 27 May 02:02 PM, Artem Bityutskiy wrote:
> > On Mon, 2014-05-05 at 07:11 -0300, Ezequiel Garcia wrote:
> > > This bug was reported on #mtd IRC. I don't have to reporter name to give proper
> > > credit. It should be applied as a fix for v3.15.
> > 
> > I've taken them, thanks.
> > 
> > However, I do not think they are important enough to send to 3.15. The
> > first patch, may be, but I doubt there is really a user suffering from
> > that. The second is just an optimization. The third is a fix, but I am
> > not sure if anyone uses this driver with such a huge MTD device. So I
> > think 3.16 should be all-right, how does this sound to you?
> > 
> 
> Sure, I'm fine with that.
> 

I think this series got lost :-(

It's not in today's -next, and I'm looking at your pull for v3.16 and it's
not there either. It seems I overlooked it, and realised just now, just before
sending a new fix.

Any idea what happened?

Anyway, I have found a new issue with ubiblock and I think this may be suitable
for -stable. Can you take a quick look at the patch below?

If the fix below is suitable for -stable, then we should apply this one now
and then rebase the large volume support on top of it, to be applied for v3.17.

>From 72b390aa14c6d2c81dfd6f3940ceed1067b9ae15 Mon Sep 17 00:00:00 2001
From: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
Date: Fri, 25 Jul 2014 19:37:03 -0300
Subject: [PATCH] UBI: block: Fix block device size set

We are currently taking the block device size from the ubi_volume_info.size
field. However, this is not the amount of data in the volume, but the
number of reserved physical eraseblocks, and hence leads to an incorrect
representation of the volume.

For static volumes, this leads to I/O errors as the block interface may
attempt to read unmapped PEBs:

$ cat /dev/ubiblock0_0 > /dev/null
UBI error: ubiblock_read_to_buf: ubiblock0_0 ubi_read error -22
end_request: I/O error, dev ubiblock0_0, sector 9536
Buffer I/O error on device ubiblock0_0, logical block 2384
[snip]

Fix this by using the ubi_volume_info.used_bytes field which is set to the
actual number of data bytes for both static and dynamic volumes.
While here, improve the error message to be less stupid and more useful:

UBI error: ubiblock_read_to_buf: ubiblock0_1 ubi_read error -9 on LEB=0, off=15872, len=512

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/mtd/ubi/block.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/ubi/block.c b/drivers/mtd/ubi/block.c
index 8457df7..295a234 100644
--- a/drivers/mtd/ubi/block.c
+++ b/drivers/mtd/ubi/block.c
@@ -188,8 +188,8 @@ static int ubiblock_read_to_buf(struct ubiblock *dev, char *buffer,
 
 	ret = ubi_read(dev->desc, leb, buffer, offset, len);
 	if (ret) {
-		ubi_err("%s ubi_read error %d",
-			dev->gd->disk_name, ret);
+		ubi_err("%s ubi_read error %d on LEB=%d, off=%d, len=%d",
+			dev->gd->disk_name, ret, leb, offset, len);
 		return ret;
 	}
 	return 0;
@@ -412,7 +412,7 @@ int ubiblock_create(struct ubi_volume_info *vi)
 	gd->first_minor = dev->ubi_num * UBI_MAX_VOLUMES + dev->vol_id;
 	gd->private_data = dev;
 	sprintf(gd->disk_name, "ubiblock%d_%d", dev->ubi_num, dev->vol_id);
-	disk_capacity = (vi->size * vi->usable_leb_size) >> 9;
+	disk_capacity = vi->used_bytes >> 9;
 	set_capacity(gd, disk_capacity);
 	dev->gd = gd;
 
@@ -516,9 +516,9 @@ static void ubiblock_resize(struct ubi_volume_info *vi)
 	}
 
 	mutex_lock(&dev->dev_mutex);
-	disk_capacity = (vi->size * vi->usable_leb_size) >> 9;
+	disk_capacity = vi->used_bytes >> 9;
 	set_capacity(dev->gd, disk_capacity);
-	ubi_msg("%s resized to %d LEBs", dev->gd->disk_name, vi->size);
+	ubi_msg("%s resized to %d bytes", dev->gd->disk_name, vi->used_bytes);
 	mutex_unlock(&dev->dev_mutex);
 	mutex_unlock(&devices_mutex);
 }
-- 
2.0.1

-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

  reply	other threads:[~2014-07-25 23:11 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-05 10:11 [PATCH v2 for v3.15 0/3] UBI: block: Support very large volumes Ezequiel Garcia
2014-05-05 10:11 ` [PATCH v2 1/3] UBI: block: Make ubiblock_resize return something Ezequiel Garcia
2014-05-05 10:11 ` [PATCH v2 2/3] UBI: block: Set disk_capacity out of the mutex Ezequiel Garcia
2014-05-05 10:11 ` [PATCH v2 3/3] UBI: block: Avoid disk size integer overflow Ezequiel Garcia
2014-05-21 16:20 ` [PATCH v2 for v3.15 0/3] UBI: block: Support very large volumes Ezequiel Garcia
2014-05-27 11:02 ` Artem Bityutskiy
2014-05-27 12:30   ` Ezequiel Garcia
2014-07-25 23:10     ` Ezequiel Garcia [this message]
2014-07-28 16:05       ` Artem Bityutskiy
2014-07-30 14:31         ` Ezequiel Garcia

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140725231001.GA29798@arch.cereza \
    --to=ezequiel.garcia@free-electrons.com \
    --cc=dedekind1@gmail.com \
    --cc=linux-mtd@lists.infradead.org \
    --cc=richard@nod.at \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox