Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1710164 > unrolled thread
| Started by | Dan Williams <dan.j.williams@intel.com> |
|---|---|
| First post | 2017-08-12 07:20 +0200 |
| Last post | 2017-08-16 07:30 +0200 |
| Articles | 6 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH 0/3] libnvdimm: export supported page size alignments Dan Williams <dan.j.williams@intel.com> - 2017-08-12 07:20 +0200
[PATCH 3/3] libnvdimm, pfn, dax: limit namespace alignments to the supported set Dan Williams <dan.j.williams@intel.com> - 2017-08-12 07:20 +0200
[PATCH 2/3] libnvdimm, pfn, dax: show supported dax/pfn region alignments in sysfs Dan Williams <dan.j.williams@intel.com> - 2017-08-12 07:20 +0200
Re: [PATCH 2/3] libnvdimm, pfn, dax: show supported dax/pfn region alignments in sysfs Oliver <oohall@gmail.com> - 2017-08-15 08:50 +0200
Re: [PATCH 2/3] libnvdimm, pfn, dax: show supported dax/pfn region alignments in sysfs Dan Williams <dan.j.williams@intel.com> - 2017-08-15 17:50 +0200
Re: [PATCH 2/3] libnvdimm, pfn, dax: show supported dax/pfn region alignments in sysfs Oliver <oohall@gmail.com> - 2017-08-16 07:30 +0200
| From | Dan Williams <dan.j.williams@intel.com> |
|---|---|
| Date | 2017-08-12 07:20 +0200 |
| Subject | [PATCH 0/3] libnvdimm: export supported page size alignments |
| Message-ID | <udx06-4QR-3@gated-at.bofh.it> |
This series is a minor rework of Oliver's original patch:
https://patchwork.kernel.org/patch/9811257/
It allows userspace to discover the system huge and gigantic page sizes
for aligning devices to support larger than PAGE_SIZE mappings for dax.
---
Dan Williams (2):
libnvdimm: rename nd_sector_size_{show,store} to nd_size_select_{show,store}
libnvdimm, pfn, dax: limit namespace alignments to the supported set
Oliver O'Halloran (1):
libnvdimm, pfn, dax: show supported dax/pfn region alignments in sysfs
drivers/nvdimm/btt_devs.c | 4 ++--
drivers/nvdimm/core.c | 10 +++++----
drivers/nvdimm/namespace_devs.c | 6 +++--
drivers/nvdimm/nd.h | 6 +++--
drivers/nvdimm/pfn_devs.c | 43 ++++++++++++++++++++++++---------------
5 files changed, 39 insertions(+), 30 deletions(-)
[toc] | [next] | [standalone]
| From | Dan Williams <dan.j.williams@intel.com> |
|---|---|
| Date | 2017-08-12 07:20 +0200 |
| Subject | [PATCH 3/3] libnvdimm, pfn, dax: limit namespace alignments to the supported set |
| Message-ID | <udx06-4QR-7@gated-at.bofh.it> |
| In reply to | #1710164 |
Now that we properly advertise the supported pte, pmd, and pud sizes,
restrict the supported alignments that can be set on a namespace. This
assumes that userspace was not previously relying on the ability to set
odd alignments. At least ndctl only ever supported setting the namespace
alignment to 4K, 2M, or 1G.
Cc: Oliver O'Halloran <oohall@gmail.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/nvdimm/pfn_devs.c | 23 ++---------------------
1 file changed, 2 insertions(+), 21 deletions(-)
diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c
index 610dd17a17f6..f447a1eb919d 100644
--- a/drivers/nvdimm/pfn_devs.c
+++ b/drivers/nvdimm/pfn_devs.c
@@ -131,26 +131,6 @@ static const unsigned long *nd_pfn_supported_alignments(void)
return supported_alignments;
}
-static ssize_t __align_store(struct nd_pfn *nd_pfn, const char *buf)
-{
- unsigned long val;
- int rc;
-
- rc = kstrtoul(buf, 0, &val);
- if (rc)
- return rc;
-
- if (!is_power_of_2(val) || val < PAGE_SIZE || val > SZ_1G)
- return -EINVAL;
-
- if (nd_pfn->dev.driver)
- return -EBUSY;
- else
- nd_pfn->align = val;
-
- return 0;
-}
-
static ssize_t align_store(struct device *dev,
struct device_attribute *attr, const char *buf, size_t len)
{
@@ -159,7 +139,8 @@ static ssize_t align_store(struct device *dev,
device_lock(dev);
nvdimm_bus_lock(dev);
- rc = __align_store(nd_pfn, buf);
+ rc = nd_size_select_store(dev, buf, &nd_pfn->align,
+ nd_pfn_supported_alignments());
dev_dbg(dev, "%s: result: %zd wrote: %s%s", __func__,
rc, buf, buf[len - 1] == '\n' ? "" : "\n");
nvdimm_bus_unlock(dev);
[toc] | [prev] | [next] | [standalone]
| From | Dan Williams <dan.j.williams@intel.com> |
|---|---|
| Date | 2017-08-12 07:20 +0200 |
| Subject | [PATCH 2/3] libnvdimm, pfn, dax: show supported dax/pfn region alignments in sysfs |
| Message-ID | <udx06-4QR-11@gated-at.bofh.it> |
| In reply to | #1710164 |
From: Oliver O'Halloran <oohall@gmail.com>
The alignment of a DAX and PFN regions dictates the page sizes that can
be used to map the region. Even if the hardware page sizes are known the
actual range of supported page sizes that can be used with DAX depends
on the kernel configuration. As a result it's best that the kernel
advertises the alignments that should be used with these region types.
This patch adds the 'supported_alignments' region attribute to expose
this information to userspace.
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
[djbw: integrate with nd_size_select_show() rename and other fixups]
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/nvdimm/pfn_devs.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c
index 2ae9a000b090..610dd17a17f6 100644
--- a/drivers/nvdimm/pfn_devs.c
+++ b/drivers/nvdimm/pfn_devs.c
@@ -111,6 +111,26 @@ static ssize_t align_show(struct device *dev,
return sprintf(buf, "%ld\n", nd_pfn->align);
}
+static const unsigned long *nd_pfn_supported_alignments(void)
+{
+ /*
+ * This needs to be a local variable because the *_SIZE macros
+ * aren't always constants.
+ */
+ static const unsigned long supported_alignments[] = {
+ PAGE_SIZE,
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+ HPAGE_PMD_SIZE,
+#ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
+ HPAGE_PUD_SIZE,
+#endif
+#endif
+ 0,
+ };
+
+ return supported_alignments;
+}
+
static ssize_t __align_store(struct nd_pfn *nd_pfn, const char *buf)
{
unsigned long val;
@@ -260,6 +280,13 @@ static ssize_t size_show(struct device *dev,
}
static DEVICE_ATTR_RO(size);
+static ssize_t supported_alignments_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return nd_size_select_show(0, nd_pfn_supported_alignments(), buf);
+}
+static DEVICE_ATTR_RO(supported_alignments);
+
static struct attribute *nd_pfn_attributes[] = {
&dev_attr_mode.attr,
&dev_attr_namespace.attr,
@@ -267,6 +294,7 @@ static struct attribute *nd_pfn_attributes[] = {
&dev_attr_align.attr,
&dev_attr_resource.attr,
&dev_attr_size.attr,
+ &dev_attr_supported_alignments.attr,
NULL,
};
[toc] | [prev] | [next] | [standalone]
| From | Oliver <oohall@gmail.com> |
|---|---|
| Date | 2017-08-15 08:50 +0200 |
| Subject | Re: [PATCH 2/3] libnvdimm, pfn, dax: show supported dax/pfn region alignments in sysfs |
| Message-ID | <ueDPQ-6jk-17@gated-at.bofh.it> |
| In reply to | #1710166 |
On Tue, Aug 15, 2017 at 4:02 PM, kbuild test robot <lkp@intel.com> wrote:
> Hi Oliver,
>
> [auto build test ERROR on linux-nvdimm/libnvdimm-for-next]
> [also build test ERROR on v4.13-rc5 next-20170811]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Dan-Williams/libnvdimm-export-supported-page-size-alignments/20170815-105258
> base: https://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm.git libnvdimm-for-next
> config: powerpc-allmodconfig (attached as .config)
> compiler: powerpc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
> reproduce:
> wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=powerpc
>
> All error/warnings (new ones prefixed by >>):
>
> In file included from include/linux/mm.h:446:0,
> from include/linux/memremap.h:3,
> from drivers//nvdimm/pfn_devs.c:13:
> drivers//nvdimm/pfn_devs.c: In function 'nd_pfn_supported_alignments':
>>> include/linux/huge_mm.h:83:24: error: initializer element is not constant
> #define HPAGE_PMD_SIZE ((1UL) << HPAGE_PMD_SHIFT)
> ^
>>> drivers//nvdimm/pfn_devs.c:123:3: note: in expansion of macro 'HPAGE_PMD_SIZE'
> HPAGE_PMD_SIZE,
> ^~~~~~~~~~~~~~
> include/linux/huge_mm.h:83:24: note: (near initialization for 'supported_alignments[1]')
> #define HPAGE_PMD_SIZE ((1UL) << HPAGE_PMD_SHIFT)
> ^
>>> drivers//nvdimm/pfn_devs.c:123:3: note: in expansion of macro 'HPAGE_PMD_SIZE'
> HPAGE_PMD_SIZE,
> ^~~~~~~~~~~~~~
> --
> In file included from include/linux/mm.h:446:0,
> from include/linux/memremap.h:3,
> from drivers/nvdimm/pfn_devs.c:13:
> drivers/nvdimm/pfn_devs.c: In function 'nd_pfn_supported_alignments':
>>> include/linux/huge_mm.h:83:24: error: initializer element is not constant
> #define HPAGE_PMD_SIZE ((1UL) << HPAGE_PMD_SHIFT)
> ^
> drivers/nvdimm/pfn_devs.c:123:3: note: in expansion of macro 'HPAGE_PMD_SIZE'
> HPAGE_PMD_SIZE,
> ^~~~~~~~~~~~~~
> include/linux/huge_mm.h:83:24: note: (near initialization for 'supported_alignments[1]')
> #define HPAGE_PMD_SIZE ((1UL) << HPAGE_PMD_SHIFT)
> ^
> drivers/nvdimm/pfn_devs.c:123:3: note: in expansion of macro 'HPAGE_PMD_SIZE'
> HPAGE_PMD_SIZE,
> ^~~~~~~~~~~~~~
>
> vim +/HPAGE_PMD_SIZE +123 drivers//nvdimm/pfn_devs.c
>
> > 13 #include <linux/memremap.h>
> 14 #include <linux/blkdev.h>
> 15 #include <linux/device.h>
> 16 #include <linux/genhd.h>
> 17 #include <linux/sizes.h>
> 18 #include <linux/slab.h>
> 19 #include <linux/fs.h>
> 20 #include <linux/mm.h>
> 21 #include "nd-core.h"
> 22 #include "pfn.h"
> 23 #include "nd.h"
> 24
> 25 static void nd_pfn_release(struct device *dev)
> 26 {
> 27 struct nd_region *nd_region = to_nd_region(dev->parent);
> 28 struct nd_pfn *nd_pfn = to_nd_pfn(dev);
> 29
> 30 dev_dbg(dev, "%s\n", __func__);
> 31 nd_detach_ndns(&nd_pfn->dev, &nd_pfn->ndns);
> 32 ida_simple_remove(&nd_region->pfn_ida, nd_pfn->id);
> 33 kfree(nd_pfn->uuid);
> 34 kfree(nd_pfn);
> 35 }
> 36
> 37 static struct device_type nd_pfn_device_type = {
> 38 .name = "nd_pfn",
> 39 .release = nd_pfn_release,
> 40 };
> 41
> 42 bool is_nd_pfn(struct device *dev)
> 43 {
> 44 return dev ? dev->type == &nd_pfn_device_type : false;
> 45 }
> 46 EXPORT_SYMBOL(is_nd_pfn);
> 47
> 48 struct nd_pfn *to_nd_pfn(struct device *dev)
> 49 {
> 50 struct nd_pfn *nd_pfn = container_of(dev, struct nd_pfn, dev);
> 51
> 52 WARN_ON(!is_nd_pfn(dev));
> 53 return nd_pfn;
> 54 }
> 55 EXPORT_SYMBOL(to_nd_pfn);
> 56
> 57 static ssize_t mode_show(struct device *dev,
> 58 struct device_attribute *attr, char *buf)
> 59 {
> 60 struct nd_pfn *nd_pfn = to_nd_pfn_safe(dev);
> 61
> 62 switch (nd_pfn->mode) {
> 63 case PFN_MODE_RAM:
> 64 return sprintf(buf, "ram\n");
> 65 case PFN_MODE_PMEM:
> 66 return sprintf(buf, "pmem\n");
> 67 default:
> 68 return sprintf(buf, "none\n");
> 69 }
> 70 }
> 71
> 72 static ssize_t mode_store(struct device *dev,
> 73 struct device_attribute *attr, const char *buf, size_t len)
> 74 {
> 75 struct nd_pfn *nd_pfn = to_nd_pfn_safe(dev);
> 76 ssize_t rc = 0;
> 77
> 78 device_lock(dev);
> 79 nvdimm_bus_lock(dev);
> 80 if (dev->driver)
> 81 rc = -EBUSY;
> 82 else {
> 83 size_t n = len - 1;
> 84
> 85 if (strncmp(buf, "pmem\n", n) == 0
> 86 || strncmp(buf, "pmem", n) == 0) {
> 87 nd_pfn->mode = PFN_MODE_PMEM;
> 88 } else if (strncmp(buf, "ram\n", n) == 0
> 89 || strncmp(buf, "ram", n) == 0)
> 90 nd_pfn->mode = PFN_MODE_RAM;
> 91 else if (strncmp(buf, "none\n", n) == 0
> 92 || strncmp(buf, "none", n) == 0)
> 93 nd_pfn->mode = PFN_MODE_NONE;
> 94 else
> 95 rc = -EINVAL;
> 96 }
> 97 dev_dbg(dev, "%s: result: %zd wrote: %s%s", __func__,
> 98 rc, buf, buf[len - 1] == '\n' ? "" : "\n");
> 99 nvdimm_bus_unlock(dev);
> 100 device_unlock(dev);
> 101
> 102 return rc ? rc : len;
> 103 }
> 104 static DEVICE_ATTR_RW(mode);
> 105
> 106 static ssize_t align_show(struct device *dev,
> 107 struct device_attribute *attr, char *buf)
> 108 {
> 109 struct nd_pfn *nd_pfn = to_nd_pfn_safe(dev);
> 110
> 111 return sprintf(buf, "%ld\n", nd_pfn->align);
> 112 }
> 113
> 114 static const unsigned long *nd_pfn_supported_alignments(void)
> 115 {
> 116 /*
> 117 * This needs to be a local variable because the *_SIZE macros
> 118 * aren't always constants.
> 119 */
I probably should have been clearer, "local" here really means
"non-static". Otherwise the array could have been made a global.
> 120 static const unsigned long supported_alignments[] = {
> 121 PAGE_SIZE,
> 122 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> > 123 HPAGE_PMD_SIZE,
> 124 #ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
> 125 HPAGE_PUD_SIZE,
> 126 #endif
> 127 #endif
> 128 0,
> 129 };
> 130
> 131 return supported_alignments;
> 132 }
> 133
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
[toc] | [prev] | [next] | [standalone]
| From | Dan Williams <dan.j.williams@intel.com> |
|---|---|
| Date | 2017-08-15 17:50 +0200 |
| Subject | Re: [PATCH 2/3] libnvdimm, pfn, dax: show supported dax/pfn region alignments in sysfs |
| Message-ID | <ueMgq-37n-35@gated-at.bofh.it> |
| In reply to | #1711901 |
On Mon, Aug 14, 2017 at 11:46 PM, Oliver <oohall@gmail.com> wrote:
> On Tue, Aug 15, 2017 at 4:02 PM, kbuild test robot <lkp@intel.com> wrote:
[..]
>> 114 static const unsigned long *nd_pfn_supported_alignments(void)
>> 115 {
>> 116 /*
>> 117 * This needs to be a local variable because the *_SIZE macros
>> 118 * aren't always constants.
>> 119 */
>
> I probably should have been clearer, "local" here really means
> "non-static". Otherwise the array could have been made a global.
>
Whoops, my fault. How about this:
@@ -127,8 +127,11 @@ static const unsigned long
*nd_pfn_supported_alignments(void)
#endif
0,
};
+ static unsigned long data[ARRAY_SIZE(supported_alignments)];
- return supported_alignments;
+ memcpy(data, supported_alignments, sizeof(data));
+
+ return data;
}
[toc] | [prev] | [next] | [standalone]
| From | Oliver <oohall@gmail.com> |
|---|---|
| Date | 2017-08-16 07:30 +0200 |
| Subject | Re: [PATCH 2/3] libnvdimm, pfn, dax: show supported dax/pfn region alignments in sysfs |
| Message-ID | <ueZ3X-2XF-1@gated-at.bofh.it> |
| In reply to | #1712285 |
On Wed, Aug 16, 2017 at 1:47 AM, Dan Williams <dan.j.williams@intel.com> wrote:
> On Mon, Aug 14, 2017 at 11:46 PM, Oliver <oohall@gmail.com> wrote:
>> On Tue, Aug 15, 2017 at 4:02 PM, kbuild test robot <lkp@intel.com> wrote:
> [..]
>>> 114 static const unsigned long *nd_pfn_supported_alignments(void)
>>> 115 {
>>> 116 /*
>>> 117 * This needs to be a local variable because the *_SIZE macros
>>> 118 * aren't always constants.
>>> 119 */
>>
>> I probably should have been clearer, "local" here really means
>> "non-static". Otherwise the array could have been made a global.
>>
>
> Whoops, my fault. How about this:
>
> @@ -127,8 +127,11 @@ static const unsigned long
> *nd_pfn_supported_alignments(void)
> #endif
> 0,
> };
> + static unsigned long data[ARRAY_SIZE(supported_alignments)];
>
> - return supported_alignments;
> + memcpy(data, supported_alignments, sizeof(data));
> +
> + return data;
> }
That should do the trick, but you'll need to fix up the source array
declaration too.
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web