From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 4FAA519EED3 for ; Tue, 13 Jan 2026 11:34:50 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1768304090; cv=none; b=tsVgy1/6cOexWNsXVvTKTlIRF7oRwi+UDRwsiLxyQJ309BgVmijq20IH3fnpbL3kYrbM1PUwOQgMWLhZePyhNw6SXmqGf01w7jvmwRkDgJJ4mn+1BoYI42MOCI21nsdm5/NMoVYeoYmatJylZaPbHHbWhQRG3Ye8TGkperKJJJ8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1768304090; c=relaxed/simple; bh=8SYRj/Bn4n03WlxO9wJ4xkhgbEiQG0xyAbcovfmvVts=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=mKjxq4Ra880NylnUsfIX8iJ59MRhOpWIYcgCBkbp4l5mCupHjUA7DT09eb11hwJ/ydowWo0PUJVmgVEsjS/iY+xTHgZu2GQR+05bySQ8DXGf7SpL+Z0NCXepPZmY3Kgb1rzWAjUSUIh/joHZrU6d75RQ/pOmIrKZW9oJcdly5BI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=LT4WJ8AU; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="LT4WJ8AU" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 7B10FC116C6; Tue, 13 Jan 2026 11:34:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1768304089; bh=8SYRj/Bn4n03WlxO9wJ4xkhgbEiQG0xyAbcovfmvVts=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=LT4WJ8AUH5aCphVvnO6bYgyhUe5bT8rop9QoZ0j0AkV6KUH6ZdnQ7WITlMY+Aroh5 XyeoZi06OaJfBC2KeAm4S+VkmZxcpym/0kNLYZyTilDlK1bwM5omG4Vt8DKYZ1bSf7 xo4x24DibF7EDTAoAq9j2UXbLez9g3LLPrxGr5BhX1Nol0s+cZSNdyurbPSS6hUWUM 3SWI4rToibpkd0bHJGKMqhhPoqZewpast2WdoS/DA6x66M+ynpqiFGsTDFflOUH4jN JGZLUrkIrqD2lHSf117dDkADBOr/T0pUrZqvvaCPOKbZuO3WeZ0vHhfHC7l3MQrhqa EaEU1mX+LRJEQ== Date: Tue, 13 Jan 2026 13:34:42 +0200 From: Mike Rapoport To: Jason Gunthorpe Cc: Jason Miu , Alexander Graf , Andrew Morton , Baoquan He , Changyuan Lyu , David Matlack , David Rientjes , Pasha Tatashin , Pratyush Yadav , kexec@lists.infradead.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH v4 1/2] kho: Adopt radix tree for preserved memory tracking Message-ID: References: <20260109001127.2596222-1-jasonmiu@google.com> <20260109001127.2596222-2-jasonmiu@google.com> <20260112143904.GA812923@nvidia.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260112143904.GA812923@nvidia.com> On Mon, Jan 12, 2026 at 10:39:04AM -0400, Jason Gunthorpe wrote: > On Mon, Jan 12, 2026 at 12:15:54PM +0200, Mike Rapoport wrote: > > > + * The tree is traversed using a key that encodes the page's physical address > > > + * (pa) and its order into a single unsigned long value. The encoded key value > > > + * is composed of two parts: the 'order bit' in the upper part and the 'page > > > + * offset' in the lower part.:: > > > + * > > > + * +------------+-----------------------------+--------------------------+ > > > + * | Page Order | Order Bit | Page Offset | > > > + * +------------+-----------------------------+--------------------------+ > > > + * | 0 | ...000100 ... (at bit 52) | pa >> (PAGE_SHIFT + 0) | > > > + * | 1 | ...000010 ... (at bit 51) | pa >> (PAGE_SHIFT + 1) | > > > + * | 2 | ...000001 ... (at bit 50) | pa >> (PAGE_SHIFT + 2) | > > > + * | ... | ... | ... | > > > + * +------------+-----------------------------+--------------------------+ > > > + * > > > + * Page Offset: > > > > To me "page offset" reads as offset from somewhere and here it's rather pfn > > on steroids :) > > Also in many places in the kernel "page offset" refers to the offset inside a > > page. > > > > Can't say I can think of a better name, but it feels that it should express > > that this is an address more explicitly. > > It is "Shifted Physical Address" > > > > + node = phys_to_virt((phys_addr_t)node->table[idx]); > > > + } > > > + > > > + /* Handle the leaf level bitmap (level 0) */ > > > + leaf = (struct kho_radix_leaf *)node; > > > + idx = kho_radix_get_index(key, 0); > > > + __clear_bit(idx, leaf->bitmap); > > > > I think I already mentioned it in earlier reviews, but I don't remember any > > response. > > > > How do we approach freeing empty bitmaps and intermediate nodes? > > If we do a few preserve/uppreserve cycles for memory that can be allocated > > and freed in between we might get many unused bitmaps. > > Surely this is an error case?? > > We shouldn't be unpreserving at all in a normal flow? That's an error case for KHO/LUO, but might not be an error case for other users of the kho_radix_tree. For example mshv intends to use kho_radix_tree to track the hypervisor memory and there unpreserving will be a part of the normal flow. I'm not saying we must implement freeing of empty bitmaps from day 1, but at least there should be a comment about it. > Jason -- Sincerely yours, Mike.