linux-kselftest.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mike Kravetz <mike.kravetz@oracle.com>
To: David Rientjes <rientjes@google.com>,
	Mina Almasry <almasrymina@google.com>
Cc: shakeelb@google.com, shuah@kernel.org, gthelen@google.com,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	linux-kselftest@vger.kernel.org, cgroups@vger.kernel.org,
	aneesh.kumar@linux.vnet.ibm.com
Subject: Re: [PATCH v10 2/8] hugetlb_cgroup: add interface for charge/uncharge hugetlb reservations
Date: Wed, 29 Jan 2020 16:41:51 -0800	[thread overview]
Message-ID: <43930e45-7505-1fc2-36ac-69a91a00a336@oracle.com> (raw)
In-Reply-To: <alpine.DEB.2.21.2001291312490.175731@chino.kir.corp.google.com>

On 1/29/20 1:21 PM, David Rientjes wrote:
> On Tue, 14 Jan 2020, Mina Almasry wrote:
> 
>> diff --git a/include/linux/hugetlb_cgroup.h b/include/linux/hugetlb_cgroup.h
>> index 063962f6dfc6a..eab8a70d5bcb5 100644
>> --- a/include/linux/hugetlb_cgroup.h
>> +++ b/include/linux/hugetlb_cgroup.h
>> @@ -20,29 +20,37 @@
>>  struct hugetlb_cgroup;
>>  /*
>>   * Minimum page order trackable by hugetlb cgroup.
>> - * At least 3 pages are necessary for all the tracking information.
>> + * At least 4 pages are necessary for all the tracking information.
>>   */
>>  #define HUGETLB_CGROUP_MIN_ORDER	2
> 
> I always struggle with a way to document and protect these types of 
> usages.  In this case, we are using the private filed of tail pages; in 
> thp code, we enumerate these usages separately in struct page: see "Tail 
> pages of compound page" comment in the union.  Using the private field is 
> fine to store a pointer to the hugetlb_cgroup, but I'm wondering if we can 
> document or protect against future patches not understanding this usage.  
> Otherwise it's implicit beyond this comment.
> 
> Maybe an expanded comment here is the only thing that is needed because 
> it's unique to struct hugetlb_cgroup that describes what struct page 
> represents for the second, third, and (now) fourth tail page.

I think that expanding the comment may be sufficient.  Let's at least
document what the private field of the of the tail pages are used for
WRT cgroups.
Second tail page (hpage[2]) usage cgroup
Third tail page (hpage[3]) reservation cgroup

BTW, we are just adding usage of the third tail page IIUC.  The comment
that 4 pages are necessary includes the head page.
-- 
Mike Kravetz

  reply	other threads:[~2020-01-30  0:44 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-15  1:26 [PATCH v10 1/8] hugetlb_cgroup: Add hugetlb_cgroup reservation counter Mina Almasry
2020-01-15  1:26 ` [PATCH v10 2/8] hugetlb_cgroup: add interface for charge/uncharge hugetlb reservations Mina Almasry
2020-01-17 19:26   ` Mike Kravetz
2020-01-17 19:34     ` Mina Almasry
2020-01-29 21:21   ` David Rientjes
2020-01-30  0:41     ` Mike Kravetz [this message]
2020-01-15  1:26 ` [PATCH v10 3/8] hugetlb_cgroup: add reservation accounting for private mappings Mina Almasry
2020-01-17 22:57   ` Mike Kravetz
2020-01-29 21:28   ` David Rientjes
2020-02-03 23:17     ` Mina Almasry
2020-01-15  1:26 ` [PATCH v10 4/8] hugetlb: disable region_add file_region coalescing Mina Almasry
2020-01-21 18:50   ` Mike Kravetz
2020-01-15  1:26 ` [PATCH v10 5/8] hugetlb_cgroup: add accounting for shared mappings Mina Almasry
2020-01-15  1:26 ` [PATCH v10 6/8] hugetlb_cgroup: support noreserve mappings Mina Almasry
2020-01-15  1:26 ` [PATCH v10 7/8] hugetlb_cgroup: Add hugetlb_cgroup reservation tests Mina Almasry
2020-01-23  9:15   ` Sandipan Das
2020-01-23 20:05     ` Mina Almasry
2020-01-29 21:00     ` David Rientjes
2020-01-30  6:11       ` Sandipan Das
2020-02-03 23:18         ` Mina Almasry
2020-01-15  1:26 ` [PATCH v10 8/8] hugetlb_cgroup: Add hugetlb_cgroup reservation docs Mina Almasry
2020-01-16 22:44 ` [PATCH v10 1/8] hugetlb_cgroup: Add hugetlb_cgroup reservation counter Mike Kravetz
2020-01-29 21:09 ` David Rientjes
2020-02-03 23:16   ` Mina Almasry

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=43930e45-7505-1fc2-36ac-69a91a00a336@oracle.com \
    --to=mike.kravetz@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=almasrymina@google.com \
    --cc=aneesh.kumar@linux.vnet.ibm.com \
    --cc=cgroups@vger.kernel.org \
    --cc=gthelen@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=rientjes@google.com \
    --cc=shakeelb@google.com \
    --cc=shuah@kernel.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 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).