On Mon, Jul 17, 2017 at 3:20 PM, Verma, Vishal L <[email protected]> wrote: > On Mon, 2017-07-17 at 15:00 -0700, Dan Williams wrote: >> On Mon, Jul 17, 2017 at 2:27 PM, Vishal Verma <[email protected] >> m> wrote: >> > Clearing errors or badblocks during a BTT write requires sending an >> > ACPI >> > DSM, which means potentially sleeping. Since a BTT IO happens in >> > atomic >> > context (preemption disabled, spinlocks may be held), we cannot >> > perform >> > error clearing in the course of an IO. Due to this error clearing >> > for >> > BTT IOs has hitherto been disabled. >> > >> > In this patch we move error clearing out of the atomic section, and >> > thus >> > re-enable error clearing with BTTs. When we are about to add a block >> > to >> > the free list, we check if it was previously marked as an error, and >> > if >> > it was, we add it to the freelist, but also set a flag that says >> > error >> > clearing will be required. We then drop the lane (ending the atomic >> > context), and send a zero buffer so that the error can be cleared. >> > The >> > error flag in the free list is protected by the nd 'lane', and is >> > set >> > only be a thread while it holds that lane. When the error is >> > cleared, >> > the flag is cleared, but while holding a mutex for that freelist >> > index. >> > >> > When writing, we check for two things - >> > 1/ If the freelist mutex is held or if the error flag is set. If so, >> > this is an error block that is being (or about to be) cleared. >> > 2/ If the block is a known badblock based on nsio->bb >> > >> > The second check is required because the BTT map error flag for a >> > map >> > entry only gets set when an error LBA is read. If we write to a new >> > location that may not have the map error flag set, but still might >> > be in >> > the region's badblock list, we can trigger an EIO on the write, >> > which is >> > undesirable and completely avoidable. >> > >> > Cc: Jeff Moyer <[email protected]> >> > Cc: Toshi Kani <[email protected]> >> > Cc: Dan Williams <[email protected]> >> > Signed-off-by: Vishal Verma <[email protected]> >> > --- >> > drivers/nvdimm/btt.c | 89 >> > +++++++++++++++++++++++++++++++++++++++++++++----- >> > drivers/nvdimm/btt.h | 5 +++ >> > drivers/nvdimm/claim.c | 8 ----- >> > 3 files changed, 85 insertions(+), 17 deletions(-) >> > > > <> > >> > +static int arena_clear_freelist_error(struct arena_info *arena, u32 >> > lane) >> > +{ >> > + int ret = 0; >> > + >> > + if (arena->freelist[lane].has_err) { >> > + u32 lba = arena->freelist[lane].block; >> > + u64 nsoff = to_namespace_offset(arena, lba); >> > + void *zerobuf; >> > + >> > + zerobuf = kzalloc(arena->sector_size, GFP_KERNEL); >> > + if (!zerobuf) >> > + return -ENOMEM; >> >> Hmm, can this just use the system zero page (empty_zero_page)? Also, >> even if it can't, I'd prefer the NOIO fix for this case be folded into >> this patch. The NOIO fixup for the acpi dsm path should be separate. >> >> Another consideration if we can't use the system zero page, do we need >> to allocate this on demand? Seems like something we could just >> pre-allocate at init time. > > I had started out using the empty_zero_page, but to use that you have to > kmap_atomic the page, which ends up creating an atomic section again :)
I think it is safe to assume that the zero page is never in highmem. _______________________________________________ Linux-nvdimm mailing list [email protected] https://lists.01.org/mailman/listinfo/linux-nvdimm
