| Message ID | 1450761298-12172-4-git-send-email-david@gibson.dropbear.id.au (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On 12/22/15, David Gibson <david@gibson.dropbear.id.au> wrote: > This patch adds a special file in /sys/kernel/mm which can be used to view > the current size of the hash page table (as a bit shift) and to trigger > a resize of the hash table on PAPR guests. > > Logically this would make more sense as a debugfs file, but that doesn't > see to provide an obvious way to have a "store" function that has an effect > other than updating a variable. > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > --- > arch/powerpc/platforms/pseries/lpar.c | 41 > +++++++++++++++++++++++++++++++++++ > 1 file changed, 41 insertions(+) > > diff --git a/arch/powerpc/platforms/pseries/lpar.c > b/arch/powerpc/platforms/pseries/lpar.c > index aadb242..2759767 100644 > --- a/arch/powerpc/platforms/pseries/lpar.c > +++ b/arch/powerpc/platforms/pseries/lpar.c > @@ -903,3 +903,44 @@ int pSeries_lpar_resize_hpt(unsigned long shift) > > return 0; > } > + > +static ssize_t ppc64_pft_size_show(struct kobject *kobj, > + struct kobj_attribute *attr, char *buf) > +{ > + return sprintf(buf, "%llu\n", (unsigned long long)ppc64_pft_size); > +} > + > +static ssize_t ppc64_pft_size_store(struct kobject *kobj, > + struct kobj_attribute *attr, > + const char *buf, size_t count) > +{ > + unsigned long new_shift; > + int rc; > + > + pr_err("lpar: ppc64_pft_size_store() count=%zd\n", count); Why pr_err? > + > + if (kstrtoul(buf, 0, &new_shift)) > + return -EINVAL; > + > + rc = pSeries_lpar_resize_hpt(new_shift); > + if (rc < 0) > + return rc; > + > + return count; > +} > + > +static struct kobj_attribute ppc64_pft_size_attr = > + __ATTR(ppc64_pft_size, 0600, ppc64_pft_size_show, ppc64_pft_size_store); > + > +static int __init pseries_lpar_sysfs(void) > +{ > + int rc; > + > + rc = sysfs_create_file(mm_kobj, &ppc64_pft_size_attr.attr); > + if (rc) > + pr_err("lpar: unable to create ppc64_pft_size sysfs file (%d)\n", > + rc); > + > + return 0; I think that we need to return rc value, right? > +} > +machine_device_initcall(pseries, pseries_lpar_sysfs); > -- > 2.5.0 > > _______________________________________________ > Linuxppc-dev mailing list > Linuxppc-dev@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/linuxppc-dev
On Tue, Dec 22, 2015 at 02:19:20PM +0300, Denis Kirjanov wrote: > On 12/22/15, David Gibson <david@gibson.dropbear.id.au> wrote: > > This patch adds a special file in /sys/kernel/mm which can be used to view > > the current size of the hash page table (as a bit shift) and to trigger > > a resize of the hash table on PAPR guests. > > > > Logically this would make more sense as a debugfs file, but that doesn't > > see to provide an obvious way to have a "store" function that has an effect > > other than updating a variable. > > > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > > --- > > arch/powerpc/platforms/pseries/lpar.c | 41 > > +++++++++++++++++++++++++++++++++++ > > 1 file changed, 41 insertions(+) > > > > diff --git a/arch/powerpc/platforms/pseries/lpar.c > > b/arch/powerpc/platforms/pseries/lpar.c > > index aadb242..2759767 100644 > > --- a/arch/powerpc/platforms/pseries/lpar.c > > +++ b/arch/powerpc/platforms/pseries/lpar.c > > @@ -903,3 +903,44 @@ int pSeries_lpar_resize_hpt(unsigned long shift) > > > > return 0; > > } > > + > > +static ssize_t ppc64_pft_size_show(struct kobject *kobj, > > + struct kobj_attribute *attr, char *buf) > > +{ > > + return sprintf(buf, "%llu\n", (unsigned long long)ppc64_pft_size); > > +} > > + > > +static ssize_t ppc64_pft_size_store(struct kobject *kobj, > > + struct kobj_attribute *attr, > > + const char *buf, size_t count) > > +{ > > + unsigned long new_shift; > > + int rc; > > + > > + pr_err("lpar: ppc64_pft_size_store() count=%zd\n", count); > Why pr_err? Uh.. good question. That's a not particularly useful debug comment I should just remove anyway. > > + > > + if (kstrtoul(buf, 0, &new_shift)) > > + return -EINVAL; > > + > > + rc = pSeries_lpar_resize_hpt(new_shift); > > + if (rc < 0) > > + return rc; > > + > > + return count; > > +} > > + > > +static struct kobj_attribute ppc64_pft_size_attr = > > + __ATTR(ppc64_pft_size, 0600, ppc64_pft_size_show, ppc64_pft_size_store); > > + > > +static int __init pseries_lpar_sysfs(void) > > +{ > > + int rc; > > + > > + rc = sysfs_create_file(mm_kobj, &ppc64_pft_size_attr.attr); > > + if (rc) > > + pr_err("lpar: unable to create ppc64_pft_size sysfs file (%d)\n", > > + rc); > > + > > + return 0; > I think that we need to return rc value, right? No. Failing to register this sysfs file shouldn't cause the boot to fail. > > +} > > +machine_device_initcall(pseries, pseries_lpar_sysfs); > > > > _______________________________________________ > > Linuxppc-dev mailing list > > Linuxppc-dev@lists.ozlabs.org > > https://lists.ozlabs.org/listinfo/linuxppc-dev >
On 12/23/15, David Gibson <david@gibson.dropbear.id.au> wrote: > On Tue, Dec 22, 2015 at 02:19:20PM +0300, Denis Kirjanov wrote: >> On 12/22/15, David Gibson <david@gibson.dropbear.id.au> wrote: >> > This patch adds a special file in /sys/kernel/mm which can be used to >> > view >> > the current size of the hash page table (as a bit shift) and to trigger >> > a resize of the hash table on PAPR guests. >> > >> > Logically this would make more sense as a debugfs file, but that >> > doesn't >> > see to provide an obvious way to have a "store" function that has an >> > effect >> > other than updating a variable. >> > >> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> >> > --- >> > arch/powerpc/platforms/pseries/lpar.c | 41 >> > +++++++++++++++++++++++++++++++++++ >> > 1 file changed, 41 insertions(+) >> > >> > diff --git a/arch/powerpc/platforms/pseries/lpar.c >> > b/arch/powerpc/platforms/pseries/lpar.c >> > index aadb242..2759767 100644 >> > --- a/arch/powerpc/platforms/pseries/lpar.c >> > +++ b/arch/powerpc/platforms/pseries/lpar.c >> > @@ -903,3 +903,44 @@ int pSeries_lpar_resize_hpt(unsigned long shift) >> > >> > return 0; >> > } >> > + >> > +static ssize_t ppc64_pft_size_show(struct kobject *kobj, >> > + struct kobj_attribute *attr, char *buf) >> > +{ >> > + return sprintf(buf, "%llu\n", (unsigned long long)ppc64_pft_size); >> > +} >> > + >> > +static ssize_t ppc64_pft_size_store(struct kobject *kobj, >> > + struct kobj_attribute *attr, >> > + const char *buf, size_t count) >> > +{ >> > + unsigned long new_shift; >> > + int rc; >> > + >> > + pr_err("lpar: ppc64_pft_size_store() count=%zd\n", count); >> Why pr_err? > > Uh.. good question. That's a not particularly useful debug comment I > should just remove anyway. > >> > + >> > + if (kstrtoul(buf, 0, &new_shift)) >> > + return -EINVAL; >> > + >> > + rc = pSeries_lpar_resize_hpt(new_shift); >> > + if (rc < 0) >> > + return rc; >> > + >> > + return count; >> > +} >> > + >> > +static struct kobj_attribute ppc64_pft_size_attr = >> > + __ATTR(ppc64_pft_size, 0600, ppc64_pft_size_show, >> > ppc64_pft_size_store); >> > + >> > +static int __init pseries_lpar_sysfs(void) >> > +{ >> > + int rc; >> > + >> > + rc = sysfs_create_file(mm_kobj, &ppc64_pft_size_attr.attr); >> > + if (rc) >> > + pr_err("lpar: unable to create ppc64_pft_size sysfs file (%d)\n", >> > + rc); >> > + >> > + return 0; >> I think that we need to return rc value, right? > > No. Failing to register this sysfs file shouldn't cause the boot to fail. Then we don't need rc at all ;) > >> > +} >> > +machine_device_initcall(pseries, pseries_lpar_sysfs); >> > >> > _______________________________________________ >> > Linuxppc-dev mailing list >> > Linuxppc-dev@lists.ozlabs.org >> > https://lists.ozlabs.org/listinfo/linuxppc-dev >> > > -- > David Gibson | I'll have my music baroque, and my code > david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ > | _way_ _around_! > http://www.ozlabs.org/~dgibson >
On Wed, Dec 23, 2015 at 03:16:39PM +0300, Denis Kirjanov wrote: > On 12/23/15, David Gibson <david@gibson.dropbear.id.au> wrote: > > On Tue, Dec 22, 2015 at 02:19:20PM +0300, Denis Kirjanov wrote: > >> On 12/22/15, David Gibson <david@gibson.dropbear.id.au> wrote: > >> > This patch adds a special file in /sys/kernel/mm which can be used to > >> > view > >> > the current size of the hash page table (as a bit shift) and to trigger > >> > a resize of the hash table on PAPR guests. > >> > > >> > Logically this would make more sense as a debugfs file, but that > >> > doesn't > >> > see to provide an obvious way to have a "store" function that has an > >> > effect > >> > other than updating a variable. > >> > > >> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > >> > --- > >> > arch/powerpc/platforms/pseries/lpar.c | 41 > >> > +++++++++++++++++++++++++++++++++++ > >> > 1 file changed, 41 insertions(+) > >> > > >> > diff --git a/arch/powerpc/platforms/pseries/lpar.c > >> > b/arch/powerpc/platforms/pseries/lpar.c > >> > index aadb242..2759767 100644 > >> > --- a/arch/powerpc/platforms/pseries/lpar.c > >> > +++ b/arch/powerpc/platforms/pseries/lpar.c > >> > @@ -903,3 +903,44 @@ int pSeries_lpar_resize_hpt(unsigned long shift) > >> > > >> > return 0; > >> > } > >> > + > >> > +static ssize_t ppc64_pft_size_show(struct kobject *kobj, > >> > + struct kobj_attribute *attr, char *buf) > >> > +{ > >> > + return sprintf(buf, "%llu\n", (unsigned long long)ppc64_pft_size); > >> > +} > >> > + > >> > +static ssize_t ppc64_pft_size_store(struct kobject *kobj, > >> > + struct kobj_attribute *attr, > >> > + const char *buf, size_t count) > >> > +{ > >> > + unsigned long new_shift; > >> > + int rc; > >> > + > >> > + pr_err("lpar: ppc64_pft_size_store() count=%zd\n", count); > >> Why pr_err? > > > > Uh.. good question. That's a not particularly useful debug comment I > > should just remove anyway. > > > >> > + > >> > + if (kstrtoul(buf, 0, &new_shift)) > >> > + return -EINVAL; > >> > + > >> > + rc = pSeries_lpar_resize_hpt(new_shift); > >> > + if (rc < 0) > >> > + return rc; > >> > + > >> > + return count; > >> > +} > >> > + > >> > +static struct kobj_attribute ppc64_pft_size_attr = > >> > + __ATTR(ppc64_pft_size, 0600, ppc64_pft_size_show, > >> > ppc64_pft_size_store); > >> > + > >> > +static int __init pseries_lpar_sysfs(void) > >> > +{ > >> > + int rc; > >> > + > >> > + rc = sysfs_create_file(mm_kobj, &ppc64_pft_size_attr.attr); > >> > + if (rc) > >> > + pr_err("lpar: unable to create ppc64_pft_size sysfs file (%d)\n", > >> > + rc); > >> > + > >> > + return 0; > >> I think that we need to return rc value, right? > > > > No. Failing to register this sysfs file shouldn't cause the boot to fail. > Then we don't need rc at all ;) Except to report the error code in the message, which is useful.
diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c index aadb242..2759767 100644 --- a/arch/powerpc/platforms/pseries/lpar.c +++ b/arch/powerpc/platforms/pseries/lpar.c @@ -903,3 +903,44 @@ int pSeries_lpar_resize_hpt(unsigned long shift) return 0; } + +static ssize_t ppc64_pft_size_show(struct kobject *kobj, + struct kobj_attribute *attr, char *buf) +{ + return sprintf(buf, "%llu\n", (unsigned long long)ppc64_pft_size); +} + +static ssize_t ppc64_pft_size_store(struct kobject *kobj, + struct kobj_attribute *attr, + const char *buf, size_t count) +{ + unsigned long new_shift; + int rc; + + pr_err("lpar: ppc64_pft_size_store() count=%zd\n", count); + + if (kstrtoul(buf, 0, &new_shift)) + return -EINVAL; + + rc = pSeries_lpar_resize_hpt(new_shift); + if (rc < 0) + return rc; + + return count; +} + +static struct kobj_attribute ppc64_pft_size_attr = + __ATTR(ppc64_pft_size, 0600, ppc64_pft_size_show, ppc64_pft_size_store); + +static int __init pseries_lpar_sysfs(void) +{ + int rc; + + rc = sysfs_create_file(mm_kobj, &ppc64_pft_size_attr.attr); + if (rc) + pr_err("lpar: unable to create ppc64_pft_size sysfs file (%d)\n", + rc); + + return 0; +} +machine_device_initcall(pseries, pseries_lpar_sysfs);
This patch adds a special file in /sys/kernel/mm which can be used to view the current size of the hash page table (as a bit shift) and to trigger a resize of the hash table on PAPR guests. Logically this would make more sense as a debugfs file, but that doesn't see to provide an obvious way to have a "store" function that has an effect other than updating a variable. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> --- arch/powerpc/platforms/pseries/lpar.c | 41 +++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+)