From nobody Thu Dec 23 08:02:30 2021 X-Original-To: dev-commits-src-branches@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id EAB49190417E; Thu, 23 Dec 2021 08:02:30 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4JKN2t517cz3CV1; Thu, 23 Dec 2021 08:02:30 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 8C1945203; Thu, 23 Dec 2021 08:02:30 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 1BN82UNM056887; Thu, 23 Dec 2021 08:02:30 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 1BN82Uml056886; Thu, 23 Dec 2021 08:02:30 GMT (envelope-from git) Date: Thu, 23 Dec 2021 08:02:30 GMT Message-Id: <202112230802.1BN82Uml056886@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Doug Moore Subject: git: 3b8062cdd5a8 - stable/13 - vm: Don't break vm reserv that can't meet align reqs List-Id: Commits to the stable branches of the FreeBSD src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-branches List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-branches@freebsd.org X-BeenThere: dev-commits-src-branches@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: dougm X-Git-Repository: src X-Git-Refname: refs/heads/stable/13 X-Git-Reftype: branch X-Git-Commit: 3b8062cdd5a86232957cfb26e6f89ca4798ecad0 Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1640246550; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=1g5Re7g7DEHvV6qWQtesmbJ7F7VgSY+H8+77fOfdY9Y=; b=XhVlaQkLUC3lYciI89Ye0oUXJTBDR0kMetj7HT+Pir4hZNfsvJvFKz4icr+rvuWNCEu/LA vwJghRPOO5BE1co4N2VBdj17KTW7BwVMuuh90oXx86FjaBY9m9aN6RfglPbn7by3RVudAb ZRvQeo13WOQd81WOZvRbs1nL5c+C0s8FAmy3HTxz6mtHT9pS/LpkanrRFNvXm/NJFrj3b7 irQwWU2cKG7B8gb4Yl+ZAb/j5DtxWned+98aaVeJbou/xM0Exv7zRtEVA1idOshNTqf6GT kfsOq3reYu7RsvKvZISIuV/QvglX301QhRNfTNvFPhHkZzJGxaSu2jsWpt17oQ== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1640246550; a=rsa-sha256; cv=none; b=vRo1tosCmicQMMLIZyac54DSZP056xfWxnKG7wSX8y7PVG9tVPj5wtFTLEIHbuP2k5AHpC X2WwCkzEMkrYR6a8+omwYG190o0GltbTH5U8q1dpGR+Xp1dcMiXP9jpMEHHWXcEuqdPhRU kq7Xv8cGQoJeVDai2X59zFbH6BMMb5uzzmff5ZjvGMFwLdEGwVLG8doEDya94jZ7cX4wry w0OVrnpDmM23Fd7wF9egPmwUg45/CNtxaPy7s8BNf0CNq0l1lSzbvpiWuuBGhB+dfp7Wlj 8lnewSDR4gfAcAa082Ljdsm69Br/NXfnqIu6+zDiiBr6c8P5DmLoQeqF/wsasA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch stable/13 has been updated by dougm: URL: https://cgit.FreeBSD.org/src/commit/?id=3b8062cdd5a86232957cfb26e6f89ca4798ecad0 commit 3b8062cdd5a86232957cfb26e6f89ca4798ecad0 Author: Doug Moore AuthorDate: 2021-12-16 18:20:56 +0000 Commit: Doug Moore CommitDate: 2021-12-23 08:01:17 +0000 vm: Don't break vm reserv that can't meet align reqs Function vm_reserv_test_contig has incorrectly used its alignment and boundary parameters to find a well-positioned range of empty pages in a reservation. Consequently, a reservation could be broken mistakenly when it was unable to provide a satisfactory set of pages. Rename the function, correct the errors, and add assertions to detect the error in case it appears again. Reviewed by: alc, markj MFC after: 1 week Differential Revision: https://reviews.freebsd.org/D33344 (cherry picked from commit 6f1c8908272f3c0a6631e001bd2eb50a5b69261d) --- sys/vm/vm_reserv.c | 94 +++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 65 insertions(+), 29 deletions(-) diff --git a/sys/vm/vm_reserv.c b/sys/vm/vm_reserv.c index ca31eac964c0..8b5f316cc420 100644 --- a/sys/vm/vm_reserv.c +++ b/sys/vm/vm_reserv.c @@ -1233,25 +1233,29 @@ vm_reserv_reclaim_inactive(int domain) /* * Determine whether this reservation has free pages that satisfy the given * request for contiguous physical memory. Start searching from the lower - * bound, defined by low_index. + * bound, defined by lo, and stop at the upper bound, hi. Return the index + * of the first satisfactory free page, or -1 if none is found. */ -static bool -vm_reserv_test_contig(vm_reserv_t rv, u_long npages, vm_paddr_t low, - vm_paddr_t high, u_long alignment, vm_paddr_t boundary) +static int +vm_reserv_find_contig(vm_reserv_t rv, int npages, int lo, + int hi, int ppn_align, int ppn_bound) { - vm_paddr_t pa, size; u_long changes; - int bitpos, bits_left, i, hi, lo, n; + int bitpos, bits_left, i, n; vm_reserv_assert_locked(rv); - size = npages << PAGE_SHIFT; - pa = VM_PAGE_TO_PHYS(&rv->pages[0]); - lo = (pa < low) ? - ((low + PAGE_MASK - pa) >> PAGE_SHIFT) : 0; + KASSERT(npages <= VM_LEVEL_0_NPAGES - 1, + ("%s: Too many pages", __func__)); + KASSERT(ppn_bound <= VM_LEVEL_0_NPAGES, + ("%s: Too big a boundary for reservation size", __func__)); + KASSERT(npages <= ppn_bound, + ("%s: Too many pages for given boundary", __func__)); + KASSERT(ppn_align != 0 && powerof2(ppn_align), + ("ppn_align is not a positive power of 2")); + KASSERT(ppn_bound != 0 && powerof2(ppn_bound), + ("ppn_bound is not a positive power of 2")); i = lo / NBPOPMAP; changes = rv->popmap[i] | ((1UL << (lo % NBPOPMAP)) - 1); - hi = (pa + VM_LEVEL_0_SIZE > high) ? - ((high + PAGE_MASK - pa) >> PAGE_SHIFT) : VM_LEVEL_0_NPAGES; n = hi / NBPOPMAP; bits_left = hi % NBPOPMAP; hi = lo = -1; @@ -1276,25 +1280,20 @@ vm_reserv_test_contig(vm_reserv_t rv, u_long npages, vm_paddr_t low, continue; } hi = NBPOPMAP * i + bitpos; - pa = VM_PAGE_TO_PHYS(&rv->pages[lo]); - if ((pa & (alignment - 1)) != 0) { + if (lo < roundup2(lo, ppn_align)) { /* Skip to next aligned page. */ - lo += (((pa - 1) | (alignment - 1)) + 1) >> - PAGE_SHIFT; + lo = roundup2(lo, ppn_align); if (lo >= VM_LEVEL_0_NPAGES) - return (false); - pa = VM_PAGE_TO_PHYS(&rv->pages[lo]); + return (-1); } - if (((pa ^ (pa + size - 1)) & ~(boundary - 1)) != 0) { + if (lo + npages > roundup2(lo, ppn_bound)) { /* Skip to next boundary-matching page. */ - lo += (((pa - 1) | (boundary - 1)) + 1) >> - PAGE_SHIFT; + lo = roundup2(lo, ppn_bound); if (lo >= VM_LEVEL_0_NPAGES) - return (false); - pa = VM_PAGE_TO_PHYS(&rv->pages[lo]); + return (-1); } - if (lo * PAGE_SIZE + size <= hi * PAGE_SIZE) - return (true); + if (lo + npages <= hi) + return (lo); lo = hi; } if (++i < n) @@ -1303,7 +1302,7 @@ vm_reserv_test_contig(vm_reserv_t rv, u_long npages, vm_paddr_t low, changes = bits_left == 0 ? -1UL : (rv->popmap[n] | (-1UL << bits_left)); else - return (false); + return (-1); } } @@ -1320,12 +1319,32 @@ vm_reserv_reclaim_contig(int domain, u_long npages, vm_paddr_t low, struct vm_reserv_queue *queue; vm_paddr_t pa, size; vm_reserv_t marker, rv, rvn; + int hi, lo, posn, ppn_align, ppn_bound; + KASSERT(npages > 0, ("npages is 0")); + KASSERT(powerof2(alignment), ("alignment is not a power of 2")); + KASSERT(powerof2(boundary), ("boundary is not a power of 2")); if (npages > VM_LEVEL_0_NPAGES - 1) return (false); + size = npages << PAGE_SHIFT; + /* + * Ensure that a free range starting at a boundary-multiple + * doesn't include a boundary-multiple within it. Otherwise, + * no boundary-constrained allocation is possible. + */ + if (size > boundary) + return (false); marker = &vm_rvd[domain].marker; queue = &vm_rvd[domain].partpop; - size = npages << PAGE_SHIFT; + /* + * Compute shifted alignment, boundary values for page-based + * calculations. Constrain to range [1, VM_LEVEL_0_NPAGES] to + * avoid overflow. + */ + ppn_align = (int)(ulmin(ulmax(PAGE_SIZE, alignment), + VM_LEVEL_0_SIZE) >> PAGE_SHIFT); + ppn_bound = (int)(MIN(MAX(PAGE_SIZE, boundary), + VM_LEVEL_0_SIZE) >> PAGE_SHIFT); vm_reserv_domain_scan_lock(domain); vm_reserv_domain_lock(domain); @@ -1339,6 +1358,10 @@ vm_reserv_reclaim_contig(int domain, u_long npages, vm_paddr_t low, /* This entire reservation is too high; go to next. */ continue; } + if ((pa & (alignment - 1)) != 0) { + /* This entire reservation is unaligned; go to next. */ + continue; + } if (vm_reserv_trylock(rv) == 0) { TAILQ_INSERT_AFTER(queue, rv, marker, partpopq); @@ -1356,8 +1379,21 @@ vm_reserv_reclaim_contig(int domain, u_long npages, vm_paddr_t low, TAILQ_REMOVE(queue, marker, partpopq); } vm_reserv_domain_unlock(domain); - if (vm_reserv_test_contig(rv, npages, low, high, - alignment, boundary)) { + lo = (pa >= low) ? 0 : + (int)((low + PAGE_MASK - pa) >> PAGE_SHIFT); + hi = (pa + VM_LEVEL_0_SIZE <= high) ? VM_LEVEL_0_NPAGES : + (int)((high - pa) >> PAGE_SHIFT); + posn = vm_reserv_find_contig(rv, (int)npages, lo, hi, + ppn_align, ppn_bound); + if (posn >= 0) { + pa = VM_PAGE_TO_PHYS(&rv->pages[posn]); + KASSERT((pa & (alignment - 1)) == 0, + ("%s: adjusted address does not align to %lx", + __func__, alignment)); + KASSERT(((pa ^ (pa + size - 1)) & -boundary) == 0, + ("%s: adjusted address spans boundary to %lx", + __func__, boundary)); + vm_reserv_domain_scan_unlock(domain); vm_reserv_reclaim(rv); vm_reserv_unlock(rv);