Patchwork [GIT,PULL] logfs: bug fixes

login
register
mail settings
Submitter Artem Bityutskiy
Date Feb. 1, 2012, 12:54 p.m.
Message ID <1328100867.28171.88.camel@sauron.fi.intel.com>
Download mbox | patch
Permalink /patch/138934/
State New
Headers show

Comments

Artem Bityutskiy - Feb. 1, 2012, 12:54 p.m.
From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Date: Wed, 1 Feb 2012 11:50:07 +0200
Subject: [PATCH] mtd: fix merge conflict resolution breakage

This patch fixes merge conflict resolution breakage introduced by merge
commit 'd3712b9dfcf44ca145cf87e7f4096fa2d923471a'.

The commit changed 'mtd_can_have_bb()' function and made it always return
zero, which is incorrect. Instead, we need it to return whether the
underlying flash device can have bad eraseblocks or not. UBI needs this
information because it affects how it handles the underlying flash.
E.g., if the underlying flash is NOR, it cannot have bad blocks and any
write or erase error is fatal, and all we can do is to switch to R/O
mode. We do not need to reserve a pool of good eraseblocks for bad
eraseblocks handling, and so on.

This patch also removes 'mtd_can_have_bb()' invocations from Logfs
to ensure correct Logfs behavior.

I've tested that with this patch UBI works on top of NOR and NAND
flashes emulated by mtdram and nandsim correspondingly.

This patch is based on patch from Linus Torvalds.

Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---

Linus, here is the emergency fix - basically your patch tested and
and slightly modified. You should probably put yourself as the author.

Also attached the patch for your convenience.

 fs/logfs/dev_mtd.c      |    6 ------
 include/linux/mtd/mtd.h |    2 +-
 2 files changed, 1 insertions(+), 7 deletions(-)
Jörn Engel - Feb. 1, 2012, 2:07 p.m.
On Wed, 1 February 2012 14:54:27 +0200, Artem Bityutskiy wrote:
> 
> This patch fixes merge conflict resolution breakage introduced by merge
> commit 'd3712b9dfcf44ca145cf87e7f4096fa2d923471a'.
> 
> The commit changed 'mtd_can_have_bb()' function and made it always return
> zero, which is incorrect. Instead, we need it to return whether the
> underlying flash device can have bad eraseblocks or not. UBI needs this
> information because it affects how it handles the underlying flash.
> E.g., if the underlying flash is NOR, it cannot have bad blocks and any
> write or erase error is fatal, and all we can do is to switch to R/O
> mode. We do not need to reserve a pool of good eraseblocks for bad
> eraseblocks handling, and so on.
> 
> This patch also removes 'mtd_can_have_bb()' invocations from Logfs
> to ensure correct Logfs behavior.
> 
> I've tested that with this patch UBI works on top of NOR and NAND
> flashes emulated by mtdram and nandsim correspondingly.
> 
> This patch is based on patch from Linus Torvalds.
> 
> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

Looks good to me.
Acked-by: Joern Engel <joern@logfs.org>

Jörn
Prasad Joshi - Feb. 1, 2012, 5:26 p.m.
On Wed, Feb 1, 2012 at 6:24 PM, Artem Bityutskiy
<artem.bityutskiy@linux.intel.com> wrote:
> From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> Date: Wed, 1 Feb 2012 11:50:07 +0200
> Subject: [PATCH] mtd: fix merge conflict resolution breakage
>
> This patch fixes merge conflict resolution breakage introduced by merge
> commit 'd3712b9dfcf44ca145cf87e7f4096fa2d923471a'.
>
> The commit changed 'mtd_can_have_bb()' function and made it always return
> zero, which is incorrect. Instead, we need it to return whether the
> underlying flash device can have bad eraseblocks or not. UBI needs this
> information because it affects how it handles the underlying flash.
> E.g., if the underlying flash is NOR, it cannot have bad blocks and any
> write or erase error is fatal, and all we can do is to switch to R/O
> mode. We do not need to reserve a pool of good eraseblocks for bad
> eraseblocks handling, and so on.
>
> This patch also removes 'mtd_can_have_bb()' invocations from Logfs
> to ensure correct Logfs behavior.
>
> I've tested that with this patch UBI works on top of NOR and NAND
> flashes emulated by mtdram and nandsim correspondingly.
>
> This patch is based on patch from Linus Torvalds.
>
> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>


Acked-by: Prasad Joshi <prasadjoshi.linux@gmail.com>

> ---
>
> Linus, here is the emergency fix - basically your patch tested and
> and slightly modified. You should probably put yourself as the author.
>
> Also attached the patch for your convenience.
>
>  fs/logfs/dev_mtd.c      |    6 ------
>  include/linux/mtd/mtd.h |    2 +-
>  2 files changed, 1 insertions(+), 7 deletions(-)
>
> diff --git a/fs/logfs/dev_mtd.c b/fs/logfs/dev_mtd.c
> index e97404d..9c50144 100644
> --- a/fs/logfs/dev_mtd.c
> +++ b/fs/logfs/dev_mtd.c
> @@ -152,9 +152,6 @@ static struct page *logfs_mtd_find_first_sb(struct super_block *sb, u64 *ofs)
>        filler_t *filler = logfs_mtd_readpage;
>        struct mtd_info *mtd = super->s_mtd;
>
> -       if (!mtd_can_have_bb(mtd))
> -               return NULL;
> -
>        *ofs = 0;
>        while (mtd_block_isbad(mtd, *ofs)) {
>                *ofs += mtd->erasesize;
> @@ -172,9 +169,6 @@ static struct page *logfs_mtd_find_last_sb(struct super_block *sb, u64 *ofs)
>        filler_t *filler = logfs_mtd_readpage;
>        struct mtd_info *mtd = super->s_mtd;
>
> -       if (!mtd_can_have_bb(mtd))
> -               return NULL;
> -
>        *ofs = mtd->size - mtd->erasesize;
>        while (mtd_block_isbad(mtd, *ofs)) {
>                *ofs -= mtd->erasesize;
> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
> index c09020a..d43dc25 100644
> --- a/include/linux/mtd/mtd.h
> +++ b/include/linux/mtd/mtd.h
> @@ -487,7 +487,7 @@ static inline int mtd_has_oob(const struct mtd_info *mtd)
>
>  static inline int mtd_can_have_bb(const struct mtd_info *mtd)
>  {
> -       return 0;
> +       return !!mtd->block_isbad;
>  }
>
>        /* Kernel-side ioctl definitions */
> --
> 1.7.9
Brian Norris - Feb. 1, 2012, 7:02 p.m.
On Wed, Feb 1, 2012 at 4:54 AM, Artem Bityutskiy
<artem.bityutskiy@linux.intel.com> wrote:
> From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> Date: Wed, 1 Feb 2012 11:50:07 +0200
> Subject: [PATCH] mtd: fix merge conflict resolution breakage
>
> This patch fixes merge conflict resolution breakage introduced by merge
> commit 'd3712b9dfcf44ca145cf87e7f4096fa2d923471a'.
>
> The commit changed 'mtd_can_have_bb()' function and made it always return
> zero, which is incorrect. Instead, we need it to return whether the
> underlying flash device can have bad eraseblocks or not. UBI needs this
> information because it affects how it handles the underlying flash.
> E.g., if the underlying flash is NOR, it cannot have bad blocks and any
> write or erase error is fatal, and all we can do is to switch to R/O
> mode. We do not need to reserve a pool of good eraseblocks for bad
> eraseblocks handling, and so on.
>
> This patch also removes 'mtd_can_have_bb()' invocations from Logfs
> to ensure correct Logfs behavior.
>
> I've tested that with this patch UBI works on top of NOR and NAND
> flashes emulated by mtdram and nandsim correspondingly.
>
> This patch is based on patch from Linus Torvalds.
>
> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

This agrees with my suggestions.

Acked-by: Brian Norris <computersforpeace@gmail.com>

Patch

From 191b90d59cb97b83d5f979d46940d0bab2b02b14 Mon Sep 17 00:00:00 2001
From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Date: Wed, 1 Feb 2012 11:50:07 +0200
Subject: [PATCH] mtd: fix merge conflict resolution breakage

This patch fixes merge conflict resolution breakage introduced by merge
commit 'd3712b9dfcf44ca145cf87e7f4096fa2d923471a'.

The commit changed 'mtd_can_have_bb()' function and made it always return
zero, which is incorrect. Instead, we need it to return whether the
underlying flash device can have bad eraseblocks or not. UBI needs this
information because it affects how it handles the underlying flash.
E.g., if the underlying flash is NOR, it cannot have bad blocks and any
write or erase error is fatal, and all we can do is to switch to R/O
mode. We do not need to reserve a pool of good eraseblocks for bad
eraseblocks handling, and so on.

This patch also removes 'mtd_can_have_bb()' invocations from Logfs
to ensure correct Logfs behavior.

I've tested that with this patch UBI works on top of NOR and NAND
flashes emulated by mtdram and nandsim correspondingly.

This patch is based on patch from Linus Torvalds.

Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---

 fs/logfs/dev_mtd.c      |    6 ------
 include/linux/mtd/mtd.h |    2 +-
 2 files changed, 1 insertions(+), 7 deletions(-)

diff --git a/fs/logfs/dev_mtd.c b/fs/logfs/dev_mtd.c

index e97404d..9c50144 100644

--- a/fs/logfs/dev_mtd.c

+++ b/fs/logfs/dev_mtd.c

@@ -152,9 +152,6 @@  static struct page *logfs_mtd_find_first_sb(struct super_block *sb, u64 *ofs)

 	filler_t *filler = logfs_mtd_readpage;
 	struct mtd_info *mtd = super->s_mtd;
 
-	if (!mtd_can_have_bb(mtd))

-		return NULL;

-

 	*ofs = 0;
 	while (mtd_block_isbad(mtd, *ofs)) {
 		*ofs += mtd->erasesize;
@@ -172,9 +169,6 @@  static struct page *logfs_mtd_find_last_sb(struct super_block *sb, u64 *ofs)

 	filler_t *filler = logfs_mtd_readpage;
 	struct mtd_info *mtd = super->s_mtd;
 
-	if (!mtd_can_have_bb(mtd))

-		return NULL;

-

 	*ofs = mtd->size - mtd->erasesize;
 	while (mtd_block_isbad(mtd, *ofs)) {
 		*ofs -= mtd->erasesize;
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h

index c09020a..d43dc25 100644

--- a/include/linux/mtd/mtd.h

+++ b/include/linux/mtd/mtd.h

@@ -487,7 +487,7 @@  static inline int mtd_has_oob(const struct mtd_info *mtd)

 
 static inline int mtd_can_have_bb(const struct mtd_info *mtd)
 {
-	return 0;

+	return !!mtd->block_isbad;

 }
 
 	/* Kernel-side ioctl definitions */
-- 

1.7.9