From nobody Thu Jun 27 19:19:15 2024 X-Original-To: dev-commits-src-main@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 4W97fb6Ts7z5QFML; Thu, 27 Jun 2024 19:19:19 +0000 (UTC) (envelope-from unkadoug@gmail.com) Received: from mail-ot1-x32c.google.com (mail-ot1-x32c.google.com [IPv6:2607:f8b0:4864:20::32c]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "WR4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4W97fZ710Tz4lk8; Thu, 27 Jun 2024 19:19:18 +0000 (UTC) (envelope-from unkadoug@gmail.com) Authentication-Results: mx1.freebsd.org; dkim=pass header.d=gmail.com header.s=20230601 header.b=acqsofRn; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (mx1.freebsd.org: domain of unkadoug@gmail.com designates 2607:f8b0:4864:20::32c as permitted sender) smtp.mailfrom=unkadoug@gmail.com Received: by mail-ot1-x32c.google.com with SMTP id 46e09a7af769-6fb840d8ffdso4391508a34.1; Thu, 27 Jun 2024 12:19:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1719515957; x=1720120757; darn=freebsd.org; h=content-transfer-encoding:in-reply-to:references:to :content-language:subject:user-agent:mime-version:date:message-id :from:from:to:cc:subject:date:message-id:reply-to; bh=T+NyAr0mdgm8LnaYLwg3YBqENlxhjE4sw2OAM/aVpyI=; b=acqsofRn9xOuJ/z4NRAaF4nZoPe9IlQ2O+yIf7BH/5rDxrFjwVS7Np3Cg87GRrlFZI Cjmy3phGR/muPpd+m3LpIwddK9xZ+7vJtMkUT1HVdiP9BgKUjPbswwPvMAweUkv6wA3a UvZ62jNfMGJzuc7L9C8wdisBG6oBOGj0QZJ3y+7lt0OB1madRadQJcx0sdrEna1APAm6 PsmSVU3V8BYw3zYoIvZiS5pbJwQvUdL2XqUFucI+z0jDx5mkXA6BXiWqxmq59Z+Cgxjb bq8d/Y0ibiSCsaIH6hkNTMmFKf2X9YYrA5QcmO1Xji1WfdaHyoHK9Ww7Ox+xOmzZPmVC r0UQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1719515957; x=1720120757; h=content-transfer-encoding:in-reply-to:references:to :content-language:subject:user-agent:mime-version:date:message-id :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=T+NyAr0mdgm8LnaYLwg3YBqENlxhjE4sw2OAM/aVpyI=; b=LxTTN2nhlGSpIUzkdRdnxpIKYeYeKyopj4z8IGDWM4jtSqC3bsvmgNdfRrQClho1gm U0qZU07s5Td6yXSFL3OqW8RGRhqw6NvvvnqDCC/G2umMCXI9+lK8owZ2HUF5StYgeoSE piEqmjOPePFlRqUXXJ4FQl9qu0dClSz8MqZStjV6pUScMFdbDWebg62YGjanH5SRF+4n o0cqme+coJomigdJ2Jga/qDlNWewTytSnZtn+stQL9LqtVj+Es95Q9lh9g43tOlZWUXQ UeU59qQHQC2/nePQ8QntaRL0JKhN7qhW0BJb4mJRFdbRN1/Em5Tyj+IFF+c0yTwBt1Fw dxCw== X-Forwarded-Encrypted: i=1; AJvYcCWnxdqUznCJuh7bx6//PB0zftWlFmtnCbrC/MfpbTOlc5NXhnHxStW4eEwkF5gyXc/gBJyrR5uoCKSc5AwiHq8PDwzRyZhhs8D8yq7qFnSLC7y63EOouWNtTyypgSzLGhtZc4VK+TnSChkeXKTIvDX0nw== X-Gm-Message-State: AOJu0YzH1HUPvgL6cPNMzxTstzmW47cUOTjgVL5AGvCkULrJUo1aaFZk rxOln8gl1RCa81/jwVBZ3qxuI09m8GXQAM57ci54feuAMXmkN/Y9mFcsI5H+ X-Google-Smtp-Source: AGHT+IGg/z2SA2zYqCxhS1ez6rbfIL+BU0fQfousD77xOdjdDmoGnGNwbVkIeEhfZyiUjS3+UaZfXA== X-Received: by 2002:a05:6830:88:b0:701:ef8b:d676 with SMTP id 46e09a7af769-701ef8bf6d8mr3672767a34.5.1719515956767; Thu, 27 Jun 2024 12:19:16 -0700 (PDT) Received: from [108.254.203.202] (108-254-203-202.lightspeed.hstntx.sbcglobal.net. [108.254.203.202]) by smtp.gmail.com with ESMTPSA id 46e09a7af769-701f7b20212sm52836a34.61.2024.06.27.12.19.16 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 27 Jun 2024 12:19:16 -0700 (PDT) From: Doug Moore X-Google-Original-From: Doug Moore Message-ID: <960d761d-5c58-9e97-c07a-d689d07bdab1@freebsd.org> Date: Thu, 27 Jun 2024 14:19:15 -0500 List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@FreeBSD.org MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:102.0) Gecko/20100101 Thunderbird/102.10.0 Subject: Re: git: 995730a635bc - main - swap_pager: cleanup swapoff_object Content-Language: en-US To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org References: <202406271917.45RJHQ31058153@gitrepo.freebsd.org> In-Reply-To: <202406271917.45RJHQ31058153@gitrepo.freebsd.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spamd-Bar: -- X-Spamd-Result: default: False [-2.94 / 15.00]; NEURAL_HAM_MEDIUM(-1.00)[-1.000]; MID_RHS_MATCH_TO(1.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000]; NEURAL_HAM_SHORT(-0.94)[-0.938]; DMARC_POLICY_ALLOW(-0.50)[gmail.com,none]; R_DKIM_ALLOW(-0.20)[gmail.com:s=20230601]; R_SPF_ALLOW(-0.20)[+ip6:2607:f8b0:4000::/36]; MIME_GOOD(-0.10)[text/plain]; TO_DN_NONE(0.00)[]; MIME_TRACE(0.00)[0:+]; FROM_HAS_DN(0.00)[]; FREEMAIL_FROM(0.00)[gmail.com]; FREEMAIL_ENVFROM(0.00)[gmail.com]; FROM_EQ_ENVFROM(0.00)[]; RCVD_IN_DNSWL_NONE(0.00)[2607:f8b0:4864:20::32c:from]; ASN(0.00)[asn:15169, ipnet:2607:f8b0::/32, country:US]; RCVD_VIA_SMTP_AUTH(0.00)[]; RCVD_COUNT_TWO(0.00)[2]; RCVD_TLS_LAST(0.00)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; ARC_NA(0.00)[]; DWL_DNSWL_NONE(0.00)[gmail.com:dkim]; RCPT_COUNT_THREE(0.00)[3]; MLMMJ_DEST(0.00)[dev-commits-src-all@freebsd.org,dev-commits-src-main@freebsd.org]; DKIM_TRACE(0.00)[gmail.com:+] X-Rspamd-Queue-Id: 4W97fZ710Tz4lk8 Omitted: Previous version tested by pho. My apologies. Doug On 6/27/24 14:17, Doug Moore wrote: > The branch main has been updated by dougm: > > URL: https://cgit.FreeBSD.org/src/commit/?id=995730a635bcb3bf6e5f2ffde950fb930cb1c23b > > commit 995730a635bcb3bf6e5f2ffde950fb930cb1c23b > Author: Doug Moore > AuthorDate: 2024-06-27 19:06:09 +0000 > Commit: Doug Moore > CommitDate: 2024-06-27 19:06:09 +0000 > > swap_pager: cleanup swapoff_object > > Function swap_pager_swapoff_object calls vm_pager_unswapped (via > swp_pager_force_dirty) for every page that must be unswapped. That > means that there's an unneeded check for lock ownership (the caller > always owns it), a needless PCTRIE_LOOKUP (the caller has already > found it), a call to free one page of swap space only, and a check to > see if all blocks are empty, when the caller usually knows that the > check is useless. > > Isolate the essential part, needed however swap_pager_unswapped is > invoked, into a smaller function swap_pager_unswapped_acct. From > swapoff_object, invoke swp_pager_update_freerange for each appropriate > page, so that there are potentially fewer calls to > swp_pager_freeswapspace. Consider freeing a set of blocks (a struct > swblk) only after having invalidated all those blocks. > > Replace the doubly-nested loops with a single loop, and refetch and > rescan a swblk only when the object write lock has been released and > reacquired. > > After getting a page from swap, dirty it immediately to address a race > condition observed by @kib. > > Reviewed by: kib > Differential Revision: https://reviews.freebsd.org/D45668 > --- > sys/vm/swap_pager.c | 184 +++++++++++++++++++++++++++++----------------------- > 1 file changed, 103 insertions(+), 81 deletions(-) > > diff --git a/sys/vm/swap_pager.c b/sys/vm/swap_pager.c > index 8d9236a2eb1a..79986842d814 100644 > --- a/sys/vm/swap_pager.c > +++ b/sys/vm/swap_pager.c > @@ -1183,6 +1183,21 @@ swap_pager_haspage(vm_object_t object, vm_pindex_t pindex, int *before, > return (TRUE); > } > > +static void > +swap_pager_unswapped_acct(vm_page_t m) > +{ > + KASSERT((m->object->flags & OBJ_SWAP) != 0, > + ("Free object not swappable")); > + if ((m->a.flags & PGA_SWAP_FREE) != 0) > + counter_u64_add(swap_free_completed, 1); > + vm_page_aflag_clear(m, PGA_SWAP_FREE | PGA_SWAP_SPACE); > + > + /* > + * The meta data only exists if the object is OBJT_SWAP > + * and even then might not be allocated yet. > + */ > +} > + > /* > * SWAP_PAGER_PAGE_UNSWAPPED() - remove swap backing store related to page > * > @@ -1229,16 +1244,7 @@ swap_pager_unswapped(vm_page_t m) > } > return; > } > - if ((m->a.flags & PGA_SWAP_FREE) != 0) > - counter_u64_add(swap_free_completed, 1); > - vm_page_aflag_clear(m, PGA_SWAP_FREE | PGA_SWAP_SPACE); > - > - /* > - * The meta data only exists if the object is OBJT_SWAP > - * and even then might not be allocated yet. > - */ > - KASSERT((m->object->flags & OBJ_SWAP) != 0, > - ("Free object not swappable")); > + swap_pager_unswapped_acct(m); > > sb = SWAP_PCTRIE_LOOKUP(&m->object->un_pager.swp.swp_blks, > rounddown(m->pindex, SWAP_META_PAGES)); > @@ -1786,11 +1792,12 @@ swap_pager_nswapdev(void) > } > > static void > -swp_pager_force_dirty(vm_page_t m) > +swp_pager_force_dirty(struct page_range *range, vm_page_t m, daddr_t *blk) > { > - > vm_page_dirty(m); > - swap_pager_unswapped(m); > + swap_pager_unswapped_acct(m); > + swp_pager_update_freerange(range, *blk); > + *blk = SWAPBLK_NONE; > vm_page_launder(m); > } > > @@ -1827,18 +1834,23 @@ swap_pager_swapped_pages(vm_object_t object) > static void > swap_pager_swapoff_object(struct swdevt *sp, vm_object_t object) > { > + struct page_range range; > struct swblk *sb; > vm_page_t m; > vm_pindex_t pi; > daddr_t blk; > - int i, nv, rahead, rv; > + int i, rahead, rv; > + bool sb_empty; > > + VM_OBJECT_ASSERT_WLOCKED(object); > KASSERT((object->flags & OBJ_SWAP) != 0, > ("%s: Object not swappable", __func__)); > > - for (pi = 0; (sb = SWAP_PCTRIE_LOOKUP_GE( > - &object->un_pager.swp.swp_blks, pi)) != NULL; ) { > - if ((object->flags & OBJ_DEAD) != 0) { > + pi = 0; > + i = 0; > + swp_pager_init_freerange(&range); > + for (;;) { > + if (i == 0 && (object->flags & OBJ_DEAD) != 0) { > /* > * Make sure that pending writes finish before > * returning. > @@ -1847,76 +1859,86 @@ swap_pager_swapoff_object(struct swdevt *sp, vm_object_t object) > swp_pager_meta_free_all(object); > break; > } > - for (i = 0; i < SWAP_META_PAGES; i++) { > - /* > - * Count the number of contiguous valid blocks. > - */ > - for (nv = 0; nv < SWAP_META_PAGES - i; nv++) { > - blk = sb->d[i + nv]; > - if (!swp_pager_isondev(blk, sp) || > - blk == SWAPBLK_NONE) > - break; > - } > - if (nv == 0) > - continue; > > - /* > - * Look for a page corresponding to the first > - * valid block and ensure that any pending paging > - * operations on it are complete. If the page is valid, > - * mark it dirty and free the swap block. Try to batch > - * this operation since it may cause sp to be freed, > - * meaning that we must restart the scan. Avoid busying > - * valid pages since we may block forever on kernel > - * stack pages. > - */ > - m = vm_page_lookup(object, sb->p + i); > - if (m == NULL) { > - m = vm_page_alloc(object, sb->p + i, > - VM_ALLOC_NORMAL | VM_ALLOC_WAITFAIL); > - if (m == NULL) > - break; > - } else { > - if ((m->oflags & VPO_SWAPINPROG) != 0) { > - m->oflags |= VPO_SWAPSLEEP; > - VM_OBJECT_SLEEP(object, &object->handle, > - PSWP, "swpoff", 0); > - break; > - } > - if (vm_page_all_valid(m)) { > - do { > - swp_pager_force_dirty(m); > - } while (--nv > 0 && > - (m = vm_page_next(m)) != NULL && > - vm_page_all_valid(m) && > - (m->oflags & VPO_SWAPINPROG) == 0); > - break; > - } > - if (!vm_page_busy_acquire(m, VM_ALLOC_WAITFAIL)) > - break; > + if (i == SWAP_META_PAGES) { > + pi = sb->p + SWAP_META_PAGES; > + if (sb_empty) { > + SWAP_PCTRIE_REMOVE( > + &object->un_pager.swp.swp_blks, sb->p); > + uma_zfree(swblk_zone, sb); > } > + i = 0; > + } > > - vm_object_pip_add(object, 1); > - rahead = SWAP_META_PAGES; > - rv = swap_pager_getpages_locked(object, &m, 1, NULL, > - &rahead); > - if (rv != VM_PAGER_OK) > - panic("%s: read from swap failed: %d", > - __func__, rv); > - VM_OBJECT_WLOCK(object); > - vm_object_pip_wakeupn(object, 1); > - vm_page_xunbusy(m); > + if (i == 0) { > + sb = SWAP_PCTRIE_LOOKUP_GE( > + &object->un_pager.swp.swp_blks, pi); > + if (sb == NULL) > + break; > + sb_empty = true; > + m = NULL; > + } > > - /* > - * The object lock was dropped so we must restart the > - * scan of this swap block. Pages paged in during this > - * iteration will be marked dirty in a future iteration. > - */ > - break; > + /* Skip an invalid block. */ > + blk = sb->d[i]; > + if (blk == SWAPBLK_NONE || !swp_pager_isondev(blk, sp)) { > + if (blk != SWAPBLK_NONE) > + sb_empty = false; > + m = NULL; > + i++; > + continue; > } > - if (i == SWAP_META_PAGES) > - pi = sb->p + SWAP_META_PAGES; > + > + /* > + * Look for a page corresponding to this block, If the found > + * page has pending operations, sleep and restart the scan. > + */ > + m = m != NULL ? vm_page_next(m) : > + vm_page_lookup(object, sb->p + i); > + if (m != NULL && (m->oflags & VPO_SWAPINPROG) != 0) { > + m->oflags |= VPO_SWAPSLEEP; > + VM_OBJECT_SLEEP(object, &object->handle, PSWP, "swpoff", > + 0); > + i = 0; /* Restart scan after object lock dropped. */ > + continue; > + } > + > + /* > + * If the found page is valid, mark it dirty and free the swap > + * block. > + */ > + if (m != NULL && vm_page_all_valid(m)) { > + swp_pager_force_dirty(&range, m, &sb->d[i]); > + i++; > + continue; > + } > + > + /* Is there a page we can acquire or allocate? */ > + if (m == NULL) { > + m = vm_page_alloc(object, sb->p + i, > + VM_ALLOC_NORMAL | VM_ALLOC_WAITFAIL); > + } else if (!vm_page_busy_acquire(m, VM_ALLOC_WAITFAIL)) > + m = NULL; > + > + /* If no page available, repeat this iteration. */ > + if (m == NULL) > + continue; > + > + /* Get the page from swap, mark it dirty, restart the scan. */ > + vm_object_pip_add(object, 1); > + rahead = SWAP_META_PAGES; > + rv = swap_pager_getpages_locked(object, &m, 1, NULL, &rahead); > + if (rv != VM_PAGER_OK) > + panic("%s: read from swap failed: %d", __func__, rv); > + VM_OBJECT_WLOCK(object); > + vm_object_pip_wakeupn(object, 1); > + KASSERT(vm_page_all_valid(m), > + ("%s: Page %p not all valid", __func__, m)); > + swp_pager_force_dirty(&range, m, &sb->d[i]); > + vm_page_xunbusy(m); > + i = 0; /* Restart scan after object lock dropped. */ > } > + swp_pager_freeswapspace(&range); > } > > /*