From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 62C06384224 for ; Wed, 4 Mar 2026 08:53:43 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772614424; cv=none; b=YHxn6/njR6VPpltYDnY3qiwARCVwIeD+BGZjxQNqi3BWo/152DCqispasx97Kli7zFBJlNT5xINEbCmpNEzFxAWAySrbGYI5km7iKlqw+KTjzVtkezzaemjqP9/sl1uDXA3hSTIAitNRR4Pr7mzVDGSLHPKQs/MQ6kXkEG1VT4Q= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772614424; c=relaxed/simple; bh=VtW+ic0NIuxW4zAacW+Hy15PNLopKuvn6PrD7jF89CE=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=bULlKKHFNIPnyveCrh/jYznQ5loy99aUjXH6eod6AX8/WZlBnmyownxEPD5gJ+bID/5AScpjudez55OPNMnhWkNr78RdnZzlXfdbhCDSdxfhygYxoptUzsW6RIVbFfQIxNN8T7943Z4PcBk+7wUCJu8dZ5zmdHGqqKxVIOWbyl8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 5EDEF339; Wed, 4 Mar 2026 00:53:36 -0800 (PST) Received: from [10.163.173.252] (unknown [10.163.173.252]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 435D93F7BD; Wed, 4 Mar 2026 00:53:38 -0800 (PST) Message-ID: Date: Wed, 4 Mar 2026 14:23:35 +0530 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH V3 2/2] arm64/mm: Reject memory removal that splits a kernel leaf mapping To: "David Hildenbrand (Arm)" , linux-arm-kernel@lists.infradead.org Cc: mark.rutland@arm.com, Catalin Marinas , Will Deacon , Ryan Roberts , Yang Shi , Christoph Lameter , linux-kernel@vger.kernel.org References: <20260224062423.972404-1-anshuman.khandual@arm.com> <20260224062423.972404-3-anshuman.khandual@arm.com> <6eee7f14-c3f2-4e16-b713-65c2ea141457@kernel.org> <2d07e814-a7a3-4cec-8a59-158420bde1f2@arm.com> <05e6034d-3493-4961-a97d-a80fff78289b@kernel.org> Content-Language: en-US From: Anshuman Khandual In-Reply-To: <05e6034d-3493-4961-a97d-a80fff78289b@kernel.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 03/03/26 2:30 PM, David Hildenbrand (Arm) wrote: >>> I can understand the desire for this check, but I am confused about Fixes: >> >> Probably nothing is broken now I guess but the original memory hot remove >> patch should have taken care of this scenario. Although don't have strong >> opinions either way. We could drop both "Fixes" and "Closes" tags here if >> that is preferred. >> > > We tend to only tag actual fixes. If we consider this a possible fix, we > should ask ourselves whether this would be stable material. Ryan had earlier asked for the Cc: stable to be dropped as this was not an actual fix. But seems like we should drop these Fixes/Closes tags as well. > > ... > >>>> + return false; >>>> + >>>> + pgdp = pgd_offset_k(addr); >>>> + pgd = pgdp_get(pgdp); >>>> + if (!pgd_present(pgd)) >>>> + return false; >>> >>> How could we end up with non-present areas in a range we hotplugged earlier? >> >> We might not in reality but in order to be sure just an additional protection. > > I'm rather wondering if this would indicate a real bug somewhere else > that we would silently swallow. > > Anyhow, no real preference from my side, just something I considered weird. > > [...] > >>>> + >>>> +static bool can_unmap_without_split(unsigned long pfn, unsigned long nr_pages) >>>> +{ >>>> + unsigned long phys_start, phys_end, size, start, end; >>>> + >>>> + phys_start = PFN_PHYS(pfn); >>>> + phys_end = phys_start + nr_pages * PAGE_SIZE; >>>> + >>>> + /* >>>> + * PFN range's linear map edges are leaf entry aligned >>>> + */ >>>> + start = __phys_to_virt(phys_start); >>>> + end = __phys_to_virt(phys_end); >>>> + if (addr_splits_kernel_leaf(start) || addr_splits_kernel_leaf(end)) { >>>> + pr_warn("[%lx %lx] splits a leaf entry in linear map\n", >>>> + phys_start, phys_end); >>>> + return false; >>>> + } >>>> + >>>> + /* >>>> + * PFN range's vmemmap edges are leaf entry aligned >>>> + */ >>>> + size = nr_pages * sizeof(struct page); >>>> + start = (unsigned long)pfn_to_page(pfn); >>>> + end = start + size; >>> >>> As arm64 cannot be used with CONFIG_SPARSEMEM (only with >>> CONFIG_SPARSEMEM_VMEMMAP, as it sets SPARSEMEM_VMEMMAP_ENABLE), >>> I think you can just do > > Just to clarify: > > What I meant here is: with CONFIG_SPARSEMEM but without > CONFIG_SPARSEMEM_VMEMMAP. Alright but does the commit message or pfn_to_page() code block here needs a comment about this ? OR it is apparent enough ?