[v5,2/4] mm/nvdimm: Add page size and struct page size to pfn superblock
diff mbox series

Message ID 20190809074520.27115-3-aneesh.kumar@linux.ibm.com
State Superseded
Headers show
Series
  • Mark the namespace disabled on pfn superblock mismatch
Related show

Commit Message

Aneesh Kumar K.V Aug. 9, 2019, 7:45 a.m. UTC
This is needed so that we don't wrongly initialize a namespace
which doesn't have enough space reserved for holding struct pages
with the current kernel.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
 drivers/nvdimm/pfn.h      |  5 ++++-
 drivers/nvdimm/pfn_devs.c | 27 ++++++++++++++++++++++++++-
 2 files changed, 30 insertions(+), 2 deletions(-)

Comments

Aneesh Kumar K.V Aug. 10, 2019, 4:21 a.m. UTC | #1
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:

  	case PFN_MODE_PMEM:
> @@ -475,6 +484,20 @@ 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) {
> +		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)) {
> +		dev_err(&nd_pfn->dev,
> +			"init failed, struct page size mismatch %d\n",
> +			le16_to_cpu(pfn_sb->page_struct_size));
> +		return -EOPNOTSUPP;
> +	}
> +

We need this here?

From 9885b2f9ed81a2438fc81507cfcdbdb1aeab756c Mon Sep 17 00:00:00 2001
From: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
Date: Fri, 9 Aug 2019 22:10:08 +0530
Subject: [PATCH] nvdimm: check struct page size only if pfn node is PMEM

We should do the check only with PFN_MODE_PMEM. If we use
memory for backing vmemmap, we should be able to enable
the namespace even if struct page size change.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
 drivers/nvdimm/pfn_devs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c
index f43d1baa6f33..f3e9a4b826da 100644
--- a/drivers/nvdimm/pfn_devs.c
+++ b/drivers/nvdimm/pfn_devs.c
@@ -509,7 +509,8 @@ int nd_pfn_validate(struct nd_pfn *nd_pfn, const char *sig)
 		return -EOPNOTSUPP;
 	}
 
-	if (le16_to_cpu(pfn_sb->page_struct_size) < sizeof(struct page)) {
+	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));
Dan Williams Aug. 14, 2019, 9:06 p.m. UTC | #2
On Fri, Aug 9, 2019 at 9:21 PM Aneesh Kumar K.V
<aneesh.kumar@linux.ibm.com> wrote:
>
> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>
>         case PFN_MODE_PMEM:
> > @@ -475,6 +484,20 @@ 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) {

I think this is too strict. It's only a potential problem in the
"pfn_sb->page_size > PAGE_SIZE" case, because only then might the
existing reservation for the memmap be too small. Otherwise, unless
I'm missing something, there should be no ill effects for
under-utilizing the reservation.

> > +             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)) {
> > +             dev_err(&nd_pfn->dev,
> > +                     "init failed, struct page size mismatch %d\n",
> > +                     le16_to_cpu(pfn_sb->page_struct_size));
> > +             return -EOPNOTSUPP;
> > +     }
> > +
>
> We need this here?

Yes, both ->page_struct_size and ->page_size are only relevant in the
PFN_MODE_PMEM case because PFN_MODE_RAM assumes all pages are
allocated dynamically and the size does not matter.


>
> From 9885b2f9ed81a2438fc81507cfcdbdb1aeab756c Mon Sep 17 00:00:00 2001
> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
> Date: Fri, 9 Aug 2019 22:10:08 +0530
> Subject: [PATCH] nvdimm: check struct page size only if pfn node is PMEM
>
> We should do the check only with PFN_MODE_PMEM. If we use
> memory for backing vmemmap, we should be able to enable
> the namespace even if struct page size change.

Same as the other patches please drop the usage of "we" in the changelog.

Patch
diff mbox series

diff --git a/drivers/nvdimm/pfn.h b/drivers/nvdimm/pfn.h
index 7381673b7b70..acb19517f678 100644
--- a/drivers/nvdimm/pfn.h
+++ b/drivers/nvdimm/pfn.h
@@ -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;
 };
 
diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c
index 3e7b11cf1aae..37e96811c2fc 100644
--- a/drivers/nvdimm/pfn_devs.c
+++ b/drivers/nvdimm/pfn_devs.c
@@ -460,6 +460,15 @@  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) {
+		/*
+		 * For a large part we use PAGE_SIZE. But we
+		 * do have some accounting code using SZ_4K.
+		 */
+		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 +484,20 @@  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) {
+		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)) {
+		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
@@ -722,8 +745,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(sizeof(struct page));
+	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);