linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Punit Agrawal <punit.agrawal@arm.com>
To: TSUKADA Koutaro <tsukada@ascade.co.jp>
Cc: Johannes Weiner <hannes@cmpxchg.org>,
	 Michal Hocko <mhocko@kernel.org>,
	 Vladimir Davydov <vdavydov.dev@gmail.com>,
	 Jonathan Corbet <corbet@lwn.net>,
	 "Luis R. Rodriguez" <mcgrof@kernel.org>,
	 Kees Cook <keescook@chromium.org>,
	 Andrew Morton <akpm@linux-foundation.org>,
	 Roman Gushchin <guro@fb.com>,
	 David Rientjes <rientjes@google.com>,
	 Mike Kravetz <mike.kravetz@oracle.com>,
	 "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>,
	 Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>,
	 Anshuman Khandual <khandual@linux.vnet.ibm.com>,
	 Marc-Andre Lureau <marcandre.lureau@redhat.com>,
	 Dan Williams <dan.j.williams@intel.com>,
	 Vlastimil Babka <vbabka@suse.cz>,
	 linux-doc@vger.kernel.org,  linux-kernel@vger.kernel.org,
	 linux-fsdevel@vger.kernel.org,  linux-mm@kvack.org,
	 cgroups@vger.kernel.org
Subject: Re: [PATCH v2 3/7] memcg: use compound_order rather than hpage_nr_pages
Date: Fri, 18 May 2018 18:51:07 +0100	[thread overview]
Message-ID: <87sh6ozwc4.fsf@e105922-lin.cambridge.arm.com> (raw)
In-Reply-To: <87wow0zwja.fsf@e105922-lin.cambridge.arm.com> (Punit Agrawal's message of "Fri, 18 May 2018 18:46:49 +0100")

Punit Agrawal <punit.agrawal@arm.com> writes:

> Tsukada-san,
>
> I am not familiar with memcg so can't comment about whether the patchset
> is the right way to solve the problem outlined in the cover letter but
> had a couple of comments about this patch.
>
> TSUKADA Koutaro <tsukada@ascade.co.jp> writes:
>
>> The current memcg implementation assumes that the compound page is THP.
>> In order to be able to charge surplus hugepage, we use compound_order.
>>
>> Signed-off-by: TSUKADA Koutaro <tsukada@ascade.co.jp>
>
> Please move this before Patch 1/7. This is to prevent wrong accounting
> of pages to memcg for size != PMD_SIZE.

I just noticed that the default state is off so the change isn't enabled
until the sysfs node is exposed in the next patch. Please ignore this
comment.

One below still applies.

>
>> ---
>>  memcontrol.c |   10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> index 2bd3df3..a8f1ff8 100644
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
>> @@ -4483,7 +4483,7 @@ static int mem_cgroup_move_account(struct page *page,
>>  				   struct mem_cgroup *to)
>>  {
>>  	unsigned long flags;
>> -	unsigned int nr_pages = compound ? hpage_nr_pages(page) : 1;
>> +	unsigned int nr_pages = compound ? (1 << compound_order(page)) : 1;
>
> Instead of replacing calls to hpage_nr_pages(), is it possible to modify
> it to do the calculation?
>
> Thanks,
> Punit
>
>>  	int ret;
>>  	bool anon;
>>
>> @@ -5417,7 +5417,7 @@ int mem_cgroup_try_charge(struct page *page, struct mm_struct *mm,
>>  			  bool compound)
>>  {
>>  	struct mem_cgroup *memcg = NULL;
>> -	unsigned int nr_pages = compound ? hpage_nr_pages(page) : 1;
>> +	unsigned int nr_pages = compound ? (1 << compound_order(page)) : 1;
>>  	int ret = 0;
>>
>>  	if (mem_cgroup_disabled())
>> @@ -5478,7 +5478,7 @@ int mem_cgroup_try_charge(struct page *page, struct mm_struct *mm,
>>  void mem_cgroup_commit_charge(struct page *page, struct mem_cgroup *memcg,
>>  			      bool lrucare, bool compound)
>>  {
>> -	unsigned int nr_pages = compound ? hpage_nr_pages(page) : 1;
>> +	unsigned int nr_pages = compound ? (1 << compound_order(page)) : 1;
>>
>>  	VM_BUG_ON_PAGE(!page->mapping, page);
>>  	VM_BUG_ON_PAGE(PageLRU(page) && !lrucare, page);
>> @@ -5522,7 +5522,7 @@ void mem_cgroup_commit_charge(struct page *page, struct mem_cgroup *memcg,
>>  void mem_cgroup_cancel_charge(struct page *page, struct mem_cgroup *memcg,
>>  		bool compound)
>>  {
>> -	unsigned int nr_pages = compound ? hpage_nr_pages(page) : 1;
>> +	unsigned int nr_pages = compound ? (1 << compound_order(page)) : 1;
>>
>>  	if (mem_cgroup_disabled())
>>  		return;
>> @@ -5729,7 +5729,7 @@ void mem_cgroup_migrate(struct page *oldpage, struct page *newpage)
>>
>>  	/* Force-charge the new page. The old one will be freed soon */
>>  	compound = PageTransHuge(newpage);
>> -	nr_pages = compound ? hpage_nr_pages(newpage) : 1;
>> +	nr_pages = compound ? (1 << compound_order(newpage)) : 1;
>>
>>  	page_counter_charge(&memcg->memory, nr_pages);
>>  	if (do_memsw_account())

  reply	other threads:[~2018-05-18 17:51 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-18  4:27 [PATCH v2 0/7] mm: pages for hugetlb's overcommit may be able to charge to memcg TSUKADA Koutaro
2018-05-18  4:29 ` [PATCH v2 1/7] hugetlb: introduce charge_surplus_huge_pages to struct hstate TSUKADA Koutaro
2018-05-18  4:32 ` [PATCH v2 2/7] hugetlb: support migrate charging for surplus hugepages TSUKADA Koutaro
2018-05-18  4:34 ` [PATCH v2 3/7] memcg: use compound_order rather than hpage_nr_pages TSUKADA Koutaro
2018-05-18 17:46   ` Punit Agrawal
2018-05-18 17:51     ` Punit Agrawal [this message]
2018-05-21  3:48       ` TSUKADA Koutaro
2018-05-21 14:53         ` Punit Agrawal
2018-05-18  4:36 ` [PATCH v2 4/7] mm, sysctl: make charging surplus hugepages controllable TSUKADA Koutaro
2018-05-18  4:37 ` [PATCH v2 5/7] hugetlb: add charge_surplus_hugepages attribute TSUKADA Koutaro
2018-05-18  4:39 ` [PATCH v2 6/7] Documentation, hugetlb: describe about charge_surplus_hugepages, TSUKADA Koutaro
2018-05-18  4:41 ` [PATCH v2 7/7] memcg: supports movement of surplus hugepages statistics TSUKADA Koutaro
2018-05-21 14:52 ` [PATCH v2 0/7] mm: pages for hugetlb's overcommit may be able to charge to memcg Punit Agrawal
2018-05-22 12:56   ` TSUKADA Koutaro
2018-05-21 18:07 ` Mike Kravetz
2018-05-22 13:04   ` TSUKADA Koutaro
2018-05-22 18:54     ` Michal Hocko
2018-05-24  4:39       ` TSUKADA Koutaro
2018-05-24  8:20         ` Michal Hocko
2018-05-24 12:58           ` TSUKADA Koutaro
2018-05-24 13:24             ` Michal Hocko
2018-05-25  1:51               ` TSUKADA Koutaro
2018-05-22 20:28     ` Mike Kravetz
2018-05-22 13:51 ` Michal Hocko
2018-05-24  4:26   ` TSUKADA Koutaro
2018-05-24  8:27     ` Michal Hocko
2018-05-24 17:45     ` Mike Kravetz
2018-05-25  1:55       ` TSUKADA Koutaro

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=87sh6ozwc4.fsf@e105922-lin.cambridge.arm.com \
    --to=punit.agrawal@arm.com \
    --cc=akpm@linux-foundation.org \
    --cc=aneesh.kumar@linux.vnet.ibm.com \
    --cc=cgroups@vger.kernel.org \
    --cc=corbet@lwn.net \
    --cc=dan.j.williams@intel.com \
    --cc=guro@fb.com \
    --cc=hannes@cmpxchg.org \
    --cc=keescook@chromium.org \
    --cc=khandual@linux.vnet.ibm.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=marcandre.lureau@redhat.com \
    --cc=mcgrof@kernel.org \
    --cc=mhocko@kernel.org \
    --cc=mike.kravetz@oracle.com \
    --cc=n-horiguchi@ah.jp.nec.com \
    --cc=rientjes@google.com \
    --cc=tsukada@ascade.co.jp \
    --cc=vbabka@suse.cz \
    --cc=vdavydov.dev@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).