Re: [PATCH v2 1/3] badblocks: Add core badblock management code

[Verma, Vishal L]

On Wed, 2015-12-23 at 10:06 +1100, NeilBrown wrote:
> On Wed, Dec 23 2015, Verma, Vishal L wrote:
> 
> > On Tue, 2015-12-22 at 16:34 +1100, NeilBrown wrote:
> > > On Sat, Dec 05 2015, Verma, Vishal L wrote:
> > > 
> > > > On Fri, 2015-12-04 at 15:30 -0800, James Bottomley wrote:
> > > > [...]
> > > > > > +ssize_t badblocks_store(struct badblocks *bb, const char
> > > > > > *page,
> > > > > > size_t len,
> > > > > > +   int unack)
> > > > > [...]
> > > > > > +int badblocks_init(struct badblocks *bb, int enable)
> > > > > > +{
> > > > > > +   bb->count = 0;
> > > > > > +   if (enable)
> > > > > > +   bb->shift = 0;
> > > > > > +   else
> > > > > > +   bb->shift = -1;
> > > > > > +   bb->page = kmalloc(PAGE_SIZE, GFP_KERNEL);
> > > > > 
> > > > > Why not __get_free_page(GFP_KERNEL)?  The problem with kmalloc
> > > > > of
> > > > > an
> > > > > exactly known page sized quantity is that the slab tracker for
> > > > > this
> > > > > requires two contiguous pages for each page because of the
> > > > > overhead.
> > > > 
> > > > Cool, I didn't know about __get_free_page - I can fix this up
> > > > too.
> > > > 
> > > 
> > > I was reminded of this just recently I thought I should clear up
> > > the
> > > misunderstanding.
> > > 
> > > kmalloc(PAGE_SIZE) does *not* incur significant overhead and
> > > certainly
> > > does not require two contiguous free pages.
> > > If you "grep kmalloc-4096 /proc/slabinfo" you will note that both
> > > objperslab and pagesperslab are 1.  So one page is used to store
> > > each
> > > 4096 byte allocation.
> > > 
> > > To quote the email from Linus which reminded me about this
> > > 
> > > > If you
> > > > want to allocate a page, and get a pointer, just use
> > > > "kmalloc()".
> > > > Boom, done!
> > > 
> > > https://lkml.org/lkml/2015/12/21/605
> > > 
> > > There probably is a small CPU overhead from using kmalloc, but no
> > > memory
> > > overhead.
> > 
> > Thanks Neil.
> > I just read the rest of that thread - and I'm wondering if we should
> > change back to kzalloc here.
> > 
> > The one thing __get_free_page gets us is PAGE_SIZE-aligned memory.
> > Do
> > you think that would be better for this use? (I can't think of any).
> > If
> > not, I can send out a new version reverting back to kzalloc.
> 
> kzalloc(PAGE_SIZE) will also always return page-aligned memory.
> kzalloc returns a void*, __get_free_page returns unsigned long.  For
> that reason alone I would prefer kzalloc.
> 
> But I'm not necessarily suggesting you change the code.  I just wanted
> to clarify a misunderstanding.  You should produce the
> code that you are
> most happy with.


I agree, the typecasting with __get_free_page is pretty ugly. I'll
change it back to kzalloc.

Thanks,
-Vishal

signature.asc
Description: This is a digitally signed message part

Re: [PATCH v2 1/3] badblocks: Add core badblock management code

[Verma, Vishal L]

On Tue, 2015-12-22 at 16:34 +1100, NeilBrown wrote:
> On Sat, Dec 05 2015, Verma, Vishal L wrote:
> 
> > On Fri, 2015-12-04 at 15:30 -0800, James Bottomley wrote:
> > [...]
> > > > +ssize_t badblocks_store(struct badblocks *bb, const char *page,
> > > > size_t len,
> > > > +   int unack)
> > > [...]
> > > > +int badblocks_init(struct badblocks *bb, int enable)
> > > > +{
> > > > +   bb->count = 0;
> > > > +   if (enable)
> > > > +   bb->shift = 0;
> > > > +   else
> > > > +   bb->shift = -1;
> > > > +   bb->page = kmalloc(PAGE_SIZE, GFP_KERNEL);
> > > 
> > > Why not __get_free_page(GFP_KERNEL)?  The problem with kmalloc of
> > > an
> > > exactly known page sized quantity is that the slab tracker for
> > > this
> > > requires two contiguous pages for each page because of the
> > > overhead.
> > 
> > Cool, I didn't know about __get_free_page - I can fix this up too.
> > 
> 
> I was reminded of this just recently I thought I should clear up the
> misunderstanding.
> 
> kmalloc(PAGE_SIZE) does *not* incur significant overhead and certainly
> does not require two contiguous free pages.
> If you "grep kmalloc-4096 /proc/slabinfo" you will note that both
> objperslab and pagesperslab are 1.  So one page is used to store each
> 4096 byte allocation.
> 
> To quote the email from Linus which reminded me about this
> 
> > If you
> > want to allocate a page, and get a pointer, just use "kmalloc()".
> > Boom, done!
> 
> https://lkml.org/lkml/2015/12/21/605
> 
> There probably is a small CPU overhead from using kmalloc, but no
> memory
> overhead.

Thanks Neil.
I just read the rest of that thread - and I'm wondering if we should
change back to kzalloc here.

The one thing __get_free_page gets us is PAGE_SIZE-aligned memory. Do
you think that would be better for this use? (I can't think of any). If
not, I can send out a new version reverting back to kzalloc.

-Vishal



signature.asc
Description: This is a digitally signed message part

Re: [PATCH v2 1/3] badblocks: Add core badblock management code

[Verma, Vishal L]

On Wed, 2015-12-09 at 08:03 +1100, NeilBrown wrote:
> On Sat, Dec 05 2015, Verma, Vishal L wrote:
> > > 
> > > > +int badblocks_clear(struct badblocks *bb, sector_t s, int
> > > > sectors)
> > > > +{
> > > [...]
> > > > +#define DO_DEBUG 1
> > > 
> > > Why have this at all if it's unconditionally defined and always
> > > set.
> > 
> > Neil - any reason or anything you had in mind for this? Or is it
> > just an
> > artifact and can be removed.
> 
> Like the comment says:
> 
>   /* Allow clearing via sysfs *only* for testing/debugging.
>* Normally only a successful write may clear a badblock
>*/
> 
> The DO_DEBUG define and ifdefs are documentation identifying bits of
> code that should be removed when it all seems to be working.
> Maybe now is a good time to remove that code.
> 
Hm, I think it would be nice to continue to have the ability to clear
badblocks using sysfs at least for a while more, as we test the various
error handling paths for NVDIMMS (Dan, thoughts?).

We could either remove it later or (I'm leaning towards) make it a
config option similar to FAIL_MAKE_REQUEST and friends..

-Vishal

signature.asc
Description: This is a digitally signed message part

Re: [PATCH v2 1/3] badblocks: Add core badblock management code

[Verma, Vishal L]

On Tue, 2015-12-08 at 13:18 -0800, Dan Williams wrote:
> On Tue, Dec 8, 2015 at 1:08 PM, Verma, Vishal L
> <vishal.l.ve...@intel.com> wrote:
> > On Wed, 2015-12-09 at 08:03 +1100, NeilBrown wrote:
> > > On Sat, Dec 05 2015, Verma, Vishal L wrote:
> > > > > 
> > > > > > +int badblocks_clear(struct badblocks *bb, sector_t s, int
> > > > > > sectors)
> > > > > > +{
> > > > > [...]
> > > > > > +#define DO_DEBUG 1
> > > > > 
> > > > > Why have this at all if it's unconditionally defined and
> > > > > always
> > > > > set.
> > > > 
> > > > Neil - any reason or anything you had in mind for this? Or is it
> > > > just an
> > > > artifact and can be removed.
> > > 
> > > Like the comment says:
> > > 
> > >   /* Allow clearing via sysfs *only* for testing/debugging.
> > >    * Normally only a successful write may clear a badblock
> > >    */
> > > 
> > > The DO_DEBUG define and ifdefs are documentation identifying bits
> > > of
> > > code that should be removed when it all seems to be working.
> > > Maybe now is a good time to remove that code.
> > > 
> > Hm, I think it would be nice to continue to have the ability to
> > clear
> > badblocks using sysfs at least for a while more, as we test the
> > various
> > error handling paths for NVDIMMS (Dan, thoughts?).
> > 
> > We could either remove it later or (I'm leaning towards) make it a
> > config option similar to FAIL_MAKE_REQUEST and friends..
> 
> "later" as in before v4.5-rc1?  We can always carry this debug feature
> locally for testing.  We don't want userspace growing ABI attachments
> to this capability now that it's more than just md tooling that will
> see this.


Agreed. The following incremental patch removes sysfs support.
All the latest badblocks patches can also be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/vishal/nvdimm.git 
gendisk-badblocks


8<-
From 5f0e7ac31d27a132f314106f1db33af22fde03ed Mon Sep 17 00:00:00 2001
From: Vishal Verma <vishal.l.ve...@intel.com>
Date: Tue, 8 Dec 2015 16:28:31 -0700
Subject: [PATCH v4 4/3] badblocks: remove support for clearing via sysfs

sysfs support for clearing badblocks was originally meant for testing
only. With the move to generalize the interface, remove this support so
that userspace doesn't start treating this as an ABI.

Signed-off-by: Vishal Verma <vishal.l.ve...@intel.com>
---
 block/badblocks.c | 15 ---
 1 file changed, 15 deletions(-)

diff --git a/block/badblocks.c b/block/badblocks.c
index f0ac279..e5d2a91 100644
--- a/block/badblocks.c
+++ b/block/badblocks.c
@@ -503,16 +503,6 @@ ssize_t badblocks_store(struct badblocks *bb, const
char *page, size_t len,
    int length;
    char newline;
 
-   /* Allow clearing via sysfs *only* for testing/debugging.
-    * Normally only a successful write may clear a badblock
-    */
-   int clear = 0;
-
-   if (page[0] == '-') {
-   clear = 1;
-   page++;
-   }
-
    switch (sscanf(page, "%llu %d%c", , , ))
{
    case 3:
    if (newline != '\n')
@@ -525,11 +515,6 @@ ssize_t badblocks_store(struct badblocks *bb, const
char *page, size_t len,
    return -EINVAL;
    }
 
-   if (clear) {
-   badblocks_clear(bb, sector, length);
-   return len;
-   }
-
    if (badblocks_set(bb, sector, length, !unack))
    return -ENOSPC;
    else
-- 
2.5.0

Re: [PATCH v2 0/3] Badblock tracking for gendisks

[Verma, Vishal L]

Oops, sorry, should've been PATCH v3..
The contents are right, just the subject line is off.

-Vishal


On Mon, 2015-12-07 at 19:52 -0700, Vishal Verma wrote:
> v3:
>   - Add kernel-doc style comments to all exported functions in
> badblocks.c (James)
>   - Make return values from badblocks functions consistent with
> themselves
> and the kernel style. Change the polarity of badblocks_set, and
> update
> all callers accordingly (James)
>   - In gendisk, don't unconditionally allocate badblocks, export the
> initializer.
> This also allows the initializer to be a non-void return type, so
> that the
> badblocks user can act upon failures better (James)
> 
> 
> v2:
>   - In badblocks_free, make 'page' NULL (patch 1)
>   - Move the core badblocks code to a new .c file (patch 1) (Jens)
>   - Fix a sizeof usage in disk_alloc_badblocks (patch 2) (Dan)
>   - Since disk_alloc_badblocks can fail, check disk->bb for NULL in
> the
> genhd wrappers (patch 2) (Jeff)
>   - Update the md conversion to also ise the badblocks init and free
> functions (patch 3)
>   - Remove the BB_* macros from md.h as they are now in badblocks.h
> (patch 3)
> 
> Patch 1 copies badblock management code into a header of its own,
> making it generally available. It follows common libraries of code
> such as linked lists, where anyone may embed a core data structure
> in another place, and use the provided accessor functions to
> manipulate the data.
> 
> Patch 2 adds badblock tracking to gendisks (in preparation for use
> by NVDIMM devices).
> 
> Patch 3 converts md over to use the new badblocks 'library'. I have
> done some pretty simple testing on this - created a raid 1 device,
> made sure the sysfs entries show up, and can be used to add and view
> badblocks. A closer look by the md folks would be nice here.
> 
> Vishal Verma (3):
>   badblocks: Add core badblock management code
>   block: Add badblock management for gendisks
>   md: convert to use the generic badblocks code
> 
>  block/Makefile|   2 +-
>  block/badblocks.c | 576
> ++
>  block/genhd.c |  76 ++
>  drivers/md/md.c   | 516 ++---
> 
>  drivers/md/md.h   |  40 +---
>  include/linux/badblocks.h |  53 +
>  include/linux/genhd.h |   7 +
>  7 files changed, 741 insertions(+), 529 deletions(-)
>  create mode 100644 block/badblocks.c
>  create mode 100644 include/linux/badblocks.h
> N�r��yb�X��ǧv�^�)޺{.n�+{���"�{ay�ʇڙ�,j��f���h���z��w���
> ���j:+v���w�j�mzZ+�ݢj"��!�i

Re: [PATCH v2 0/3] Badblock tracking for gendisks

[Verma, Vishal L]

On Wed, 2015-11-25 at 11:43 -0700, Vishal Verma wrote:
> v2:
>   - In badblocks_free, make 'page' NULL (patch 1)
>   - Move the core badblocks code to a new .c file (patch 1) (Jens)
>   - Fix a sizeof usage in disk_alloc_badblocks (patch 2) (Dan)
>   - Since disk_alloc_badblocks can fail, check disk->bb for NULL in
> the
> genhd wrappers (patch 2) (Jeff)
>   - Update the md conversion to also ise the badblocks init and free
> functions (patch 3)
>   - Remove the BB_* macros from md.h as they are now in badblocks.h
> (patch 3)
> 
> Patch 1 copies badblock management code into a header of its own,
> making it generally available. It follows common libraries of code
> such as linked lists, where anyone may embed a core data structure
> in another place, and use the provided accessor functions to
> manipulate the data.
> 
> Patch 2 adds badblock tracking to gendisks (in preparation for use
> by NVDIMM devices). Right now, it is turned on unconditionally - I'd
> appreciate comments on if that is the right path.
> 
> Patch 3 converts md over to use the new badblocks 'library'. I have
> done some pretty simple testing on this - created a raid 1 device,
> made sure the sysfs entries show up, and can be used to add and view
> badblocks. A closer look by the md folks would be nice here.
> 
> 
> Vishal Verma (3):
>   badblocks: Add core badblock management code
>   block: Add badblock management for gendisks
>   md: convert to use the generic badblocks code
> 

Ping.

Jens, are you ok taking this through the block tree?
Any other comments from anyone else?

Thanks,
-Vishal

Re: [PATCH v2 1/3] badblocks: Add core badblock management code

[Verma, Vishal L]

On Fri, 2015-12-04 at 15:30 -0800, James Bottomley wrote:
[...]
> > + * We return
> > + *  0 if there are no known bad blocks in the range
> > + *  1 if there are known bad block which are all acknowledged
> > + * -1 if there are bad blocks which have not yet been acknowledged
> > in metadata.
> > + * plus the start/length of the first bad section we overlap.
> > + */
> 
> This comment should be docbook.

Applicable to all your comments - (and they are all valid), I simply
copied over all this from md. I'm happy to make the changes to comments,
and the other two things (see below) if that's the right thing to do --
I just tried to keep my own changes to the original md badblocks code
minimal.
Would it be better (for review-ability) if I made these changes in a new
patch on top of this, or should I just squash them into this one?

> 
> > +int badblocks_check(struct badblocks *bb, sector_t s, int sectors,
> > +   sector_t *first_bad, int *bad_sectors)
> [...]
> > +
> > +/*
> > + * Add a range of bad blocks to the table.
> > + * This might extend the table, or might contract it
> > + * if two adjacent ranges can be merged.
> > + * We binary-search to find the 'insertion' point, then
> > + * decide how best to handle it.
> > + */
> 
> And this one, plus you don't document returns.  It looks like this
> function returns 1 on success and zero on failure, which is really
> counter-intuitive for the kernel: zero is usually returned on success
> and negative error on failure.
> 
> > +int badblocks_set(struct badblocks *bb, sector_t s, int sectors,
> > +   int acknowledged)
> [...]
> > +
> > +/*
> > + * Remove a range of bad blocks from the table.
> > + * This may involve extending the table if we spilt a region,
> > + * but it must not fail.  So if the table becomes full, we just
> > + * drop the remove request.
> > + */
> 
> Docbook and document returns.  This time they're the kernel standard
> of
> 0 on success and negative error on failure making the convention for
> badblocks_set even more counterintuitive.
> 
> > +int badblocks_clear(struct badblocks *bb, sector_t s, int sectors)
> > +{
> [...]
> > +#define DO_DEBUG 1
> 
> Why have this at all if it's unconditionally defined and always set.

Neil - any reason or anything you had in mind for this? Or is it just an
artifact and can be removed.

> 
> > +ssize_t badblocks_store(struct badblocks *bb, const char *page,
> > size_t len,
> > +   int unack)
> [...]
> > +int badblocks_init(struct badblocks *bb, int enable)
> > +{
> > +   bb->count = 0;
> > +   if (enable)
> > +   bb->shift = 0;
> > +   else
> > +   bb->shift = -1;
> > +   bb->page = kmalloc(PAGE_SIZE, GFP_KERNEL);
> 
> Why not __get_free_page(GFP_KERNEL)?  The problem with kmalloc of an
> exactly known page sized quantity is that the slab tracker for this
> requires two contiguous pages for each page because of the overhead.

Cool, I didn't know about __get_free_page - I can fix this up too.

> 
> James
> 
> 

Re: [PATCH v2 1/3] badblocks: Add core badblock management code

[Verma, Vishal L]

On Fri, 2015-12-04 at 16:06 -0800, James Bottomley wrote:
> On Fri, 2015-12-04 at 23:58 +0000, Verma, Vishal L wrote:
> > On Fri, 2015-12-04 at 15:30 -0800, James Bottomley wrote:
> > [...]
> > > > + * We return
> > > > + *  0 if there are no known bad blocks in the range
> > > > + *  1 if there are known bad block which are all acknowledged
> > > > + * -1 if there are bad blocks which have not yet been
> > > > acknowledged
> > > > in metadata.
> > > > + * plus the start/length of the first bad section we overlap.
> > > > + */
> > > 
> > > This comment should be docbook.
> > 
> > Applicable to all your comments - (and they are all valid), I simply
> > copied over all this from md. I'm happy to make the changes to
> > comments,
> > and the other two things (see below) if that's the right thing to do
> > --
> > I just tried to keep my own changes to the original md badblocks
> > code
> > minimal.
> > Would it be better (for review-ability) if I made these changes in a
> > new
> > patch on top of this, or should I just squash them into this one?
> 
> If you were moving it, that might be appropriate.  However, this is
> effectively new code because you're not removing the original, so we
> should begin at least with a coherent API. (i.e. corrections to the
> original patch rather than incremental).
> 

Patch 3 does remove the original code, but yes, I agree. Will send
another version.

Thanks for the review.

-Vishal

Re: [PATCH v2 2/3] block: Add badblock management for gendisks

[Verma, Vishal L]

On Fri, 2015-12-04 at 15:33 -0800, James Bottomley wrote:
[...]
> >  static void register_disk(struct gendisk *disk)
> >  {
> >     struct device *ddev = disk_to_dev(disk);
> > @@ -609,6 +624,7 @@ void add_disk(struct gendisk *disk)
> >     disk->first_minor = MINOR(devt);
> >  
> >     disk_alloc_events(disk);
> > +   disk_alloc_badblocks(disk);
> 
> Why unconditionally do this?  No-one currently uses the interface, but
> every disk will now pay the price of an additional structure plus a
> page
> for no benefit.  You should probably either export the initializer for
> those who want to use it or, perhaps even better, make it lazily
> allocated the first time anyone tries to set a bad block.
> 
> If you come up with a really good reason for allocating it
> unconditionally, then it should probably be an embedded structure in
> the gendisk.
> 
Agreed - I'll fix for v3.

I'm considering an embedded structure in gendisk (same as md) (why is
this preferred to pointer chasing, especially when this wastes more
space?), and a new exported initializer that is used by anyone who wants
to use gendisk's badblocks.


-VishalN�r��yb�X��ǧv�^�)޺{.n�+{���"�{ay�ʇڙ�,j��f���h���z��w���
���j:+v���w�j�mzZ+�ݢj"��!�i

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

Re: [PATCH 2/3] block: Add badblock management for gendisks

[Verma, Vishal L]

On Wed, 2015-11-25 at 10:37 -0500, Jeff Moyer wrote:
> "Verma, Vishal L" <vishal.l.ve...@intel.com> writes:
> 
> > On Tue, 2015-11-24 at 14:14 -0500, Jeff Moyer wrote:
> > > 
> > > I'm not sure whether it makes sense to continue without badblock
> > > management for the RAID code.  I was hoping Neil would comment on
> > > that.
> > > 
> > > -Jeff
> > 
> > Not sure I follow? I believe I've kept all the badblocks
> > functionality
> > RAID already had..
> 
> What I mean to say is that the RAID code had previously embedded the
> badblocks structure in one of its other data structures.  As a result,
> you would never get an allocation failure for it.
> 
Ah I see - I don't think that has effectively changed. 'rdev' still
contains a statically allocated badblocks structure (as opposed to
gendisk, which just stores a pointer). md used to dynamically allocate
the storage space inside badblocks (bb->page), and that is still the
case using badblocks_init.

-Vishal

Re: [PATCH 2/3] block: Add badblock management for gendisks

[Verma, Vishal L]

On Tue, 2015-11-24 at 14:14 -0500, Jeff Moyer wrote:
> 
> I'm not sure whether it makes sense to continue without badblock
> management for the RAID code.  I was hoping Neil would comment on
> that.
> 
> -Jeff

Not sure I follow? I believe I've kept all the badblocks functionality
RAID already had..


On a related note, something I observed when testing with md:

md's badblock list is per-device, and can be seen at:
/sys/block/md0/md/dev-pmem0/bad_blocks

Now if we have badblocks in the gendisks too, there is also:
/sys/block/pmem0/bad_blocks

The two are separate 'accounts' maintained by separate drivers (md for
the former, and pmem for the latter). This can potentially be
confusing..

Should we consolidate the two, i.e. make md (re)use the gendisk
badblocks for its purposes too?

-Vishal

Re: [PATCH 2/3] block: Add badblock management for gendisks

[Verma, Vishal L]

On Tue, 2015-11-24 at 10:34 -0500, Jeff Moyer wrote:
> Vishal Verma  writes:
> 
> > NVDIMM devices, which can behave more like DRAM rather than block
> > devices, may develop bad cache lines, or 'poison'. A block device
> > exposed by the pmem driver can then consume poison via a read (or
> > write), and cause a machine check. On platforms without machine
> > check recovery features, this would mean a crash.
> > 
> > The block device maintaining a runtime list of all known sectors
> > that
> > have poison can directly avoid this, and also provide a path forward
> > to enable proper handling/recovery for DAX faults on such a device.
> > 
> > Use the new badblock management interfaces to add a badblocks list
> > to
> > gendisks.
> 
> Because disk_alloc_badblocks can fail, you need to check for a NULL
> disk->bb in all of the utility functions you've defined.
> 

Thanks, Jeff - I'll fix this. I have a handful of other fixes queued up
too, will send out a v2 soon.


-VishalN�r��yb�X��ǧv�^�)޺{.n�+{���"�{ay�ʇڙ�,j��f���h���z��w���
���j:+v���w�j�mzZ+�ݢj"��!�i