Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 30 Sep 2020 17:01:24 -0400
From:   Johannes Weiner <hannes@cmpxchg.org>
To:     Roman Gushchin <guro@fb.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        Shakeel Butt <shakeelb@google.com>,
        Michal Hocko <mhocko@kernel.org>, linux-kernel@vger.kernel.org,
        linux-mm@kvack.org, kernel-team@fb.com
Subject: Re: [PATCH v3 4/4] mm: convert page kmemcg type to a page memcg flag
Message-ID: <20200930210124.GB469663@cmpxchg.org>
References: <20200929235920.537849-1-guro@fb.com>
 <20200929235920.537849-5-guro@fb.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200929235920.537849-5-guro@fb.com>
Precedence: bulk

On Tue, Sep 29, 2020 at 04:59:20PM -0700, Roman Gushchin wrote:
> @@ -3087,7 +3087,7 @@ int __memcg_kmem_charge_page(struct page *page, gfp_t gfp, int order)
>  		ret = __memcg_kmem_charge(memcg, gfp, 1 << order);
>  		if (!ret) {
>  			set_page_memcg(page, memcg);
> -			__SetPageKmemcg(page);
> +			__SetPageMemcgKmem(page);
>  			return 0;
>  		}
>  		css_put(&memcg->css);
> @@ -3112,10 +3112,6 @@ void __memcg_kmem_uncharge_page(struct page *page, int order)
>  	__memcg_kmem_uncharge(memcg, nr_pages);
>  	clear_page_memcg(page);
>  	css_put(&memcg->css);
> -
> -	/* slab pages do not have PageKmemcg flag set */
> -	if (PageKmemcg(page))
> -		__ClearPageKmemcg(page);

Hm, the named flags are great, and the getter functions are complex
enough to justify helpers.

But both flags are set along with the object pointer, and cleared when
the pointer is cleared (which makes sense, because pages can not
change their type while they're alive). page_clear_memcg() and
page_clear_objcgs() do the same thing.

	page->memcg_data = (unsigned long)pointer | MEMCG_DATA_TYPE;

and

	page->memcg_data = 0;

are straight-forward and really don't require abstraction. Please
open-code these.