From: Luiz Capitulino <lcapitulino@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: David Rientjes <rientjes@google.com>,
Joonsoo Kim <iamjoonsoo.kim@lge.com>,
Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>,
"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [patch] mm, hugetlb: generalize writes to nr_hugepages
Date: Wed, 9 Jul 2014 09:31:37 -0400 [thread overview]
Message-ID: <20140709093137.5a6ab051@redhat.com> (raw)
In-Reply-To: <20140708151113.dd1469faea6177959356620b@linux-foundation.org>
On Tue, 8 Jul 2014 15:11:13 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:
> On Wed, 2 Jul 2014 17:44:46 -0700 (PDT) David Rientjes <rientjes@google.com> wrote:
>
> > > > @@ -2248,36 +2257,18 @@ static int hugetlb_sysctl_handler_common(bool obey_mempolicy,
> > > > void __user *buffer, size_t *length, loff_t *ppos)
> > > > {
> > > > struct hstate *h = &default_hstate;
> > > > - unsigned long tmp;
> > > > + unsigned long tmp = h->max_huge_pages;
> > > > int ret;
> > > >
> > > > - if (!hugepages_supported())
> > > > - return -ENOTSUPP;
> > >
> > > Shouldn't you add this check to __nr_hugepages_store_common()? Otherwise
> > > looks good to me.
> > >
> >
> > Hmm, I think you're right but I don't think __nr_hugepages_store_common()
> > is the right place: if we have a legitimate hstate for the sysfs tunables
> > then we should support hugepages. I think this should be kept in
> > hugetlb_sysctl_handler_common().
>
> This?
Yes.
>
> --- a/mm/hugetlb.c~mm-hugetlb-generalize-writes-to-nr_hugepages-fix
> +++ a/mm/hugetlb.c
> @@ -2260,6 +2260,9 @@ static int hugetlb_sysctl_handler_common
> unsigned long tmp = h->max_huge_pages;
> int ret;
>
> + if (!hugepages_supported())
> + return -ENOTSUPP;
> +
> table->data = &tmp;
> table->maxlen = sizeof(unsigned long);
> ret = proc_doulongvec_minmax(table, write, buffer, length, ppos);
> _
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
WARNING: multiple messages have this Message-ID (diff)
From: Luiz Capitulino <lcapitulino@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: David Rientjes <rientjes@google.com>,
Joonsoo Kim <iamjoonsoo.kim@lge.com>,
Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>,
"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [patch] mm, hugetlb: generalize writes to nr_hugepages
Date: Wed, 9 Jul 2014 09:31:37 -0400 [thread overview]
Message-ID: <20140709093137.5a6ab051@redhat.com> (raw)
In-Reply-To: <20140708151113.dd1469faea6177959356620b@linux-foundation.org>
On Tue, 8 Jul 2014 15:11:13 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:
> On Wed, 2 Jul 2014 17:44:46 -0700 (PDT) David Rientjes <rientjes@google.com> wrote:
>
> > > > @@ -2248,36 +2257,18 @@ static int hugetlb_sysctl_handler_common(bool obey_mempolicy,
> > > > void __user *buffer, size_t *length, loff_t *ppos)
> > > > {
> > > > struct hstate *h = &default_hstate;
> > > > - unsigned long tmp;
> > > > + unsigned long tmp = h->max_huge_pages;
> > > > int ret;
> > > >
> > > > - if (!hugepages_supported())
> > > > - return -ENOTSUPP;
> > >
> > > Shouldn't you add this check to __nr_hugepages_store_common()? Otherwise
> > > looks good to me.
> > >
> >
> > Hmm, I think you're right but I don't think __nr_hugepages_store_common()
> > is the right place: if we have a legitimate hstate for the sysfs tunables
> > then we should support hugepages. I think this should be kept in
> > hugetlb_sysctl_handler_common().
>
> This?
Yes.
>
> --- a/mm/hugetlb.c~mm-hugetlb-generalize-writes-to-nr_hugepages-fix
> +++ a/mm/hugetlb.c
> @@ -2260,6 +2260,9 @@ static int hugetlb_sysctl_handler_common
> unsigned long tmp = h->max_huge_pages;
> int ret;
>
> + if (!hugepages_supported())
> + return -ENOTSUPP;
> +
> table->data = &tmp;
> table->maxlen = sizeof(unsigned long);
> ret = proc_doulongvec_minmax(table, write, buffer, length, ppos);
> _
>
next prev parent reply other threads:[~2014-07-09 13:31 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-06-30 23:57 [patch] mm, hugetlb: generalize writes to nr_hugepages David Rientjes
2014-06-30 23:57 ` David Rientjes
2014-07-01 0:46 ` [patch] mm, hugetlb: remove hugetlb_zero and hugetlb_infinity David Rientjes
2014-07-01 0:46 ` David Rientjes
2014-07-01 14:05 ` Naoya Horiguchi
2014-07-01 14:05 ` Naoya Horiguchi
2014-07-02 21:38 ` Luiz Capitulino
2014-07-02 21:38 ` Luiz Capitulino
2014-07-01 13:09 ` [patch] mm, hugetlb: generalize writes to nr_hugepages Naoya Horiguchi
2014-07-01 13:09 ` Naoya Horiguchi
2014-07-02 21:25 ` Luiz Capitulino
2014-07-02 21:25 ` Luiz Capitulino
2014-07-03 0:44 ` David Rientjes
2014-07-03 0:44 ` David Rientjes
2014-07-03 3:37 ` Luiz Capitulino
2014-07-03 3:37 ` Luiz Capitulino
2014-07-08 22:11 ` Andrew Morton
2014-07-08 22:11 ` Andrew Morton
2014-07-09 13:31 ` Luiz Capitulino [this message]
2014-07-09 13:31 ` Luiz Capitulino
2014-07-03 2:04 ` Davidlohr Bueso
2014-07-03 2:04 ` Davidlohr Bueso
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140709093137.5a6ab051@redhat.com \
--to=lcapitulino@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=iamjoonsoo.kim@lge.com \
--cc=kirill.shutemov@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=n-horiguchi@ah.jp.nec.com \
--cc=rientjes@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.