From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-yb1-f171.google.com (mail-yb1-f171.google.com [209.85.219.171]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 85E99342C93 for ; Fri, 5 Sep 2025 11:26:56 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.219.171 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1757071620; cv=none; b=Ldei4VDv84DaA1Bd4PNRzoKaQCbxaNsQPaCkB2yayuCpe7+9gbZ7JuEK5RP4h1GmVPuoEhSZvA1Q/FUxw2FA7AhXGL+EUJVBMHXV6aTV/Oo+P2xDRv9VLh5DpoFT7uJUdP7Pmi1+gON9o2iVHR4iKS2l/txn3+9UQ2seFfl4GTc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1757071620; c=relaxed/simple; bh=izaO1BR35Fqya77sIICkCHEPey84lEeqJmCseg1mS9I=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=niiDnBTxNFArUL3VIMnVGddEZCU+GJuAmCcc0uZPgE+3hdvCdVImi4RjyxE88YH4DnibUGK7hv2pxiwVLNux/73wxWLFAlW7XkctqaCYy51eE1RkuZmScaVfW4JjUIponRqhduIa2OnT6PtLxWd7K2eHGewOpXK6f2c8O3OBE3o= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.dk; spf=pass smtp.mailfrom=kernel.dk; dkim=pass (2048-bit key) header.d=kernel-dk.20230601.gappssmtp.com header.i=@kernel-dk.20230601.gappssmtp.com header.b=CnmtDgbf; arc=none smtp.client-ip=209.85.219.171 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.dk Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=kernel.dk Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel-dk.20230601.gappssmtp.com header.i=@kernel-dk.20230601.gappssmtp.com header.b="CnmtDgbf" Received: by mail-yb1-f171.google.com with SMTP id 3f1490d57ef6-e96d8722c6eso2224069276.3 for ; Fri, 05 Sep 2025 04:26:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel-dk.20230601.gappssmtp.com; s=20230601; t=1757071615; x=1757676415; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=yNzF16PbnrymzXw8XwNj1X2cjJHo1hQQnEraT/RlzJM=; b=CnmtDgbfe5tx1THODz/ci1n4In1WFrcRusjah2wZs+jf2hB56JQjXfJ7ATtpVom76c 61hqjlMZi1VFPhasNBrwfMHh9u/meMHuv3TN01wxweWG/qm/nxbbiEwDD5AOCKb+8F1J kqf3SLR5bFzPbJ66alN0+i5bf3paHUZgZnveUXtd/dKpZA89r+cvj5GhEPawFzrOqIGe BFTrneTpqHaT3P0E8di18lRVlobqmkBYoANZyLpCHBsruQuELWugu9BuisPtgQT3CPKc ldGSg6HF51kNkdeuhoPWeW/TZrB5OLZSUJIMA+AXIAsWJ5Cw+nfCxuAyZ2Y68HdN8Sjs o/QA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1757071615; x=1757676415; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=yNzF16PbnrymzXw8XwNj1X2cjJHo1hQQnEraT/RlzJM=; b=EGfapQf9Kf7C/q3AhxuNwE4i/W7Ik4xdkEtK/WUDHzKn+99Q+sy8ObE9UxWaQIFTTZ aQxOu//QY6qaky+sHvtLGnVFrpbT8RC/9KCOj1geiMJ2REtStCx2al1z2VtDzBh662On eNgnBWMdJX5YoE3HGelBjzt08wgMQXKaKT3F7AXAHE7JrQ7ZIRdJu9B5MPPjgJZDjw// wi72Zc8YsyOBICHR6RpKUa0qWK7XJcNneFV2wbjoLw7SUETNRuJvFPnetegv39T3a/tk Y0j4E9SeHxPuqlnH7bQBvHXAvn5ftqnQV+pEgnZpJooFgGUcuH6qEAZNcmeRWSKbwt8y NKDQ== X-Forwarded-Encrypted: i=1; AJvYcCWTp2KWWgouvhaf3JucUc8tpNEfP6H4p4fkGROmQkbCMD0vm/WL9tHJsS0WzFbHlajStqf5MwRXqRIEpbo=@vger.kernel.org X-Gm-Message-State: AOJu0Ywx+vPcXwCkrLicczWTeHrEbyu0yXjFTSIwoXFXD/A5acQ+qY0M VeZ/ooeEoH3mugwbql+LS2/rnXXTYEE/becBz05mVOe99uxAlxRdSz7DwPB7OHQ2KLE= X-Gm-Gg: ASbGncsUoch0vjTkBANeSubDr6kHHXc2g4TRW4ehiW1C5lHJ059h1+Gw0w8pOLoxcSD gYvztfSpVUZFgS9wbAkpzmJHRoVOegI7EjOesReR6O0EmKrw2Db7XhqLBeERnzdAMLygYRyfxeR hJ87Nc63xPIovbt0+swD9qjAOu+E0Q9nQT1EypQEg71m2yF+tkOgJP90p1xDYczSMndOacfbSyh glYeCtljt8bvBkhMXaDFNqwvT0m9HEJ/OCL9xEfYY0KXpB6ULTXGIrg1kEQVkF32o0tn7zAt0ZT WVOggKYHckJZROH1+tXjc0diLIaZAKuLlncpcFWXEiRidrzWKZJ1sbtfVnyCOOlH+4fY7ahOG5k lEr6foFfVStf+An+3/g== X-Google-Smtp-Source: AGHT+IEXVquWXmj0Yd3oyr4uhZoLxM2opNHaeihf8Xp8KBtnXc2guOYW42U1nSR+kmrTjCZ7Op/enQ== X-Received: by 2002:a05:6902:18ce:b0:e96:fac0:60bc with SMTP id 3f1490d57ef6-e98a58455f2mr22114649276.41.1757071615282; Fri, 05 Sep 2025 04:26:55 -0700 (PDT) Received: from [10.0.3.24] ([50.227.229.138]) by smtp.gmail.com with ESMTPSA id 3f1490d57ef6-e9bbdf504e0sm3031724276.11.2025.09.05.04.26.53 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 05 Sep 2025 04:26:54 -0700 (PDT) Message-ID: <1513d5fd-14ef-4cd0-a9a5-1016e9be6540@kernel.dk> Date: Fri, 5 Sep 2025 05:26:53 -0600 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 19/37] mm/gup: remove record_subpages() To: David Hildenbrand , linux-kernel@vger.kernel.org Cc: Alexander Potapenko , Andrew Morton , Brendan Jackman , Christoph Lameter , Dennis Zhou , Dmitry Vyukov , dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org, iommu@lists.linux.dev, io-uring@vger.kernel.org, Jason Gunthorpe , Johannes Weiner , John Hubbard , kasan-dev@googlegroups.com, kvm@vger.kernel.org, "Liam R. Howlett" , Linus Torvalds , linux-arm-kernel@axis.com, linux-arm-kernel@lists.infradead.org, linux-crypto@vger.kernel.org, linux-ide@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-mips@vger.kernel.org, linux-mmc@vger.kernel.org, linux-mm@kvack.org, linux-riscv@lists.infradead.org, linux-s390@vger.kernel.org, linux-scsi@vger.kernel.org, Lorenzo Stoakes , Marco Elver , Marek Szyprowski , Michal Hocko , Mike Rapoport , Muchun Song , netdev@vger.kernel.org, Oscar Salvador , Peter Xu , Robin Murphy , Suren Baghdasaryan , Tejun Heo , virtualization@lists.linux.dev, Vlastimil Babka , wireguard@lists.zx2c4.com, x86@kernel.org, Zi Yan References: <20250901150359.867252-1-david@redhat.com> <20250901150359.867252-20-david@redhat.com> <5090355d-546a-4d06-99e1-064354d156b5@redhat.com> Content-Language: en-US From: Jens Axboe In-Reply-To: <5090355d-546a-4d06-99e1-064354d156b5@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 9/5/25 12:41 AM, David Hildenbrand wrote: > On 01.09.25 17:03, David Hildenbrand wrote: >> We can just cleanup the code by calculating the #refs earlier, >> so we can just inline what remains of record_subpages(). >> >> Calculate the number of references/pages ahead of times, and record them >> only once all our tests passed. >> >> Signed-off-by: David Hildenbrand >> --- >> mm/gup.c | 25 ++++++++----------------- >> 1 file changed, 8 insertions(+), 17 deletions(-) >> >> diff --git a/mm/gup.c b/mm/gup.c >> index c10cd969c1a3b..f0f4d1a68e094 100644 >> --- a/mm/gup.c >> +++ b/mm/gup.c >> @@ -484,19 +484,6 @@ static inline void mm_set_has_pinned_flag(struct mm_struct *mm) >> #ifdef CONFIG_MMU >> #ifdef CONFIG_HAVE_GUP_FAST >> -static int record_subpages(struct page *page, unsigned long sz, >> - unsigned long addr, unsigned long end, >> - struct page **pages) >> -{ >> - int nr; >> - >> - page += (addr & (sz - 1)) >> PAGE_SHIFT; >> - for (nr = 0; addr != end; nr++, addr += PAGE_SIZE) >> - pages[nr] = page++; >> - >> - return nr; >> -} >> - >> /** >> * try_grab_folio_fast() - Attempt to get or pin a folio in fast path. >> * @page: pointer to page to be grabbed >> @@ -2967,8 +2954,8 @@ static int gup_fast_pmd_leaf(pmd_t orig, pmd_t *pmdp, unsigned long addr, >> if (pmd_special(orig)) >> return 0; >> - page = pmd_page(orig); >> - refs = record_subpages(page, PMD_SIZE, addr, end, pages + *nr); >> + refs = (end - addr) >> PAGE_SHIFT; >> + page = pmd_page(orig) + ((addr & ~PMD_MASK) >> PAGE_SHIFT); >> folio = try_grab_folio_fast(page, refs, flags); >> if (!folio) >> @@ -2989,6 +2976,8 @@ static int gup_fast_pmd_leaf(pmd_t orig, pmd_t *pmdp, unsigned long addr, >> } >> *nr += refs; >> + for (; refs; refs--) >> + *(pages++) = page++; >> folio_set_referenced(folio); >> return 1; >> } >> @@ -3007,8 +2996,8 @@ static int gup_fast_pud_leaf(pud_t orig, pud_t *pudp, unsigned long addr, >> if (pud_special(orig)) >> return 0; >> - page = pud_page(orig); >> - refs = record_subpages(page, PUD_SIZE, addr, end, pages + *nr); >> + refs = (end - addr) >> PAGE_SHIFT; >> + page = pud_page(orig) + ((addr & ~PUD_MASK) >> PAGE_SHIFT); >> folio = try_grab_folio_fast(page, refs, flags); >> if (!folio) >> @@ -3030,6 +3019,8 @@ static int gup_fast_pud_leaf(pud_t orig, pud_t *pudp, unsigned long addr, >> } >> *nr += refs; >> + for (; refs; refs--) >> + *(pages++) = page++; >> folio_set_referenced(folio); >> return 1; >> } > > Okay, this code is nasty. We should rework this code to just return the nr and receive a the proper > pages pointer, getting rid of the "*nr" parameter. > > For the time being, the following should do the trick: > > commit bfd07c995814354f6b66c5b6a72e96a7aa9fb73b (HEAD -> nth_page) > Author: David Hildenbrand > Date: Fri Sep 5 08:38:43 2025 +0200 > > fixup: mm/gup: remove record_subpages() > pages is not adjusted by the caller, but idnexed by existing *nr. > Signed-off-by: David Hildenbrand > > diff --git a/mm/gup.c b/mm/gup.c > index 010fe56f6e132..22420f2069ee1 100644 > --- a/mm/gup.c > +++ b/mm/gup.c > @@ -2981,6 +2981,7 @@ static int gup_fast_pmd_leaf(pmd_t orig, pmd_t *pmdp, unsigned long addr, > return 0; > } > > + pages += *nr; > *nr += refs; > for (; refs; refs--) > *(pages++) = page++; > @@ -3024,6 +3025,7 @@ static int gup_fast_pud_leaf(pud_t orig, pud_t *pudp, unsigned long addr, > return 0; > } > > + pages += *nr; > *nr += refs; > for (; refs; refs--) > *(pages++) = page++; > Tested as fixing the issue for me, thanks. -- Jens Axboe