From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-174.mta0.migadu.com (out-174.mta0.migadu.com [91.218.175.174]) (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 249173AD514 for ; Wed, 3 Jun 2026 10:20:39 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=91.218.175.174 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780482044; cv=none; b=TAlcEoSh17Yn95Ix2yWYx/FaOt4Gn84SihQbS5pbOOos8zm35/F/K4HXrU3m+OObynVurUMLgtP4KfDsV3ikBrbvZroqg6EAL3ZCENI1dJ2PlV+gkSti4imVXJ4XRRbPUeywDI4WCtSydJOQnpE7zvjhtAeZnhHpG6tqORUwrgk= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780482044; c=relaxed/simple; bh=/2zhvjvqsgtj27u2IjBEQe6hOCfDrVdjHCnPf05pSxQ=; h=Mime-Version:Content-Type:Date:Message-Id:Cc:Subject:From:To: References:In-Reply-To; b=uuBy5O7/DYmxdMUQD49qBIt3uI46uR8tV5UY1DkxzR/ht4F8RQiwMVfKllcuBL+uovdCddM7aT2ntmB2v0lWy6uWMsQEytEQO24I1q/Qqs9WAySCuByIv4iSz1cgJWZQTRC5BaFuK2I2VhZUjCtpWQRwFd3fbJ72cnNwWNdTxKk= 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=r3mH7nFJ; arc=none smtp.client-ip=91.218.175.174 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="r3mH7nFJ" Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1780482037; 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=SwJ/oDUEZoUVeWaKj5YumT6ozbOZBJ5GODC2Y/CoUmA=; b=r3mH7nFJkK2HgcQdu2MTumDv3yOTDprezVGASn+Qnln1fYr8CIT4f554AVPN/GjhUWqW06 WyVy88vK7/vB6oHvdNQnb2gEqMBa/yv+YBnC68x+8nBtOKnPgTJ6ODeyVMHzLcmQEhT27D P1ERc4JpChuu8xUCwLLhtuOcZdor+ZY= Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Wed, 03 Jun 2026 10:20:35 +0000 Message-Id: Cc: Subject: Re: [PATCH v2 2/3] x86/mm: simplify calculation of max_pfn_mapped X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: "Brendan Jackman" To: "Dave Hansen" , "Brendan Jackman" , "Ingo Molnar" , "Borislav Petkov" , "Dave Hansen" , , "H. Peter Anvin" , "Andy Lutomirski" , "Peter Zijlstra" , "Thomas Gleixner" References: <20260503-x86-init-cleanup-v2-0-bb690bd2477c@google.com> <20260503-x86-init-cleanup-v2-2-bb690bd2477c@google.com> In-Reply-To: X-Migadu-Flow: FLOW_OUT On Tue Jun 2, 2026 at 9:39 PM UTC, Dave Hansen wrote: > On 5/3/26 06:04, Brendan Jackman wrote: > ... >> Luckily, init_memory_mapping() avoids all these conditions. In that >> case, the return value is just paddr_end. And that value is already >> present, no need to depend on the confusing return value. > > It feels like we should say something about split_mem_range() here. All > of the guaranteed non-fiddly behavior originates in there, right? [pasting back the conditions from the commit message for context] >> but only in these conditions: >>=20 >> 1. There is a mismatch between the alignment of the requested range a= nd >> the page sizes allowed by page_size_mask >> >> 2. The range ends in a region that is not mapped according to >> e820. >> >> 3. The range ends in a region that was already mapped (note this case= is >> particularly fiddly because the return value depends on what level >> the existing mapping is at. This is probably a bug, see [0] for >> discussion). split_mem_range() is responsible for excluding point 1, since it returns the correct page_size_mask. The other two are actually down to the callers, right?=20 So how about for point 1 I mention that in the commit message, then for points 2 and 3 maybe they should actually be code comments, i.e. documented as preconditions for calling init_memory_mapping()? >> diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c >> index ae3e9e0820153..1a6a6fc700bb5 100644 >> --- a/arch/x86/mm/init.c >> +++ b/arch/x86/mm/init.c >> @@ -544,10 +544,11 @@ void __ref init_memory_mapping(unsigned long start= , >> memset(mr, 0, sizeof(mr)); >> nr_range =3D split_mem_range(mr, 0, start, end); >> =20 >> - for (i =3D 0; i < nr_range; i++) >> - paddr_last =3D kernel_physical_mapping_init(mr[i].start, mr[i].end, >> - mr[i].page_size_mask, >> - prot); >> + for (i =3D 0; i < nr_range; i++) { >> + kernel_physical_mapping_init(mr[i].start, mr[i].end, >> + mr[i].page_size_mask, prot); >> + paddr_last =3D mr[i].end; >> + } > > I guess this is actually: > > for (i =3D 0; i < nr_range; i++) > kernel_physical_mapping_init(...); > > paddr_last =3D mr[nr_range-1].end; > > Right? But what you have is probably just as compact. Oh, weird. My code might be just as compact but it's confusing, it should be written your way for sure.