Re: [RFC 3/3] pseries: sysfs hack to trigger a hash page table resize

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3147 bytes --]

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.

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]