| Message ID | 20220420020435.90326-5-jane.chu@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | DAX poison recovery | expand |
> + if (bb->count && > + badblocks_check(bb, sector, num, &first_bad, &num_bad)) { Weird alignment here, continuing lines for conditionals are aligned either after the opening brace: if (bb->count && badblocks_check(bb, sector, num, &first_bad, &num_bad)) { or with double tabs. I tend to prefer the version I posted above. The being said, shouldn't this change even be in this patch and not just added once we add actual recovery support?
On Tue, Apr 19, 2022 at 7:05 PM Jane Chu <jane.chu@oracle.com> wrote: > > Up till now, dax_direct_access() is used implicitly for normal > access, but for the purpose of recovery write, dax range with > poison is requested. To make the interface clear, introduce > enum dax_access_mode { > DAX_ACCESS, > DAX_RECOVERY_WRITE, > } > where DAX_ACCESS is used for normal dax access, and > DAX_RECOVERY_WRITE is used for dax recovery write. > > Suggested-by: Dan Williams <dan.j.williams@intel.com> > Signed-off-by: Jane Chu <jane.chu@oracle.com> > --- > drivers/dax/super.c | 5 ++-- > drivers/md/dm-linear.c | 5 ++-- > drivers/md/dm-log-writes.c | 5 ++-- > drivers/md/dm-stripe.c | 5 ++-- > drivers/md/dm-target.c | 4 ++- > drivers/md/dm-writecache.c | 7 +++--- > drivers/md/dm.c | 5 ++-- > drivers/nvdimm/pmem.c | 44 +++++++++++++++++++++++++-------- > drivers/nvdimm/pmem.h | 5 +++- > drivers/s390/block/dcssblk.c | 9 ++++--- > fs/dax.c | 9 ++++--- > fs/fuse/dax.c | 4 +-- > include/linux/dax.h | 9 +++++-- > include/linux/device-mapper.h | 4 ++- > tools/testing/nvdimm/pmem-dax.c | 3 ++- > 15 files changed, 85 insertions(+), 38 deletions(-) > > diff --git a/drivers/dax/super.c b/drivers/dax/super.c > index 0211e6f7b47a..5405eb553430 100644 > --- a/drivers/dax/super.c > +++ b/drivers/dax/super.c > @@ -117,6 +117,7 @@ enum dax_device_flags { > * @dax_dev: a dax_device instance representing the logical memory range > * @pgoff: offset in pages from the start of the device to translate > * @nr_pages: number of consecutive pages caller can handle relative to @pfn > + * @mode: indicator on normal access or recovery write > * @kaddr: output parameter that returns a virtual address mapping of pfn > * @pfn: output parameter that returns an absolute pfn translation of @pgoff > * > @@ -124,7 +125,7 @@ enum dax_device_flags { > * pages accessible at the device relative @pgoff. > */ > long dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff, long nr_pages, > - void **kaddr, pfn_t *pfn) > + enum dax_access_mode mode, void **kaddr, pfn_t *pfn) > { > long avail; > > @@ -138,7 +139,7 @@ long dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff, long nr_pages, > return -EINVAL; > > avail = dax_dev->ops->direct_access(dax_dev, pgoff, nr_pages, > - kaddr, pfn); > + mode, kaddr, pfn); > if (!avail) > return -ERANGE; > return min(avail, nr_pages); > diff --git a/drivers/md/dm-linear.c b/drivers/md/dm-linear.c > index 76b486e4d2be..13e263299c9c 100644 > --- a/drivers/md/dm-linear.c > +++ b/drivers/md/dm-linear.c > @@ -172,11 +172,12 @@ static struct dax_device *linear_dax_pgoff(struct dm_target *ti, pgoff_t *pgoff) > } > > static long linear_dax_direct_access(struct dm_target *ti, pgoff_t pgoff, > - long nr_pages, void **kaddr, pfn_t *pfn) > + long nr_pages, enum dax_access_mode mode, void **kaddr, > + pfn_t *pfn) > { > struct dax_device *dax_dev = linear_dax_pgoff(ti, &pgoff); > > - return dax_direct_access(dax_dev, pgoff, nr_pages, kaddr, pfn); > + return dax_direct_access(dax_dev, pgoff, nr_pages, mode, kaddr, pfn); > } > > static int linear_dax_zero_page_range(struct dm_target *ti, pgoff_t pgoff, > diff --git a/drivers/md/dm-log-writes.c b/drivers/md/dm-log-writes.c > index c9d036d6bb2e..06bdbed65eb1 100644 > --- a/drivers/md/dm-log-writes.c > +++ b/drivers/md/dm-log-writes.c > @@ -889,11 +889,12 @@ static struct dax_device *log_writes_dax_pgoff(struct dm_target *ti, > } > > static long log_writes_dax_direct_access(struct dm_target *ti, pgoff_t pgoff, > - long nr_pages, void **kaddr, pfn_t *pfn) > + long nr_pages, enum dax_access_mode mode, void **kaddr, > + pfn_t *pfn) > { > struct dax_device *dax_dev = log_writes_dax_pgoff(ti, &pgoff); > > - return dax_direct_access(dax_dev, pgoff, nr_pages, kaddr, pfn); > + return dax_direct_access(dax_dev, pgoff, nr_pages, mode, kaddr, pfn); > } > > static int log_writes_dax_zero_page_range(struct dm_target *ti, pgoff_t pgoff, > diff --git a/drivers/md/dm-stripe.c b/drivers/md/dm-stripe.c > index c81d331d1afe..77d72900e997 100644 > --- a/drivers/md/dm-stripe.c > +++ b/drivers/md/dm-stripe.c > @@ -315,11 +315,12 @@ static struct dax_device *stripe_dax_pgoff(struct dm_target *ti, pgoff_t *pgoff) > } > > static long stripe_dax_direct_access(struct dm_target *ti, pgoff_t pgoff, > - long nr_pages, void **kaddr, pfn_t *pfn) > + long nr_pages, enum dax_access_mode mode, void **kaddr, > + pfn_t *pfn) > { > struct dax_device *dax_dev = stripe_dax_pgoff(ti, &pgoff); > > - return dax_direct_access(dax_dev, pgoff, nr_pages, kaddr, pfn); > + return dax_direct_access(dax_dev, pgoff, nr_pages, mode, kaddr, pfn); > } > > static int stripe_dax_zero_page_range(struct dm_target *ti, pgoff_t pgoff, > diff --git a/drivers/md/dm-target.c b/drivers/md/dm-target.c > index 64dd0b34fcf4..8cd5184e62f0 100644 > --- a/drivers/md/dm-target.c > +++ b/drivers/md/dm-target.c > @@ -10,6 +10,7 @@ > #include <linux/init.h> > #include <linux/kmod.h> > #include <linux/bio.h> > +#include <linux/dax.h> > > #define DM_MSG_PREFIX "target" > > @@ -142,7 +143,8 @@ static void io_err_release_clone_rq(struct request *clone, > } > > static long io_err_dax_direct_access(struct dm_target *ti, pgoff_t pgoff, > - long nr_pages, void **kaddr, pfn_t *pfn) > + long nr_pages, enum dax_access_mode mode, void **kaddr, > + pfn_t *pfn) > { > return -EIO; > } > diff --git a/drivers/md/dm-writecache.c b/drivers/md/dm-writecache.c > index 5630b470ba42..d74c5a7a0ab4 100644 > --- a/drivers/md/dm-writecache.c > +++ b/drivers/md/dm-writecache.c > @@ -286,7 +286,8 @@ static int persistent_memory_claim(struct dm_writecache *wc) > > id = dax_read_lock(); > > - da = dax_direct_access(wc->ssd_dev->dax_dev, offset, p, &wc->memory_map, &pfn); > + da = dax_direct_access(wc->ssd_dev->dax_dev, offset, p, DAX_ACCESS, > + &wc->memory_map, &pfn); > if (da < 0) { > wc->memory_map = NULL; > r = da; > @@ -308,8 +309,8 @@ static int persistent_memory_claim(struct dm_writecache *wc) > i = 0; > do { > long daa; > - daa = dax_direct_access(wc->ssd_dev->dax_dev, offset + i, p - i, > - NULL, &pfn); > + daa = dax_direct_access(wc->ssd_dev->dax_dev, offset + i, > + p - i, DAX_ACCESS, NULL, &pfn); > if (daa <= 0) { > r = daa ? daa : -EINVAL; > goto err3; > diff --git a/drivers/md/dm.c b/drivers/md/dm.c > index 3c5fad7c4ee6..8258676a352f 100644 > --- a/drivers/md/dm.c > +++ b/drivers/md/dm.c > @@ -1093,7 +1093,8 @@ static struct dm_target *dm_dax_get_live_target(struct mapped_device *md, > } > > static long dm_dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff, > - long nr_pages, void **kaddr, pfn_t *pfn) > + long nr_pages, enum dax_access_mode mode, void **kaddr, > + pfn_t *pfn) > { > struct mapped_device *md = dax_get_private(dax_dev); > sector_t sector = pgoff * PAGE_SECTORS; > @@ -1111,7 +1112,7 @@ static long dm_dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff, > if (len < 1) > goto out; > nr_pages = min(len, nr_pages); > - ret = ti->type->direct_access(ti, pgoff, nr_pages, kaddr, pfn); > + ret = ti->type->direct_access(ti, pgoff, nr_pages, mode, kaddr, pfn); > > out: > dm_put_live_table(md, srcu_idx); > diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c > index 4aa17132a557..c77b7cf19639 100644 > --- a/drivers/nvdimm/pmem.c > +++ b/drivers/nvdimm/pmem.c > @@ -239,24 +239,47 @@ static int pmem_rw_page(struct block_device *bdev, sector_t sector, > > /* see "strong" declaration in tools/testing/nvdimm/pmem-dax.c */ > __weak long __pmem_direct_access(struct pmem_device *pmem, pgoff_t pgoff, > - long nr_pages, void **kaddr, pfn_t *pfn) > + long nr_pages, enum dax_access_mode mode, void **kaddr, > + pfn_t *pfn) > { > resource_size_t offset = PFN_PHYS(pgoff) + pmem->data_offset; > - > - if (unlikely(is_bad_pmem(&pmem->bb, PFN_PHYS(pgoff) / 512, > - PFN_PHYS(nr_pages)))) > - return -EIO; > + sector_t sector = PFN_PHYS(pgoff) >> SECTOR_SHIFT; > + unsigned int num = PFN_PHYS(nr_pages) >> SECTOR_SHIFT; > + struct badblocks *bb = &pmem->bb; > + sector_t first_bad; > + int num_bad; > > if (kaddr) > *kaddr = pmem->virt_addr + offset; > if (pfn) > *pfn = phys_to_pfn_t(pmem->phys_addr + offset, pmem->pfn_flags); > > + if (bb->count && > + badblocks_check(bb, sector, num, &first_bad, &num_bad)) { > + long actual_nr; > + > + if (mode != DAX_RECOVERY_WRITE) > + return -EIO; > + > + /* > + * Set the recovery stride is set to kernel page size because > + * the underlying driver and firmware clear poison functions > + * don't appear to handle large chunk(such as 2MiB) reliably. > + */ > + actual_nr = PHYS_PFN( > + PAGE_ALIGN((first_bad - sector) << SECTOR_SHIFT)); > + dev_dbg(pmem->bb.dev, "start sector(%llu), nr_pages(%ld), first_bad(%llu), actual_nr(%ld)\n", > + sector, nr_pages, first_bad, actual_nr); > + if (actual_nr) > + return actual_nr; > + return 1; > + } > + Similar feedback as Christoph, lets keep this patch just to the simple task of plumbing the @mode argument to dax_direct_access() and save the logic changes for DAX_RECOVERY_WRITE to a later patch. The idea being that in general you want to limit the blast radius of regressions so that it simplifies reverts if necessary. If these logic changes have a regression then reverting this patch also undoes all the other innocent boilerplate plumbing.
On 4/20/2022 11:57 PM, Christoph Hellwig wrote: >> + if (bb->count && >> + badblocks_check(bb, sector, num, &first_bad, &num_bad)) { > > Weird alignment here, continuing lines for conditionals are aligned > either after the opening brace: > > if (bb->count && > badblocks_check(bb, sector, num, &first_bad, &num_bad)) { > > or with double tabs. I tend to prefer the version I posted above. > > The being said, shouldn't this change even be in this patch and not just > added once we add actual recovery support? Right, will move the recovery_write related changes to next patch. thanks! -jane
On 4/21/2022 12:35 PM, Dan Williams wrote: > On Tue, Apr 19, 2022 at 7:05 PM Jane Chu <jane.chu@oracle.com> wrote: >> >> Up till now, dax_direct_access() is used implicitly for normal >> access, but for the purpose of recovery write, dax range with >> poison is requested. To make the interface clear, introduce >> enum dax_access_mode { >> DAX_ACCESS, >> DAX_RECOVERY_WRITE, >> } >> where DAX_ACCESS is used for normal dax access, and >> DAX_RECOVERY_WRITE is used for dax recovery write. >> >> Suggested-by: Dan Williams <dan.j.williams@intel.com> >> Signed-off-by: Jane Chu <jane.chu@oracle.com> >> --- >> drivers/dax/super.c | 5 ++-- >> drivers/md/dm-linear.c | 5 ++-- >> drivers/md/dm-log-writes.c | 5 ++-- >> drivers/md/dm-stripe.c | 5 ++-- >> drivers/md/dm-target.c | 4 ++- >> drivers/md/dm-writecache.c | 7 +++--- >> drivers/md/dm.c | 5 ++-- >> drivers/nvdimm/pmem.c | 44 +++++++++++++++++++++++++-------- >> drivers/nvdimm/pmem.h | 5 +++- >> drivers/s390/block/dcssblk.c | 9 ++++--- >> fs/dax.c | 9 ++++--- >> fs/fuse/dax.c | 4 +-- >> include/linux/dax.h | 9 +++++-- >> include/linux/device-mapper.h | 4 ++- >> tools/testing/nvdimm/pmem-dax.c | 3 ++- >> 15 files changed, 85 insertions(+), 38 deletions(-) >> >> diff --git a/drivers/dax/super.c b/drivers/dax/super.c >> index 0211e6f7b47a..5405eb553430 100644 >> --- a/drivers/dax/super.c >> +++ b/drivers/dax/super.c >> @@ -117,6 +117,7 @@ enum dax_device_flags { >> * @dax_dev: a dax_device instance representing the logical memory range >> * @pgoff: offset in pages from the start of the device to translate >> * @nr_pages: number of consecutive pages caller can handle relative to @pfn >> + * @mode: indicator on normal access or recovery write >> * @kaddr: output parameter that returns a virtual address mapping of pfn >> * @pfn: output parameter that returns an absolute pfn translation of @pgoff >> * >> @@ -124,7 +125,7 @@ enum dax_device_flags { >> * pages accessible at the device relative @pgoff. >> */ >> long dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff, long nr_pages, >> - void **kaddr, pfn_t *pfn) >> + enum dax_access_mode mode, void **kaddr, pfn_t *pfn) >> { >> long avail; >> >> @@ -138,7 +139,7 @@ long dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff, long nr_pages, >> return -EINVAL; >> >> avail = dax_dev->ops->direct_access(dax_dev, pgoff, nr_pages, >> - kaddr, pfn); >> + mode, kaddr, pfn); >> if (!avail) >> return -ERANGE; >> return min(avail, nr_pages); >> diff --git a/drivers/md/dm-linear.c b/drivers/md/dm-linear.c >> index 76b486e4d2be..13e263299c9c 100644 >> --- a/drivers/md/dm-linear.c >> +++ b/drivers/md/dm-linear.c >> @@ -172,11 +172,12 @@ static struct dax_device *linear_dax_pgoff(struct dm_target *ti, pgoff_t *pgoff) >> } >> >> static long linear_dax_direct_access(struct dm_target *ti, pgoff_t pgoff, >> - long nr_pages, void **kaddr, pfn_t *pfn) >> + long nr_pages, enum dax_access_mode mode, void **kaddr, >> + pfn_t *pfn) >> { >> struct dax_device *dax_dev = linear_dax_pgoff(ti, &pgoff); >> >> - return dax_direct_access(dax_dev, pgoff, nr_pages, kaddr, pfn); >> + return dax_direct_access(dax_dev, pgoff, nr_pages, mode, kaddr, pfn); >> } >> >> static int linear_dax_zero_page_range(struct dm_target *ti, pgoff_t pgoff, >> diff --git a/drivers/md/dm-log-writes.c b/drivers/md/dm-log-writes.c >> index c9d036d6bb2e..06bdbed65eb1 100644 >> --- a/drivers/md/dm-log-writes.c >> +++ b/drivers/md/dm-log-writes.c >> @@ -889,11 +889,12 @@ static struct dax_device *log_writes_dax_pgoff(struct dm_target *ti, >> } >> >> static long log_writes_dax_direct_access(struct dm_target *ti, pgoff_t pgoff, >> - long nr_pages, void **kaddr, pfn_t *pfn) >> + long nr_pages, enum dax_access_mode mode, void **kaddr, >> + pfn_t *pfn) >> { >> struct dax_device *dax_dev = log_writes_dax_pgoff(ti, &pgoff); >> >> - return dax_direct_access(dax_dev, pgoff, nr_pages, kaddr, pfn); >> + return dax_direct_access(dax_dev, pgoff, nr_pages, mode, kaddr, pfn); >> } >> >> static int log_writes_dax_zero_page_range(struct dm_target *ti, pgoff_t pgoff, >> diff --git a/drivers/md/dm-stripe.c b/drivers/md/dm-stripe.c >> index c81d331d1afe..77d72900e997 100644 >> --- a/drivers/md/dm-stripe.c >> +++ b/drivers/md/dm-stripe.c >> @@ -315,11 +315,12 @@ static struct dax_device *stripe_dax_pgoff(struct dm_target *ti, pgoff_t *pgoff) >> } >> >> static long stripe_dax_direct_access(struct dm_target *ti, pgoff_t pgoff, >> - long nr_pages, void **kaddr, pfn_t *pfn) >> + long nr_pages, enum dax_access_mode mode, void **kaddr, >> + pfn_t *pfn) >> { >> struct dax_device *dax_dev = stripe_dax_pgoff(ti, &pgoff); >> >> - return dax_direct_access(dax_dev, pgoff, nr_pages, kaddr, pfn); >> + return dax_direct_access(dax_dev, pgoff, nr_pages, mode, kaddr, pfn); >> } >> >> static int stripe_dax_zero_page_range(struct dm_target *ti, pgoff_t pgoff, >> diff --git a/drivers/md/dm-target.c b/drivers/md/dm-target.c >> index 64dd0b34fcf4..8cd5184e62f0 100644 >> --- a/drivers/md/dm-target.c >> +++ b/drivers/md/dm-target.c >> @@ -10,6 +10,7 @@ >> #include <linux/init.h> >> #include <linux/kmod.h> >> #include <linux/bio.h> >> +#include <linux/dax.h> >> >> #define DM_MSG_PREFIX "target" >> >> @@ -142,7 +143,8 @@ static void io_err_release_clone_rq(struct request *clone, >> } >> >> static long io_err_dax_direct_access(struct dm_target *ti, pgoff_t pgoff, >> - long nr_pages, void **kaddr, pfn_t *pfn) >> + long nr_pages, enum dax_access_mode mode, void **kaddr, >> + pfn_t *pfn) >> { >> return -EIO; >> } >> diff --git a/drivers/md/dm-writecache.c b/drivers/md/dm-writecache.c >> index 5630b470ba42..d74c5a7a0ab4 100644 >> --- a/drivers/md/dm-writecache.c >> +++ b/drivers/md/dm-writecache.c >> @@ -286,7 +286,8 @@ static int persistent_memory_claim(struct dm_writecache *wc) >> >> id = dax_read_lock(); >> >> - da = dax_direct_access(wc->ssd_dev->dax_dev, offset, p, &wc->memory_map, &pfn); >> + da = dax_direct_access(wc->ssd_dev->dax_dev, offset, p, DAX_ACCESS, >> + &wc->memory_map, &pfn); >> if (da < 0) { >> wc->memory_map = NULL; >> r = da; >> @@ -308,8 +309,8 @@ static int persistent_memory_claim(struct dm_writecache *wc) >> i = 0; >> do { >> long daa; >> - daa = dax_direct_access(wc->ssd_dev->dax_dev, offset + i, p - i, >> - NULL, &pfn); >> + daa = dax_direct_access(wc->ssd_dev->dax_dev, offset + i, >> + p - i, DAX_ACCESS, NULL, &pfn); >> if (daa <= 0) { >> r = daa ? daa : -EINVAL; >> goto err3; >> diff --git a/drivers/md/dm.c b/drivers/md/dm.c >> index 3c5fad7c4ee6..8258676a352f 100644 >> --- a/drivers/md/dm.c >> +++ b/drivers/md/dm.c >> @@ -1093,7 +1093,8 @@ static struct dm_target *dm_dax_get_live_target(struct mapped_device *md, >> } >> >> static long dm_dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff, >> - long nr_pages, void **kaddr, pfn_t *pfn) >> + long nr_pages, enum dax_access_mode mode, void **kaddr, >> + pfn_t *pfn) >> { >> struct mapped_device *md = dax_get_private(dax_dev); >> sector_t sector = pgoff * PAGE_SECTORS; >> @@ -1111,7 +1112,7 @@ static long dm_dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff, >> if (len < 1) >> goto out; >> nr_pages = min(len, nr_pages); >> - ret = ti->type->direct_access(ti, pgoff, nr_pages, kaddr, pfn); >> + ret = ti->type->direct_access(ti, pgoff, nr_pages, mode, kaddr, pfn); >> >> out: >> dm_put_live_table(md, srcu_idx); >> diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c >> index 4aa17132a557..c77b7cf19639 100644 >> --- a/drivers/nvdimm/pmem.c >> +++ b/drivers/nvdimm/pmem.c >> @@ -239,24 +239,47 @@ static int pmem_rw_page(struct block_device *bdev, sector_t sector, >> >> /* see "strong" declaration in tools/testing/nvdimm/pmem-dax.c */ >> __weak long __pmem_direct_access(struct pmem_device *pmem, pgoff_t pgoff, >> - long nr_pages, void **kaddr, pfn_t *pfn) >> + long nr_pages, enum dax_access_mode mode, void **kaddr, >> + pfn_t *pfn) >> { >> resource_size_t offset = PFN_PHYS(pgoff) + pmem->data_offset; >> - >> - if (unlikely(is_bad_pmem(&pmem->bb, PFN_PHYS(pgoff) / 512, >> - PFN_PHYS(nr_pages)))) >> - return -EIO; >> + sector_t sector = PFN_PHYS(pgoff) >> SECTOR_SHIFT; >> + unsigned int num = PFN_PHYS(nr_pages) >> SECTOR_SHIFT; >> + struct badblocks *bb = &pmem->bb; >> + sector_t first_bad; >> + int num_bad; >> >> if (kaddr) >> *kaddr = pmem->virt_addr + offset; >> if (pfn) >> *pfn = phys_to_pfn_t(pmem->phys_addr + offset, pmem->pfn_flags); >> >> + if (bb->count && >> + badblocks_check(bb, sector, num, &first_bad, &num_bad)) { >> + long actual_nr; >> + >> + if (mode != DAX_RECOVERY_WRITE) >> + return -EIO; >> + >> + /* >> + * Set the recovery stride is set to kernel page size because >> + * the underlying driver and firmware clear poison functions >> + * don't appear to handle large chunk(such as 2MiB) reliably. >> + */ >> + actual_nr = PHYS_PFN( >> + PAGE_ALIGN((first_bad - sector) << SECTOR_SHIFT)); >> + dev_dbg(pmem->bb.dev, "start sector(%llu), nr_pages(%ld), first_bad(%llu), actual_nr(%ld)\n", >> + sector, nr_pages, first_bad, actual_nr); >> + if (actual_nr) >> + return actual_nr; >> + return 1; >> + } >> + > > Similar feedback as Christoph, lets keep this patch just to the simple > task of plumbing the @mode argument to dax_direct_access() and save > the logic changes for DAX_RECOVERY_WRITE to a later patch. The idea > being that in general you want to limit the blast radius of > regressions so that it simplifies reverts if necessary. If these logic > changes have a regression then reverting this patch also undoes all > the other innocent boilerplate plumbing. Got it, will do. thanks! -jane
diff --git a/drivers/dax/super.c b/drivers/dax/super.c index 0211e6f7b47a..5405eb553430 100644 --- a/drivers/dax/super.c +++ b/drivers/dax/super.c @@ -117,6 +117,7 @@ enum dax_device_flags { * @dax_dev: a dax_device instance representing the logical memory range * @pgoff: offset in pages from the start of the device to translate * @nr_pages: number of consecutive pages caller can handle relative to @pfn + * @mode: indicator on normal access or recovery write * @kaddr: output parameter that returns a virtual address mapping of pfn * @pfn: output parameter that returns an absolute pfn translation of @pgoff * @@ -124,7 +125,7 @@ enum dax_device_flags { * pages accessible at the device relative @pgoff. */ long dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff, long nr_pages, - void **kaddr, pfn_t *pfn) + enum dax_access_mode mode, void **kaddr, pfn_t *pfn) { long avail; @@ -138,7 +139,7 @@ long dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff, long nr_pages, return -EINVAL; avail = dax_dev->ops->direct_access(dax_dev, pgoff, nr_pages, - kaddr, pfn); + mode, kaddr, pfn); if (!avail) return -ERANGE; return min(avail, nr_pages); diff --git a/drivers/md/dm-linear.c b/drivers/md/dm-linear.c index 76b486e4d2be..13e263299c9c 100644 --- a/drivers/md/dm-linear.c +++ b/drivers/md/dm-linear.c @@ -172,11 +172,12 @@ static struct dax_device *linear_dax_pgoff(struct dm_target *ti, pgoff_t *pgoff) } static long linear_dax_direct_access(struct dm_target *ti, pgoff_t pgoff, - long nr_pages, void **kaddr, pfn_t *pfn) + long nr_pages, enum dax_access_mode mode, void **kaddr, + pfn_t *pfn) { struct dax_device *dax_dev = linear_dax_pgoff(ti, &pgoff); - return dax_direct_access(dax_dev, pgoff, nr_pages, kaddr, pfn); + return dax_direct_access(dax_dev, pgoff, nr_pages, mode, kaddr, pfn); } static int linear_dax_zero_page_range(struct dm_target *ti, pgoff_t pgoff, diff --git a/drivers/md/dm-log-writes.c b/drivers/md/dm-log-writes.c index c9d036d6bb2e..06bdbed65eb1 100644 --- a/drivers/md/dm-log-writes.c +++ b/drivers/md/dm-log-writes.c @@ -889,11 +889,12 @@ static struct dax_device *log_writes_dax_pgoff(struct dm_target *ti, } static long log_writes_dax_direct_access(struct dm_target *ti, pgoff_t pgoff, - long nr_pages, void **kaddr, pfn_t *pfn) + long nr_pages, enum dax_access_mode mode, void **kaddr, + pfn_t *pfn) { struct dax_device *dax_dev = log_writes_dax_pgoff(ti, &pgoff); - return dax_direct_access(dax_dev, pgoff, nr_pages, kaddr, pfn); + return dax_direct_access(dax_dev, pgoff, nr_pages, mode, kaddr, pfn); } static int log_writes_dax_zero_page_range(struct dm_target *ti, pgoff_t pgoff, diff --git a/drivers/md/dm-stripe.c b/drivers/md/dm-stripe.c index c81d331d1afe..77d72900e997 100644 --- a/drivers/md/dm-stripe.c +++ b/drivers/md/dm-stripe.c @@ -315,11 +315,12 @@ static struct dax_device *stripe_dax_pgoff(struct dm_target *ti, pgoff_t *pgoff) } static long stripe_dax_direct_access(struct dm_target *ti, pgoff_t pgoff, - long nr_pages, void **kaddr, pfn_t *pfn) + long nr_pages, enum dax_access_mode mode, void **kaddr, + pfn_t *pfn) { struct dax_device *dax_dev = stripe_dax_pgoff(ti, &pgoff); - return dax_direct_access(dax_dev, pgoff, nr_pages, kaddr, pfn); + return dax_direct_access(dax_dev, pgoff, nr_pages, mode, kaddr, pfn); } static int stripe_dax_zero_page_range(struct dm_target *ti, pgoff_t pgoff, diff --git a/drivers/md/dm-target.c b/drivers/md/dm-target.c index 64dd0b34fcf4..8cd5184e62f0 100644 --- a/drivers/md/dm-target.c +++ b/drivers/md/dm-target.c @@ -10,6 +10,7 @@ #include <linux/init.h> #include <linux/kmod.h> #include <linux/bio.h> +#include <linux/dax.h> #define DM_MSG_PREFIX "target" @@ -142,7 +143,8 @@ static void io_err_release_clone_rq(struct request *clone, } static long io_err_dax_direct_access(struct dm_target *ti, pgoff_t pgoff, - long nr_pages, void **kaddr, pfn_t *pfn) + long nr_pages, enum dax_access_mode mode, void **kaddr, + pfn_t *pfn) { return -EIO; } diff --git a/drivers/md/dm-writecache.c b/drivers/md/dm-writecache.c index 5630b470ba42..d74c5a7a0ab4 100644 --- a/drivers/md/dm-writecache.c +++ b/drivers/md/dm-writecache.c @@ -286,7 +286,8 @@ static int persistent_memory_claim(struct dm_writecache *wc) id = dax_read_lock(); - da = dax_direct_access(wc->ssd_dev->dax_dev, offset, p, &wc->memory_map, &pfn); + da = dax_direct_access(wc->ssd_dev->dax_dev, offset, p, DAX_ACCESS, + &wc->memory_map, &pfn); if (da < 0) { wc->memory_map = NULL; r = da; @@ -308,8 +309,8 @@ static int persistent_memory_claim(struct dm_writecache *wc) i = 0; do { long daa; - daa = dax_direct_access(wc->ssd_dev->dax_dev, offset + i, p - i, - NULL, &pfn); + daa = dax_direct_access(wc->ssd_dev->dax_dev, offset + i, + p - i, DAX_ACCESS, NULL, &pfn); if (daa <= 0) { r = daa ? daa : -EINVAL; goto err3; diff --git a/drivers/md/dm.c b/drivers/md/dm.c index 3c5fad7c4ee6..8258676a352f 100644 --- a/drivers/md/dm.c +++ b/drivers/md/dm.c @@ -1093,7 +1093,8 @@ static struct dm_target *dm_dax_get_live_target(struct mapped_device *md, } static long dm_dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff, - long nr_pages, void **kaddr, pfn_t *pfn) + long nr_pages, enum dax_access_mode mode, void **kaddr, + pfn_t *pfn) { struct mapped_device *md = dax_get_private(dax_dev); sector_t sector = pgoff * PAGE_SECTORS; @@ -1111,7 +1112,7 @@ static long dm_dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff, if (len < 1) goto out; nr_pages = min(len, nr_pages); - ret = ti->type->direct_access(ti, pgoff, nr_pages, kaddr, pfn); + ret = ti->type->direct_access(ti, pgoff, nr_pages, mode, kaddr, pfn); out: dm_put_live_table(md, srcu_idx); diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c index 4aa17132a557..c77b7cf19639 100644 --- a/drivers/nvdimm/pmem.c +++ b/drivers/nvdimm/pmem.c @@ -239,24 +239,47 @@ static int pmem_rw_page(struct block_device *bdev, sector_t sector, /* see "strong" declaration in tools/testing/nvdimm/pmem-dax.c */ __weak long __pmem_direct_access(struct pmem_device *pmem, pgoff_t pgoff, - long nr_pages, void **kaddr, pfn_t *pfn) + long nr_pages, enum dax_access_mode mode, void **kaddr, + pfn_t *pfn) { resource_size_t offset = PFN_PHYS(pgoff) + pmem->data_offset; - - if (unlikely(is_bad_pmem(&pmem->bb, PFN_PHYS(pgoff) / 512, - PFN_PHYS(nr_pages)))) - return -EIO; + sector_t sector = PFN_PHYS(pgoff) >> SECTOR_SHIFT; + unsigned int num = PFN_PHYS(nr_pages) >> SECTOR_SHIFT; + struct badblocks *bb = &pmem->bb; + sector_t first_bad; + int num_bad; if (kaddr) *kaddr = pmem->virt_addr + offset; if (pfn) *pfn = phys_to_pfn_t(pmem->phys_addr + offset, pmem->pfn_flags); + if (bb->count && + badblocks_check(bb, sector, num, &first_bad, &num_bad)) { + long actual_nr; + + if (mode != DAX_RECOVERY_WRITE) + return -EIO; + + /* + * Set the recovery stride is set to kernel page size because + * the underlying driver and firmware clear poison functions + * don't appear to handle large chunk(such as 2MiB) reliably. + */ + actual_nr = PHYS_PFN( + PAGE_ALIGN((first_bad - sector) << SECTOR_SHIFT)); + dev_dbg(pmem->bb.dev, "start sector(%llu), nr_pages(%ld), first_bad(%llu), actual_nr(%ld)\n", + sector, nr_pages, first_bad, actual_nr); + if (actual_nr) + return actual_nr; + return 1; + } + /* - * If badblocks are present, limit known good range to the - * requested range. + * If badblocks are present but not in the range, limit known good range + * to the requested range. */ - if (unlikely(pmem->bb.count)) + if (bb->count) return nr_pages; return PHYS_PFN(pmem->size - pmem->pfn_pad - offset); } @@ -278,11 +301,12 @@ static int pmem_dax_zero_page_range(struct dax_device *dax_dev, pgoff_t pgoff, } static long pmem_dax_direct_access(struct dax_device *dax_dev, - pgoff_t pgoff, long nr_pages, void **kaddr, pfn_t *pfn) + pgoff_t pgoff, long nr_pages, enum dax_access_mode mode, + void **kaddr, pfn_t *pfn) { struct pmem_device *pmem = dax_get_private(dax_dev); - return __pmem_direct_access(pmem, pgoff, nr_pages, kaddr, pfn); + return __pmem_direct_access(pmem, pgoff, nr_pages, mode, kaddr, pfn); } static const struct dax_operations pmem_dax_ops = { diff --git a/drivers/nvdimm/pmem.h b/drivers/nvdimm/pmem.h index 1f51a2361429..392b0b38acb9 100644 --- a/drivers/nvdimm/pmem.h +++ b/drivers/nvdimm/pmem.h @@ -8,6 +8,8 @@ #include <linux/pfn_t.h> #include <linux/fs.h> +enum dax_access_mode; + /* this definition is in it's own header for tools/testing/nvdimm to consume */ struct pmem_device { /* One contiguous memory region per device */ @@ -28,7 +30,8 @@ struct pmem_device { }; long __pmem_direct_access(struct pmem_device *pmem, pgoff_t pgoff, - long nr_pages, void **kaddr, pfn_t *pfn); + long nr_pages, enum dax_access_mode mode, void **kaddr, + pfn_t *pfn); #ifdef CONFIG_MEMORY_FAILURE static inline bool test_and_clear_pmem_poison(struct page *page) diff --git a/drivers/s390/block/dcssblk.c b/drivers/s390/block/dcssblk.c index d614843caf6c..8d0d0eaa3059 100644 --- a/drivers/s390/block/dcssblk.c +++ b/drivers/s390/block/dcssblk.c @@ -32,7 +32,8 @@ static int dcssblk_open(struct block_device *bdev, fmode_t mode); static void dcssblk_release(struct gendisk *disk, fmode_t mode); static void dcssblk_submit_bio(struct bio *bio); static long dcssblk_dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff, - long nr_pages, void **kaddr, pfn_t *pfn); + long nr_pages, enum dax_access_mode mode, void **kaddr, + pfn_t *pfn); static char dcssblk_segments[DCSSBLK_PARM_LEN] = "\0"; @@ -50,7 +51,8 @@ static int dcssblk_dax_zero_page_range(struct dax_device *dax_dev, long rc; void *kaddr; - rc = dax_direct_access(dax_dev, pgoff, nr_pages, &kaddr, NULL); + rc = dax_direct_access(dax_dev, pgoff, nr_pages, DAX_ACCESS, + &kaddr, NULL); if (rc < 0) return rc; memset(kaddr, 0, nr_pages << PAGE_SHIFT); @@ -927,7 +929,8 @@ __dcssblk_direct_access(struct dcssblk_dev_info *dev_info, pgoff_t pgoff, static long dcssblk_dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff, - long nr_pages, void **kaddr, pfn_t *pfn) + long nr_pages, enum dax_access_mode mode, void **kaddr, + pfn_t *pfn) { struct dcssblk_dev_info *dev_info = dax_get_private(dax_dev); diff --git a/fs/dax.c b/fs/dax.c index 67a08a32fccb..ef3103107104 100644 --- a/fs/dax.c +++ b/fs/dax.c @@ -721,7 +721,8 @@ static int copy_cow_page_dax(struct vm_fault *vmf, const struct iomap_iter *iter int id; id = dax_read_lock(); - rc = dax_direct_access(iter->iomap.dax_dev, pgoff, 1, &kaddr, NULL); + rc = dax_direct_access(iter->iomap.dax_dev, pgoff, 1, DAX_ACCESS, + &kaddr, NULL); if (rc < 0) { dax_read_unlock(id); return rc; @@ -1013,7 +1014,7 @@ static int dax_iomap_pfn(const struct iomap *iomap, loff_t pos, size_t size, id = dax_read_lock(); length = dax_direct_access(iomap->dax_dev, pgoff, PHYS_PFN(size), - NULL, pfnp); + DAX_ACCESS, NULL, pfnp); if (length < 0) { rc = length; goto out; @@ -1122,7 +1123,7 @@ static int dax_memzero(struct dax_device *dax_dev, pgoff_t pgoff, void *kaddr; long ret; - ret = dax_direct_access(dax_dev, pgoff, 1, &kaddr, NULL); + ret = dax_direct_access(dax_dev, pgoff, 1, DAX_ACCESS, &kaddr, NULL); if (ret > 0) { memset(kaddr + offset, 0, size); dax_flush(dax_dev, kaddr + offset, size); @@ -1247,7 +1248,7 @@ static loff_t dax_iomap_iter(const struct iomap_iter *iomi, } map_len = dax_direct_access(dax_dev, pgoff, PHYS_PFN(size), - &kaddr, NULL); + DAX_ACCESS, &kaddr, NULL); if (map_len < 0) { ret = map_len; break; diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c index d7d3a7f06862..10eb50cbf398 100644 --- a/fs/fuse/dax.c +++ b/fs/fuse/dax.c @@ -1241,8 +1241,8 @@ static int fuse_dax_mem_range_init(struct fuse_conn_dax *fcd) INIT_DELAYED_WORK(&fcd->free_work, fuse_dax_free_mem_worker); id = dax_read_lock(); - nr_pages = dax_direct_access(fcd->dev, 0, PHYS_PFN(dax_size), NULL, - NULL); + nr_pages = dax_direct_access(fcd->dev, 0, PHYS_PFN(dax_size), + DAX_ACCESS, NULL, NULL); dax_read_unlock(id); if (nr_pages < 0) { pr_debug("dax_direct_access() returned %ld\n", nr_pages); diff --git a/include/linux/dax.h b/include/linux/dax.h index 9fc5f99a0ae2..3f1339bce3c0 100644 --- a/include/linux/dax.h +++ b/include/linux/dax.h @@ -14,6 +14,11 @@ struct iomap_ops; struct iomap_iter; struct iomap; +enum dax_access_mode { + DAX_ACCESS, + DAX_RECOVERY_WRITE, +}; + struct dax_operations { /* * direct_access: translate a device-relative @@ -21,7 +26,7 @@ struct dax_operations { * number of pages available for DAX at that pfn. */ long (*direct_access)(struct dax_device *, pgoff_t, long, - void **, pfn_t *); + enum dax_access_mode, void **, pfn_t *); /* * Validate whether this device is usable as an fsdax backing * device. @@ -178,7 +183,7 @@ static inline void dax_read_unlock(int id) bool dax_alive(struct dax_device *dax_dev); void *dax_get_private(struct dax_device *dax_dev); long dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff, long nr_pages, - void **kaddr, pfn_t *pfn); + enum dax_access_mode mode, void **kaddr, pfn_t *pfn); size_t dax_copy_from_iter(struct dax_device *dax_dev, pgoff_t pgoff, void *addr, size_t bytes, struct iov_iter *i); size_t dax_copy_to_iter(struct dax_device *dax_dev, pgoff_t pgoff, void *addr, diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h index c2a3758c4aaa..acdedda0d12b 100644 --- a/include/linux/device-mapper.h +++ b/include/linux/device-mapper.h @@ -20,6 +20,7 @@ struct dm_table; struct dm_report_zones_args; struct mapped_device; struct bio_vec; +enum dax_access_mode; /* * Type of table, mapped_device's mempool and request_queue @@ -146,7 +147,8 @@ typedef int (*dm_busy_fn) (struct dm_target *ti); * >= 0 : the number of bytes accessible at the address */ typedef long (*dm_dax_direct_access_fn) (struct dm_target *ti, pgoff_t pgoff, - long nr_pages, void **kaddr, pfn_t *pfn); + long nr_pages, enum dax_access_mode node, void **kaddr, + pfn_t *pfn); typedef int (*dm_dax_zero_page_range_fn)(struct dm_target *ti, pgoff_t pgoff, size_t nr_pages); diff --git a/tools/testing/nvdimm/pmem-dax.c b/tools/testing/nvdimm/pmem-dax.c index af19c85558e7..dcc328eba811 100644 --- a/tools/testing/nvdimm/pmem-dax.c +++ b/tools/testing/nvdimm/pmem-dax.c @@ -8,7 +8,8 @@ #include <nd.h> long __pmem_direct_access(struct pmem_device *pmem, pgoff_t pgoff, - long nr_pages, void **kaddr, pfn_t *pfn) + long nr_pages, enum dax_access_mode mode, void **kaddr, + pfn_t *pfn) { resource_size_t offset = PFN_PHYS(pgoff) + pmem->data_offset;
Up till now, dax_direct_access() is used implicitly for normal access, but for the purpose of recovery write, dax range with poison is requested. To make the interface clear, introduce enum dax_access_mode { DAX_ACCESS, DAX_RECOVERY_WRITE, } where DAX_ACCESS is used for normal dax access, and DAX_RECOVERY_WRITE is used for dax recovery write. Suggested-by: Dan Williams <dan.j.williams@intel.com> Signed-off-by: Jane Chu <jane.chu@oracle.com> --- drivers/dax/super.c | 5 ++-- drivers/md/dm-linear.c | 5 ++-- drivers/md/dm-log-writes.c | 5 ++-- drivers/md/dm-stripe.c | 5 ++-- drivers/md/dm-target.c | 4 ++- drivers/md/dm-writecache.c | 7 +++--- drivers/md/dm.c | 5 ++-- drivers/nvdimm/pmem.c | 44 +++++++++++++++++++++++++-------- drivers/nvdimm/pmem.h | 5 +++- drivers/s390/block/dcssblk.c | 9 ++++--- fs/dax.c | 9 ++++--- fs/fuse/dax.c | 4 +-- include/linux/dax.h | 9 +++++-- include/linux/device-mapper.h | 4 ++- tools/testing/nvdimm/pmem-dax.c | 3 ++- 15 files changed, 85 insertions(+), 38 deletions(-)