All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wanpeng Li <liwp.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: "Aneesh Kumar K.V"
	<aneesh.kumar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Cc: Andrew Morton
	<akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>,
	Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>,
	Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Wanpeng Li <liwp.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Subject: Re: [PATCH] mm/hugetlb_cgroup: Add list_del to remove unused page from hugepage_activelist when hugepage migration
Date: Wed, 11 Jul 2012 18:16:18 +0800	[thread overview]
Message-ID: <20120711101618.GA12033@kernel> (raw)
In-Reply-To: <87txxeiqc0.fsf-6yE53ggjAfyqSkle7U1LjlaTQe2KTcn/@public.gmane.org>

On Wed, Jul 11, 2012 at 03:15:19PM +0530, Aneesh Kumar K.V wrote:
>Wanpeng Li <liwp.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
>
>> On Wed, Jul 11, 2012 at 02:02:23PM +0530, Aneesh Kumar K.V wrote:
>>>Wanpeng Li <liwp.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
>>>
>>>> From: Wanpeng Li <liwp-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
>>>>
>>>> hugepage_activelist is used to track currently used HugeTLB pages.
>>>> We can find the in-use HugeTLB pages to support HugeTLB cgroup
>>>> removal. Don't keep unused page in hugetlb_activelist too long.
>>>> Otherwise, on cgroup removal we should update the unused page's 
>>>> charge to parent count. To reduce this overhead, remove unused page 
>>>> from hugepage_activelist immediately.
>>>>
>>>> Signed-off-by: Wanpeng Li <liwp.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>> ---
>>>>  mm/hugetlb_cgroup.c |    1 +
>>>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/mm/hugetlb_cgroup.c b/mm/hugetlb_cgroup.c
>>>> index b834e8d..d819d66 100644
>>>> --- a/mm/hugetlb_cgroup.c
>>>> +++ b/mm/hugetlb_cgroup.c
>>>> @@ -398,6 +398,7 @@ void hugetlb_cgroup_migrate(struct page *oldhpage, struct page *newhpage)
>>>>  	spin_lock(&hugetlb_lock);
>>>>  	h_cg = hugetlb_cgroup_from_page(oldhpage);
>>>>  	set_hugetlb_cgroup(oldhpage, NULL);
>>>> +	list_del(&oldhpage->lru);
>>>>
>>>>  	/* move the h_cg details to new cgroup */
>>>>  	set_hugetlb_cgroup(newhpage, h_cg);
>>>
>>>put_page on the oldhpage will do that. If we do list_del here
>>>free_huge_page() will have error list_move().
>>
>> I don't know if move the page to other lists when migration in order 
>> to reduce overhead I mentioned make sense. :-)
>>
>
>Even if we are in the active_list, we don't get anything wrong because
>hugetlb cgroup is set to NULL for the old hugetlb page. The above patch is
>wrong and will result in crash when we try to put_page the oldhpage.

Hi aneesh, I am happy we are in the same company. I am a guy in LTC
China. I want to know if there any features or works about hugetlb
cgroup I can do with you!

Regards,
Wanpeng Li

>
>-aneesh
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at  http://www.tux.org/lkml/

WARNING: multiple messages have this Message-ID (diff)
From: Wanpeng Li <liwp.linux@gmail.com>
To: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Li Zefan <lizefan@huawei.com>, Tejun Heo <tj@kernel.org>,
	cgroups@vger.kernel.org, linux-kernel@vger.kernel.org,
	Wanpeng Li <liwp.linux@gmail.com>
Subject: Re: [PATCH] mm/hugetlb_cgroup: Add list_del to remove unused page from hugepage_activelist when hugepage migration
Date: Wed, 11 Jul 2012 18:16:18 +0800	[thread overview]
Message-ID: <20120711101618.GA12033@kernel> (raw)
In-Reply-To: <87txxeiqc0.fsf@skywalker.in.ibm.com>

On Wed, Jul 11, 2012 at 03:15:19PM +0530, Aneesh Kumar K.V wrote:
>Wanpeng Li <liwp.linux@gmail.com> writes:
>
>> On Wed, Jul 11, 2012 at 02:02:23PM +0530, Aneesh Kumar K.V wrote:
>>>Wanpeng Li <liwp.linux@gmail.com> writes:
>>>
>>>> From: Wanpeng Li <liwp@linux.vnet.ibm.com>
>>>>
>>>> hugepage_activelist is used to track currently used HugeTLB pages.
>>>> We can find the in-use HugeTLB pages to support HugeTLB cgroup
>>>> removal. Don't keep unused page in hugetlb_activelist too long.
>>>> Otherwise, on cgroup removal we should update the unused page's 
>>>> charge to parent count. To reduce this overhead, remove unused page 
>>>> from hugepage_activelist immediately.
>>>>
>>>> Signed-off-by: Wanpeng Li <liwp.linux@gmail.com>
>>>> ---
>>>>  mm/hugetlb_cgroup.c |    1 +
>>>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/mm/hugetlb_cgroup.c b/mm/hugetlb_cgroup.c
>>>> index b834e8d..d819d66 100644
>>>> --- a/mm/hugetlb_cgroup.c
>>>> +++ b/mm/hugetlb_cgroup.c
>>>> @@ -398,6 +398,7 @@ void hugetlb_cgroup_migrate(struct page *oldhpage, struct page *newhpage)
>>>>  	spin_lock(&hugetlb_lock);
>>>>  	h_cg = hugetlb_cgroup_from_page(oldhpage);
>>>>  	set_hugetlb_cgroup(oldhpage, NULL);
>>>> +	list_del(&oldhpage->lru);
>>>>
>>>>  	/* move the h_cg details to new cgroup */
>>>>  	set_hugetlb_cgroup(newhpage, h_cg);
>>>
>>>put_page on the oldhpage will do that. If we do list_del here
>>>free_huge_page() will have error list_move().
>>
>> I don't know if move the page to other lists when migration in order 
>> to reduce overhead I mentioned make sense. :-)
>>
>
>Even if we are in the active_list, we don't get anything wrong because
>hugetlb cgroup is set to NULL for the old hugetlb page. The above patch is
>wrong and will result in crash when we try to put_page the oldhpage.

Hi aneesh, I am happy we are in the same company. I am a guy in LTC
China. I want to know if there any features or works about hugetlb
cgroup I can do with you!

Regards,
Wanpeng Li

>
>-aneesh
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at  http://www.tux.org/lkml/

  parent reply	other threads:[~2012-07-11 10:16 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-11  3:51 [PATCH] mm/hugetlb_cgroup: Add list_del to remove unused page from hugepage_activelist when hugepage migration Wanpeng Li
2012-07-11  3:51 ` Wanpeng Li
2012-07-11  8:32 ` Aneesh Kumar K.V
2012-07-11  8:32   ` Aneesh Kumar K.V
2012-07-11  8:32   ` Aneesh Kumar K.V
     [not found]   ` <87wr2aitpk.fsf-6yE53ggjAfyqSkle7U1LjlaTQe2KTcn/@public.gmane.org>
2012-07-11  9:17     ` Wanpeng Li
2012-07-11  9:17       ` Wanpeng Li
2012-07-11  9:45       ` Aneesh Kumar K.V
     [not found]         ` <87txxeiqc0.fsf-6yE53ggjAfyqSkle7U1LjlaTQe2KTcn/@public.gmane.org>
2012-07-11 10:16           ` Wanpeng Li [this message]
2012-07-11 10:16             ` Wanpeng Li

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=20120711101618.GA12033@kernel \
    --to=liwp.linux-re5jqeeqqe8avxtiumwx3w@public.gmane.org \
    --cc=akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org \
    --cc=aneesh.kumar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org \
    --cc=cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org \
    --cc=tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    /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.