* mtd-utils: ubiformat: writing images on flash with badblocks.
@ 2011-08-25 8:26 Anton Olofsson
2011-09-11 10:59 ` Artem Bityutskiy
0 siblings, 1 reply; 7+ messages in thread
From: Anton Olofsson @ 2011-08-25 8:26 UTC (permalink / raw)
To: linux-mtd
Hi!
We encountered some problems with ubiformat
and writing ubi-images onto flash with badblocks.
If a badblock was encountered during write ubiformat would
skip writing that “file-block” altogether, and this would eventually
result in EOF while trying to read the image file.
The quick fix was to rewind the filedescriptor for the next pass.
Im not sure if others have encountered this problem
or even if this is the correct way to handle this situation.
Here is the change made though:
--- a/ubi-utils/ubiformat.c
+++ b/ubi-utils/ubiformat.c
@@ -546,6 +546,11 @@ static int flash_image(libmtd_t libmtd, const
struct mtd_dev_info *mtd,
if (mark_bad(mtd, si, eb))
goto out_close;
}
+ /*rewind fd so next read_all(...) reads correct block*/
+ if (lseek(fd, -mtd->eb_size, SEEK_CUR) == -1) {
+ sys_errmsg("unable to rewind file");
+ goto out_close;
+ }
continue;
}
if (++written_ebs >= img_ebs)
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: mtd-utils: ubiformat: writing images on flash with badblocks.
2011-08-25 8:26 mtd-utils: ubiformat: writing images on flash with badblocks Anton Olofsson
@ 2011-09-11 10:59 ` Artem Bityutskiy
2011-09-12 6:41 ` Ricard Wanderlof
2011-09-13 9:33 ` Anton Olofsson
0 siblings, 2 replies; 7+ messages in thread
From: Artem Bityutskiy @ 2011-09-11 10:59 UTC (permalink / raw)
To: Anton Olofsson; +Cc: linux-mtd
On Thu, 2011-08-25 at 10:26 +0200, Anton Olofsson wrote:
> Hi!
>
> We encountered some problems with ubiformat
> and writing ubi-images onto flash with badblocks.
>
> If a badblock was encountered during write ubiformat would
> skip writing that “file-block” altogether, and this would eventually
> result in EOF while trying to read the image file.
>
> The quick fix was to rewind the filedescriptor for the next pass.
>
> Im not sure if others have encountered this problem
> or even if this is the correct way to handle this situation.
>
> Here is the change made though:
Hi, thanks for the fix, but I have few notes:
1. The patch does not have your Signed-off-by:
2. The patch is line-wrapped, so does not apply.
> --- a/ubi-utils/ubiformat.c
> +++ b/ubi-utils/ubiformat.c
> @@ -546,6 +546,11 @@ static int flash_image(libmtd_t libmtd, const
> struct mtd_dev_info *mtd,
> if (mark_bad(mtd, si, eb))
> goto out_close;
> }
> + /*rewind fd so next read_all(...) reads correct block*/
> + if (lseek(fd, -mtd->eb_size, SEEK_CUR) == -1) {
> + sys_errmsg("unable to rewind file");
> + goto out_close;
> + }
> continue;
But most importantly, I think this will break the case when the input
is stdout (pipe), not a file. So I quickly cooked the below patch
instead. I did not test it - would it be possible for you to give it a
test and let me know if it is ok, in which case I'll push it to the
mtd-utils repository.
>From 737ca8095332bf2d8110135de6e522fdb07a42df Mon Sep 17 00:00:00 2001
From: Artem Bityutskiy <artem.bityutskiy@intel.com>
Date: Sun, 11 Sep 2011 13:52:08 +0300
Subject: [PATCH] ubiformat: handle write errors correctly
This issue was reported and analyzed by
Anton Olofsson <anol.martinsson@gmail.com>:
when ubiformat encounters a write error while flashing the UBI image (which may
come from a file of from stdout), it correctly marks the faulty eraseblock as
bad and skips it. However, it also incorrectly drops the data buffer which was
supposed to be written, and reads next block of data.
This patch fixes this issue - in case of a write error, we preserve the current
data and write it to the next eraseblock, instead of dropping it.
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@intel.com>
---
ubi-utils/ubiformat.c | 22 ++++++++++++++++------
1 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/ubi-utils/ubiformat.c b/ubi-utils/ubiformat.c
index bfa1730..c396b09 100644
--- a/ubi-utils/ubiformat.c
+++ b/ubi-utils/ubiformat.c
@@ -442,7 +442,7 @@ static int mark_bad(const struct mtd_dev_info *mtd, struct ubi_scan_info *si, in
static int flash_image(libmtd_t libmtd, const struct mtd_dev_info *mtd,
const struct ubigen_info *ui, struct ubi_scan_info *si)
{
- int fd, img_ebs, eb, written_ebs = 0, divisor;
+ int fd, img_ebs, eb, written_ebs = 0, divisor, skip_data_read = 0;
off_t st_size;
fd = open_file(&st_size);
@@ -501,12 +501,15 @@ static int flash_image(libmtd_t libmtd, const struct mtd_dev_info *mtd,
continue;
}
- err = read_all(fd, buf, mtd->eb_size);
- if (err) {
- sys_errmsg("failed to read eraseblock %d from \"%s\"",
- written_ebs, args.image);
- goto out_close;
+ if (!skip_data_read) {
+ err = read_all(fd, buf, mtd->eb_size);
+ if (err) {
+ sys_errmsg("failed to read eraseblock %d from \"%s\"",
+ written_ebs, args.image);
+ goto out_close;
+ }
}
+ skip_data_read = 0;
if (args.override_ec)
ec = args.ec;
@@ -546,6 +549,13 @@ static int flash_image(libmtd_t libmtd, const struct mtd_dev_info *mtd,
if (mark_bad(mtd, si, eb))
goto out_close;
}
+
+ /*
+ * We have to make sure that we do not read next block
+ * of data from the input image or stdin - we have to
+ * write buf first instead.
+ */
+ skip_data_read = 1;
continue;
}
if (++written_ebs >= img_ebs)
--
1.7.6
--
Best Regards,
Artem Bityutskiy
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: mtd-utils: ubiformat: writing images on flash with badblocks.
2011-09-11 10:59 ` Artem Bityutskiy
@ 2011-09-12 6:41 ` Ricard Wanderlof
2011-09-12 9:33 ` Artem Bityutskiy
2011-09-13 9:33 ` Anton Olofsson
1 sibling, 1 reply; 7+ messages in thread
From: Ricard Wanderlof @ 2011-09-12 6:41 UTC (permalink / raw)
To: Artem Bityutskiy; +Cc: linux-mtd, Anton Olofsson
> On Sun, 11 Sep 2011, Artem Bityutskiy wrote:
>
> This issue was reported and analyzed by
> Anton Olofsson <anol.martinsson@gmail.com>:
>
> when ubiformat encounters a write error while flashing the UBI image (which may
> come from a file of from stdout), it correctly marks the faulty eraseblock as
> bad and skips it. However, it also incorrectly drops the data buffer which was
> supposed to be written, and reads next block of data.
I'm just curious, how come this has gone unnoticed for so long? One would
think that someone would have tried to flash an image to a chip with bad
blocks a long time ago?
/Ricard
--
Ricard Wolf Wanderlöf ricardw(at)axis.com
Axis Communications AB, Lund, Sweden www.axis.com
Phone +46 46 272 2016 Fax +46 46 13 61 30
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: mtd-utils: ubiformat: writing images on flash with badblocks.
2011-09-12 6:41 ` Ricard Wanderlof
@ 2011-09-12 9:33 ` Artem Bityutskiy
2011-09-12 9:40 ` Ricard Wanderlof
0 siblings, 1 reply; 7+ messages in thread
From: Artem Bityutskiy @ 2011-09-12 9:33 UTC (permalink / raw)
To: Ricard Wanderlof; +Cc: linux-mtd, Anton Olofsson
On Mon, 2011-09-12 at 08:41 +0200, Ricard Wanderlof wrote:
> > On Sun, 11 Sep 2011, Artem Bityutskiy wrote:
> >
> > This issue was reported and analyzed by
> > Anton Olofsson <anol.martinsson@gmail.com>:
> >
> > when ubiformat encounters a write error while flashing the UBI image (which may
> > come from a file of from stdout), it correctly marks the faulty eraseblock as
> > bad and skips it. However, it also incorrectly drops the data buffer which was
> > supposed to be written, and reads next block of data.
>
> I'm just curious, how come this has gone unnoticed for so long? One would
> think that someone would have tried to flash an image to a chip with bad
> blocks a long time ago?
No, bad blocks are handled fine. This is about the situation when an
eraseblock is good, then we write to it, and we get an error, then we
torture it, and the test fails, and we (ubiformat) mark it as bad. This
is very rare situation indeed.
--
Best Regards,
Artem Bityutskiy
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: mtd-utils: ubiformat: writing images on flash with badblocks.
2011-09-12 9:33 ` Artem Bityutskiy
@ 2011-09-12 9:40 ` Ricard Wanderlof
0 siblings, 0 replies; 7+ messages in thread
From: Ricard Wanderlof @ 2011-09-12 9:40 UTC (permalink / raw)
To: Artem Bityutskiy; +Cc: linux-mtd, Ricard Wanderlöf, Anton Olofsson
On Mon, 12 Sep 2011, Artem Bityutskiy wrote:
>>> when ubiformat encounters a write error while flashing the UBI image (which may
>>> come from a file of from stdout), it correctly marks the faulty eraseblock as
>>> bad and skips it. However, it also incorrectly drops the data buffer which was
>>> supposed to be written, and reads next block of data.
>>
>> I'm just curious, how come this has gone unnoticed for so long? One would
>> think that someone would have tried to flash an image to a chip with bad
>> blocks a long time ago?
>
> No, bad blocks are handled fine. This is about the situation when an
> eraseblock is good, then we write to it, and we get an error, then we
> torture it, and the test fails, and we (ubiformat) mark it as bad. This
> is very rare situation indeed.
Agreed. Sorry I missed the complete background.
/Ricard
--
Ricard Wolf Wanderlöf ricardw(at)axis.com
Axis Communications AB, Lund, Sweden www.axis.com
Phone +46 46 272 2016 Fax +46 46 13 61 30
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: mtd-utils: ubiformat: writing images on flash with badblocks.
2011-09-11 10:59 ` Artem Bityutskiy
2011-09-12 6:41 ` Ricard Wanderlof
@ 2011-09-13 9:33 ` Anton Olofsson
2011-09-19 4:53 ` Artem Bityutskiy
1 sibling, 1 reply; 7+ messages in thread
From: Anton Olofsson @ 2011-09-13 9:33 UTC (permalink / raw)
To: dedekind1; +Cc: linux-mtd
Hi,
Great patch!
As it turns out this issue was a symptom of our nand driver not working 100%.
Reverting to the faulty driver I was able to recreate the error and
check your patch.
As far as I can see this works as expected for both file and pipe input
and image is written correctly.
Best Regards
Anton Olofsson
On Sun, Sep 11, 2011 at 12:59 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> On Thu, 2011-08-25 at 10:26 +0200, Anton Olofsson wrote:
>> Hi!
>>
>> We encountered some problems with ubiformat
>> and writing ubi-images onto flash with badblocks.
>>
>> If a badblock was encountered during write ubiformat would
>> skip writing that “file-block” altogether, and this would eventually
>> result in EOF while trying to read the image file.
>>
>> The quick fix was to rewind the filedescriptor for the next pass.
>>
>> Im not sure if others have encountered this problem
>> or even if this is the correct way to handle this situation.
>>
>> Here is the change made though:
>
>
> Hi, thanks for the fix, but I have few notes:
>
> 1. The patch does not have your Signed-off-by:
> 2. The patch is line-wrapped, so does not apply.
>
>
>> --- a/ubi-utils/ubiformat.c
>> +++ b/ubi-utils/ubiformat.c
>> @@ -546,6 +546,11 @@ static int flash_image(libmtd_t libmtd, const
>> struct mtd_dev_info *mtd,
>> if (mark_bad(mtd, si, eb))
>> goto out_close;
>> }
>> + /*rewind fd so next read_all(...) reads correct block*/
>> + if (lseek(fd, -mtd->eb_size, SEEK_CUR) == -1) {
>> + sys_errmsg("unable to rewind file");
>> + goto out_close;
>> + }
>> continue;
>
> But most importantly, I think this will break the case when the input
> is stdout (pipe), not a file. So I quickly cooked the below patch
> instead. I did not test it - would it be possible for you to give it a
> test and let me know if it is ok, in which case I'll push it to the
> mtd-utils repository.
>
> From 737ca8095332bf2d8110135de6e522fdb07a42df Mon Sep 17 00:00:00 2001
> From: Artem Bityutskiy <artem.bityutskiy@intel.com>
> Date: Sun, 11 Sep 2011 13:52:08 +0300
> Subject: [PATCH] ubiformat: handle write errors correctly
>
> This issue was reported and analyzed by
> Anton Olofsson <anol.martinsson@gmail.com>:
>
> when ubiformat encounters a write error while flashing the UBI image (which may
> come from a file of from stdout), it correctly marks the faulty eraseblock as
> bad and skips it. However, it also incorrectly drops the data buffer which was
> supposed to be written, and reads next block of data.
>
> This patch fixes this issue - in case of a write error, we preserve the current
> data and write it to the next eraseblock, instead of dropping it.
>
> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@intel.com>
> ---
> ubi-utils/ubiformat.c | 22 ++++++++++++++++------
> 1 files changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/ubi-utils/ubiformat.c b/ubi-utils/ubiformat.c
> index bfa1730..c396b09 100644
> --- a/ubi-utils/ubiformat.c
> +++ b/ubi-utils/ubiformat.c
> @@ -442,7 +442,7 @@ static int mark_bad(const struct mtd_dev_info *mtd, struct ubi_scan_info *si, in
> static int flash_image(libmtd_t libmtd, const struct mtd_dev_info *mtd,
> const struct ubigen_info *ui, struct ubi_scan_info *si)
> {
> - int fd, img_ebs, eb, written_ebs = 0, divisor;
> + int fd, img_ebs, eb, written_ebs = 0, divisor, skip_data_read = 0;
> off_t st_size;
>
> fd = open_file(&st_size);
> @@ -501,12 +501,15 @@ static int flash_image(libmtd_t libmtd, const struct mtd_dev_info *mtd,
> continue;
> }
>
> - err = read_all(fd, buf, mtd->eb_size);
> - if (err) {
> - sys_errmsg("failed to read eraseblock %d from \"%s\"",
> - written_ebs, args.image);
> - goto out_close;
> + if (!skip_data_read) {
> + err = read_all(fd, buf, mtd->eb_size);
> + if (err) {
> + sys_errmsg("failed to read eraseblock %d from \"%s\"",
> + written_ebs, args.image);
> + goto out_close;
> + }
> }
> + skip_data_read = 0;
>
> if (args.override_ec)
> ec = args.ec;
> @@ -546,6 +549,13 @@ static int flash_image(libmtd_t libmtd, const struct mtd_dev_info *mtd,
> if (mark_bad(mtd, si, eb))
> goto out_close;
> }
> +
> + /*
> + * We have to make sure that we do not read next block
> + * of data from the input image or stdin - we have to
> + * write buf first instead.
> + */
> + skip_data_read = 1;
> continue;
> }
> if (++written_ebs >= img_ebs)
> --
> 1.7.6
>
> --
> Best Regards,
> Artem Bityutskiy
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: mtd-utils: ubiformat: writing images on flash with badblocks.
2011-09-13 9:33 ` Anton Olofsson
@ 2011-09-19 4:53 ` Artem Bityutskiy
0 siblings, 0 replies; 7+ messages in thread
From: Artem Bityutskiy @ 2011-09-19 4:53 UTC (permalink / raw)
To: Anton Olofsson; +Cc: linux-mtd
On Tue, 2011-09-13 at 11:33 +0200, Anton Olofsson wrote:
> As far as I can see this works as expected for both file and pipe input
> and image is written correctly.
Thanks! Pushed to mtd-utils.git.
--
Best Regards,
Artem Bityutskiy
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-09-19 4:51 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-25 8:26 mtd-utils: ubiformat: writing images on flash with badblocks Anton Olofsson
2011-09-11 10:59 ` Artem Bityutskiy
2011-09-12 6:41 ` Ricard Wanderlof
2011-09-12 9:33 ` Artem Bityutskiy
2011-09-12 9:40 ` Ricard Wanderlof
2011-09-13 9:33 ` Anton Olofsson
2011-09-19 4:53 ` Artem Bityutskiy
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.