From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from fhigh-b8-smtp.messagingengine.com (fhigh-b8-smtp.messagingengine.com [202.12.124.159]) (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 9E4E14A4832 for ; Thu, 24 Sep 2026 15:22:58 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=202.12.124.159 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1790263385; cv=none; b=VnPt+rwitjRcosbg1wBKRUBqLVamefSqjz/YvuWgXk4weD6/kZBA+ZuW8Nx7rBzjZVXFcHHMFazAra96oRbYKhffctHYeKhfJ6Ief9BrpoY8y+fZBkdgZLvz/0ODDgaxy8Ocl+V1eXLEOrvXtoHczfkXx5yCx5U7iJg3sNcVrb8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1790263385; c=relaxed/simple; bh=0NPQiXmcxX+mcYT+VPLSV1Q+UOJdUjBb6qH1Fe3KQ+k=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=pHehaVbeZdjzdGbgP/jI/K5ZU4JNqhYVhEDG9uPB2cGRzI3iHjsErU+goL2U0t2ZhE6uNo1pAE4Hf9+OTx0lcFnLnueUCQpHZZhUj3n0R2mEMv1iriC+2a95qILfFRWXU5yaZNxv953ojL5XetJk8N2KA5C4EGEZOz9URfcOAE0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=shutemov.name; spf=pass smtp.mailfrom=shutemov.name; dkim=pass (2048-bit key) header.d=shutemov.name header.i=@shutemov.name header.b=rHiW/DHe; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=ldzREHgy; arc=none smtp.client-ip=202.12.124.159 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=shutemov.name Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=shutemov.name Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=shutemov.name header.i=@shutemov.name header.b="rHiW/DHe"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="ldzREHgy" Received: from phl-compute-02.internal (phl-compute-02.internal [10.202.2.42]) by mailfhigh.stl.internal (Postfix) with ESMTP id CEAF57A00F7; Thu, 24 Sep 2026 11:22:56 -0400 (EDT) Received: from phl-frontend-04 ([10.202.2.163]) by phl-compute-02.internal (MEProxy); Thu, 24 Sep 2026 11:22:57 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=shutemov.name; h=cc:cc:content-type:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:subject:subject:to:to; s=fm3; t=1790263376; x= 1790349776; bh=i9829kPweuklhix3Z1AJfyPME8Dj1E3vcoJGw3M8wqk=; b=r HiW/DHek+JmfrjHge6+5C9Qq7tzQKeVot5KGDx5KDOk5tTqMfrK6MTIm/uxK0DS9 CuCwOTUvtLTyX/0bl/25EPfDPwCSYVOSVzlNa8mMvqdBVsuJdveRt1F296iDdBBu vCT9AYnoDTn5ubo8QggHHPcWpPGWn059EUeol9yFM/doqKWQjPOOgx3sp+VOkOf0 Cv2yp3RiF77jwASVIitq9UMKDySW5O5FmhuHE6LiR0yrbBRLBJqmLEDLGZojj+JI 4sJ6tCR81IwhrcAqF/fkMegwBFCAn6tGnyq++AsCI6iyoGzckbXIWJyoTqZspLYh ZA3z8Ojfn/8AYJ3JxJv/Q== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t= 1790263376; x=1790349776; bh=i9829kPweuklhix3Z1AJfyPME8Dj1E3vcoJ Gw3M8wqk=; b=ldzREHgy8qn/M67eVWZG1M9WNO1oVErf0HZcYvoi6NqEQMUKbhn bOIAoaO3fnBQJ1WPq/JhdwNu4HtWJWR89hyjxbZqK5FgRiRAXp9JSLI2hYyP6lnq ZBx5YU8oQtzpqTaIOUY+AJVKidm3Djfk1r7VvsIzP287bozKM5En+/qmHQg110EH 3340bNAwp/ZA6oXHEYbpS9l/EKnHz14FG66Lbx8opVCVAZFtsrfj02ECbmhwvmM+ bacjNYbemrGG8plMCyNspk6r+KxNl+y0nPjzrv8x0dInmO+0aHDBbZac9cMLFIkV C4AlljPvACf5D3Lvhua8CkTI+WspfqUOqQw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: dmFkZTFOEUn76qAsDivzyhzaxVg7PBFGRMw+JOhFLqg+cNl/nhm8GytXTnazGyDSxwsGhK j/+NkLfyi36dqu7YdqiPFUiCwIsC1ecyqSlqD3Tm3hsRlecNHu92tTNpFDWN+dz3gvKG6F kN5sIFxDwtuWxWxoOY5Cuvq4sRqgoamTjVSTdtIc3wp8gBL3r56ubVMWhtzdXQ/CCaCogi 3u0GJXH7g/d5NIYGaXEA/71nb/j3UTqjEE5ZBwkqiRSN6z8w6QuD75ih3hLZEhI0hzYuY5 9UTL2y/fCg3Qga4bEbwRZkYBSwgzIa1EHyrLI7bgmDJR4jI8MMpa/XGhfqVoCL41U4VdPA 3jolhBlpuiuG5Ou3hokvPOnz/Q89mLkYMrjPAYjITIbsEpktlB6Cg4Mm5mEVLFfU/6gQf8 kklLUDvf7klk/P/ynJ3R9L9w+IMxpUNzHXVF5ENjbheqjRJzYBJmmvxXzkpErjl5zHRW0z 63kXRk0zRkkKEe6BuHAbHnnC5chSxOxfJZUlk1b3Wgbodb+Kh7os4JoftfVzuIOy9tFkzC cqGP/6YUQ3FDRGn5Aurmxcgv01jUlsvAG2Why4sdj6Ah8fac1ngNEwwBAUyIbGqzP/ohmM tmuGP1q0Bx56xS8dr2K0kCqihfpR48oEVX3WlAUE4EXHWZHKoV23ua1WITsA X-ME-Proxy: Feedback-ID: ie3994620:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 24 Sep 2026 11:22:55 -0400 (EDT) Date: Thu, 24 Sep 2026 16:22:54 +0100 From: Kiryl Shutsemau To: "David Hildenbrand (Arm)" Cc: Andrew Morton , Lorenzo Stoakes , Zi Yan , Baolin Wang , linux-mm@kvack.org, linux-kernel@vger.kernel.org, kernel-team@meta.com, "Liam R. Howlett" , Nico Pache , Ryan Roberts , Dev Jain , Barry Song , Lance Yang , Usama Arif , Vlastimil Babka , Jann Horn Subject: Re: [PATCH v3 11/12] mm/collapse: declare the collapse interface in collapse.h Message-ID: References: <20260916093145.4022188-1-kirill@shutemov.name> <20260916093145.4022188-12-kirill@shutemov.name> <63f49a8b-cabf-4b93-a7e2-76b62c463a90@kernel.org> 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: <63f49a8b-cabf-4b93-a7e2-76b62c463a90@kernel.org> On Wed, Sep 23, 2026 at 03:34:11PM +0200, David Hildenbrand (Arm) wrote: > On 9/16/26 11:31, Kiryl Shutsemau wrote: > > From: "Kiryl Shutsemau (Meta)" > > > > A collapse takes four calls: > > > > - collapse_control_init() - set up the control a caller carries; > > - collapse_scan_pmd() - scan one PTE table, under mmap_lock; > > - collapse_run_pmd() - collapse what the scan found, no mmap_lock; > > As discussed, having a single collapse_pmd() function might be cleaner if that's > easily possible. Answered on patch 9: the two calls are the point. > > - collapse_control_release() - done with the control. > > And as discussed, I hope we can just get rid of a release function that's not > actually supposed to release anything right now (unless I was missing an update > in one of the patches). Will remove in v4. > > All four are static in khugepaged.c, as are collapse_possible_orders(), > > which says what a VMA allows, and the revalidate a caller needs once a > > collapse has given the mmap_lock up. No other file can ask for a collapse > > without them. > > > > Declare them in collapse.h, with a comment stating the order they are > > called in and who holds the lock over each step. Each function says > > what it needs and what it does where it is defined. > > > > hugepage_vma_revalidate() becomes collapse_vma_revalidate(): it is part of > > what a collapse offers now, not a helper of the daemon. > > Is it just me or is collapse_vma_revalidate() an odd part of this interface? > > You'd expect a matching function that performs the initial validation on a given > vma. > > Maybe we should have a > > orders = collapse_vma_validate(vma) > > that really just wraps collapse_possible_orders(), an expose that instead to the > collapse users? > > So they'd use collapse_vma_validate() to then call collapse_vma_revalidate() > after temporarily dropping the mmap lock? collapse_vma_revalidate() re-checks what the scan ran under: a VMA at the address, anonymous if the scan went that way, covering the PMD range, allowing this order. A collapse_vma_validate() could bundle what the callers check before a scan, for the sake of symmetry. Feels like overkill to me. > > -static enum scan_result hugepage_vma_revalidate(struct mm_struct *mm, unsigned long address, > > +enum scan_result collapse_vma_revalidate(struct mm_struct *mm, unsigned long address, > > bool expect_anon, struct vm_area_struct **vmap, > > struct collapse_control *cc, unsigned int order) > > { > > @@ -1264,7 +1264,7 @@ static enum scan_result collapse_huge_page(struct mm_struct *mm, > > } > > > > mmap_read_lock(mm); > > - result = hugepage_vma_revalidate(mm, pmd_addr, /*expect_anon=*/ true, > > + result = collapse_vma_revalidate(mm, pmd_addr, /*expect_anon=*/ true, > > &vma, cc, order); > > if (result != SCAN_SUCCEED) { > > mmap_read_unlock(mm); > > @@ -1299,7 +1299,7 @@ static enum scan_result collapse_huge_page(struct mm_struct *mm, > > * mmap_lock. > > */ > > mmap_write_lock(mm); > > - result = hugepage_vma_revalidate(mm, pmd_addr, /*expect_anon=*/ true, > > + result = collapse_vma_revalidate(mm, pmd_addr, /*expect_anon=*/ true, > > &vma, cc, order); > > if (result != SCAN_SUCCEED) > > goto out_up_write; > > @@ -2741,7 +2741,8 @@ static enum scan_result collapse_scan_file(struct mm_struct *mm, > > return result; > > } > > > > -static void collapse_control_init(struct collapse_control *cc) > > +/* Set up a control before its first scan; cc->policy is the caller's to fill */ > > > Kerneldoc please. Applies to the other ones exposed as part of the same > interface as well. Will do, for all of them. The overview in collapse.h stays; it is what ties the calls together and says who holds the lock between them. -- Kiryl Shutsemau / Kirill A. Shutemov