Re: [PATCH 3/3] pmem: Use the poison list to expose badblocks

["Verma, Vishal L" <vishal.l.verma@intel.com>]

On Sun, 2015-12-20 at 17:20 -0800, Dan Williams wrote:
> On Sun, Dec 20, 2015 at 1:18 AM,  <vishal@kernel.org> wrote:
> > From: Vishal Verma <vishal.l.verma@intel.com>
> > 
> > Enable the gendisk badblocks feature for pmem namespaces.
> > If the pmem namespace being created has any known poison associated
> > with
> > its physical address space, convert the poison ranges to bad sectors
> > exposed using the badblocks interface.
> > 
> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> > ---
> >  drivers/nvdimm/pmem.c | 124
> > ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 124 insertions(+)
> 
> I think we should move this new functionality to the core because
> there is not much pmem driver specific.  It's all generic nvdimm-core
> and block-core functionality.  The only missing information the core
> routine needs is the gendisk and a data offset (if sector-zero is at
> an offset from the base address range of the namespace).  Something
> like:
> 
> nvdimm_namespace_disk_poison(struct nd_namespace_common *ndns,
>     resource_size_t offset, struct gendisk *disk)

This should be easy to do, however isn't it a bit counter-intuitive to
move this into core? The lower level drivers pmem/blk/btt are all owners
of their respective gendisks, and so doesn't it make more sense for them
to be in control of manipulating their gendisk data. Also, I can see
moving them if this was a common operation, but only pmem will ever need
to do this..

I'm not too strongly opposed to this however - the one thing that did
feel a bit awkward being in pmem was that we ask core for a struct
list_head and then walk it ourselves - pmem doesn't normally know about
the internals of nvdimm_bus, but with this we implicitly make it aware.