Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6;
Date:   Tue, 3 Oct 2023 10:13:29 -0700
From:   Mike Kravetz <mike.kravetz@oracle.com>
To:     Nhat Pham <nphamcs@gmail.com>
Cc:     akpm@linux-foundation.org, riel@surriel.com, hannes@cmpxchg.org,
        mhocko@kernel.org, roman.gushchin@linux.dev, shakeelb@google.com,
        muchun.song@linux.dev, tj@kernel.org, lizefan.x@bytedance.com,
        shuah@kernel.org, yosryahmed@google.com, fvdl@google.com,
        linux-mm@kvack.org, kernel-team@meta.com,
        linux-kernel@vger.kernel.org, cgroups@vger.kernel.org
Subject: Re: [PATCH v3 2/3] hugetlb: memcg: account hugetlb-backed memory in
 memory controller
Message-ID: <20231003171329.GB314430@monkey>
References: <20231003001828.2554080-1-nphamcs@gmail.com>
 <20231003001828.2554080-3-nphamcs@gmail.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231003001828.2554080-3-nphamcs@gmail.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?qoFiFaBNy4ZTq/I3I6kPuhkV7vmdrKuml62DezlFoc2Rjs/YbSh6JhK+mezv?=
 =?us-ascii?Q?w79vfGyWw5o+Z6HddXs4V3wqB4ILELA4ZFnbZ4wSWZMzAEbQsZg5VhvcDxb+?=
 =?us-ascii?Q?eld7YVwHmh36wAjxAFCPi7XK9UxnhmZVgiFov1lSW7oltJAmLnhq5VdM7CDL?=
 =?us-ascii?Q?CfCEncwlcqBPW5zTe87OL4/92h/JR3OHXlG4P2J16s+TeTObzBJDZmFT3Jqk?=
 =?us-ascii?Q?Fi79lvHSKt2PZrQX88wSuRnlOnYzYCzYDUZ7lQekWYwmCChRdHXr+HXB0aXB?=
 =?us-ascii?Q?1r1CmUeY7abiDNcEYg3jjBgMAdXMx2mVKtsnJdO9MONsPzdv8XawY0YmoNKV?=
 =?us-ascii?Q?x+XIL4/VCeeHhvlYp60ulyCW8q6SFKg7hp/gmdaJ8eGhjcO7lao6EARw1w9n?=
 =?us-ascii?Q?7xqBzs8ulJ3XKfwMk00BLNunHapxUp29xMRpbIpsI/QcLqQR/hJIHQ3ExzsW?=
 =?us-ascii?Q?V1emRzUdIpWiE5xvKJZjexmhQgA3cOh+cZXubw9ZU3l3xAe59X8UVs0Zx97T?=
 =?us-ascii?Q?h46y1uFroxsqFQnTBT2Sn+aEtGVr2no+6P+az1Tr/r1yueUneI3D2omW5qpj?=
 =?us-ascii?Q?Kb6znW3BswShDGdI3GoTK38r6VeCSJH4w9uM7yQB0VOG6ZRyI6mczKG+XR69?=
 =?us-ascii?Q?K9db2guHUsN1xgVPgBEELzn1dxwrz7uTNEghC4qlCy1NTSRBCFRZlgs+k6vM?=
 =?us-ascii?Q?pENLRr+bGA33gh5w12ImBzmrKcqzqv0kxGJ6Io6AiTXNTlgU+cO6IXYjA/5b?=
 =?us-ascii?Q?VPNgIRmsXedPRWWO1XK2ONJYPSzIlUcrZcK4GollevcfVqdph7JYBgi5toV8?=
 =?us-ascii?Q?VnUWb28wThsAMUamFbdZg20yvpsjlaSeGz/JaEPAi1qnEIXV4cIrZS5IXcNp?=
 =?us-ascii?Q?lfhicG9YRcRRgsAck1TGgTsiPPOeUsFSWP7XhV0GirqiiK8xvNQofLikQWyU?=
 =?us-ascii?Q?WMW9ib+NAS1+3szp7k2aFQeGZVepjLAFnCI/eurVuCTBQpWBP7QGGAndiF5k?=
 =?us-ascii?Q?HrilL0l0nzdipgqwMgkajjaBA2mx8IMjtBznanHKMNhRNhGh9IZdTFdA0G2Q?=
 =?us-ascii?Q?U5xgqtmj+H2a6y0Gag/KiQHIyWHIgTxJ4IkuER/eLcEutSNp4YQ0QH+rjwGG?=
 =?us-ascii?Q?DCWyceJ+Eq35ZaKryoQg1pUZgSxmMh2e9WJeU606LIBHQD7ME0nvFGrbn432?=
 =?us-ascii?Q?UQ7KMAJ7MOQGetXj1rAP8jgk4dl2bFJWqnohDsG6K5yChQb6HymlN1KEn3Uv?=
 =?us-ascii?Q?OBqW9RSid8FeXM+1SBghJfq/j7rW58aPMw9pngNw+/+UK4TkRpK2ZHLjjSw4?=
 =?us-ascii?Q?WZDyB3+quPhWX68H5vL/MV5uAF+vPqSl7j05hiGKudU3SwlmEyjteAw9fbiN?=
 =?us-ascii?Q?jEfw5tZmonsgvV1hyzn61e0zeGiOv8LCSrz7wVoIy9Ie7IX0uFuIc4jTLBpJ?=
 =?us-ascii?Q?Qfn0uU3TNyXnBJTNxgx3RnCi2KvmTu2O8MScTlg0PsORntYPHIF3H0pCX1nB?=
 =?us-ascii?Q?asCinTPWAAbj8ACpOYTN+CQ2Y9Z/jeEwrSVwdYIqYItiXMmTEKTYLY1AUkF3?=
 =?us-ascii?Q?iS2N0bIoZ0xmnhanl0r6o4tNIxoilcIiqw0oiiaY?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: =?us-ascii?Q?NjCPopDPxnMtR/zTNFShWeJOVc2+JoNUe0PGEAFeISUyR141bHHnSjqL3EFN?=
 =?us-ascii?Q?3mivfPhg0NV8uAS7kogQHQUt1hqDtNvFui+//Xj0PHz/ENUtDWCnIZhimjlO?=
 =?us-ascii?Q?J9kVho4NYVrTyoQ+zvYR/gntjs1Ja1pNX0KhCCHi+4+gilMovSdwDnIUQQVy?=
 =?us-ascii?Q?l6qDQ9nnJetOwGEkFFf3nXDByc4VTLpS21B6AM15qYjZlzXtG61De6BLXjgA?=
 =?us-ascii?Q?jXVZdD/AYoc7x12wjlgDsOAiys9fWIRZS3pnxsIGM2pcPW9oRt3YXbwJUKB3?=
 =?us-ascii?Q?wLyuTgz0xIqeWz7MOFL0KqXIM8AnpOhhNwFB6A9ocajWTR6PiKR0EG+e/ZLU?=
 =?us-ascii?Q?CMWgeiQR1fTmu/dummVlkjqVA/u4HnH66+GWW5a6WnRBfjJ9xQSmQYkmVIMu?=
 =?us-ascii?Q?ASTa2/F8FgqnOT15+3oEOEw26v/zF9/aNe6Kci56d2OgNigOSpjks+tb8lyY?=
 =?us-ascii?Q?eCxGNvPj3BAysXd7kvtmjKwdS6cl9m1TUh6s/C9hdJ5MhzVNOcC63gblEuYH?=
 =?us-ascii?Q?UOU3VSg8q18/3po0hMtN1Ham77+eKM8LkvY6qIHsow4J0L7YSnA97YMmF2Bc?=
 =?us-ascii?Q?h0a23NNYNEIvceJzOaaFvyey4gbh4lzx8TRCCU/SI9uSR2LCLiC3+mUZnGJu?=
 =?us-ascii?Q?c5vFjnx8lppNrl3Tl/H6SJH4QoVvy863NCIf0hjzhuPBmfxDgNabuScwGwEa?=
 =?us-ascii?Q?xM8ijp/c3s3l7ehpe1c3PsQE/IF6hV3EyPMMw2VtYZneeEkVS67MjU0H0GZg?=
 =?us-ascii?Q?9nm+i0fWHbV3yZzCKJDqKYYM0ljyN5AdVbB3Byx9WehDeNIfurXc3a1LNZjj?=
 =?us-ascii?Q?cF+Up9kKavYFZdsqef+C1SsUnZbjUIlGITKO59/M4e/f9cZgfNpGbF6wQFqE?=
 =?us-ascii?Q?EwWX/vWLWxVq4O87cJAJcLuJ8DOTPOqQMCCfq32i9F/BkMxaKMTfodt+x6yL?=
 =?us-ascii?Q?ulzDh+jR2PqZ+2xultaim9jbuRWnqEW6AMrqA8iL6XuHrS1xXGulPHryPOtL?=
 =?us-ascii?Q?IoHByDCmsHZ1DBAJ69jYBfIBpPEsen5BDwLXkalANN/1sSeg6LQ3Za3tdw6V?=
 =?us-ascii?Q?ZzN/vsWKaTVE5saI/hX4xAlDn48vdA=3D=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4e027521-f06a-45b2-eb0b-08dbc4341294
X-MS-Exchange-CrossTenant-AuthSource: BY5PR10MB4196.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Oct 2023 17:13:33.3805
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 1Lv3+eLfRlOCIEMi9gv1qxEAqZhbjFl1kS8nR0BlG8wR1WilHVJSHua3tle/UfXG123CU0pLUlll7XBfFtejug==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH2PR10MB4279
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.267,Aquarius:18.0.980,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2023-10-03_15,2023-10-02_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 mlxscore=0 spamscore=0
 phishscore=0 bulkscore=0 mlxlogscore=999 suspectscore=0 adultscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2309180000
 definitions=main-2310030131
X-Proofpoint-GUID: 0AwUZvipXCGtrR0up2Edwigy8cppLqQJ
X-Proofpoint-ORIG-GUID: 0AwUZvipXCGtrR0up2Edwigy8cppLqQJ
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no
	version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on pete.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (pete.vger.email [0.0.0.0]); Tue, 03 Oct 2023 10:14:29 -0700 (PDT)

On 10/02/23 17:18, Nhat Pham wrote:
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index de220e3ff8be..74472e911b0a 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -1902,6 +1902,7 @@ void free_huge_folio(struct folio *folio)
>  				     pages_per_huge_page(h), folio);
>  	hugetlb_cgroup_uncharge_folio_rsvd(hstate_index(h),
>  					  pages_per_huge_page(h), folio);
> +	mem_cgroup_uncharge(folio);
>  	if (restore_reserve)
>  		h->resv_huge_pages++;
>  
> @@ -3009,11 +3010,20 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma,
>  	struct hugepage_subpool *spool = subpool_vma(vma);
>  	struct hstate *h = hstate_vma(vma);
>  	struct folio *folio;
> -	long map_chg, map_commit;
> +	long map_chg, map_commit, nr_pages = pages_per_huge_page(h);
>  	long gbl_chg;
> -	int ret, idx;
> +	int memcg_charge_ret, ret, idx;
>  	struct hugetlb_cgroup *h_cg = NULL;
> +	struct mem_cgroup *memcg;
>  	bool deferred_reserve;
> +	gfp_t gfp = htlb_alloc_mask(h) | __GFP_RETRY_MAYFAIL;
> +
> +	memcg = get_mem_cgroup_from_current();
> +	memcg_charge_ret = mem_cgroup_hugetlb_try_charge(memcg, gfp, nr_pages);
> +	if (memcg_charge_ret == -ENOMEM) {
> +		mem_cgroup_put(memcg);
> +		return ERR_PTR(-ENOMEM);
> +	}
>  
>  	idx = hstate_index(h);
>  	/*
> @@ -3022,8 +3032,12 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma,
>  	 * code of zero indicates a reservation exists (no change).
>  	 */
>  	map_chg = gbl_chg = vma_needs_reservation(h, vma, addr);
> -	if (map_chg < 0)
> +	if (map_chg < 0) {
> +		if (!memcg_charge_ret)
> +			mem_cgroup_cancel_charge(memcg, nr_pages);
> +		mem_cgroup_put(memcg);
>  		return ERR_PTR(-ENOMEM);
> +	}
>  
>  	/*
>  	 * Processes that did not create the mapping will have no
> @@ -3034,10 +3048,8 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma,
>  	 */
>  	if (map_chg || avoid_reserve) {
>  		gbl_chg = hugepage_subpool_get_pages(spool, 1);
> -		if (gbl_chg < 0) {
> -			vma_end_reservation(h, vma, addr);
> -			return ERR_PTR(-ENOSPC);
> -		}
> +		if (gbl_chg < 0)
> +			goto out_end_reservation;
>  
>  		/*
>  		 * Even though there was no reservation in the region/reserve
> @@ -3119,6 +3131,11 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma,
>  			hugetlb_cgroup_uncharge_folio_rsvd(hstate_index(h),
>  					pages_per_huge_page(h), folio);
>  	}
> +
> +	if (!memcg_charge_ret)
> +		mem_cgroup_commit_charge(folio, memcg);
> +	mem_cgroup_put(memcg);
> +
>  	return folio;
>  
>  out_uncharge_cgroup:
> @@ -3130,7 +3147,11 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma,
>  out_subpool_put:
>  	if (map_chg || avoid_reserve)
>  		hugepage_subpool_put_pages(spool, 1);
> +out_end_reservation:
>  	vma_end_reservation(h, vma, addr);
> +	if (!memcg_charge_ret)
> +		mem_cgroup_cancel_charge(memcg, nr_pages);
> +	mem_cgroup_put(memcg);
>  	return ERR_PTR(-ENOSPC);
>  }
>  

IIUC, huge page usage is charged in alloc_hugetlb_folio and uncharged in
free_huge_folio.  During migration, huge pages are allocated via
alloc_migrate_hugetlb_folio, not alloc_hugetlb_folio.  So, there is no
charging for the migration target page and we uncharge the source page.
It looks like there will be no charge for the huge page after migration?

If my analysis above is correct, then we may need to be careful about
this accounting.  We may not want both source and target pages to be
charged at the same time.
-- 
Mike Kravetz