Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 25 Jun 2025 19:10:15 GMT
From:      Alan Cox <alc@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 07297aee35f2 - main - vm_page: update comments and KASSERT()s concerning page allocation
Message-ID:  <202506251910.55PJAF9S076800@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by alc:

URL: https://cgit.FreeBSD.org/src/commit/?id=07297aee35f26cfd37aef078ff6c0cbe67bbba15

commit 07297aee35f26cfd37aef078ff6c0cbe67bbba15
Author:     Alan Cox <alc@FreeBSD.org>
AuthorDate: 2025-06-19 05:45:42 +0000
Commit:     Alan Cox <alc@FreeBSD.org>
CommitDate: 2025-06-25 19:09:49 +0000

    vm_page: update comments and KASSERT()s concerning page allocation
    
    Update the legend describing the arguments to the most commonly used
    page allocation functions.  Notably, eliminate a reference to a function
    that no longer exists; and update to reflect the elimination of
    VM_ALLOC_NOOBJ, specifically, VM_ALLOC_WAITOK is no longer a legal
    option to vm_page_alloc{,_contig}().
    
    Eliminate a nonsensical KASSERT(). VM_ALLOC_SBUSY is forbidden as an
    argument to vm_page_alloc_noobj{,_contig,}_domain() by a KASSERT(), so
    having a different KASSERT() that tests for it is nonsensical.
    
    Strengthen other KASSERT()s involving VM_ALLOC_NOBUSY and
    VM_ALLOC_NOFREE.
    
    Reviewed by:    kib, markj, dougm (an earlier version)
    Differential Revision:  https://reviews.freebsd.org/D49391
---
 sys/vm/vm_page.c | 55 ++++++++++++++++++++++++++++++++-----------------------
 sys/vm/vm_page.h | 36 +++++++++++++++++++-----------------
 2 files changed, 51 insertions(+), 40 deletions(-)

diff --git a/sys/vm/vm_page.c b/sys/vm/vm_page.c
index 5b05f0dc11c9..bbae55895c2c 100644
--- a/sys/vm/vm_page.c
+++ b/sys/vm/vm_page.c
@@ -1983,7 +1983,10 @@ vm_page_iter_rename(struct pctrie_iter *old_pages, vm_page_t m,
  *				intends to allocate
  *	VM_ALLOC_NOBUSY		do not exclusive busy the page
  *	VM_ALLOC_NODUMP		do not include the page in a kernel core dump
+ *	VM_ALLOC_NOFREE		page will never be freed
+ *	VM_ALLOC_NOWAIT		ignored (default behavior)
  *	VM_ALLOC_SBUSY		shared busy the allocated page
+ *	VM_ALLOC_WAITFAIL	in case of failure, sleep before returning
  *	VM_ALLOC_WIRED		wire the allocated page
  *	VM_ALLOC_ZERO		prefer a zeroed page
  */
@@ -2081,11 +2084,12 @@ vm_page_alloc_domain_iter(vm_object_t object, vm_pindex_t pindex, int domain,
 	vm_page_t m;
 	int flags;
 
-#define	VPA_FLAGS	(VM_ALLOC_CLASS_MASK | VM_ALLOC_WAITFAIL |	\
-			 VM_ALLOC_NOWAIT | VM_ALLOC_NOBUSY |		\
-			 VM_ALLOC_SBUSY | VM_ALLOC_WIRED |		\
-			 VM_ALLOC_NODUMP | VM_ALLOC_ZERO |		\
-			 VM_ALLOC_NOFREE | VM_ALLOC_COUNT_MASK)
+#define	VM_ALLOC_COMMON	(VM_ALLOC_CLASS_MASK | VM_ALLOC_NODUMP |	\
+			 VM_ALLOC_NOWAIT | VM_ALLOC_WAITFAIL |		\
+			 VM_ALLOC_WIRED | VM_ALLOC_ZERO)
+#define	VPA_FLAGS	(VM_ALLOC_COMMON | VM_ALLOC_COUNT_MASK |	\
+			 VM_ALLOC_NOBUSY | VM_ALLOC_NOFREE |		\
+			 VM_ALLOC_SBUSY)
 	KASSERT((req & ~VPA_FLAGS) == 0,
 	    ("invalid request %#x", req));
 	KASSERT(((req & (VM_ALLOC_NOBUSY | VM_ALLOC_SBUSY)) !=
@@ -2234,15 +2238,18 @@ found:
  *
  *	allocation classes:
  *	VM_ALLOC_NORMAL		normal process request
- *	VM_ALLOC_SYSTEM		system *really* needs a page
+ *	VM_ALLOC_SYSTEM		system *really* needs the pages
  *	VM_ALLOC_INTERRUPT	interrupt time request
  *
  *	optional allocation flags:
- *	VM_ALLOC_NOBUSY		do not exclusive busy the page
- *	VM_ALLOC_NODUMP		do not include the page in a kernel core dump
- *	VM_ALLOC_SBUSY		shared busy the allocated page
- *	VM_ALLOC_WIRED		wire the allocated page
- *	VM_ALLOC_ZERO		prefer a zeroed page
+ *	VM_ALLOC_NOBUSY		do not exclusive busy the pages
+ *	VM_ALLOC_NODUMP		do not include the pages in a kernel core dump
+ *	VM_ALLOC_NORECLAIM	do not reclaim after initial failure
+ *	VM_ALLOC_NOWAIT		ignored (default behavior)
+ *	VM_ALLOC_SBUSY		shared busy the allocated pages
+ *	VM_ALLOC_WAITFAIL	in case of failure, sleep before returning
+ *	VM_ALLOC_WIRED		wire the allocated pages
+ *	VM_ALLOC_ZERO		prefer zeroed pages
  */
 vm_page_t
 vm_page_alloc_contig(vm_object_t object, vm_pindex_t pindex, int req,
@@ -2321,7 +2328,9 @@ vm_page_alloc_contig_domain(vm_object_t object, vm_pindex_t pindex, int domain,
 	vm_page_t m, m_ret, mpred;
 	u_int busy_lock, flags, oflags;
 
-#define	VPAC_FLAGS	(VPA_FLAGS | VM_ALLOC_NORECLAIM)
+#define	VPAC_FLAGS	(VM_ALLOC_COMMON | VM_ALLOC_COUNT_MASK |	\
+			 VM_ALLOC_NOBUSY | VM_ALLOC_NORECLAIM |		\
+			 VM_ALLOC_SBUSY)
 	KASSERT((req & ~VPAC_FLAGS) == 0,
 	    ("invalid request %#x", req));
 	KASSERT(((req & (VM_ALLOC_NOBUSY | VM_ALLOC_SBUSY)) !=
@@ -2422,11 +2431,8 @@ vm_page_alloc_noobj_domain(int domain, int req)
 	vm_page_t m;
 	int flags;
 
-#define	VPAN_FLAGS	(VM_ALLOC_CLASS_MASK | VM_ALLOC_WAITFAIL |      \
-			 VM_ALLOC_NOWAIT | VM_ALLOC_WAITOK |		\
-			 VM_ALLOC_NOBUSY | VM_ALLOC_WIRED |		\
-			 VM_ALLOC_NODUMP | VM_ALLOC_ZERO |		\
-			 VM_ALLOC_NOFREE | VM_ALLOC_COUNT_MASK)
+#define	VPAN_FLAGS	(VM_ALLOC_COMMON | VM_ALLOC_COUNT_MASK |	\
+			 VM_ALLOC_NOFREE | VM_ALLOC_WAITOK)
 	KASSERT((req & ~VPAN_FLAGS) == 0,
 	    ("invalid request %#x", req));
 
@@ -2624,15 +2630,13 @@ vm_page_alloc_noobj_contig_domain(int domain, int req, u_long npages,
 	vm_page_t m, m_ret;
 	u_int flags;
 
-#define	VPANC_FLAGS	(VPAN_FLAGS | VM_ALLOC_NORECLAIM)
+#define	VPANC_FLAGS	(VM_ALLOC_COMMON | VM_ALLOC_COUNT_MASK |	\
+			 VM_ALLOC_NORECLAIM | VM_ALLOC_WAITOK)
 	KASSERT((req & ~VPANC_FLAGS) == 0,
 	    ("invalid request %#x", req));
 	KASSERT((req & (VM_ALLOC_WAITOK | VM_ALLOC_NORECLAIM)) !=
 	    (VM_ALLOC_WAITOK | VM_ALLOC_NORECLAIM),
 	    ("invalid request %#x", req));
-	KASSERT(((req & (VM_ALLOC_NOBUSY | VM_ALLOC_SBUSY)) !=
-	    (VM_ALLOC_NOBUSY | VM_ALLOC_SBUSY)),
-	    ("invalid request %#x", req));
 	KASSERT(npages > 0, ("vm_page_alloc_contig: npages is zero"));
 
 	while ((m_ret = vm_page_find_contig_domain(domain, req, npages,
@@ -5141,15 +5145,20 @@ vm_page_grab_valid_unlocked(vm_page_t *mp, vm_object_t object,
  * allocation classes:
  *	VM_ALLOC_NORMAL		normal process request
  *	VM_ALLOC_SYSTEM		system *really* needs the pages
+ *	VM_ALLOC_INTERRUPT	interrupt time request
  *
  * The caller must always specify that the pages are to be busied and/or
  * wired.
  *
  * optional allocation flags:
  *	VM_ALLOC_IGN_SBUSY	do not sleep on soft busy pages
- *	VM_ALLOC_NOBUSY		do not exclusive busy the page
+ *	VM_ALLOC_NOBUSY		do not exclusive busy the pages
+ *	VM_ALLOC_NODUMP		do not include the pages in a kernel core dump
+ *	VM_ALLOC_NOFREE		pages will never be freed
  *	VM_ALLOC_NOWAIT		do not sleep
- *	VM_ALLOC_SBUSY		set page to sbusy state
+ *	VM_ALLOC_SBUSY		set pages to sbusy state
+ *	VM_ALLOC_WAITFAIL	in case of failure, sleep before returning
+ *	VM_ALLOC_WAITOK		ignored (default behavior)
  *	VM_ALLOC_WIRED		wire the pages
  *	VM_ALLOC_ZERO		zero and validate any invalid pages
  *
diff --git a/sys/vm/vm_page.h b/sys/vm/vm_page.h
index ba3f88864661..8f2d5aee3cd4 100644
--- a/sys/vm/vm_page.h
+++ b/sys/vm/vm_page.h
@@ -462,13 +462,15 @@ extern long first_page;			/* first physical page number */
 vm_page_t PHYS_TO_VM_PAGE(vm_paddr_t pa);
 
 /*
- * Page allocation parameters for vm_page for the functions
- * vm_page_alloc(), vm_page_grab(), vm_page_alloc_contig() and
- * vm_page_alloc_freelist().  Some functions support only a subset
- * of the flags, and ignore others, see the flags legend.
+ * vm_page allocation arguments for the functions vm_page_alloc(),
+ * vm_page_alloc_contig(), vm_page_alloc_noobj(), vm_page_grab(), and
+ * vm_page_grab_pages().  Each function supports only a subset of the flags.
+ * See the flags legend.
  *
- * The meaning of VM_ALLOC_ZERO differs slightly between the vm_page_alloc*()
- * and the vm_page_grab*() functions.  See these functions for details.
+ * The meaning of VM_ALLOC_ZERO varies: vm_page_alloc_noobj(), vm_page_grab(),
+ * and vm_page_grab_pages() guarantee that the returned pages are zeroed; in
+ * contrast vm_page_alloc() and vm_page_alloc_contig() do not, leaving it to
+ * the caller to test the page's flags for PG_ZERO.
  *
  * Bits 0 - 1 define class.
  * Bits 2 - 15 dedicated for flags.
@@ -476,7 +478,7 @@ vm_page_t PHYS_TO_VM_PAGE(vm_paddr_t pa);
  * (a) - vm_page_alloc() supports the flag.
  * (c) - vm_page_alloc_contig() supports the flag.
  * (g) - vm_page_grab() supports the flag.
- * (n) - vm_page_alloc_noobj() and vm_page_alloc_freelist() support the flag.
+ * (n) - vm_page_alloc_noobj() supports the flag.
  * (p) - vm_page_grab_pages() supports the flag.
  * Bits above 15 define the count of additional pages that the caller
  * intends to allocate.
@@ -485,26 +487,26 @@ vm_page_t PHYS_TO_VM_PAGE(vm_paddr_t pa);
 #define VM_ALLOC_INTERRUPT	1
 #define VM_ALLOC_SYSTEM		2
 #define	VM_ALLOC_CLASS_MASK	3
-#define	VM_ALLOC_WAITOK		0x0008	/* (acn) Sleep and retry */
-#define	VM_ALLOC_WAITFAIL	0x0010	/* (acn) Sleep and return error */
+#define	VM_ALLOC_WAITOK		0x0008	/* (gnp) Sleep and retry */
+#define	VM_ALLOC_WAITFAIL	0x0010	/* (acgnp) Sleep and return error */
 #define	VM_ALLOC_WIRED		0x0020	/* (acgnp) Allocate a wired page */
 #define	VM_ALLOC_ZERO		0x0040	/* (acgnp) Allocate a zeroed page */
 #define	VM_ALLOC_NORECLAIM	0x0080	/* (c) Do not reclaim after failure */
-#define	VM_ALLOC_NOFREE		0x0100	/* (an) Page will never be released */
+#define	VM_ALLOC_NOFREE		0x0100	/* (agnp) Page will never be freed */
 #define	VM_ALLOC_NOBUSY		0x0200	/* (acgp) Do not excl busy the page */
-#define	VM_ALLOC_NOCREAT	0x0400	/* (gp) Don't create a page */
+#define	VM_ALLOC_NOCREAT	0x0400	/* (gp) Do not allocate a page */
 #define	VM_ALLOC_AVAIL1		0x0800
-#define	VM_ALLOC_IGN_SBUSY	0x1000	/* (gp) Ignore shared busy flag */
-#define	VM_ALLOC_NODUMP		0x2000	/* (ag) don't include in dump */
+#define	VM_ALLOC_IGN_SBUSY	0x1000	/* (gp) Ignore shared busy state */
+#define	VM_ALLOC_NODUMP		0x2000	/* (acgnp) Do not include in dump */
 #define	VM_ALLOC_SBUSY		0x4000	/* (acgp) Shared busy the page */
 #define	VM_ALLOC_NOWAIT		0x8000	/* (acgnp) Do not sleep */
 #define	VM_ALLOC_COUNT_MAX	0xffff
 #define	VM_ALLOC_COUNT_SHIFT	16
 #define	VM_ALLOC_COUNT_MASK	(VM_ALLOC_COUNT(VM_ALLOC_COUNT_MAX))
-#define	VM_ALLOC_COUNT(count)	({				\
-	KASSERT((count) <= VM_ALLOC_COUNT_MAX,			\
-	    ("%s: invalid VM_ALLOC_COUNT value", __func__));	\
-	(count) << VM_ALLOC_COUNT_SHIFT;			\
+#define	VM_ALLOC_COUNT(count)	({ 	/* (acgn) Additional pages */	\
+	KASSERT((count) <= VM_ALLOC_COUNT_MAX,				\
+	    ("%s: invalid VM_ALLOC_COUNT value", __func__));		\
+	(count) << VM_ALLOC_COUNT_SHIFT;				\
 })
 
 #ifdef M_NOWAIT



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202506251910.55PJAF9S076800>