From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-11.4 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5B07DC3F68F for ; Mon, 3 Feb 2020 23:18:06 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id F02182087E for ; Mon, 3 Feb 2020 23:18:05 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="P9vxuWfA" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org F02182087E Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id A38EB6B0003; Mon, 3 Feb 2020 18:18:05 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 9E9566B0006; Mon, 3 Feb 2020 18:18:05 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8B0876B0007; Mon, 3 Feb 2020 18:18:05 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0041.hostedemail.com [216.40.44.41]) by kanga.kvack.org (Postfix) with ESMTP id 71AFD6B0003 for ; Mon, 3 Feb 2020 18:18:05 -0500 (EST) Received: from smtpin24.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 134FB283A for ; Mon, 3 Feb 2020 23:18:05 +0000 (UTC) X-FDA: 76450380930.24.dogs00_80bcfa8375c3b X-HE-Tag: dogs00_80bcfa8375c3b X-Filterd-Recvd-Size: 7765 Received: from mail-oi1-f195.google.com (mail-oi1-f195.google.com [209.85.167.195]) by imf25.hostedemail.com (Postfix) with ESMTP for ; Mon, 3 Feb 2020 23:18:04 +0000 (UTC) Received: by mail-oi1-f195.google.com with SMTP id a142so16550899oii.7 for ; Mon, 03 Feb 2020 15:18:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=4S8vHNwaTJoy9iVF47eyFtZHq86ah3z1R3R7OSO3+2M=; b=P9vxuWfA+Xz9O02Hf3pPI55HK5oO3beJiNSR+DkSHarWvGckjpVxj3+V9IaIcAJtZW mL9Vq5fV6HDoY5T3XjZwRYKZLxdyCMu0xYN0cx5DzBXtkfW6vhkiA1BBfH7tc+MnyXlT ooGWOmIwhM4GBBfvs9TLPhmk/Eq8kz7ojqO1i+vD6/wUHy2iP6866ydb5+Y6V0VY7xWD a/YvrUcthf9AcOfwYbCSNM7QKCs55X7EkSbGzpe2VhlEnkcQX70cj2cTugm1D9SnjfYm s+cZHlRxeIrDbRqXpGiNjtXGTQGo5xThTMF5DlS+ke3zqph4Cmd8k4bTjnhD35NTohrd vuOw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=4S8vHNwaTJoy9iVF47eyFtZHq86ah3z1R3R7OSO3+2M=; b=r0vt73s3WhkICuYwK2fGpu2enOP4xkDqUs/XIYmb4i/Jj9NE8abfHvdBTp0swdFLlO 7wYLIhlWeNuuUAGe02KH1NrXZV8SWVPU6OK5rKXzsmRfkNQKaJpfWFSzf27O5HpqdsYN +GXBYcaxDq5pQCwUYQgX1NNbOEKBPKexfbE6uHU7/fzkO9EKQEsQBUo1JG9+gGmkM8sy fwvwpAT05MYcXxUD36pPZ8KOAQjZFhUD0t23d/JZmaFK8h2OD2+GDxlyXHWrlV3Bn6do xLCoUPdzBinD2isrJM3nVW2GbVfU09KjW5j0+hP9k+D5KI83rzlq08I8ngosjXVbMEuC T7WA== X-Gm-Message-State: APjAAAXA5L4RfBZzhP98C0XZwPnFKkCY51dmuc92ruiEcIdr5oC16Edn MebMtLUxuCVoqnuTNPoOG4fZSyjE83fIKXlDsV///Q== X-Google-Smtp-Source: APXvYqyuj43djmIIaXiJRQDMfPEY4O5rvJgfItz8Cm1XFY7pfC3bE45El1OJ8nh1wyvDG/uukiMcMl2vAZyBV5IHz8Y= X-Received: by 2002:a05:6808:7dd:: with SMTP id f29mr1176345oij.67.1580771883610; Mon, 03 Feb 2020 15:18:03 -0800 (PST) MIME-Version: 1.0 References: <20200115012651.228058-1-almasrymina@google.com> <20200115012651.228058-3-almasrymina@google.com> In-Reply-To: From: Mina Almasry Date: Mon, 3 Feb 2020 15:17:52 -0800 Message-ID: Subject: Re: [PATCH v10 3/8] hugetlb_cgroup: add reservation accounting for private mappings To: David Rientjes Cc: Mike Kravetz , Shakeel Butt , shuah , Greg Thelen , Andrew Morton , open list , linux-mm@kvack.org, linux-kselftest@vger.kernel.org, cgroups@vger.kernel.org, Aneesh Kumar Content-Type: text/plain; charset="UTF-8" X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Wed, Jan 29, 2020 at 1:28 PM David Rientjes wrote: > > On Tue, 14 Jan 2020, Mina Almasry wrote: > > > diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h > > index dea6143aa0685..5491932ea5758 100644 > > --- a/include/linux/hugetlb.h > > +++ b/include/linux/hugetlb.h > > @@ -46,6 +46,16 @@ struct resv_map { > > long adds_in_progress; > > struct list_head region_cache; > > long region_cache_count; > > +#ifdef CONFIG_CGROUP_HUGETLB > > + /* > > + * On private mappings, the counter to uncharge reservations is stored > > + * here. If these fields are 0, then either the mapping is shared, or > > + * cgroup accounting is disabled for this resv_map. > > + */ > > + struct page_counter *reservation_counter; > > + unsigned long pages_per_hpage; > > + struct cgroup_subsys_state *css; > > +#endif > > }; > > extern struct resv_map *resv_map_alloc(void); > > void resv_map_release(struct kref *ref); > > diff --git a/include/linux/hugetlb_cgroup.h b/include/linux/hugetlb_cgroup.h > > index eab8a70d5bcb5..8c320accefe87 100644 > > --- a/include/linux/hugetlb_cgroup.h > > +++ b/include/linux/hugetlb_cgroup.h > > @@ -25,6 +25,33 @@ struct hugetlb_cgroup; > > #define HUGETLB_CGROUP_MIN_ORDER 2 > > > > #ifdef CONFIG_CGROUP_HUGETLB > > +enum hugetlb_memory_event { > > + HUGETLB_MAX, > > + HUGETLB_NR_MEMORY_EVENTS, > > +}; > > + > > +struct hugetlb_cgroup { > > + struct cgroup_subsys_state css; > > + > > + /* > > + * the counter to account for hugepages from hugetlb. > > + */ > > + struct page_counter hugepage[HUGE_MAX_HSTATE]; > > + > > + /* > > + * the counter to account for hugepage reservations from hugetlb. > > + */ > > + struct page_counter reserved_hugepage[HUGE_MAX_HSTATE]; > > + > > + atomic_long_t events[HUGE_MAX_HSTATE][HUGETLB_NR_MEMORY_EVENTS]; > > + atomic_long_t events_local[HUGE_MAX_HSTATE][HUGETLB_NR_MEMORY_EVENTS]; > > + > > + /* Handle for "hugetlb.events" */ > > + struct cgroup_file events_file[HUGE_MAX_HSTATE]; > > + > > + /* Handle for "hugetlb.events.local" */ > > + struct cgroup_file events_local_file[HUGE_MAX_HSTATE]; > > +}; > > > > static inline struct hugetlb_cgroup *hugetlb_cgroup_from_page(struct page *page, > > bool reserved) > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > > index 62a4cf3db4090..f1b63946ee95c 100644 > > --- a/mm/hugetlb.c > > +++ b/mm/hugetlb.c > > @@ -666,6 +666,17 @@ struct resv_map *resv_map_alloc(void) > > INIT_LIST_HEAD(&resv_map->regions); > > > > resv_map->adds_in_progress = 0; > > +#ifdef CONFIG_CGROUP_HUGETLB > > + /* > > + * Initialize these to 0. On shared mappings, 0's here indicate these > > + * fields don't do cgroup accounting. On private mappings, these will be > > + * re-initialized to the proper values, to indicate that hugetlb cgroup > > + * reservations are to be un-charged from here. > > + */ > > + resv_map->reservation_counter = NULL; > > + resv_map->pages_per_hpage = 0; > > + resv_map->css = NULL; > > +#endif > > Might be better to extract out a resv_map_init() that does the > initialization when CONFIG_CGROUP_HUGETLB is enabled? Could be used here > as well as hugetlb_reserve_pages(). > > > > > INIT_LIST_HEAD(&resv_map->region_cache); > > list_add(&rg->link, &resv_map->region_cache); > > @@ -3194,7 +3205,11 @@ static void hugetlb_vm_op_close(struct vm_area_struct *vma) > > > > reserve = (end - start) - region_count(resv, start, end); > > > > - kref_put(&resv->refs, resv_map_release); > > +#ifdef CONFIG_CGROUP_HUGETLB > > + hugetlb_cgroup_uncharge_counter(resv->reservation_counter, > > + (end - start) * resv->pages_per_hpage, > > + resv->css); > > +#endif > > > > if (reserve) { > > /* > > Mike has given is Reviewed-by so likely not a big concern for the generic > hugetlb code, but I was wondering if we can reduce the number of #ifdef's > if we defined a CONFIG_CGROUP_HUGETLB helper to take the resv, end, and > start? If CONFIG_CGROUP_HUGETLB is defined, it converts into the above, > otherwise it's a no-op and we don't run into any compile errors because we > are accessing fields that don't exist without the option. > Yes wherever possible I refactored the code a bit to remove #ifdefs in the middle of hugetlb logic. > Otherwise looks good! > > Acked-by: David Rientjes