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

From: "Verma, Vishal L" <vishal.l.verma@intel.com>
To: "shli@kernel.org" <shli@kernel.org>
Cc: "linux-raid@vger.kernel.org" <linux-raid@vger.kernel.org>,
	"linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
	"linux-nvdimm@lists.01.org" <linux-nvdimm@lists.01.org>,
	"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	"neilb@suse.com" <neilb@suse.com>, "axboe@fb.com" <axboe@fb.com>,
	"jmoyer@redhat.com" <jmoyer@redhat.com>
Subject: Re: [PATCH v2 3/3] md: convert to use the generic badblocks code
Date: Tue, 1 Dec 2015 19:52:05 +0000	[thread overview]
Message-ID: <1448999525.5911.8.camel@intel.com> (raw)
In-Reply-To: <20151201185518.GA13926@kernel.org>

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 <vishal.l.verma@intel.com>
> > ---
> >  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 <linux/kthread.h>
> >  #include <linux/blkdev.h>
> > +#include <linux/badblocks.h>
> >  #include <linux/sysctl.h>
> >  #include <linux/seq_file.h>
> >  #include <linux/fs.h>
> > @@ -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(&rdev->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