| Message ID | 1437518265-14294-3-git-send-email-vishal.l.verma@intel.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Vishal Verma |
| Headers | show |
On Tue, Jul 21, 2015 at 3:37 PM, Vishal Verma <vishal.l.verma@intel.com> wrote: > When a BTT is instantiated on a namespace it must validate the namespace > uuid matches the 'parent_uuid' stored in the btt superblock. This > property enforces that changing the namespace UUID invalidates all > former BTT instances on that storage. For "IO namespaces" that don't > have a label or UUID, the parent_uuid is set to zero, and this > validation is skipped. For such cases, old BTTs have to be invalidated > by forcing the namespace to raw mode, and overwriting the BTT info > blocks. > > Based on a patch by Dan Williams <dan.j.williams@intel.com> > > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> > --- > drivers/nvdimm/btt.c | 17 +++++++++++++++++ > drivers/nvdimm/core.c | 20 ++++++++++++++++++++ > drivers/nvdimm/namespace_devs.c | 6 +++--- > drivers/nvdimm/nd-core.h | 3 +++ > drivers/nvdimm/nd.h | 1 + > 5 files changed, 44 insertions(+), 3 deletions(-) > > diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c > index 87e6a96..81781db 100644 > --- a/drivers/nvdimm/btt.c > +++ b/drivers/nvdimm/btt.c > @@ -582,16 +582,31 @@ static void free_arenas(struct btt *btt) > } > } > > +static int uuid_is_null(u8 *uuid) Seems this should be returning bool instead of int? > +{ > + static const u8 null_uuid[16]; > + > + return !memcmp(uuid, null_uuid, 16); I prefer "memcmp() == 0" for the positive match case. "!memcmp()" reads like failure. > +} > + > /* > * This function checks if the metadata layout is valid and error free > */ > static int arena_is_valid(struct nd_btt *nd_btt, struct btt_sb *super) > { > + const u8 *parent_uuid = nd_dev_to_uuid(&nd_btt->ndns->dev); > u64 checksum; > > + if (memcmp(super->signature, BTT_SIG, BTT_SIG_LEN) != 0) > + return 0; > + > if (memcmp(super->uuid, nd_btt->uuid, 16)) > return 0; > > + if (!uuid_is_null(super->parent_uuid)) > + if (memcmp(super->parent_uuid, parent_uuid, 16)) Similar memcmp polarity comment. I'd expect to error out early in this routine in the "if (memcmp() != 0)" case. > + return 0; > + > checksum = le64_to_cpu(super->checksum); > super->checksum = 0; > if (checksum != nd_btt_sb_checksum(super)) > @@ -759,6 +774,7 @@ static int btt_arena_write_layout(struct arena_info *arena) > int ret; > struct btt_sb *super; > struct nd_btt *nd_btt = arena->nd_btt; > + const u8 *parent_uuid = nd_dev_to_uuid(&nd_btt->ndns->dev); > > ret = btt_map_init(arena); > if (ret) > @@ -774,6 +790,7 @@ static int btt_arena_write_layout(struct arena_info *arena) > > strncpy(super->signature, BTT_SIG, BTT_SIG_LEN); > memcpy(super->uuid, nd_btt->uuid, 16); > + memcpy(super->parent_uuid, parent_uuid, 16); > super->flags = cpu_to_le32(arena->flags); > super->version_major = cpu_to_le16(arena->version_major); > super->version_minor = cpu_to_le16(arena->version_minor); > diff --git a/drivers/nvdimm/core.c b/drivers/nvdimm/core.c > index cb62ec6..4d6e7ad 100644 > --- a/drivers/nvdimm/core.c > +++ b/drivers/nvdimm/core.c > @@ -174,6 +174,26 @@ int nd_uuid_store(struct device *dev, u8 **uuid_out, const char *buf, > return 0; > } > > +const u8 *nd_dev_to_uuid(struct device *dev) > +{ > + static const u8 null_uuid[16]; > + > + if (!dev) > + return null_uuid; > + > + if (is_namespace_pmem(dev)) { > + struct nd_namespace_pmem *nspm = to_nd_namespace_pmem(dev); > + > + return nspm->uuid; > + } else if (is_namespace_blk(dev)) { > + struct nd_namespace_blk *nsblk = to_nd_namespace_blk(dev); > + > + return nsblk->uuid; > + } else > + return null_uuid; > +} > +EXPORT_SYMBOL(nd_dev_to_uuid); This nd_dev_to_uuid() helper really wants to be in drivers/nvdimm/namespace_devs.c... > + > ssize_t nd_sector_size_show(unsigned long current_lbasize, > const unsigned long *supported, char *buf) > { > diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c > index fef0dd8..1be0e3b 100644 > --- a/drivers/nvdimm/namespace_devs.c > +++ b/drivers/nvdimm/namespace_devs.c > @@ -61,17 +61,17 @@ static struct device_type namespace_blk_device_type = { > .release = namespace_blk_release, > }; > > -static bool is_namespace_pmem(struct device *dev) > +bool is_namespace_pmem(struct device *dev) > { > return dev ? dev->type == &namespace_pmem_device_type : false; > } > > -static bool is_namespace_blk(struct device *dev) > +bool is_namespace_blk(struct device *dev) > { > return dev ? dev->type == &namespace_blk_device_type : false; > } > > -static bool is_namespace_io(struct device *dev) > +bool is_namespace_io(struct device *dev) > { > return dev ? dev->type == &namespace_io_device_type : false; > } ...so that we don't have to make these is_namespace_*() helpers public... > diff --git a/drivers/nvdimm/nd-core.h b/drivers/nvdimm/nd-core.h > index e1970c7..51f1e0b 100644 > --- a/drivers/nvdimm/nd-core.h > +++ b/drivers/nvdimm/nd-core.h > @@ -45,6 +45,9 @@ struct nvdimm { > bool is_nvdimm(struct device *dev); > bool is_nd_pmem(struct device *dev); > bool is_nd_blk(struct device *dev); > +bool is_namespace_pmem(struct device *dev); > +bool is_namespace_blk(struct device *dev); > +bool is_namespace_io(struct device *dev); ...and don't need to declare them.
diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c index 87e6a96..81781db 100644 --- a/drivers/nvdimm/btt.c +++ b/drivers/nvdimm/btt.c @@ -582,16 +582,31 @@ static void free_arenas(struct btt *btt) } } +static int uuid_is_null(u8 *uuid) +{ + static const u8 null_uuid[16]; + + return !memcmp(uuid, null_uuid, 16); +} + /* * This function checks if the metadata layout is valid and error free */ static int arena_is_valid(struct nd_btt *nd_btt, struct btt_sb *super) { + const u8 *parent_uuid = nd_dev_to_uuid(&nd_btt->ndns->dev); u64 checksum; + if (memcmp(super->signature, BTT_SIG, BTT_SIG_LEN) != 0) + return 0; + if (memcmp(super->uuid, nd_btt->uuid, 16)) return 0; + if (!uuid_is_null(super->parent_uuid)) + if (memcmp(super->parent_uuid, parent_uuid, 16)) + return 0; + checksum = le64_to_cpu(super->checksum); super->checksum = 0; if (checksum != nd_btt_sb_checksum(super)) @@ -759,6 +774,7 @@ static int btt_arena_write_layout(struct arena_info *arena) int ret; struct btt_sb *super; struct nd_btt *nd_btt = arena->nd_btt; + const u8 *parent_uuid = nd_dev_to_uuid(&nd_btt->ndns->dev); ret = btt_map_init(arena); if (ret) @@ -774,6 +790,7 @@ static int btt_arena_write_layout(struct arena_info *arena) strncpy(super->signature, BTT_SIG, BTT_SIG_LEN); memcpy(super->uuid, nd_btt->uuid, 16); + memcpy(super->parent_uuid, parent_uuid, 16); super->flags = cpu_to_le32(arena->flags); super->version_major = cpu_to_le16(arena->version_major); super->version_minor = cpu_to_le16(arena->version_minor); diff --git a/drivers/nvdimm/core.c b/drivers/nvdimm/core.c index cb62ec6..4d6e7ad 100644 --- a/drivers/nvdimm/core.c +++ b/drivers/nvdimm/core.c @@ -174,6 +174,26 @@ int nd_uuid_store(struct device *dev, u8 **uuid_out, const char *buf, return 0; } +const u8 *nd_dev_to_uuid(struct device *dev) +{ + static const u8 null_uuid[16]; + + if (!dev) + return null_uuid; + + if (is_namespace_pmem(dev)) { + struct nd_namespace_pmem *nspm = to_nd_namespace_pmem(dev); + + return nspm->uuid; + } else if (is_namespace_blk(dev)) { + struct nd_namespace_blk *nsblk = to_nd_namespace_blk(dev); + + return nsblk->uuid; + } else + return null_uuid; +} +EXPORT_SYMBOL(nd_dev_to_uuid); + ssize_t nd_sector_size_show(unsigned long current_lbasize, const unsigned long *supported, char *buf) { diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c index fef0dd8..1be0e3b 100644 --- a/drivers/nvdimm/namespace_devs.c +++ b/drivers/nvdimm/namespace_devs.c @@ -61,17 +61,17 @@ static struct device_type namespace_blk_device_type = { .release = namespace_blk_release, }; -static bool is_namespace_pmem(struct device *dev) +bool is_namespace_pmem(struct device *dev) { return dev ? dev->type == &namespace_pmem_device_type : false; } -static bool is_namespace_blk(struct device *dev) +bool is_namespace_blk(struct device *dev) { return dev ? dev->type == &namespace_blk_device_type : false; } -static bool is_namespace_io(struct device *dev) +bool is_namespace_io(struct device *dev) { return dev ? dev->type == &namespace_io_device_type : false; } diff --git a/drivers/nvdimm/nd-core.h b/drivers/nvdimm/nd-core.h index e1970c7..51f1e0b 100644 --- a/drivers/nvdimm/nd-core.h +++ b/drivers/nvdimm/nd-core.h @@ -45,6 +45,9 @@ struct nvdimm { bool is_nvdimm(struct device *dev); bool is_nd_pmem(struct device *dev); bool is_nd_blk(struct device *dev); +bool is_namespace_pmem(struct device *dev); +bool is_namespace_blk(struct device *dev); +bool is_namespace_io(struct device *dev); struct nvdimm_bus *walk_to_nvdimm_bus(struct device *nd_dev); int __init nvdimm_bus_init(void); void nvdimm_bus_exit(void); diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h index c41f53e74..7e03122 100644 --- a/drivers/nvdimm/nd.h +++ b/drivers/nvdimm/nd.h @@ -217,4 +217,5 @@ static inline bool nd_iostat_start(struct bio *bio, unsigned long *start) } void nd_iostat_end(struct bio *bio, unsigned long start); resource_size_t nd_namespace_blk_validate(struct nd_namespace_blk *nsblk); +const u8 *nd_dev_to_uuid(struct device *dev); #endif /* __ND_H__ */
When a BTT is instantiated on a namespace it must validate the namespace uuid matches the 'parent_uuid' stored in the btt superblock. This property enforces that changing the namespace UUID invalidates all former BTT instances on that storage. For "IO namespaces" that don't have a label or UUID, the parent_uuid is set to zero, and this validation is skipped. For such cases, old BTTs have to be invalidated by forcing the namespace to raw mode, and overwriting the BTT info blocks. Based on a patch by Dan Williams <dan.j.williams@intel.com> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> --- drivers/nvdimm/btt.c | 17 +++++++++++++++++ drivers/nvdimm/core.c | 20 ++++++++++++++++++++ drivers/nvdimm/namespace_devs.c | 6 +++--- drivers/nvdimm/nd-core.h | 3 +++ drivers/nvdimm/nd.h | 1 + 5 files changed, 44 insertions(+), 3 deletions(-)