From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-180.mta1.migadu.com (out-180.mta1.migadu.com [95.215.58.180]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E571C3D6CAC for ; Mon, 13 Apr 2026 14:06:12 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=95.215.58.180 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776089177; cv=none; b=JiC+Jay9LDFGfTSqRXK4GbeBvsk2Qlm/VZNof9L+2HeY52G3MEB6xwkRiB9LwMcBWLGDfVdvwxWrbKSPGRRpeEiOquN3nBTvn41+3gPqJ7VYGztBvZDO5umI+cmx8nXVL3fBvv7xo6RZVgIL00QhUEaurbiTgzVPX05SINZ95Zg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776089177; c=relaxed/simple; bh=iIX+oGc6zhMMbKtddF0iNVuPkDYCMDnxIKZusE7tJ4M=; h=Content-Type:From:Mime-Version:Subject:Date:Message-Id:References: Cc:In-Reply-To:To; b=bLiSNLa7IS9uTjoor0feA/Si2cHic9NabBsNJlQFXUpmcOBjjxMwkx4Igx7/pI4xs5Lxgvj0cMMhWElB0pUMj3ES6VZH/cEULNIgrTpmaIL3bGQ8W/MVr4f1IeKcLZXlN5eO08PVQSiZHMfkPdel0tkPHtgjvGuBABoi0K0QLDM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=K0xst7Ry; arc=none smtp.client-ip=95.215.58.180 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="K0xst7Ry" Content-Type: text/plain; charset=utf-8 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1776089168; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=LUfrQZBP/ZyxZwE2DiN7BB9ru8IXfCgKr+W6yEpjREU=; b=K0xst7RydEesK7u/I0oErZaYxbMcx5zzyAsnS8H6GKxD9Ov5nuyoHtuE9XB5fGt4bZHMD0 p2LcX5tCqoEK0KvpClSKLPtcK+QYnapEe/SKzlBMfyIDgptXBNEvufSuDbVkGcOjB3oEau nqdpsWfAeywtCPt8EBRTdgheme5+qTI= Content-Transfer-Encoding: quoted-printable X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Muchun Song Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 (1.0) Subject: Re: [PATCH 01/49] mm/sparse: fix vmemmap accounting imbalance on memory hotplug error Date: Mon, 13 Apr 2026 22:05:34 +0800 Message-Id: <9642F4D9-8F1B-47A4-90BE-C72BB8DE9A11@linux.dev> References: Cc: Muchun Song , Andrew Morton , David Hildenbrand , Oscar Salvador , Michael Ellerman , Madhavan Srinivasan , Lorenzo Stoakes , Liam R Howlett , Vlastimil Babka , Suren Baghdasaryan , Michal Hocko , Nicholas Piggin , Christophe Leroy , aneesh.kumar@linux.ibm.com, joao.m.martins@oracle.com, linux-mm@kvack.org, linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org In-Reply-To: To: Mike Rapoport X-Migadu-Flow: FLOW_OUT > On Apr 13, 2026, at 21:36, Mike Rapoport wrote: >=20 > =EF=BB=BFHi Muchun, Hi, >=20 > On Mon, Apr 13, 2026 at 08:47:45PM +0800, Muchun Song wrote: >>>> On Apr 13, 2026, at 20:05, Mike Rapoport wrote: >>>>>>>>=20 >>>>>>>> diff --git a/mm/sparse-vmemmap.c b/mm/sparse-vmemmap.c >>>>>>>> index 6eadb9d116e4..ee27d0c0efe2 100644 >>>>>>>> --- a/mm/sparse-vmemmap.c >>>>>>>> +++ b/mm/sparse-vmemmap.c >>>>>>>> @@ -822,11 +822,11 @@ static struct page * __meminit section_activa= te(int nid, unsigned long pfn, >>>>>>>> return pfn_to_page(pfn); >>>>>>>>=20 >>>>>>>> memmap =3D populate_section_memmap(pfn, nr_pages, nid, altmap, pgma= p); >>>>>>>> + memmap_pages_add(DIV_ROUND_UP(nr_pages * sizeof(struct page), PAG= E_SIZE)); >>>>>>>=20 >>>>>>> This logically belongs to success path in populate_section_memmap().= If we >>>>>>> update the counter there, we won't need to temporarily increase it a= t all. >>>>>>=20 >>>>>> Not strictly related to this patchset, but it seems, we can have a si= ngle >>>>>> memmap_boot_pages_add() in memmap_alloc() rather than to update the c= ounter >>>>>> in memmap_alloc() callers. >>>>>=20 >>>>> It will indeed become simpler and is a good cleanup direction, but the= re >>>>> is a slight change in semantics: the page tables used for vmemmap page= >>>>> mapping will also be counted in memmap_boot_pages_add(). This might no= t >>>>> be an issue (after all, the size of the page tables is very small comp= ared >>>>> to struct pages, right?). >>>>>=20 >>>>> Additionally, I still lean toward making no changes to this patch, bec= ause >>>>> this is a pure bugfix patch =E2=80=94 of course, it is meant to facili= tate backporting >>>>> for those who need it. The cleanup would involve many more changes, so= I >>>>> prefer to do that in a separate patch. What do you think? >>>=20 >>> For this patch and easy backporting I still think that cleaner to have t= he >>> counter incremented in populate_section_memmap() rather immediately afte= r >>> it. >>=20 >> Hi Mike, >>=20 >> Alright, let=E2=80=99s revisit your solution. After we=E2=80=99ve moved t= he counter into the >> populate_section_memmap(), we still need to increase the counter temporar= ily >> (but in populate_section_memmap()) even if we fail to populate. That=E2=80= =99s >> because section_deactivate() reduces the counter without exception, isn=E2= =80=99t it? >> Just want to make sure we are on the same page on the meaning of =E2=80=9C= temporarily >> increase=E2=80=9D. Maybe you do not mean =E2=80=9Ctemporarily=E2=80=9D i= n this case. >=20 > I suggest to increase the counter only if we succeeded to populate: >=20 > diff --git a/mm/sparse-vmemmap.c b/mm/sparse-vmemmap.c > index 6eadb9d116e4..247fd54f1003 100644 > --- a/mm/sparse-vmemmap.c > +++ b/mm/sparse-vmemmap.c > @@ -656,7 +656,13 @@ static struct page * __meminit populate_section_memma= p(unsigned long pfn, > unsigned long nr_pages, int nid, struct vmem_altmap *altmap, > struct dev_pagemap *pgmap) > { > - return __populate_section_memmap(pfn, nr_pages, nid, altmap, pgmap); > + struct page *p =3D __populate_section_memmap(pfn, nr_pages, nid, altm= ap, > + pgmap); > + > + if (p) > + memmap_pages_add(DIV_ROUND_UP(nr_pages * sizeof(struct page), PAG= E_SIZE)); We don=E2=80=99t increase the counter on failure, then > + > + return p; > } >=20 > static void depopulate_section_memmap(unsigned long pfn, unsigned long nr_= pages, > @@ -826,7 +832,6 @@ static struct page * __meminit section_activate(int ni= d, unsigned long pfn, > section_deactivate(pfn, nr_pages, altmap); here section_deactivate() is called, which decrease the counter unconditiona= lly, the issue still exists. We didn't fix anything. Thanks. > return ERR_PTR(-ENOMEM); > } > - memmap_pages_add(DIV_ROUND_UP(nr_pages * sizeof(struct page), PAGE_SI= ZE)); >=20 > return memmap; > } >=20 >=20 > Then we'll better follow "all or nothing" principle and won't have > exceptional cases in section_deactivate(). >=20 >> Thanks, >> Muchun. >=20 > -- > Sincerely yours, > Mike.