| Message ID | 147916930808.236748.8215175004635279925.stgit@djiang5-desk3.ch.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 2016-11-14 at 17:21 -0700, Dave Jiang wrote: > Clearing out the poison in the metadata block of the namespace before > we use it. Range from start + 8k to pfn_sb->dataoff. > > Signed-off-by: Dave Jiang <dave.jiang@intel.com> > --- > drivers/nvdimm/pfn_devs.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c > index cea8350..ff3b55d 100644 > --- a/drivers/nvdimm/pfn_devs.c > +++ b/drivers/nvdimm/pfn_devs.c > @@ -527,11 +527,35 @@ static struct vmem_altmap > *__nvdimm_setup_pfn(struct nd_pfn *nd_pfn, > .base_pfn = init_altmap_base(base), > .reserve = init_altmap_reserve(base), > }; > + sector_t sector; > + resource_size_t meta_start, meta_size; > + long cleared; > + unsigned int sz_align; > > memcpy(res, &nsio->res, sizeof(*res)); > res->start += start_pad; > res->end -= end_trunc; > > + meta_start = res->start + SZ_8K; > + meta_size = offset - meta_start + 1; > + > + if (meta_start + meta_size > offset) > + return ERR_PTR(-EINVAL); > + > + sector = meta_start >> 9; > + sz_align = ALIGN(meta_size + (meta_start & (512 - 1)), 512); Should this be ALIGN_UP? > + > + if (unlikely(is_bad_pmem(&nsio->bb, sector, sz_align))) { > + if (IS_ALIGNED(meta_start, 512) && > IS_ALIGNED(meta_size, 512)) { > + cleared = nvdimm_clear_poison(&nd_pfn->dev, > + meta_start, > meta_size); > + badblocks_clear(&nsio->bb, sector, cleared > >> 9); 'cleared' can be a negative error value, so I do not think you can simply pass it to badblocks_clear(). Thanks, -Toshi
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 On 11/17/2016 09:35 AM, Kani, Toshimitsu wrote: > On Mon, 2016-11-14 at 17:21 -0700, Dave Jiang wrote: >> Clearing out the poison in the metadata block of the namespace >> before we use it. Range from start + 8k to pfn_sb->dataoff. >> >> Signed-off-by: Dave Jiang <dave.jiang@intel.com> --- >> drivers/nvdimm/pfn_devs.c | 24 ++++++++++++++++++++++++ 1 file >> changed, 24 insertions(+) >> >> diff --git a/drivers/nvdimm/pfn_devs.c >> b/drivers/nvdimm/pfn_devs.c index cea8350..ff3b55d 100644 --- >> a/drivers/nvdimm/pfn_devs.c +++ b/drivers/nvdimm/pfn_devs.c @@ >> -527,11 +527,35 @@ static struct vmem_altmap >> *__nvdimm_setup_pfn(struct nd_pfn *nd_pfn, .base_pfn = >> init_altmap_base(base), .reserve = init_altmap_reserve(base), }; >> +sector_t sector; +resource_size_t meta_start, meta_size; +long >> cleared; +unsigned int sz_align; >> >> memcpy(res, &nsio->res, sizeof(*res)); res->start += start_pad; >> res->end -= end_trunc; >> >> +meta_start = res->start + SZ_8K; +meta_size = offset - >> meta_start + 1; + +if (meta_start + meta_size > offset) +return >> ERR_PTR(-EINVAL); + +sector = meta_start >> 9; +sz_align = >> ALIGN(meta_size + (meta_start & (512 - 1)), 512); > > Should this be ALIGN_UP? I guess that makes sense to me, are there any bad side effects if we round up? Dan? I think there are a few other places we should update if we make the change since this code was borrowed from somewhere else. > >> + +if (unlikely(is_bad_pmem(&nsio->bb, sector, sz_align))) { +if >> (IS_ALIGNED(meta_start, 512) && IS_ALIGNED(meta_size, 512)) { >> +cleared = nvdimm_clear_poison(&nd_pfn->dev, + meta_start, >> meta_size); +badblocks_clear(&nsio->bb, sector, cleared >>>> 9); > > 'cleared' can be a negative error value, so I do not think you can > simply pass it to badblocks_clear(). Good catch! I'll fix. > > Thanks, -Toshi > -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iF4EAREIAAYFAlgt5/YACgkQZBXE8WajuT6iqAD7B6x6kN3ZyakCk7Q5wimnCkdN CZNUPXFhvuw0w8EBSxoA+gJgyL8i3dGd7/JUDrMXPrsi640o2LGVx77U4t9zT8zH =Y+NF -----END PGP SIGNATURE-----
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 On 11/17/2016 10:25 AM, Dave Jiang wrote: > On 11/17/2016 09:35 AM, Kani, Toshimitsu wrote: >> On Mon, 2016-11-14 at 17:21 -0700, Dave Jiang wrote: >>> Clearing out the poison in the metadata block of the namespace >>> before we use it. Range from start + 8k to pfn_sb->dataoff. >>> >>> Signed-off-by: Dave Jiang <dave.jiang@intel.com> --- >>> drivers/nvdimm/pfn_devs.c | 24 ++++++++++++++++++++++++ 1 >>> file changed, 24 insertions(+) >>> >>> diff --git a/drivers/nvdimm/pfn_devs.c >>> b/drivers/nvdimm/pfn_devs.c index cea8350..ff3b55d 100644 --- >>> a/drivers/nvdimm/pfn_devs.c +++ b/drivers/nvdimm/pfn_devs.c @@ >>> -527,11 +527,35 @@ static struct vmem_altmap >>> *__nvdimm_setup_pfn(struct nd_pfn *nd_pfn, .base_pfn = >>> init_altmap_base(base), .reserve = init_altmap_reserve(base), >>> }; +sector_t sector; +resource_size_t meta_start, meta_size; >>> +long cleared; +unsigned int sz_align; >>> >>> memcpy(res, &nsio->res, sizeof(*res)); res->start += start_pad; >>> res->end -= end_trunc; >>> >>> +meta_start = res->start + SZ_8K; +meta_size = offset - >>> meta_start + 1; + +if (meta_start + meta_size > offset) >>> +return ERR_PTR(-EINVAL); + +sector = meta_start >> 9; >>> +sz_align = ALIGN(meta_size + (meta_start & (512 - 1)), 512); > >> Should this be ALIGN_UP? > > I guess that makes sense to me, are there any bad side effects if > we round up? Dan? I think there are a few other places we should > update if we make the change since this code was borrowed from > somewhere else. Actually ALIGN() already aligns up looks like. > > >>> + +if (unlikely(is_bad_pmem(&nsio->bb, sector, sz_align))) { >>> +if (IS_ALIGNED(meta_start, 512) && IS_ALIGNED(meta_size, 512)) >>> { +cleared = nvdimm_clear_poison(&nd_pfn->dev, + >>> meta_start, meta_size); +badblocks_clear(&nsio->bb, sector, >>> cleared >>>>> 9); > >> 'cleared' can be a negative error value, so I do not think you >> can simply pass it to badblocks_clear(). > > Good catch! I'll fix. > > >> Thanks, -Toshi > > -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iF4EAREIAAYFAlgvNAgACgkQZBXE8WajuT4GLgEAp9E4u8yaKPUPy3Zv/VXUqpln VzyJXofVi9Bpji2WJlIBAJ9dw7sinFc4fQLRZTb71BmMDhL0z90uCr5tTJpH/Ij5 =trfV -----END PGP SIGNATURE-----
diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c index cea8350..ff3b55d 100644 --- a/drivers/nvdimm/pfn_devs.c +++ b/drivers/nvdimm/pfn_devs.c @@ -527,11 +527,35 @@ static struct vmem_altmap *__nvdimm_setup_pfn(struct nd_pfn *nd_pfn, .base_pfn = init_altmap_base(base), .reserve = init_altmap_reserve(base), }; + sector_t sector; + resource_size_t meta_start, meta_size; + long cleared; + unsigned int sz_align; memcpy(res, &nsio->res, sizeof(*res)); res->start += start_pad; res->end -= end_trunc; + meta_start = res->start + SZ_8K; + meta_size = offset - meta_start + 1; + + if (meta_start + meta_size > offset) + return ERR_PTR(-EINVAL); + + sector = meta_start >> 9; + sz_align = ALIGN(meta_size + (meta_start & (512 - 1)), 512); + + if (unlikely(is_bad_pmem(&nsio->bb, sector, sz_align))) { + if (IS_ALIGNED(meta_start, 512) && IS_ALIGNED(meta_size, 512)) { + cleared = nvdimm_clear_poison(&nd_pfn->dev, + meta_start, meta_size); + badblocks_clear(&nsio->bb, sector, cleared >> 9); + if (cleared != meta_size) + return ERR_PTR(-EIO); + } else + return ERR_PTR(-EIO); + } + if (nd_pfn->mode == PFN_MODE_RAM) { if (offset < SZ_8K) return ERR_PTR(-EINVAL);
Clearing out the poison in the metadata block of the namespace before we use it. Range from start + 8k to pfn_sb->dataoff. Signed-off-by: Dave Jiang <dave.jiang@intel.com> --- drivers/nvdimm/pfn_devs.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)