| Message ID | 155572004134.2603218.13620079500159805086.stgit@dwillia2-desk3.amr.corp.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Commit | c4703ce11c23423d4b46e3d59aef7979814fd608 |
| Headers | show |
| Series | libnvdimm/namespace: Fix label tracking error | expand |
Hi,
[This is an automated email]
This commit has been processed because it contains a "Fixes:" tag,
fixing commit: bf9bccc14c05 libnvdimm: pmem label sets and namespace instantiation..
The bot has tested the following trees: v5.0.8, v4.19.35, v4.14.112, v4.9.169, v4.4.178,
v5.0.8: Build OK!
v4.19.35: Build OK!
v4.14.112: Failed to apply! Possible dependencies:
426824d63b77 ("libnvdimm: remove redundant __func__ in dev_dbg")
v4.9.169: Failed to apply! Possible dependencies:
006358b35c73 ("libnvdimm: add support for clear poison list and badblocks for device dax")
355d838878e1 ("libnvdimm, label: add v1.2 label checksum support")
426824d63b77 ("libnvdimm: remove redundant __func__ in dev_dbg")
53b85a449b15 ("libnvdimm: passthru functions clear to send")
564e871aa66f ("libnvdimm, label: add v1.2 nvdimm label definitions")
b3fde74ea195 ("libnvdimm, label: add address abstraction identifiers")
c12c48ce869d ("libnvdimm, label: add v1.2 interleave-set-cookie algorithm")
f979b13c3cc5 ("libnvdimm, label: honor the lba size specified in v1.2 labels")
faec6f8a1cd2 ("libnvdimm, label: populate the type_guid property for v1.2 namespaces")
v4.4.178: Failed to apply! Possible dependencies:
0731de0dd95b ("libnvdimm, pfn: move 'memory mode' indication to sysfs")
0bfb8dd3edd6 ("libnvdimm: cleanup nvdimm_namespace_common_probe(), kill 'host'")
0caeef63e6d2 ("libnvdimm: Add a poison list and export badblocks")
0e749e54244e ("dax: increase granularity of dax_clear_blocks() operations")
2dc43331e34f ("libnvdimm, pfn: fix pfn seed creation")
315c562536c4 ("libnvdimm, pfn: add 'align' attribute, default to HPAGE_SIZE")
34c0fd540e79 ("mm, dax, pmem: introduce pfn_t")
4b94ffdc4163 ("x86, mm: introduce vmem_altmap to augment vmemmap_populate()")
52db400fcd50 ("pmem, dax: clean up clear_pmem()")
87ba05dff351 ("libnvdimm: don't fail init for full badblocks list")
9476df7d80df ("mm: introduce find_dev_pagemap()")
9c41242817f4 ("libnvdimm: fix mode determination for e820 devices")
ad9a8bde2cb1 ("libnvdimm, pmem: move definition of nvdimm_namespace_add_poison to nd.h")
b2e0d1625e19 ("dax: fix lifetime of in-kernel dax mappings with dax_map_atomic()")
b3fde74ea195 ("libnvdimm, label: add address abstraction identifiers")
b95f5f4391fa ("libnvdimm: convert to statically allocated badblocks")
bd032943b5b2 ("libnvdimm, pfn, convert nd_pfn_probe() to devm")
c5ed9268643c ("libnvdimm, dax: autodetect support")
cd03412a51ac ("libnvdimm, dax: introduce device-dax infrastructure")
cfe30b872058 ("libnvdimm, pmem: adjust for section collisions with 'System RAM'")
d2c0f041e1bb ("libnvdimm, pfn, pmem: allocate memmap array in persistent memory")
d9cbe09d39aa ("libnvdimm, pmem: fix 'pfn' support for section-misaligned namespaces")
f6ed58c70d14 ("libnvdimm, pfn: 'resource'-address and 'size' attributes for pfn devices")
f7c6ab80fa5f ("libnvdimm, pfn: clean up pfn create parameters")
How should we proceed with this patch?
--
Thanks,
Sasha
diff --git a/drivers/nvdimm/label.c b/drivers/nvdimm/label.c index f3d753d3169c..2030805aa216 100644 --- a/drivers/nvdimm/label.c +++ b/drivers/nvdimm/label.c @@ -756,6 +756,17 @@ static const guid_t *to_abstraction_guid(enum nvdimm_claim_class claim_class, return &guid_null; } +static void reap_victim(struct nd_mapping *nd_mapping, + struct nd_label_ent *victim) +{ + struct nvdimm_drvdata *ndd = to_ndd(nd_mapping); + u32 slot = to_slot(ndd, victim->label); + + dev_dbg(ndd->dev, "free: %d\n", slot); + nd_label_free_slot(ndd, slot); + victim->label = NULL; +} + static int __pmem_label_update(struct nd_region *nd_region, struct nd_mapping *nd_mapping, struct nd_namespace_pmem *nspm, int pos, unsigned long flags) @@ -763,9 +774,9 @@ static int __pmem_label_update(struct nd_region *nd_region, struct nd_namespace_common *ndns = &nspm->nsio.common; struct nd_interleave_set *nd_set = nd_region->nd_set; struct nvdimm_drvdata *ndd = to_ndd(nd_mapping); - struct nd_label_ent *label_ent, *victim = NULL; struct nd_namespace_label *nd_label; struct nd_namespace_index *nsindex; + struct nd_label_ent *label_ent; struct nd_label_id label_id; struct resource *res; unsigned long *free; @@ -834,18 +845,10 @@ static int __pmem_label_update(struct nd_region *nd_region, list_for_each_entry(label_ent, &nd_mapping->labels, list) { if (!label_ent->label) continue; - if (memcmp(nspm->uuid, label_ent->label->uuid, - NSLABEL_UUID_LEN) != 0) - continue; - victim = label_ent; - list_move_tail(&victim->list, &nd_mapping->labels); - break; - } - if (victim) { - dev_dbg(ndd->dev, "free: %d\n", slot); - slot = to_slot(ndd, victim->label); - nd_label_free_slot(ndd, slot); - victim->label = NULL; + if (test_and_clear_bit(ND_LABEL_REAP, &label_ent->flags) + || memcmp(nspm->uuid, label_ent->label->uuid, + NSLABEL_UUID_LEN) == 0) + reap_victim(nd_mapping, label_ent); } /* update index */ diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c index 7849bf1812c4..3e2fdfdbe7b8 100644 --- a/drivers/nvdimm/namespace_devs.c +++ b/drivers/nvdimm/namespace_devs.c @@ -1247,12 +1247,27 @@ static int namespace_update_uuid(struct nd_region *nd_region, for (i = 0; i < nd_region->ndr_mappings; i++) { struct nd_mapping *nd_mapping = &nd_region->mapping[i]; struct nvdimm_drvdata *ndd = to_ndd(nd_mapping); + struct nd_label_ent *label_ent; struct resource *res; for_each_dpa_resource(ndd, res) if (strcmp(res->name, old_label_id.id) == 0) sprintf((void *) res->name, "%s", new_label_id.id); + + mutex_lock(&nd_mapping->lock); + list_for_each_entry(label_ent, &nd_mapping->labels, list) { + struct nd_namespace_label *nd_label = label_ent->label; + struct nd_label_id label_id; + + if (!nd_label) + continue; + nd_label_gen_id(&label_id, nd_label->uuid, + __le32_to_cpu(nd_label->flags)); + if (strcmp(old_label_id.id, label_id.id) == 0) + set_bit(ND_LABEL_REAP, &label_ent->flags); + } + mutex_unlock(&nd_mapping->lock); } kfree(*old_uuid); out: diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h index a5ac3b240293..191d62af0e51 100644 --- a/drivers/nvdimm/nd.h +++ b/drivers/nvdimm/nd.h @@ -113,8 +113,12 @@ struct nd_percpu_lane { spinlock_t lock; }; +enum nd_label_flags { + ND_LABEL_REAP, +}; struct nd_label_ent { struct list_head list; + unsigned long flags; struct nd_namespace_label *label; };
Users have reported intermittent occurrences of DIMM initialization failures due to duplicate allocations of address capacity detected in the labels, or errors of the form below, both have the same root cause. nd namespace1.4: failed to track label: 0 WARNING: CPU: 17 PID: 1381 at drivers/nvdimm/label.c:863 RIP: 0010:__pmem_label_update+0x56c/0x590 [libnvdimm] Call Trace: ? nd_pmem_namespace_label_update+0xd6/0x160 [libnvdimm] nd_pmem_namespace_label_update+0xd6/0x160 [libnvdimm] uuid_store+0x17e/0x190 [libnvdimm] kernfs_fop_write+0xf0/0x1a0 vfs_write+0xb7/0x1b0 ksys_write+0x57/0xd0 do_syscall_64+0x60/0x210 Unfortunately those reports were typically with a busy parallel namespace creation / destruction loop making it difficult to see the components of the bug. However, Jane provided a simple reproducer using the work-in-progress sub-section implementation. When ndctl is reconfiguring a namespace it may take an existing defunct / disabled namespace and reconfigure it with a new uuid and other parameters. Critically namespace_update_uuid() takes existing address resources and renames them for the new namespace to use / reconfigure as it sees fit. The bug is that this rename only happens in the resource tracking tree. Existing labels with the old uuid are not reaped leading to a scenario where multiple active labels reference the same span of address range. Teach namespace_update_uuid() to flag any references to the old uuid for reaping at the next label update attempt. Cc: <stable@vger.kernel.org> Fixes: bf9bccc14c05 ("libnvdimm: pmem label sets and namespace instantiation") Link: https://github.com/pmem/ndctl/issues/91 Reported-by: Jane Chu <jane.chu@oracle.com> Reported-by: Jeff Moyer <jmoyer@redhat.com> Reported-by: Erwin Tsaur <erwin.tsaur@oracle.com> Cc: Johannes Thumshirn <jthumshirn@suse.de> Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- drivers/nvdimm/label.c | 29 ++++++++++++++++------------- drivers/nvdimm/namespace_devs.c | 15 +++++++++++++++ drivers/nvdimm/nd.h | 4 ++++ 3 files changed, 35 insertions(+), 13 deletions(-)