[PATCH v2 3/3] md: convert to use the generic badblocks code

[Vishal Verma]

Retain badblocks as part of rdev, but use the accessor functions from
include/linux/badblocks for all manipulation.

Signed-off-by: Vishal Verma 
---
 drivers/md/md.c | 516 +++-
 drivers/md/md.h |  40 +
 2 files changed, 28 insertions(+), 528 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index c702de1..afdc3ea 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -34,6 +34,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -707,8 +708,7 @@ void md_rdev_clear(struct md_rdev *rdev)
put_page(rdev->bb_page);
rdev->bb_page = NULL;
}
-   kfree(rdev->badblocks.page);
-   rdev->badblocks.page = NULL;
+   badblocks_free(>badblocks);
 }
 EXPORT_SYMBOL_GPL(md_rdev_clear);
 
@@ -1358,8 +1358,6 @@ static __le32 calc_sb_1_csum(struct mdp_superblock_1 *sb)
return cpu_to_le32(csum);
 }
 
-static int md_set_badblocks(struct badblocks *bb, sector_t s, int sectors,
-   int acknowledged);
 static int super_1_load(struct md_rdev *rdev, struct md_rdev *refdev, int 
minor_version)
 {
struct mdp_superblock_1 *sb;
@@ -1484,8 +1482,7 @@ static int super_1_load(struct md_rdev *rdev, struct 
md_rdev *refdev, int minor_
count <<= sb->bblog_shift;
if (bb + 1 == 0)
break;
-   if (md_set_badblocks(>badblocks,
-sector, count, 1) == 0)
+   if (badblocks_set(>badblocks, sector, count, 1))
return -EINVAL;
}
} else if (sb->bblog_offset != 0)
@@ -2226,7 +2223,7 @@ repeat:
rdev_for_each(rdev, mddev) {
if (rdev->badblocks.changed) {
rdev->badblocks.changed = 0;
-   md_ack_all_badblocks(>badblocks);
+   ack_all_badblocks(>badblocks);
md_error(mddev, rdev);
}
clear_bit(Blocked, >flags);
@@ -2352,7 +2349,7 @@ repeat:
clear_bit(Blocked, >flags);
 
if (any_badblocks_changed)
-   md_ack_all_badblocks(>badblocks);
+   ack_all_badblocks(>badblocks);
clear_bit(BlockedBadBlocks, >flags);
wake_up(>blocked_wait);
}
@@ -2944,11 +2941,17 @@ static ssize_t recovery_start_store(struct md_rdev 
*rdev, const char *buf, size_
 static struct rdev_sysfs_entry rdev_recovery_start =
 __ATTR(recovery_start, S_IRUGO|S_IWUSR, recovery_start_show, 
recovery_start_store);
 
-static ssize_t
-badblocks_show(struct badblocks *bb, char *page, int unack);
-static ssize_t
-badblocks_store(struct badblocks *bb, const char *page, size_t len, int unack);
-
+/* sysfs access to bad-blocks list.
+ * We present two files.
+ * 'bad-blocks' lists sector numbers and lengths of ranges that
+ *are recorded as bad.  The list is truncated to fit within
+ *the one-page limit of sysfs.
+ *Writing "sector length" to this file adds an acknowledged
+ *bad block list.
+ * 'unacknowledged-bad-blocks' lists bad blocks that have not yet
+ *been acknowledged.  Writing to this file adds bad blocks
+ *without acknowledging them.  This is largely for testing.
+ */
 static ssize_t bb_show(struct md_rdev *rdev, char *page)
 {
return badblocks_show(>badblocks, page, 0);
@@ -3063,14 +3066,7 @@ int md_rdev_init(struct md_rdev *rdev)
 * This reserves the space even on arrays where it cannot
 * be used - I wonder if that matters
 */
-   rdev->badblocks.count = 0;
-   rdev->badblocks.shift = -1; /* disabled until explicitly enabled */
-   rdev->badblocks.page = kmalloc(PAGE_SIZE, GFP_KERNEL);
-   seqlock_init(>badblocks.lock);
-   if (rdev->badblocks.page == NULL)
-   return -ENOMEM;
-
-   return 0;
+   return badblocks_init(>badblocks, 0);
 }
 EXPORT_SYMBOL_GPL(md_rdev_init);
 /*
@@ -8348,254 +8344,9 @@ void md_finish_reshape(struct mddev *mddev)
 }
 EXPORT_SYMBOL(md_finish_reshape);
 
-/* Bad block management.
- * We can record which blocks on each device are 'bad' and so just
- * fail those blocks, or that stripe, rather than the whole device.
- * Entries in the bad-block table are 64bits wide.  This comprises:
- * Length of bad-range, in sectors: 0-511 for lengths 1-512
- * Start of bad-range, sector offset, 54 bits (allows 8 exbibytes)
- *  A 'shift' can be set so that larger blocks are tracked and
- *  consequently larger devices can be covered.
- * 'Acknowledged' flag - 1 bit. - the most significant bit.
- *
- * Locking of the bad-block table uses a seqlock so md_is_badblock
- * might need to retry if 

Re: [PATCH v2 3/3] md: convert to use the generic badblocks code

[Shaohua Li]

On Wed, Nov 25, 2015 at 11:43:33AM -0700, Vishal Verma wrote:
> Retain badblocks as part of rdev, but use the accessor functions from
> include/linux/badblocks for all manipulation.
> 
> Signed-off-by: Vishal Verma 
> ---
>  drivers/md/md.c | 507 
> +++-
>  drivers/md/md.h |  40 +
>  2 files changed, 23 insertions(+), 524 deletions(-)
> 
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index c702de1..63eab20 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -34,6 +34,7 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -707,8 +708,7 @@ void md_rdev_clear(struct md_rdev *rdev)
>   put_page(rdev->bb_page);
>   rdev->bb_page = NULL;
>   }
> - kfree(rdev->badblocks.page);
> - rdev->badblocks.page = NULL;
> + badblocks_free(>badblocks);
>  }

why does rdev have extra badblocks? the gendisk already had one.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 3/3] md: convert to use the generic badblocks code

[Verma, Vishal L]

On Tue, 2015-12-01 at 10:55 -0800, Shaohua Li wrote:
> On Wed, Nov 25, 2015 at 11:43:33AM -0700, Vishal Verma wrote:
> > Retain badblocks as part of rdev, but use the accessor functions
> > from
> > include/linux/badblocks for all manipulation.
> > 
> > Signed-off-by: Vishal Verma 
> > ---
> >  drivers/md/md.c | 507 +++
> > -
> >  drivers/md/md.h |  40 +
> >  2 files changed, 23 insertions(+), 524 deletions(-)
> > 
> > diff --git a/drivers/md/md.c b/drivers/md/md.c
> > index c702de1..63eab20 100644
> > --- a/drivers/md/md.c
> > +++ b/drivers/md/md.c
> > @@ -34,6 +34,7 @@
> >  
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -707,8 +708,7 @@ void md_rdev_clear(struct md_rdev *rdev)
> >     put_page(rdev->bb_page);
> >     rdev->bb_page = NULL;
> >     }
> > -   kfree(rdev->badblocks.page);
> > -   rdev->badblocks.page = NULL;
> > +   badblocks_free(>badblocks);
> >  }
> 
> why does rdev have extra badblocks? the gendisk already had one.

rdev originally had badblocks, and this path set adds badblocks to
gendisk. It does appear that md's badblock tracking will be a bit
redundant if/once gendisk has badblocks support - see the discussion
here:
https://lists.01.org/pipermail/linux-nvdimm/2015-November/002980.html

-Vishal

[PATCH v2 3/3] md: convert to use the generic badblocks code

[Vishal Verma]

Retain badblocks as part of rdev, but use the accessor functions from
include/linux/badblocks for all manipulation.

Signed-off-by: Vishal Verma 
---
 drivers/md/md.c | 507 +++-
 drivers/md/md.h |  40 +
 2 files changed, 23 insertions(+), 524 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index c702de1..63eab20 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -34,6 +34,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -707,8 +708,7 @@ void md_rdev_clear(struct md_rdev *rdev)
put_page(rdev->bb_page);
rdev->bb_page = NULL;
}
-   kfree(rdev->badblocks.page);
-   rdev->badblocks.page = NULL;
+   badblocks_free(>badblocks);
 }
 EXPORT_SYMBOL_GPL(md_rdev_clear);
 
@@ -1358,8 +1358,6 @@ static __le32 calc_sb_1_csum(struct mdp_superblock_1 *sb)
return cpu_to_le32(csum);
 }
 
-static int md_set_badblocks(struct badblocks *bb, sector_t s, int sectors,
-   int acknowledged);
 static int super_1_load(struct md_rdev *rdev, struct md_rdev *refdev, int 
minor_version)
 {
struct mdp_superblock_1 *sb;
@@ -1484,7 +1482,7 @@ static int super_1_load(struct md_rdev *rdev, struct 
md_rdev *refdev, int minor_
count <<= sb->bblog_shift;
if (bb + 1 == 0)
break;
-   if (md_set_badblocks(>badblocks,
+   if (badblocks_set(>badblocks,
 sector, count, 1) == 0)
return -EINVAL;
}
@@ -2226,7 +2224,7 @@ repeat:
rdev_for_each(rdev, mddev) {
if (rdev->badblocks.changed) {
rdev->badblocks.changed = 0;
-   md_ack_all_badblocks(>badblocks);
+   ack_all_badblocks(>badblocks);
md_error(mddev, rdev);
}
clear_bit(Blocked, >flags);
@@ -2352,7 +2350,7 @@ repeat:
clear_bit(Blocked, >flags);
 
if (any_badblocks_changed)
-   md_ack_all_badblocks(>badblocks);
+   ack_all_badblocks(>badblocks);
clear_bit(BlockedBadBlocks, >flags);
wake_up(>blocked_wait);
}
@@ -2944,11 +2942,17 @@ static ssize_t recovery_start_store(struct md_rdev 
*rdev, const char *buf, size_
 static struct rdev_sysfs_entry rdev_recovery_start =
 __ATTR(recovery_start, S_IRUGO|S_IWUSR, recovery_start_show, 
recovery_start_store);
 
-static ssize_t
-badblocks_show(struct badblocks *bb, char *page, int unack);
-static ssize_t
-badblocks_store(struct badblocks *bb, const char *page, size_t len, int unack);
-
+/* sysfs access to bad-blocks list.
+ * We present two files.
+ * 'bad-blocks' lists sector numbers and lengths of ranges that
+ *are recorded as bad.  The list is truncated to fit within
+ *the one-page limit of sysfs.
+ *Writing "sector length" to this file adds an acknowledged
+ *bad block list.
+ * 'unacknowledged-bad-blocks' lists bad blocks that have not yet
+ *been acknowledged.  Writing to this file adds bad blocks
+ *without acknowledging them.  This is largely for testing.
+ */
 static ssize_t bb_show(struct md_rdev *rdev, char *page)
 {
return badblocks_show(>badblocks, page, 0);
@@ -3063,14 +3067,7 @@ int md_rdev_init(struct md_rdev *rdev)
 * This reserves the space even on arrays where it cannot
 * be used - I wonder if that matters
 */
-   rdev->badblocks.count = 0;
-   rdev->badblocks.shift = -1; /* disabled until explicitly enabled */
-   rdev->badblocks.page = kmalloc(PAGE_SIZE, GFP_KERNEL);
-   seqlock_init(>badblocks.lock);
-   if (rdev->badblocks.page == NULL)
-   return -ENOMEM;
-
-   return 0;
+   return badblocks_init(>badblocks, 0);
 }
 EXPORT_SYMBOL_GPL(md_rdev_init);
 /*
@@ -8348,253 +8345,7 @@ void md_finish_reshape(struct mddev *mddev)
 }
 EXPORT_SYMBOL(md_finish_reshape);
 
-/* Bad block management.
- * We can record which blocks on each device are 'bad' and so just
- * fail those blocks, or that stripe, rather than the whole device.
- * Entries in the bad-block table are 64bits wide.  This comprises:
- * Length of bad-range, in sectors: 0-511 for lengths 1-512
- * Start of bad-range, sector offset, 54 bits (allows 8 exbibytes)
- *  A 'shift' can be set so that larger blocks are tracked and
- *  consequently larger devices can be covered.
- * 'Acknowledged' flag - 1 bit. - the most significant bit.
- *
- * Locking of the bad-block table uses a seqlock so md_is_badblock
- * might need to retry if it is very unlucky.
- * We will sometimes want to check for