On Thu, Jan 7, 2016 at 3:15 PM, Linda Knippers <linda.knippers(a)hpe.com> wrote:
On 1/7/2016 5:49 PM, Dan Williams wrote:
> On Thu, Jan 7, 2016 at 1:18 PM, Linda Knippers <linda.knippers(a)hpe.com> wrote:
>> On 12/24/2015 9:21 PM, Vishal Verma wrote:
>>> During region creation, perform Address Range Scrubs (ARS) for the SPA
>>> (System Physical Address) ranges to retrieve known poison locations from
>>> firmware. Add a new data structure 'nd_poison' which is used as a
>>> in nvdimm_bus to store these poison locations.
>>> When creating a pmem namespace, if there is any known poison associated
>>> with its physical address space, convert the poison ranges to bad sectors
>>> that are exposed using the badblocks interface.
>>> Signed-off-by: Vishal Verma <vishal.l.verma(a)intel.com>
>>> drivers/acpi/nfit.c | 203
>>> drivers/nvdimm/core.c | 187 ++++++++++++++++++++++++++++++++++++++++++
>>> drivers/nvdimm/nd-core.h | 3 +
>>> drivers/nvdimm/nd.h | 6 ++
>>> drivers/nvdimm/pmem.c | 6 ++
>>> include/linux/libnvdimm.h | 1 +
>>> 6 files changed, 406 insertions(+)
>>> diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c
>> Why are the ARS function in acpi/nfit.c rather than in the core?
>> I realize that the interfaces are/will be defined in the ACPI spec but
>> it's not really part of the NFIT.
>> Why isn't this part of the nvdimm core, where the calls are made?
> True. acpi_nfit_find_poison() could easily become
> nvdimm_bus_find_poison() and move to the core. However we only need
> to take that step when / if another nvdimm bus type show up that isn't
> nfit defined. For now only nfit-defined nvdimms have a concept of
Why not go ahead and move it now?
I agree that ARS is related to NFIT, but practically everything else
in the core is as well. It doesn't seem to belongs in nfit.c. I
didn't even notice that's where it was since the subject line says
Should be straightforward, will do.
>>> index aa45d48..ad6d8c6 100644
>>> --- a/drivers/acpi/nfit.c
>>> +++ b/drivers/acpi/nfit.c
>>> @@ -15,6 +15,7 @@
>> I think you should get status, start, wait for completion,
>> records, at least that's how I read the example DSM spec and other
>> specs. It says "You must first issue a Query ARS Status command...".
> We do the lead-in ars_status check above...
Oh right, I missed that.
The way the locking works, the registration of regions are serialized so we
can't never have overlapping calls to this function, right? A comment
about how this is synchronized might be helpful since the lock is many