Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1372124 > unrolled thread

[patch] mm, hugetlb_cgroup: round limit_in_bytes down to hugepage size

Started byDavid Rientjes <rientjes@google.com>
First post2016-04-06 03:30 +0200
Last post2016-04-25 23:40 +0200
Articles 12 — 4 participants

Back to article view | Back to linux.kernel


Contents

  [patch] mm, hugetlb_cgroup: round limit_in_bytes down to hugepage  size David Rientjes <rientjes@google.com> - 2016-04-06 03:30 +0200
    Re: [patch] mm, hugetlb_cgroup: round limit_in_bytes down to hugepage  size Nikolay Borisov <kernel@kyup.com> - 2016-04-06 09:30 +0200
      Re: [patch] mm, hugetlb_cgroup: round limit_in_bytes down to hugepage  size Nikolay Borisov <kernel@kyup.com> - 2016-04-06 09:40 +0200
        Re: [patch] mm, hugetlb_cgroup: round limit_in_bytes down to  hugepage size Michal Hocko <mhocko@kernel.org> - 2016-04-06 11:20 +0200
        [patch v2] mm, hugetlb_cgroup: round limit_in_bytes down to hugepage  size David Rientjes <rientjes@google.com> - 2016-04-07 00:20 +0200
          Re: [patch v2] mm, hugetlb_cgroup: round limit_in_bytes down to  hugepage size Michal Hocko <mhocko@kernel.org> - 2016-04-07 15:00 +0200
            Re: [patch v2] mm, hugetlb_cgroup: round limit_in_bytes down to  hugepage size David Rientjes <rientjes@google.com> - 2016-04-14 22:30 +0200
              Re: [patch v2] mm, hugetlb_cgroup: round limit_in_bytes down to  hugepage size Michal Hocko <mhocko@kernel.org> - 2016-04-15 15:30 +0200
                Re: [patch v2] mm, hugetlb_cgroup: round limit_in_bytes down to  hugepage size David Rientjes <rientjes@google.com> - 2016-04-18 23:30 +0200
                  Re: [patch v2] mm, hugetlb_cgroup: round limit_in_bytes down to  hugepage size Andrew Morton <akpm@linux-foundation.org> - 2016-04-26 00:00 +0200
                    [patch v3] mm, hugetlb_cgroup: round limit_in_bytes down to hugepage  size David Rientjes <rientjes@google.com> - 2016-04-26 02:00 +0200
          Re: [patch v2] mm, hugetlb_cgroup: round limit_in_bytes down to  hugepage size David Rientjes <rientjes@google.com> - 2016-04-25 23:40 +0200

#1372124 — [patch] mm, hugetlb_cgroup: round limit_in_bytes down to hugepage size

FromDavid Rientjes <rientjes@google.com>
Date2016-04-06 03:30 +0200
Subject[patch] mm, hugetlb_cgroup: round limit_in_bytes down to hugepage size
Message-ID<rkKsa-4u3-5@gated-at.bofh.it>
The page_counter rounds limits down to page size values.  This makes
sense, except in the case of hugetlb_cgroup where it's not possible to
charge partial hugepages.

Round the hugetlb_cgroup limit down to hugepage size.

Signed-off-by: David Rientjes <rientjes@google.com>
---
 mm/hugetlb_cgroup.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mm/hugetlb_cgroup.c b/mm/hugetlb_cgroup.c
--- a/mm/hugetlb_cgroup.c
+++ b/mm/hugetlb_cgroup.c
@@ -288,6 +288,7 @@ static ssize_t hugetlb_cgroup_write(struct kernfs_open_file *of,
 
 	switch (MEMFILE_ATTR(of_cft(of)->private)) {
 	case RES_LIMIT:
+		nr_pages &= ~((1 << huge_page_order(&hstates[idx])) - 1);
 		mutex_lock(&hugetlb_limit_mutex);
 		ret = page_counter_limit(&h_cg->hugepage[idx], nr_pages);
 		mutex_unlock(&hugetlb_limit_mutex);

[toc] | [next] | [standalone]


#1372255

FromNikolay Borisov <kernel@kyup.com>
Date2016-04-06 09:30 +0200
Message-ID<rkQ4y-nZ-9@gated-at.bofh.it>
In reply to#1372124

On 04/06/2016 04:25 AM, David Rientjes wrote:
> The page_counter rounds limits down to page size values.  This makes
> sense, except in the case of hugetlb_cgroup where it's not possible to
> charge partial hugepages.
> 
> Round the hugetlb_cgroup limit down to hugepage size.
> 
> Signed-off-by: David Rientjes <rientjes@google.com>
> ---
>  mm/hugetlb_cgroup.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/mm/hugetlb_cgroup.c b/mm/hugetlb_cgroup.c
> --- a/mm/hugetlb_cgroup.c
> +++ b/mm/hugetlb_cgroup.c
> @@ -288,6 +288,7 @@ static ssize_t hugetlb_cgroup_write(struct kernfs_open_file *of,
>  
>  	switch (MEMFILE_ATTR(of_cft(of)->private)) {
>  	case RES_LIMIT:
> +		nr_pages &= ~((1 << huge_page_order(&hstates[idx])) - 1);

Why not:

nr_pages = round_down(nr_pages, huge_page_order(&hstates[idx]));


>  		mutex_lock(&hugetlb_limit_mutex);
>  		ret = page_counter_limit(&h_cg->hugepage[idx], nr_pages);
>  		mutex_unlock(&hugetlb_limit_mutex);
> 

[toc] | [prev] | [next] | [standalone]


#1372258

FromNikolay Borisov <kernel@kyup.com>
Date2016-04-06 09:40 +0200
Message-ID<rkQee-sD-11@gated-at.bofh.it>
In reply to#1372255

On 04/06/2016 10:26 AM, Nikolay Borisov wrote:
> 
> 
> On 04/06/2016 04:25 AM, David Rientjes wrote:
>> The page_counter rounds limits down to page size values.  This makes
>> sense, except in the case of hugetlb_cgroup where it's not possible to
>> charge partial hugepages.
>>
>> Round the hugetlb_cgroup limit down to hugepage size.
>>
>> Signed-off-by: David Rientjes <rientjes@google.com>
>> ---
>>  mm/hugetlb_cgroup.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/mm/hugetlb_cgroup.c b/mm/hugetlb_cgroup.c
>> --- a/mm/hugetlb_cgroup.c
>> +++ b/mm/hugetlb_cgroup.c
>> @@ -288,6 +288,7 @@ static ssize_t hugetlb_cgroup_write(struct kernfs_open_file *of,
>>  
>>  	switch (MEMFILE_ATTR(of_cft(of)->private)) {
>>  	case RES_LIMIT:
>> +		nr_pages &= ~((1 << huge_page_order(&hstates[idx])) - 1);
> 
> Why not:
> 
> nr_pages = round_down(nr_pages, huge_page_order(&hstates[idx]));

Oops, that should be:

round_down(nr_pages, 1 << huge_page_order(&hstates[idx]));

> 
> 
>>  		mutex_lock(&hugetlb_limit_mutex);
>>  		ret = page_counter_limit(&h_cg->hugepage[idx], nr_pages);
>>  		mutex_unlock(&hugetlb_limit_mutex);
>>

[toc] | [prev] | [next] | [standalone]


#1372318 — Re: [patch] mm, hugetlb_cgroup: round limit_in_bytes down to hugepage size

FromMichal Hocko <mhocko@kernel.org>
Date2016-04-06 11:20 +0200
SubjectRe: [patch] mm, hugetlb_cgroup: round limit_in_bytes down to hugepage size
Message-ID<rkRN0-1BH-17@gated-at.bofh.it>
In reply to#1372258
On Wed 06-04-16 10:33:19, Nikolay Borisov wrote:
> 
> 
> On 04/06/2016 10:26 AM, Nikolay Borisov wrote:
> > 
> > 
> > On 04/06/2016 04:25 AM, David Rientjes wrote:
> >> The page_counter rounds limits down to page size values.  This makes
> >> sense, except in the case of hugetlb_cgroup where it's not possible to
> >> charge partial hugepages.
> >>
> >> Round the hugetlb_cgroup limit down to hugepage size.
> >>
> >> Signed-off-by: David Rientjes <rientjes@google.com>
> >> ---
> >>  mm/hugetlb_cgroup.c | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/mm/hugetlb_cgroup.c b/mm/hugetlb_cgroup.c
> >> --- a/mm/hugetlb_cgroup.c
> >> +++ b/mm/hugetlb_cgroup.c
> >> @@ -288,6 +288,7 @@ static ssize_t hugetlb_cgroup_write(struct kernfs_open_file *of,
> >>  
> >>  	switch (MEMFILE_ATTR(of_cft(of)->private)) {
> >>  	case RES_LIMIT:
> >> +		nr_pages &= ~((1 << huge_page_order(&hstates[idx])) - 1);
> > 
> > Why not:
> > 
> > nr_pages = round_down(nr_pages, huge_page_order(&hstates[idx]));
> 
> Oops, that should be:
> 
> round_down(nr_pages, 1 << huge_page_order(&hstates[idx]));

round_down is a bit nicer.

Anyway
Acked-by: Michal Hocko <mhocko@suse.com>

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]


#1372891 — [patch v2] mm, hugetlb_cgroup: round limit_in_bytes down to hugepage size

FromDavid Rientjes <rientjes@google.com>
Date2016-04-07 00:20 +0200
Subject[patch v2] mm, hugetlb_cgroup: round limit_in_bytes down to hugepage size
Message-ID<rl3XP-2ep-3@gated-at.bofh.it>
In reply to#1372258
The page_counter rounds limits down to page size values.  This makes
sense, except in the case of hugetlb_cgroup where it's not possible to
charge partial hugepages.

Round the hugetlb_cgroup limit down to hugepage size.

Signed-off-by: David Rientjes <rientjes@google.com>
---
 mm/hugetlb_cgroup.c | 35 ++++++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/mm/hugetlb_cgroup.c b/mm/hugetlb_cgroup.c
--- a/mm/hugetlb_cgroup.c
+++ b/mm/hugetlb_cgroup.c
@@ -67,26 +67,42 @@ static inline bool hugetlb_cgroup_have_usage(struct hugetlb_cgroup *h_cg)
 	return false;
 }
 
+static void hugetlb_cgroup_init(struct hugetlb_cgroup *h_cgroup,
+				struct hugetlb_cgroup *parent_h_cgroup)
+{
+	int idx;
+
+	for (idx = 0; idx < HUGE_MAX_HSTATE; idx++) {
+		struct page_counter *counter = &h_cgroup->hugepage[idx];
+		struct page_counter *parent = NULL;
+		unsigned long limit;
+		int ret;
+
+		if (parent_h_cgroup)
+			parent = &parent_h_cgroup->hugepage[idx];
+		page_counter_init(counter, parent);
+
+		limit = round_down(PAGE_COUNTER_MAX,
+				   1 << huge_page_order(&hstates[idx]));
+		ret = page_counter_limit(counter, limit);
+		VM_BUG_ON(ret);
+	}
+}
+
 static struct cgroup_subsys_state *
 hugetlb_cgroup_css_alloc(struct cgroup_subsys_state *parent_css)
 {
 	struct hugetlb_cgroup *parent_h_cgroup = hugetlb_cgroup_from_css(parent_css);
 	struct hugetlb_cgroup *h_cgroup;
-	int idx;
 
 	h_cgroup = kzalloc(sizeof(*h_cgroup), GFP_KERNEL);
 	if (!h_cgroup)
 		return ERR_PTR(-ENOMEM);
 
-	if (parent_h_cgroup) {
-		for (idx = 0; idx < HUGE_MAX_HSTATE; idx++)
-			page_counter_init(&h_cgroup->hugepage[idx],
-					  &parent_h_cgroup->hugepage[idx]);
-	} else {
+	if (!parent_h_cgroup)
 		root_h_cgroup = h_cgroup;
-		for (idx = 0; idx < HUGE_MAX_HSTATE; idx++)
-			page_counter_init(&h_cgroup->hugepage[idx], NULL);
-	}
+
+	hugetlb_cgroup_init(h_cgroup, parent_h_cgroup);
 	return &h_cgroup->css;
 }
 
@@ -285,6 +301,7 @@ static ssize_t hugetlb_cgroup_write(struct kernfs_open_file *of,
 		return ret;
 
 	idx = MEMFILE_IDX(of_cft(of)->private);
+	nr_pages = round_down(nr_pages, 1 << huge_page_order(&hstates[idx]));
 
 	switch (MEMFILE_ATTR(of_cft(of)->private)) {
 	case RES_LIMIT:

[toc] | [prev] | [next] | [standalone]


#1373372 — Re: [patch v2] mm, hugetlb_cgroup: round limit_in_bytes down to hugepage size

FromMichal Hocko <mhocko@kernel.org>
Date2016-04-07 15:00 +0200
SubjectRe: [patch v2] mm, hugetlb_cgroup: round limit_in_bytes down to hugepage size
Message-ID<rlhHs-3TE-19@gated-at.bofh.it>
In reply to#1372891
On Wed 06-04-16 15:10:23, David Rientjes wrote:
[...]
> +static void hugetlb_cgroup_init(struct hugetlb_cgroup *h_cgroup,
> +				struct hugetlb_cgroup *parent_h_cgroup)
> +{
> +	int idx;
> +
> +	for (idx = 0; idx < HUGE_MAX_HSTATE; idx++) {
> +		struct page_counter *counter = &h_cgroup->hugepage[idx];
> +		struct page_counter *parent = NULL;
> +		unsigned long limit;
> +		int ret;
> +
> +		if (parent_h_cgroup)
> +			parent = &parent_h_cgroup->hugepage[idx];
> +		page_counter_init(counter, parent);
> +
> +		limit = round_down(PAGE_COUNTER_MAX,
> +				   1 << huge_page_order(&hstates[idx]));
> +		ret = page_counter_limit(counter, limit);
> +		VM_BUG_ON(ret);
> +	}
> +}

I fail to see the point for this. Why would want to round down
PAGE_COUNTER_MAX? It will never make a real difference. Or am I missing
something?
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]


#1379289 — Re: [patch v2] mm, hugetlb_cgroup: round limit_in_bytes down to hugepage size

FromDavid Rientjes <rientjes@google.com>
Date2016-04-14 22:30 +0200
SubjectRe: [patch v2] mm, hugetlb_cgroup: round limit_in_bytes down to hugepage size
Message-ID<rnW3L-7VI-5@gated-at.bofh.it>
In reply to#1373372
On Thu, 7 Apr 2016, Michal Hocko wrote:

> > +static void hugetlb_cgroup_init(struct hugetlb_cgroup *h_cgroup,
> > +				struct hugetlb_cgroup *parent_h_cgroup)
> > +{
> > +	int idx;
> > +
> > +	for (idx = 0; idx < HUGE_MAX_HSTATE; idx++) {
> > +		struct page_counter *counter = &h_cgroup->hugepage[idx];
> > +		struct page_counter *parent = NULL;
> > +		unsigned long limit;
> > +		int ret;
> > +
> > +		if (parent_h_cgroup)
> > +			parent = &parent_h_cgroup->hugepage[idx];
> > +		page_counter_init(counter, parent);
> > +
> > +		limit = round_down(PAGE_COUNTER_MAX,
> > +				   1 << huge_page_order(&hstates[idx]));
> > +		ret = page_counter_limit(counter, limit);
> > +		VM_BUG_ON(ret);
> > +	}
> > +}
> 
> I fail to see the point for this. Why would want to round down
> PAGE_COUNTER_MAX? It will never make a real difference. Or am I missing
> something?

Did you try the patch?

If we're rounding down the user value, it makes sense to be consistent 
with the upper bound default to specify intent.

[toc] | [prev] | [next] | [standalone]


#1379809 — Re: [patch v2] mm, hugetlb_cgroup: round limit_in_bytes down to hugepage size

FromMichal Hocko <mhocko@kernel.org>
Date2016-04-15 15:30 +0200
SubjectRe: [patch v2] mm, hugetlb_cgroup: round limit_in_bytes down to hugepage size
Message-ID<robYR-3E4-3@gated-at.bofh.it>
In reply to#1379289
On Thu 14-04-16 13:22:30, David Rientjes wrote:
> On Thu, 7 Apr 2016, Michal Hocko wrote:
> 
> > > +static void hugetlb_cgroup_init(struct hugetlb_cgroup *h_cgroup,
> > > +				struct hugetlb_cgroup *parent_h_cgroup)
> > > +{
> > > +	int idx;
> > > +
> > > +	for (idx = 0; idx < HUGE_MAX_HSTATE; idx++) {
> > > +		struct page_counter *counter = &h_cgroup->hugepage[idx];
> > > +		struct page_counter *parent = NULL;
> > > +		unsigned long limit;
> > > +		int ret;
> > > +
> > > +		if (parent_h_cgroup)
> > > +			parent = &parent_h_cgroup->hugepage[idx];
> > > +		page_counter_init(counter, parent);
> > > +
> > > +		limit = round_down(PAGE_COUNTER_MAX,
> > > +				   1 << huge_page_order(&hstates[idx]));
> > > +		ret = page_counter_limit(counter, limit);
> > > +		VM_BUG_ON(ret);
> > > +	}
> > > +}
> > 
> > I fail to see the point for this. Why would want to round down
> > PAGE_COUNTER_MAX? It will never make a real difference. Or am I missing
> > something?
> 
> Did you try the patch?
> 
> If we're rounding down the user value, it makes sense to be consistent 
> with the upper bound default to specify intent.

The point I've tried to raise is why do we care and add a code if we can
never reach that value? Does actually anybody checks for the alignment.
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]


#1382070 — Re: [patch v2] mm, hugetlb_cgroup: round limit_in_bytes down to hugepage size

FromDavid Rientjes <rientjes@google.com>
Date2016-04-18 23:30 +0200
SubjectRe: [patch v2] mm, hugetlb_cgroup: round limit_in_bytes down to hugepage size
Message-ID<rpoU1-422-11@gated-at.bofh.it>
In reply to#1379809
On Fri, 15 Apr 2016, Michal Hocko wrote:

> > > > +static void hugetlb_cgroup_init(struct hugetlb_cgroup *h_cgroup,
> > > > +				struct hugetlb_cgroup *parent_h_cgroup)
> > > > +{
> > > > +	int idx;
> > > > +
> > > > +	for (idx = 0; idx < HUGE_MAX_HSTATE; idx++) {
> > > > +		struct page_counter *counter = &h_cgroup->hugepage[idx];
> > > > +		struct page_counter *parent = NULL;
> > > > +		unsigned long limit;
> > > > +		int ret;
> > > > +
> > > > +		if (parent_h_cgroup)
> > > > +			parent = &parent_h_cgroup->hugepage[idx];
> > > > +		page_counter_init(counter, parent);
> > > > +
> > > > +		limit = round_down(PAGE_COUNTER_MAX,
> > > > +				   1 << huge_page_order(&hstates[idx]));
> > > > +		ret = page_counter_limit(counter, limit);
> > > > +		VM_BUG_ON(ret);
> > > > +	}
> > > > +}
> > > 
> > > I fail to see the point for this. Why would want to round down
> > > PAGE_COUNTER_MAX? It will never make a real difference. Or am I missing
> > > something?
> > 
> > Did you try the patch?
> > 
> > If we're rounding down the user value, it makes sense to be consistent 
> > with the upper bound default to specify intent.
> 
> The point I've tried to raise is why do we care and add a code if we can
> never reach that value? Does actually anybody checks for the alignment.

If the user modifies the value successfully, it can never be restored to 
the default since the write handler rounds down.  It's a matter of 
consistency for a long-term maintainable kernel and prevents bug reports.

[toc] | [prev] | [next] | [standalone]


#1386945 — Re: [patch v2] mm, hugetlb_cgroup: round limit_in_bytes down to hugepage size

FromAndrew Morton <akpm@linux-foundation.org>
Date2016-04-26 00:00 +0200
SubjectRe: [patch v2] mm, hugetlb_cgroup: round limit_in_bytes down to hugepage size
Message-ID<rrWHV-4pq-25@gated-at.bofh.it>
In reply to#1382070
On Mon, 18 Apr 2016 14:23:58 -0700 (PDT) David Rientjes <rientjes@google.com> wrote:

> On Fri, 15 Apr 2016, Michal Hocko wrote:
> 
> > > > > +static void hugetlb_cgroup_init(struct hugetlb_cgroup *h_cgroup,
> > > > > +				struct hugetlb_cgroup *parent_h_cgroup)
> > > > > +{
> > > > > +	int idx;
> > > > > +
> > > > > +	for (idx = 0; idx < HUGE_MAX_HSTATE; idx++) {
> > > > > +		struct page_counter *counter = &h_cgroup->hugepage[idx];
> > > > > +		struct page_counter *parent = NULL;
> > > > > +		unsigned long limit;
> > > > > +		int ret;
> > > > > +
> > > > > +		if (parent_h_cgroup)
> > > > > +			parent = &parent_h_cgroup->hugepage[idx];
> > > > > +		page_counter_init(counter, parent);
> > > > > +
> > > > > +		limit = round_down(PAGE_COUNTER_MAX,
> > > > > +				   1 << huge_page_order(&hstates[idx]));
> > > > > +		ret = page_counter_limit(counter, limit);
> > > > > +		VM_BUG_ON(ret);
> > > > > +	}
> > > > > +}
> > > > 
> > > > I fail to see the point for this. Why would want to round down
> > > > PAGE_COUNTER_MAX? It will never make a real difference. Or am I missing
> > > > something?
> > > 
> > > Did you try the patch?
> > > 
> > > If we're rounding down the user value, it makes sense to be consistent 
> > > with the upper bound default to specify intent.
> > 
> > The point I've tried to raise is why do we care and add a code if we can
> > never reach that value? Does actually anybody checks for the alignment.
> 
> If the user modifies the value successfully, it can never be restored to 
> the default since the write handler rounds down.  It's a matter of 
> consistency for a long-term maintainable kernel and prevents bug reports.

Can we please get the above reasoning into the changelog?

Also, the runtime effects of the patch are unclear - "not possible to
charge partial hugepages" sounds serious, but there's no cc:stable. 
Some clarification there also please.

[toc] | [prev] | [next] | [standalone]


#1387013 — [patch v3] mm, hugetlb_cgroup: round limit_in_bytes down to hugepage size

FromDavid Rientjes <rientjes@google.com>
Date2016-04-26 02:00 +0200
Subject[patch v3] mm, hugetlb_cgroup: round limit_in_bytes down to hugepage size
Message-ID<rrYA2-67A-9@gated-at.bofh.it>
In reply to#1386945
The page_counter rounds limits down to page size values.  This makes
sense, except in the case of hugetlb_cgroup where it's not possible to
charge partial hugepages.  If the hugetlb_cgroup margin is less than the
hugepage size being charged, it will fail as expected.

Round the hugetlb_cgroup limit down to hugepage size, since it is the
effective limit of the cgroup.

For consistency, round down PAGE_COUNTER_MAX as well when a
hugetlb_cgroup is created: this prevents error reports when a user cannot
restore the value to the kernel default.

Signed-off-by: David Rientjes <rientjes@google.com>
---
 v3: update changelog per akpm
     no stable backport needed

 mm/hugetlb_cgroup.c | 35 ++++++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/mm/hugetlb_cgroup.c b/mm/hugetlb_cgroup.c
--- a/mm/hugetlb_cgroup.c
+++ b/mm/hugetlb_cgroup.c
@@ -67,26 +67,42 @@ static inline bool hugetlb_cgroup_have_usage(struct hugetlb_cgroup *h_cg)
 	return false;
 }
 
+static void hugetlb_cgroup_init(struct hugetlb_cgroup *h_cgroup,
+				struct hugetlb_cgroup *parent_h_cgroup)
+{
+	int idx;
+
+	for (idx = 0; idx < HUGE_MAX_HSTATE; idx++) {
+		struct page_counter *counter = &h_cgroup->hugepage[idx];
+		struct page_counter *parent = NULL;
+		unsigned long limit;
+		int ret;
+
+		if (parent_h_cgroup)
+			parent = &parent_h_cgroup->hugepage[idx];
+		page_counter_init(counter, parent);
+
+		limit = round_down(PAGE_COUNTER_MAX,
+				   1 << huge_page_order(&hstates[idx]));
+		ret = page_counter_limit(counter, limit);
+		VM_BUG_ON(ret);
+	}
+}
+
 static struct cgroup_subsys_state *
 hugetlb_cgroup_css_alloc(struct cgroup_subsys_state *parent_css)
 {
 	struct hugetlb_cgroup *parent_h_cgroup = hugetlb_cgroup_from_css(parent_css);
 	struct hugetlb_cgroup *h_cgroup;
-	int idx;
 
 	h_cgroup = kzalloc(sizeof(*h_cgroup), GFP_KERNEL);
 	if (!h_cgroup)
 		return ERR_PTR(-ENOMEM);
 
-	if (parent_h_cgroup) {
-		for (idx = 0; idx < HUGE_MAX_HSTATE; idx++)
-			page_counter_init(&h_cgroup->hugepage[idx],
-					  &parent_h_cgroup->hugepage[idx]);
-	} else {
+	if (!parent_h_cgroup)
 		root_h_cgroup = h_cgroup;
-		for (idx = 0; idx < HUGE_MAX_HSTATE; idx++)
-			page_counter_init(&h_cgroup->hugepage[idx], NULL);
-	}
+
+	hugetlb_cgroup_init(h_cgroup, parent_h_cgroup);
 	return &h_cgroup->css;
 }
 
@@ -285,6 +301,7 @@ static ssize_t hugetlb_cgroup_write(struct kernfs_open_file *of,
 		return ret;
 
 	idx = MEMFILE_IDX(of_cft(of)->private);
+	nr_pages = round_down(nr_pages, 1 << huge_page_order(&hstates[idx]));
 
 	switch (MEMFILE_ATTR(of_cft(of)->private)) {
 	case RES_LIMIT:

[toc] | [prev] | [next] | [standalone]


#1386922 — Re: [patch v2] mm, hugetlb_cgroup: round limit_in_bytes down to hugepage size

FromDavid Rientjes <rientjes@google.com>
Date2016-04-25 23:40 +0200
SubjectRe: [patch v2] mm, hugetlb_cgroup: round limit_in_bytes down to hugepage size
Message-ID<rrWoy-4fh-21@gated-at.bofh.it>
In reply to#1372891
On Wed, 6 Apr 2016, David Rientjes wrote:

> The page_counter rounds limits down to page size values.  This makes
> sense, except in the case of hugetlb_cgroup where it's not possible to
> charge partial hugepages.
> 
> Round the hugetlb_cgroup limit down to hugepage size.
> 
> Signed-off-by: David Rientjes <rientjes@google.com>

May this be merged into -mm?

> ---
>  mm/hugetlb_cgroup.c | 35 ++++++++++++++++++++++++++---------
>  1 file changed, 26 insertions(+), 9 deletions(-)
> 
> diff --git a/mm/hugetlb_cgroup.c b/mm/hugetlb_cgroup.c
> --- a/mm/hugetlb_cgroup.c
> +++ b/mm/hugetlb_cgroup.c
> @@ -67,26 +67,42 @@ static inline bool hugetlb_cgroup_have_usage(struct hugetlb_cgroup *h_cg)
>  	return false;
>  }
>  
> +static void hugetlb_cgroup_init(struct hugetlb_cgroup *h_cgroup,
> +				struct hugetlb_cgroup *parent_h_cgroup)
> +{
> +	int idx;
> +
> +	for (idx = 0; idx < HUGE_MAX_HSTATE; idx++) {
> +		struct page_counter *counter = &h_cgroup->hugepage[idx];
> +		struct page_counter *parent = NULL;
> +		unsigned long limit;
> +		int ret;
> +
> +		if (parent_h_cgroup)
> +			parent = &parent_h_cgroup->hugepage[idx];
> +		page_counter_init(counter, parent);
> +
> +		limit = round_down(PAGE_COUNTER_MAX,
> +				   1 << huge_page_order(&hstates[idx]));
> +		ret = page_counter_limit(counter, limit);
> +		VM_BUG_ON(ret);
> +	}
> +}
> +
>  static struct cgroup_subsys_state *
>  hugetlb_cgroup_css_alloc(struct cgroup_subsys_state *parent_css)
>  {
>  	struct hugetlb_cgroup *parent_h_cgroup = hugetlb_cgroup_from_css(parent_css);
>  	struct hugetlb_cgroup *h_cgroup;
> -	int idx;
>  
>  	h_cgroup = kzalloc(sizeof(*h_cgroup), GFP_KERNEL);
>  	if (!h_cgroup)
>  		return ERR_PTR(-ENOMEM);
>  
> -	if (parent_h_cgroup) {
> -		for (idx = 0; idx < HUGE_MAX_HSTATE; idx++)
> -			page_counter_init(&h_cgroup->hugepage[idx],
> -					  &parent_h_cgroup->hugepage[idx]);
> -	} else {
> +	if (!parent_h_cgroup)
>  		root_h_cgroup = h_cgroup;
> -		for (idx = 0; idx < HUGE_MAX_HSTATE; idx++)
> -			page_counter_init(&h_cgroup->hugepage[idx], NULL);
> -	}
> +
> +	hugetlb_cgroup_init(h_cgroup, parent_h_cgroup);
>  	return &h_cgroup->css;
>  }
>  
> @@ -285,6 +301,7 @@ static ssize_t hugetlb_cgroup_write(struct kernfs_open_file *of,
>  		return ret;
>  
>  	idx = MEMFILE_IDX(of_cft(of)->private);
> +	nr_pages = round_down(nr_pages, 1 << huge_page_order(&hstates[idx]));
>  
>  	switch (MEMFILE_ATTR(of_cft(of)->private)) {
>  	case RES_LIMIT:
> 

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web