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.
Or, we could go even more explicit and define an enum that defines the
third tail page is for hugetlb_cgroup limits and fourth is for
hugetlb_cgroup reservation limits.
enum {
...
H_CG_LIMIT,
H_CG_RESV,
MAX_H_CG_PRIVATE,
};
and then do a
BUILD_BUG_ON(MAX_H_CG_PRIVATE > (1 << HUGETLB_CGROUP_MIN_ORDER));
somewhere? And then use H_CG_RESV (or a better name) as the offset from
the head page in the code?
>
> #ifdef CONFIG_CGROUP_HUGETLB
>
> -static inline struct hugetlb_cgroup *hugetlb_cgroup_from_page(struct page *page)
> +static inline struct hugetlb_cgroup *hugetlb_cgroup_from_page(struct page *page,
> + bool reserved)
When implementing specs, I immediately see "bool reserved" or
"int reserved:1" as reserved for future use :) Maybe simple enough to
just name these formals as resv?
Otherwise looks like a simple conversion of existing functionality and
extension to the new limits.
Acked-by: David Rientjes <rientjes@xxxxxxxxxx>
