[PATCH] UBI: flush wl before clearing update marker

[PATCH] UBI: flush wl before clearing update marker

[Artem Bityutskiy]

From: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>

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 <sebastian@breakpoint.cc>
Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
Cc: stable@kernel.org
Cc: linux-kernel@vger.kernel.org
---
 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

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

[Greg KH]

On Wed, Dec 02, 2009 at 08:48:43AM +0200, Artem Bityutskiy wrote:
> From: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
> 
> 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 <sebastian@breakpoint.cc>
> Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
> Cc: stable@kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  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

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

[Artem Bityutskiy]

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 <sebastian@breakpoint.cc>
> > 
> > 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 <sebastian@breakpoint.cc>
> > Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
> > Cc: stable@kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > ---
> >  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 (Артём Битюцкий)

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

[Greg KH]

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 <sebastian@breakpoint.cc>
> > > 
> > > 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 <sebastian@breakpoint.cc>
> > > Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
> > > Cc: stable@kernel.org
> > > Cc: linux-kernel@vger.kernel.org
> > > ---
> > >  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: stable@kernel.org 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

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

[Artem Bityutskiy]

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 <sebastian@breakpoint.cc>
> > > > 
> > > > 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 <sebastian@breakpoint.cc>
> > > > Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
> > > > Cc: stable@kernel.org
> > > > Cc: linux-kernel@vger.kernel.org
> > > > ---
> > > >  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: stable@kernel.org 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 (Артём Битюцкий)

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

[Sebastian Andrzej Siewior]

- it is possible to submit patches for the stable queue without sending
  them directly stable@kernel.org. If the tag (Cc: stable@kernel.org) 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 <sebastian@breakpoint.cc>
---
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
-   stable@kernel.org.
+   stable@kernel.org. An alternative is to have the tag
+    Cc: stable@kernel.org
+   in the sign-off area. Once the patch is merged it will be filtered
+   into the stable tree even without sending directly to
+   stable@kernel.org
+ - 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: <stable@kernel.org> # .32.x: a1f84a3: sched: Check for an idle shared cache
+    Cc: <stable@kernel.org> # .32.x: 1b9508f: sched: Rate-limit newidle
+    Cc: <stable@kernel.org> # .32.x: fd21073: sched: Fix affinity logic
+    Cc: <stable@kernel.org> # .32.x
+    Signed-off-by: Ingo Molnar <mingo@elte.hu>
+
+   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

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

[Greg KH]

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 stable@kernel.org. If the tag (Cc: stable@kernel.org) 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 <sebastian@breakpoint.cc>
> ---
> 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
> -   stable@kernel.org.
> +   stable@kernel.org. An alternative is to have the tag
> +    Cc: stable@kernel.org
> +   in the sign-off area. Once the patch is merged it will be filtered
> +   into the stable tree even without sending directly to
> +   stable@kernel.org

This is already documented (somewhat poorly) in the paragraph a few
bullets below this one that says:
	 - If the stable@kernel.org 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

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

[Sebastian Andrzej Siewior]

- it is possible to submit patches for the stable queue without sending
  them directly stable@kernel.org. If the tag (Cc: stable@kernel.org) 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 <sebastian@breakpoint.cc>
---
 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
-   stable@kernel.org.
+   stable@kernel.org. An alternative is to have the tag
+    Cc: stable@kernel.org
+   in the sign-off area. Once the patch is merged it will be filtered
+   into the stable tree even without sending directly to
+   stable@kernel.org
+ - 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: <stable@kernel.org> # .32.x: a1f84a3: sched: Check for an idle shared cache
+    Cc: <stable@kernel.org> # .32.x: 1b9508f: sched: Rate-limit newidle
+    Cc: <stable@kernel.org> # .32.x: fd21073: sched: Fix affinity logic
+    Cc: <stable@kernel.org> # .32.x
+    Signed-off-by: Ingo Molnar <mingo@elte.hu>
+
+   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 stable@kernel.org 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 security@kernel.org address.
 
-- 
1.6.5.2