Mark the namespace disabled on pfn superblock mismatch
Subsystems (4)
v9 cover Mark the namespace disabled on pfn superblock mismatch
We add new members to pfn superblock (PAGE_SIZE and struct page size) in this series. This is now checked while initializing the namespace. If we find a mismatch we mark the namespace disabled.
This series also handle configs where hugepage support is not enabled by default. This can result in different align restrictions for dax namespace. We mark the dax namespace disabled if we find the alignment not supported.
Changes from v8:
* updated patch 7 for addressing review feedback
Changes from v6:
* Formatting changes
Changes from v5:
* Split patch 3
* Update commit message
* Add MAX_STRUCT_PAGE_SIZE with value 64 and use that when allocating reserve block
* Add BUILD_BUG_ON if we find sizeof(struct page) > 64
Aneesh Kumar K.V (6):
libnvdimm/pmem: Advance namespace seed for specific probe errors libnvdimm/pfn_dev: Add a build check to make sure we notice when
struct page size change
libnvdimm/pfn_dev: Add page size and struct page size to pfn
superblock
libnvdimm/label: Remove the dpa align check
libnvdimm: Use PAGE_SIZE instead of SZ_4K for align check libnvdimm/dax: Pick the right alignment default when creating dax
devices
Dan Williams (1):
libnvdimm/region: Rewrite _probe_success() to _advance_seeds()
drivers/nvdimm/bus.c | 8 +-- drivers/nvdimm/label.c | 5 -- drivers/nvdimm/namespace_devs.c | 40 +++++++++--- drivers/nvdimm/nd-core.h | 3 +- drivers/nvdimm/nd.h | 10 +-- drivers/nvdimm/pfn.h | 5 +- drivers/nvdimm/pfn_devs.c | 110 +++++++++++++++++++++++++------- drivers/nvdimm/pmem.c | 29 +++++++-- drivers/nvdimm/region_devs.c | 76 ++++------------------ include/linux/huge_mm.h | 7 +- 10 files changed, 176 insertions(+), 117 deletions(-)
Signature · 1 line
2.21.0
A message that is not in the archive (sent off-list or not yet received): 20190905154603.10349-1-aneesh.kumar-tEXmvtCZX7AybS5Ee8rs3A@public.gmane.org
PATCH v9 1/7 libnvdimm/region: Rewrite _probe_success() to _advance_seeds()
From: Dan Williams <[email protected]>
The nd_region_probe_success() helper collides seed management with nvdimm->busy tracking. Given the 'busy' increment is handled internal to the nd_region driver 'probe' path move the decrement to the 'remove' path. With that cleanup the routine can be renamed to the more descriptive nd_region_advance_seeds().
The change is prompted by an incoming need to optionally advance the seeds on other events besides 'probe' success.
--- drivers/nvdimm/bus.c | 7 +--- drivers/nvdimm/namespace_devs.c | 34 ++++++++++++++--- drivers/nvdimm/nd-core.h | 3 +- drivers/nvdimm/region_devs.c | 68 +++++---------------------------- 4 files changed, 41 insertions(+), 71 deletions(-)
drivers/nvdimm/bus.c +2 -5
@@ -95,10 +95,8 @@ static int nvdimm_bus_probe(struct device *dev) rc = nd_drv->probe (dev);debug_nvdimm_unlock (dev); -if (rc ==0 ) -nd_region_probe_success (nvdimm_bus, dev); -else -nd_region_disable (nvdimm_bus, dev); +if (rc ==0 && dev->parent &&is_nd_region (dev->parent)) +nd_region_advance_seeds (to_nd_region (dev->parent), dev);nvdimm_bus_probe_end (nvdimm_bus); dev_dbg(&nvdimm_bus->dev, "END: %s.probe(%s) = %d\n", dev->driver->name,
@@ -121,7 +119,6 @@ static int nvdimm_bus_remove(struct device *dev) rc = nd_drv->remove (dev);debug_nvdimm_unlock (dev); } -nd_region_disable (nvdimm_bus, dev);dev_dbg (&nvdimm_bus->dev,"%s.remove(%s) = %d\n" , dev->driver->name,dev_name (dev), rc);
drivers/nvdimm/namespace_devs.c +28 -6
@@ -2462,6 +2462,27 @@ static struct device **create_namespaces(struct nd_region *nd_region)return devs; } +static void deactivate_labels (void *region) +{ +struct nd_region *nd_region = region; +int i; + +for (i =0 ; i < nd_region->ndr_mappings; i++) { +struct nd_mapping *nd_mapping = &nd_region->mapping[i]; +struct nvdimm_drvdata *ndd = nd_mapping->ndd; +struct nvdimm *nvdimm = nd_mapping->nvdimm; + +mutex_lock (&nd_mapping->lock); +nd_mapping_free_labels (nd_mapping); +mutex_unlock (&nd_mapping->lock); + +put_ndd (ndd); + nd_mapping->ndd =NULL ; +if (ndd) +atomic_dec (&nvdimm->busy); + } +} +static int init_active_labels (struct nd_region *nd_region) {int i;
@@ -2519,16 +2540,17 @@ static int init_active_labels(struct nd_region *nd_region)mutex_unlock (&nd_mapping->lock); } -if (j >= count) -continue ; +if (j < count) +break ; + } -mutex_lock (&nd_mapping->lock); -nd_mapping_free_labels (nd_mapping); -mutex_unlock (&nd_mapping->lock); +if (i < nd_region->ndr_mappings) { +deactivate_labels (nd_region);return -ENOMEM ; } -return 0 ; +return devm_add_action_or_reset (&nd_region->dev, deactivate_labels, + nd_region); }int nd_region_register_namespaces (struct nd_region *nd_region,int *err)
drivers/nvdimm/nd-core.h +1 -2
@@ -128,13 +128,12 @@ int __init nvdimm_bus_init(void);void nvdimm_bus_exit (void );void nvdimm_devs_exit (void );void nd_region_devs_exit (void ); -void nd_region_probe_success (struct nvdimm_bus *nvdimm_bus,struct device *dev);struct nd_region ; +void nd_region_advance_seeds (struct nd_region *nd_region,struct device *dev);void nd_region_create_ns_seed (struct nd_region *nd_region);void nd_region_create_btt_seed (struct nd_region *nd_region);void nd_region_create_pfn_seed (struct nd_region *nd_region);void nd_region_create_dax_seed (struct nd_region *nd_region); -void nd_region_disable (struct nvdimm_bus *nvdimm_bus,struct device *dev);int nvdimm_bus_create_ndctl (struct nvdimm_bus *nvdimm_bus);void nvdimm_bus_destroy_ndctl (struct nvdimm_bus *nvdimm_bus);void nd_synchronize (void );
drivers/nvdimm/region_devs.c +10 -58
95 lines not on this page · raw message
Signature · 1 line
2.21.0
PATCH v9 2/7 libnvdimm/pmem: Advance namespace seed for specific probe errors
In reply to a message not in the archiveIn order to support marking namespaces with unsupported feature/versions disabled, nvdimm core should advance the namespace seed on these probe failures. Otherwise, these failed namespaces will be considered a seed namespace and will be wrongly used while creating new namespaces.
Add -EOPNOTSUPP as return from pmem probe callback to indicate a namespace initialization failures due to pfn superblock feature/version mismatch.
--- drivers/nvdimm/bus.c | 3 ++- drivers/nvdimm/pmem.c | 29 +++++++++++++++++++++++++---- 2 files changed, 27 insertions(+), 5 deletions(-)
drivers/nvdimm/bus.c +2 -1
@@ -95,7 +95,8 @@ static int nvdimm_bus_probe(struct device *dev) rc = nd_drv->probe (dev);debug_nvdimm_unlock (dev); -if (rc ==0 && dev->parent &&is_nd_region (dev->parent)) +if ((rc ==0 || rc == -EOPNOTSUPP ) && + dev->parent &&is_nd_region (dev->parent))nd_region_advance_seeds (to_nd_region (dev->parent), dev);nvdimm_bus_probe_end (nvdimm_bus);
drivers/nvdimm/pmem.c +25 -4
@@ -490,6 +490,7 @@ static int pmem_attach_disk(struct device *dev,static int nd_pmem_probe (struct device *dev) { +int ret;struct nd_namespace_common *ndns; ndns =nvdimm_namespace_common_probe (dev);
@@ -505,12 +506,32 @@ static int nd_pmem_probe(struct device *dev)if (is_nd_pfn (dev))return pmem_attach_disk (dev, ndns); -/* if we find a valid info-block we'll come back as that personality */ -if (nd_btt_probe (dev, ndns) ==0 ||nd_pfn_probe (dev, ndns) ==0 - ||nd_dax_probe (dev, ndns) ==0 ) + ret =nd_btt_probe (dev, ndns); +if (ret ==0 )return -ENXIO ; -/* ...otherwise we're just a raw pmem device */ +/* +* We have two failure conditions here, there is no +* info reserver block or we found a valid info reserve block +* but failed to initialize the pfn superblock. +* +* For the first case consider namespace as a raw pmem namespace +* and attach a disk. +* +* For the latter, consider this a success and advance the namespace +* seed. +*/ + ret =nd_pfn_probe (dev, ndns); +if (ret ==0 ) +return -ENXIO ; +else if (ret == -EOPNOTSUPP ) +return ret; + + ret =nd_dax_probe (dev, ndns); +if (ret ==0 ) +return -ENXIO ; +else if (ret == -EOPNOTSUPP ) +return ret;return pmem_attach_disk (dev, ndns); }
Signature · 1 line
2.21.0
PATCH v9 3/7 libnvdimm/pfn_dev: Add a build check to make sure we notice when struct page size change
In reply to a message not in the archiveNamespaces created with PFN_MODE_PMEM mode stores struct page in the reserve block area. We need to make sure we account for the right struct page size while doing this. Instead of directly depending on sizeof(struct page) which can change based on different kernel config option, use the max struct page size (64) while calculating the reserve block area. This makes sure pmem device can be used across kernels built with different configs.
If the above assumption of max struct page size change, we need to update the reserve block allocation space for new namespaces created.
--- drivers/nvdimm/nd.h | 4 ++++ drivers/nvdimm/pfn_devs.c | 10 +++++++++- 2 files changed, 13 insertions(+), 1 deletion(-)
drivers/nvdimm/nd.h +4 -0
@@ -375,6 +375,10 @@ unsigned int pmem_sector_size(struct nd_namespace_common *ndns);void nvdimm_badblocks_populate (struct nd_region *nd_region,struct badblocks *bb,const struct resource *res);#if IS_ENABLED (CONFIG_ND_CLAIM ) + +/* max struct page size independent of kernel config */ +#define MAX_STRUCT_PAGE_SIZE 64 +int nvdimm_setup_pfn (struct nd_pfn *nd_pfn,struct dev_pagemap *pgmap);int devm_nsio_enable (struct device *dev,struct nd_namespace_io *nsio);void devm_nsio_disable (struct device *dev,struct nd_namespace_io *nsio);
drivers/nvdimm/pfn_devs.c +9 -1
@@ -701,8 +701,16 @@ static int nd_pfn_init(struct nd_pfn *nd_pfn)* The altmap should be padded out to the block size used * when populating the vmemmap. This *should* be equal to * PMD_SIZE for most architectures. +* +* Also make sure size of struct page is less than 64. We +* want to make sure we use large enough size here so that +* we don't have a dynamic reserve space depending on +* struct page size. But we also want to make sure we notice +* when we end up adding new elements to struct page. */ - offset =ALIGN (start +SZ_8K +64 * npfns, align) - start; +BUILD_BUG_ON (sizeof (struct page ) >MAX_STRUCT_PAGE_SIZE ); + offset =ALIGN (start +SZ_8K +MAX_STRUCT_PAGE_SIZE * npfns, align) + - start; }else if (nd_pfn->mode ==PFN_MODE_RAM ) offset =ALIGN (start +SZ_8K , align) - start;else
Signature · 1 line
2.21.0
PATCH v9 4/7 libnvdimm/pfn_dev: Add page size and struct page size to pfn superblock
In reply to a message not in the archiveThis is needed so that pmem probe don't wrongly initialize a namespace which doesn't have enough space reserved for holding struct pages with the current kernel.
--- drivers/nvdimm/pfn.h | 5 ++++- drivers/nvdimm/pfn_devs.c | 25 ++++++++++++++++++++++++- 2 files changed, 28 insertions(+), 2 deletions(-)
drivers/nvdimm/pfn.h +4 -1
@@ -29,7 +29,10 @@ struct nd_pfn_sb {/* minor-version-2 record the base alignment of the mapping */ __le32 align;/* minor-version-3 guarantee the padding and flags are zero */ -u8 padding[4000 ]; +/* minor-version-4 record the page size and struct page size */ +__le32 page_size; +__le16 page_struct_size; +u8 padding[3994 ];__le64 checksum; };
drivers/nvdimm/pfn_devs.c +24 -1
@@ -460,6 +460,11 @@ int nd_pfn_validate(struct nd_pfn *nd_pfn, const char *sig)if (__le16_to_cpu (pfn_sb->version_minor) <2 ) pfn_sb->align =0 ; +if (__le16_to_cpu (pfn_sb->version_minor) <4 ) { + pfn_sb->page_struct_size =cpu_to_le16 (64 ); + pfn_sb->page_size =cpu_to_le32 (PAGE_SIZE ); + } +switch (le32_to_cpu (pfn_sb->mode)) {case PFN_MODE_RAM :case PFN_MODE_PMEM :
@@ -475,6 +480,22 @@ int nd_pfn_validate(struct nd_pfn *nd_pfn, const char *sig) align =1UL <<ilog2 (offset); mode =le32_to_cpu (pfn_sb->mode); +if ((le32_to_cpu (pfn_sb->page_size) >PAGE_SIZE ) && + (mode ==PFN_MODE_PMEM )) { +dev_err (&nd_pfn->dev, +"init failed, page size mismatch %d\n" , +le32_to_cpu (pfn_sb->page_size)); +return -EOPNOTSUPP ; + } + +if ((le16_to_cpu (pfn_sb->page_struct_size) <sizeof (struct page )) && + (mode ==PFN_MODE_PMEM )) { +dev_err (&nd_pfn->dev, +"init failed, struct page size mismatch %d\n" , +le16_to_cpu (pfn_sb->page_struct_size)); +return -EOPNOTSUPP ; + } +if (!nd_pfn->uuid) {/* * When probing a namepace via nd_pfn_probe() the uuid
@@ -730,8 +751,10 @@ static int nd_pfn_init(struct nd_pfn *nd_pfn)memcpy (pfn_sb->uuid, nd_pfn->uuid,16 );memcpy (pfn_sb->parent_uuid,nd_dev_to_uuid (&ndns->dev),16 ); pfn_sb->version_major =cpu_to_le16 (1 ); - pfn_sb->version_minor =cpu_to_le16 (3 ); + pfn_sb->version_minor =cpu_to_le16 (4 ); pfn_sb->align =cpu_to_le32 (nd_pfn->align); + pfn_sb->page_struct_size =cpu_to_le16 (MAX_STRUCT_PAGE_SIZE ); + pfn_sb->page_size =cpu_to_le32 (PAGE_SIZE ); checksum =nd_sb_checksum ((struct nd_gen_sb *) pfn_sb); pfn_sb->checksum =cpu_to_le64 (checksum);
Signature · 1 line
2.21.0
PATCH v9 5/7 libnvdimm/label: Remove the dpa align check
In reply to a message not in the archiveThere's no strict requirement why slot_valid() needs to check for page alignment and it would seem to actively hurt cross-page-size compatibility. Let's delete the check and rely on checksum validation.
--- drivers/nvdimm/label.c | 5 ----- 1 file changed, 5 deletions(-)
drivers/nvdimm/label.c +0 -5
@@ -353,11 +353,6 @@ static bool slot_valid(struct nvdimm_drvdata *ndd,if (slot !=__le32_to_cpu (nd_label->slot))return false; -/* check that DPA allocations are page aligned */ -if ((__le64_to_cpu (nd_label->dpa) - |__le64_to_cpu (nd_label->rawsize)) %SZ_4K ) -return false; -/* check checksum */ if (namespace_label_has (ndd, checksum)) {u64 sum, sum_save;
Signature · 1 line
2.21.0
PATCH v9 6/7 libnvdimm: Use PAGE_SIZE instead of SZ_4K for align check
In reply to a message not in the archiveArchitectures have different page size than 4K. Use the PAGE_SIZE to make sure ranges are correctly aligned.
--- drivers/nvdimm/namespace_devs.c | 6 +++--- drivers/nvdimm/region_devs.c | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-)
drivers/nvdimm/namespace_devs.c +3 -3
@@ -1006,10 +1006,10 @@ static ssize_t __size_store(struct device *dev, unsigned long long val)return -ENXIO ; } -div_u64_rem (val,SZ_4K * nd_region->ndr_mappings, &remainder); +div_u64_rem (val,PAGE_SIZE * nd_region->ndr_mappings, &remainder);if (remainder) { -dev_dbg (dev,"%llu is not %dK aligned\n" , val, - (SZ_4K * nd_region->ndr_mappings) /SZ_1K ); +dev_dbg (dev,"%llu is not %ldK aligned\n" , val, + (PAGE_SIZE * nd_region->ndr_mappings) /SZ_1K );return -EINVAL ; }
drivers/nvdimm/region_devs.c +4 -4
@@ -944,10 +944,10 @@ static struct nd_region *nd_region_create(struct nvdimm_bus *nvdimm_bus,struct nd_mapping_desc *mapping = &ndr_desc->mapping[i];struct nvdimm *nvdimm = mapping->nvdimm; -if ((mapping->start | mapping->size) %SZ_4K ) { -dev_err (&nvdimm_bus->dev,"%s: %s mapping%d is not 4K aligned\n" , - caller,dev_name (&nvdimm->dev), i); - +if ((mapping->start | mapping->size) %PAGE_SIZE ) { +dev_err (&nvdimm_bus->dev, +"%s: %s mapping%d is not %ld aligned\n" , + caller,dev_name (&nvdimm->dev), i,PAGE_SIZE );return NULL ; }
Signature · 1 line
2.21.0
Quoted 37 lines · Aneesh Kumar K.V
We add new members to pfn superblock (
PAGE_SIZEand struct page size) in this series. This is now checked while initializing the namespace. If we find a mismatch we mark the namespace disabled.This series also handle configs where hugepage support is not enabled by default. This can result in different align restrictions for dax namespace. We mark the dax namespace disabled if we find the alignment not supported.
Changes from v8:
* updated patch 7 for addressing review feedbackChanges from v6:
* Formatting changesChanges from v5:
* Split patch 3
* Update commit message
* AddMAX_STRUCT_PAGE_SIZEwith value 64 and use that when allocating reserve block
* AddBUILD_BUG_ONif we find sizeof(struct page) > 64Aneesh Kumar K.V (6):
libnvdimm/pmem: Advance namespace seed for specific probe errors libnvdimm/pfn_dev: Add a build check to make sure we notice when
struct page size change
libnvdimm/pfn_dev: Add page size and struct page size to pfn
superblock
libnvdimm/label: Remove the dpa align check
libnvdimm: UsePAGE_SIZEinstead ofSZ_4Kfor align check libnvdimm/dax: Pick the right alignment default when creating dax
devicesDan Williams (1):
libnvdimm/region: Rewrite_probe_success()to_advance_seeds()
Quoted 11 lines
drivers/nvdimm/bus.c | 8 +-- drivers/nvdimm/label.c | 5 -- drivers/nvdimm/namespace_devs.c | 40 +++++++++--- drivers/nvdimm/nd-core.h | 3 +- drivers/nvdimm/nd.h | 10 +-- drivers/nvdimm/pfn.h | 5 +- drivers/nvdimm/pfn_devs.c | 110 +++++++++++++++++++++++++------- drivers/nvdimm/pmem.c | 29 +++++++-- drivers/nvdimm/region_devs.c | 76 ++++------------------ include/linux/huge_mm.h | 7 +- 10 files changed, 176 insertions(+), 117 deletions(-)
Looks good, 0day has no complaints and this passes all the current unit tests. I'll get it pushed out for v5.4 consideration. Thanks Aneesh!
PATCH v9 7/7 libnvdimm/dax: Pick the right alignment default when creating dax devices
In reply to Aneesh Kumar K.VAllow arch to provide the supported alignments and use hugepage alignment only if we support hugepage. Right now we depend on compile time configs whereas this patch switch this to runtime discovery.
Architectures like ppc64 can have THP enabled in code, but then can have hugepage size disabled by the hypervisor. This allows us to create dax devices with PAGE_SIZE alignment in this case.
Existing dax namespace with alignment larger than PAGE_SIZE will fail to initialize in this specific case. We still allow fsdax namespace initialization.
With respect to identifying whether to enable hugepage fault for a dax device, if THP is enabled during compile, we default to taking hugepage fault and in dax fault handler if we find the fault size > alignment we retry with PAGE_SIZE fault size.
This also addresses the below failure scenario on ppc64
ndctl create-namespace --mode=devdax | grep align
"align":16777216,
"align":16777216
cat /sys/devices/ndbus0/region0/dax0.0/supported_alignments
65536 16777216
daxio.static-debug -z -o /dev/dax0.0
Bus error (core dumped)
$ dmesg | tail lpar: Failed hash pte insert with error -4 hash-mmu: mm: Hashing failure ! EA=0x7fff17000000 access=0x8000000000000006 current=daxio hash-mmu: trap=0x300 vsid=0x22cb7a3 ssize=1 base psize=2 psize 10 pte=0xc000000501002b86 daxio[3860]: bus error (7) at 7fff17000000 nip 7fff973c007c lr 7fff973bff34 code 2 in libpmem.so.1.0.0[7fff973b0000+20000] daxio[3860]: code: 792945e4 7d494b78 e95f0098 7d494b78 f93f00a0 4800012c e93f0088 f93f0120 daxio[3860]: code: e93f00a0 f93f0128 e93f0120 e95f0128 <f9490000> e93f0088 39290008 f93f0110
The failure was due to guest kernel using wrong page size.
The namespaces created with 16M alignment will appear as below on a config with 16M page size disabled.
$ ndctl list -Ni
[
{
"dev":"namespace0.1",
"mode":"fsdax",
"map":"dev",
"size":5351931904,
"uuid":"fc6e9667-461a-4718-82b4-69b24570bddb",
"align":16777216,
"blockdev":"pmem0.1",
"supported_alignments":[
65536
]
},
{
"dev":"namespace0.0",
"mode":"fsdax", <==== devdax 16M alignment marked disabled.
"map":"mem",
"size":5368709120,
"uuid":"a4bdf81a-f2ee-4bc6-91db-7b87eddd0484",
"state":"disabled"
}
]--- drivers/nvdimm/nd.h | 6 +--- drivers/nvdimm/pfn_devs.c | 75 ++++++++++++++++++++++++++++----------- include/linux/huge_mm.h | 7 +++- 3 files changed, 61 insertions(+), 27 deletions(-)
drivers/nvdimm/nd.h +1 -5
@@ -289,11 +289,7 @@ static inline struct device *nd_btt_create(struct nd_region *nd_region)struct nd_pfn *to_nd_pfn (struct device *dev);#if IS_ENABLED (CONFIG_NVDIMM_PFN ) -#ifdef CONFIG_TRANSPARENT_HUGEPAGE -#define PFN_DEFAULT_ALIGNMENT HPAGE_PMD_SIZE -#else -#define PFN_DEFAULT_ALIGNMENT PAGE_SIZE -#endif +#define MAX_NVDIMM_ALIGN 4int nd_pfn_probe (struct device *dev,struct nd_namespace_common *ndns);bool is_nd_pfn (struct device *dev);
drivers/nvdimm/pfn_devs.c +54 -21
119 lines not on this page · raw message
include/linux/huge_mm.h +6 -1
@@ -108,7 +108,12 @@ static inline bool __transparent_hugepage_enabled(struct vm_area_struct *vma)if (transparent_hugepage_flags & (1 <<TRANSPARENT_HUGEPAGE_FLAG ))return true; - +/* +* For dax vmas, try to always use hugepage mappings. If the kernel does +* not support hugepages, fsdax mappings will fallback to PAGE_SIZE +* mappings, and device-dax namespaces, that try to guarantee a given +* mapping size, will fail to enable +*/ if (vma_is_dax (vma))return true;
Signature · 1 line
2.21.0