2009-12-02 06:49:57

by Artem Bityutskiy

[permalink] [raw]
Subject: [PATCH] UBI: flush wl before clearing update marker

From: Sebastian Andrzej Siewior <[email protected]>

ubiupdatevol -t does the following:
- ubi_start_update()
- set_update_marker()
- for all LEBs ubi_eba_unmap_leb()
- clear_update_marker()
- ubi_wl_flush()

ubi_wl_flush() physically erases all PEB, once it returns all PEBs are
empty. clear_update_marker() has the update marker written after return.
If there is a power cut between the last two functions then the UBI
volume has no longer the "update" marker set and may have some valid
LEBs while some of them may be gone.
If that volume in question happens to be a UBIFS volume, then mount
will fail with

|UBIFS error (pid 1361): ubifs_read_node: bad node type (255 but expected 6)
|UBIFS error (pid 1361): ubifs_read_node: bad node at LEB 0:0
|Not a node, first 24 bytes:
|00000000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff

if there is at least one valid LEB and the wear-leveling worker managed
to clear LEB 0.

The patch waits for the wl worker to finish prior clearing the "update"
marker on flash. The two new LEB which are scheduled for erasing after
clear_update_marker() should not matter because they are only visible to
UBI.

Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
Signed-off-by: Artem Bityutskiy <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
drivers/mtd/ubi/upd.c | 20 +++++++++++---------
1 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/mtd/ubi/upd.c b/drivers/mtd/ubi/upd.c
index 74fdc40..c1d7b88 100644
--- a/drivers/mtd/ubi/upd.c
+++ b/drivers/mtd/ubi/upd.c
@@ -147,12 +147,14 @@ int ubi_start_update(struct ubi_device *ubi, struct ubi_volume *vol,
}

if (bytes == 0) {
+ err = ubi_wl_flush(ubi);
+ if (err)
+ return err;
+
err = clear_update_marker(ubi, vol, 0);
if (err)
return err;
- err = ubi_wl_flush(ubi);
- if (!err)
- vol->updating = 0;
+ vol->updating = 0;
}

vol->upd_buf = vmalloc(ubi->leb_size);
@@ -362,16 +364,16 @@ int ubi_more_update_data(struct ubi_device *ubi, struct ubi_volume *vol,

ubi_assert(vol->upd_received <= vol->upd_bytes);
if (vol->upd_received == vol->upd_bytes) {
+ err = ubi_wl_flush(ubi);
+ if (err)
+ return err;
/* The update is finished, clear the update marker */
err = clear_update_marker(ubi, vol, vol->upd_bytes);
if (err)
return err;
- err = ubi_wl_flush(ubi);
- if (err == 0) {
- vol->updating = 0;
- err = to_write;
- vfree(vol->upd_buf);
- }
+ vol->updating = 0;
+ err = to_write;
+ vfree(vol->upd_buf);
}

return err;
--
1.6.2.5


2009-12-02 15:45:37

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] UBI: flush wl before clearing update marker

On Wed, Dec 02, 2009 at 08:48:43AM +0200, Artem Bityutskiy wrote:
> From: Sebastian Andrzej Siewior <[email protected]>
>
> ubiupdatevol -t does the following:
> - ubi_start_update()
> - set_update_marker()
> - for all LEBs ubi_eba_unmap_leb()
> - clear_update_marker()
> - ubi_wl_flush()
>
> ubi_wl_flush() physically erases all PEB, once it returns all PEBs are
> empty. clear_update_marker() has the update marker written after return.
> If there is a power cut between the last two functions then the UBI
> volume has no longer the "update" marker set and may have some valid
> LEBs while some of them may be gone.
> If that volume in question happens to be a UBIFS volume, then mount
> will fail with
>
> |UBIFS error (pid 1361): ubifs_read_node: bad node type (255 but expected 6)
> |UBIFS error (pid 1361): ubifs_read_node: bad node at LEB 0:0
> |Not a node, first 24 bytes:
> |00000000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>
> if there is at least one valid LEB and the wear-leveling worker managed
> to clear LEB 0.
>
> The patch waits for the wl worker to finish prior clearing the "update"
> marker on flash. The two new LEB which are scheduled for erasing after
> clear_update_marker() should not matter because they are only visible to
> UBI.
>
> Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
> Signed-off-by: Artem Bityutskiy <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> ---
> drivers/mtd/ubi/upd.c | 20 +++++++++++---------


I'm not the mtd or ubi maintainer, so why did you send this to me?

confused,

greg k-h

2009-12-03 07:02:48

by Artem Bityutskiy

[permalink] [raw]
Subject: Re: [PATCH] UBI: flush wl before clearing update marker

On Wed, 2009-12-02 at 16:42 +0100, ext Greg KH wrote:
> On Wed, Dec 02, 2009 at 08:48:43AM +0200, Artem Bityutskiy wrote:
> > From: Sebastian Andrzej Siewior <[email protected]>
> >
> > ubiupdatevol -t does the following:
> > - ubi_start_update()
> > - set_update_marker()
> > - for all LEBs ubi_eba_unmap_leb()
> > - clear_update_marker()
> > - ubi_wl_flush()
> >
> > ubi_wl_flush() physically erases all PEB, once it returns all PEBs are
> > empty. clear_update_marker() has the update marker written after return.
> > If there is a power cut between the last two functions then the UBI
> > volume has no longer the "update" marker set and may have some valid
> > LEBs while some of them may be gone.
> > If that volume in question happens to be a UBIFS volume, then mount
> > will fail with
> >
> > |UBIFS error (pid 1361): ubifs_read_node: bad node type (255 but expected 6)
> > |UBIFS error (pid 1361): ubifs_read_node: bad node at LEB 0:0
> > |Not a node, first 24 bytes:
> > |00000000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> >
> > if there is at least one valid LEB and the wear-leveling worker managed
> > to clear LEB 0.
> >
> > The patch waits for the wl worker to finish prior clearing the "update"
> > marker on flash. The two new LEB which are scheduled for erasing after
> > clear_update_marker() should not matter because they are only visible to
> > UBI.
> >
> > Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
> > Signed-off-by: Artem Bityutskiy <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > ---
> > drivers/mtd/ubi/upd.c | 20 +++++++++++---------
>
>
> I'm not the mtd or ubi maintainer, so why did you send this to me?

Sorry, I thought the protocol to get to -stable is to send to you and CC
stable. I maintain UBI and just wanted to send this patch to -stable. I
googled a bit and could not find the right way.

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

2009-12-04 02:00:30

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] UBI: flush wl before clearing update marker

On Thu, Dec 03, 2009 at 09:01:45AM +0200, Artem Bityutskiy wrote:
> On Wed, 2009-12-02 at 16:42 +0100, ext Greg KH wrote:
> > On Wed, Dec 02, 2009 at 08:48:43AM +0200, Artem Bityutskiy wrote:
> > > From: Sebastian Andrzej Siewior <[email protected]>
> > >
> > > ubiupdatevol -t does the following:
> > > - ubi_start_update()
> > > - set_update_marker()
> > > - for all LEBs ubi_eba_unmap_leb()
> > > - clear_update_marker()
> > > - ubi_wl_flush()
> > >
> > > ubi_wl_flush() physically erases all PEB, once it returns all PEBs are
> > > empty. clear_update_marker() has the update marker written after return.
> > > If there is a power cut between the last two functions then the UBI
> > > volume has no longer the "update" marker set and may have some valid
> > > LEBs while some of them may be gone.
> > > If that volume in question happens to be a UBIFS volume, then mount
> > > will fail with
> > >
> > > |UBIFS error (pid 1361): ubifs_read_node: bad node type (255 but expected 6)
> > > |UBIFS error (pid 1361): ubifs_read_node: bad node at LEB 0:0
> > > |Not a node, first 24 bytes:
> > > |00000000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> > >
> > > if there is at least one valid LEB and the wear-leveling worker managed
> > > to clear LEB 0.
> > >
> > > The patch waits for the wl worker to finish prior clearing the "update"
> > > marker on flash. The two new LEB which are scheduled for erasing after
> > > clear_update_marker() should not matter because they are only visible to
> > > UBI.
> > >
> > > Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
> > > Signed-off-by: Artem Bityutskiy <[email protected]>
> > > Cc: [email protected]
> > > Cc: [email protected]
> > > ---
> > > drivers/mtd/ubi/upd.c | 20 +++++++++++---------
> >
> >
> > I'm not the mtd or ubi maintainer, so why did you send this to me?
>
> Sorry, I thought the protocol to get to -stable is to send to you and CC
> stable. I maintain UBI and just wanted to send this patch to -stable. I
> googled a bit and could not find the right way.

Look in the Documentation/stable_kernel_rules.txt that shows all you
need is the Cc: [email protected] section in the Signed-off-by area like
you did.

So when this goes into Linus's tree, I'll automatically get it in the
right inbox and know to apply it to the -stable tree.

thanks,

greg k-h

2009-12-04 05:43:20

by Artem Bityutskiy

[permalink] [raw]
Subject: Re: [PATCH] UBI: flush wl before clearing update marker

On Fri, 2009-12-04 at 02:58 +0100, ext Greg KH wrote:
> On Thu, Dec 03, 2009 at 09:01:45AM +0200, Artem Bityutskiy wrote:
> > On Wed, 2009-12-02 at 16:42 +0100, ext Greg KH wrote:
> > > On Wed, Dec 02, 2009 at 08:48:43AM +0200, Artem Bityutskiy wrote:
> > > > From: Sebastian Andrzej Siewior <[email protected]>
> > > >
> > > > ubiupdatevol -t does the following:
> > > > - ubi_start_update()
> > > > - set_update_marker()
> > > > - for all LEBs ubi_eba_unmap_leb()
> > > > - clear_update_marker()
> > > > - ubi_wl_flush()
> > > >
> > > > ubi_wl_flush() physically erases all PEB, once it returns all PEBs are
> > > > empty. clear_update_marker() has the update marker written after return.
> > > > If there is a power cut between the last two functions then the UBI
> > > > volume has no longer the "update" marker set and may have some valid
> > > > LEBs while some of them may be gone.
> > > > If that volume in question happens to be a UBIFS volume, then mount
> > > > will fail with
> > > >
> > > > |UBIFS error (pid 1361): ubifs_read_node: bad node type (255 but expected 6)
> > > > |UBIFS error (pid 1361): ubifs_read_node: bad node at LEB 0:0
> > > > |Not a node, first 24 bytes:
> > > > |00000000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> > > >
> > > > if there is at least one valid LEB and the wear-leveling worker managed
> > > > to clear LEB 0.
> > > >
> > > > The patch waits for the wl worker to finish prior clearing the "update"
> > > > marker on flash. The two new LEB which are scheduled for erasing after
> > > > clear_update_marker() should not matter because they are only visible to
> > > > UBI.
> > > >
> > > > Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
> > > > Signed-off-by: Artem Bityutskiy <[email protected]>
> > > > Cc: [email protected]
> > > > Cc: [email protected]
> > > > ---
> > > > drivers/mtd/ubi/upd.c | 20 +++++++++++---------
> > >
> > >
> > > I'm not the mtd or ubi maintainer, so why did you send this to me?
> >
> > Sorry, I thought the protocol to get to -stable is to send to you and CC
> > stable. I maintain UBI and just wanted to send this patch to -stable. I
> > googled a bit and could not find the right way.
>
> Look in the Documentation/stable_kernel_rules.txt that shows all you
> need is the Cc: [email protected] section in the Signed-off-by area like
> you did.
>
> So when this goes into Linus's tree, I'll automatically get it in the
> right inbox and know to apply it to the -stable tree.

Oh, thanks a lot!

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

Subject: [PATCH] Doc/stable rules: add new cherry-pick logic

- it is possible to submit patches for the stable queue without sending
them directly [email protected]. If the tag (Cc: [email protected]) is
available in the sign-off area then hpa's script will filter them into
the stable mailbox once it hits Linus' tree.
- Patches which require others to be applied first can be also specified.
This was discussued in http://lkml.org/lkml/2009/11/9/474

Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
---
Greg, the cherry pick format got acked by stable team didn't it?

Documentation/stable_kernel_rules.txt | 22 +++++++++++++++++++++-
1 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/Documentation/stable_kernel_rules.txt b/Documentation/stable_kernel_rules.txt
index a452227..8c0aaad 100644
--- a/Documentation/stable_kernel_rules.txt
+++ b/Documentation/stable_kernel_rules.txt
@@ -25,7 +25,27 @@ Rules on what kind of patches are accepted, and which ones are not, into the
Procedure for submitting patches to the -stable tree:

- Send the patch, after verifying that it follows the above rules, to
- [email protected].
+ [email protected]. An alternative is to have the tag
+ Cc: [email protected]
+ in the sign-off area. Once the patch is merged it will be filtered
+ into the stable tree even without sending directly to
+ [email protected]
+ - If the patch requires other patches as prerequisites which can be
+ cherry-picked then this can be specified in the following format in
+ the sign-off area:
+
+ Cc: <[email protected]> # .32.x: a1f84a3: sched: Check for an idle shared cache
+ Cc: <[email protected]> # .32.x: 1b9508f: sched: Rate-limit newidle
+ Cc: <[email protected]> # .32.x: fd21073: sched: Fix affinity logic
+ Cc: <[email protected]> # .32.x
+ Signed-off-by: Ingo Molnar <[email protected]>
+
+ The tag sequence has the meaning of:
+ git cherry-pick a1f84a3
+ git cherry-pick 1b9508f
+ git cherry-pick fd21073
+ git cherry-pick <this commit>
+
- The sender will receive an ACK when the patch has been accepted into the
queue, or a NAK if the patch is rejected. This response might take a few
days, according to the developer's schedules.
--
1.6.5.2

2009-12-04 16:41:31

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] Doc/stable rules: add new cherry-pick logic

On Fri, Dec 04, 2009 at 12:16:35PM +0100, Sebastian Andrzej Siewior wrote:
> - it is possible to submit patches for the stable queue without sending
> them directly [email protected]. If the tag (Cc: [email protected]) is
> available in the sign-off area then hpa's script will filter them into
> the stable mailbox once it hits Linus' tree.
> - Patches which require others to be applied first can be also specified.
> This was discussued in http://lkml.org/lkml/2009/11/9/474
>
> Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
> ---
> Greg, the cherry pick format got acked by stable team didn't it?
>
> Documentation/stable_kernel_rules.txt | 22 +++++++++++++++++++++-
> 1 files changed, 21 insertions(+), 1 deletions(-)
>
> diff --git a/Documentation/stable_kernel_rules.txt b/Documentation/stable_kernel_rules.txt
> index a452227..8c0aaad 100644
> --- a/Documentation/stable_kernel_rules.txt
> +++ b/Documentation/stable_kernel_rules.txt
> @@ -25,7 +25,27 @@ Rules on what kind of patches are accepted, and which ones are not, into the
> Procedure for submitting patches to the -stable tree:
>
> - Send the patch, after verifying that it follows the above rules, to
> - [email protected].
> + [email protected]. An alternative is to have the tag
> + Cc: [email protected]
> + in the sign-off area. Once the patch is merged it will be filtered
> + into the stable tree even without sending directly to
> + [email protected]

This is already documented (somewhat poorly) in the paragraph a few
bullets below this one that says:
- If the [email protected] address is added to a patch, when it
goes into Linus's tree it will automatically be emailed to
the stable team.

Can you delete this paragraph, as your documentation is much nicer and
makes more sense?

Care to redo this patch that way?

thanks,

greg k-h

Subject: [PATCH v2] Doc/stable rules: add new cherry-pick logic

- it is possible to submit patches for the stable queue without sending
them directly [email protected]. If the tag (Cc: [email protected]) is
available in the sign-off area than hpa's script will filter them into
the stable mailbox once it hits Linus' tree.
- Patches which require others to be applied first can be also specified.
This was discussued in http://lkml.org/lkml/2009/11/9/474

Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
---
Documentation/stable_kernel_rules.txt | 24 +++++++++++++++++++++---
1 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/Documentation/stable_kernel_rules.txt b/Documentation/stable_kernel_rules.txt
index a452227..bc52bbb 100644
--- a/Documentation/stable_kernel_rules.txt
+++ b/Documentation/stable_kernel_rules.txt
@@ -25,14 +25,32 @@ Rules on what kind of patches are accepted, and which ones are not, into the
Procedure for submitting patches to the -stable tree:

- Send the patch, after verifying that it follows the above rules, to
- [email protected].
+ [email protected]. An alternative is to have the tag
+ Cc: [email protected]
+ in the sign-off area. Once the patch is merged it will be filtered
+ into the stable tree even without sending directly to
+ [email protected]
+ - If the patch requires other patches as prerequisites which can be
+ cherry-picked than this can be specified in the following format in
+ the sign-off area:
+
+ Cc: <[email protected]> # .32.x: a1f84a3: sched: Check for an idle shared cache
+ Cc: <[email protected]> # .32.x: 1b9508f: sched: Rate-limit newidle
+ Cc: <[email protected]> # .32.x: fd21073: sched: Fix affinity logic
+ Cc: <[email protected]> # .32.x
+ Signed-off-by: Ingo Molnar <[email protected]>
+
+ The tag sequence has the meaning of:
+ git cherry-pick a1f84a3
+ git cherry-pick 1b9508f
+ git cherry-pick fd21073
+ git cherry-pick <this commit>
+
- The sender will receive an ACK when the patch has been accepted into the
queue, or a NAK if the patch is rejected. This response might take a few
days, according to the developer's schedules.
- If accepted, the patch will be added to the -stable queue, for review by
other developers and by the relevant subsystem maintainer.
- - If the [email protected] address is added to a patch, when it goes into
- Linus's tree it will automatically be emailed to the stable team.
- Security patches should not be sent to this alias, but instead to the
documented [email protected] address.

--
1.6.5.2