From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1C7AEC54798 for ; Tue, 27 Feb 2024 08:23:52 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 60292440205; Tue, 27 Feb 2024 03:23:51 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 5B1D14401FB; Tue, 27 Feb 2024 03:23:51 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4793A440205; Tue, 27 Feb 2024 03:23:51 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 393004401FB for ; Tue, 27 Feb 2024 03:23:51 -0500 (EST) Received: from smtpin09.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 062931A05CF for ; Tue, 27 Feb 2024 08:23:50 +0000 (UTC) X-FDA: 81836895462.09.BD149E4 Received: from mail-yw1-f181.google.com (mail-yw1-f181.google.com [209.85.128.181]) by imf13.hostedemail.com (Postfix) with ESMTP id 3C29020005 for ; Tue, 27 Feb 2024 08:23:49 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=C9h2ruPQ; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf13.hostedemail.com: domain of ioworker0@gmail.com designates 209.85.128.181 as permitted sender) smtp.mailfrom=ioworker0@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1709022229; a=rsa-sha256; cv=none; b=cbNw6mmOsQXNxeY77lF73tiNLb6a1/kUryccds1h6jlKHCeQwabC8rK1Dlq3KDK0SJZu5p 1zAJjJz+hkvJLa820YHtbgkrNSpXtb8IRJh6zpqTqT7UtadIz31eVsmx2CjPMsytwKLm4g mJIOaBRYO+m+WzeMc3ZCsWujv48t940= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=C9h2ruPQ; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf13.hostedemail.com: domain of ioworker0@gmail.com designates 209.85.128.181 as permitted sender) smtp.mailfrom=ioworker0@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1709022229; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=6B8RP6a8YyuQVMWpLc3aNKulJBA75T/Z2hdZPd6pvYc=; b=y86+nyyaOYFLO6OE9ui+tyt7oqBzh4QPYdPiU/vRn42hk/tgsE7Ze9tuisGSzylYSv7JqO B/4/ooo+acOUmQFnXZGaPczbt0A4fN3hG4ccIFMP2/ykbSxvt2hk+q0nGEVul/M6jEZr7q KU9Ts+1cckbCGK7SY4/lzDb82CT/lv0= Received: by mail-yw1-f181.google.com with SMTP id 00721157ae682-60908e5fb9eso17619437b3.2 for ; Tue, 27 Feb 2024 00:23:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1709022228; x=1709627028; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=6B8RP6a8YyuQVMWpLc3aNKulJBA75T/Z2hdZPd6pvYc=; b=C9h2ruPQuMFmYyo85kFBitvfutdqUHfU5m1NHVC+bSciLBiQibFaI0W36b+q7T+WG+ iVL4CPKramE4jRR4348kJ5IaDVekBJEfKio5QGb1vgUqxPOVDS00sGz0gYZ1WbQqcjsg MX41NqRbjQFtbSV6Qc3s63Unbl4G38ifGgwBKAiKiYU3YSgWkVXOJ3+MtnYnMCKz9x6b 531Ano9A0R7HrVtVX+99aRLMjshpJM7AUdTJqy7/FGoaM+8xmiT1jXnLOiT2U6rOFwmf cWUUbfnw3jJnh5iMceXW5p5xwH2mfyiVah4Ryf6Nqi6tfpJnYQ7m2wrow17DGvL+YU87 t7cg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709022228; x=1709627028; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=6B8RP6a8YyuQVMWpLc3aNKulJBA75T/Z2hdZPd6pvYc=; b=ah++cw5cw/GeQumE1+6XUmZqJQT37eTY7gaVtsXsdPmautwUo/NJvSuv+GPxGXr3no 5yIqTfSKz/zzQj+Q1cZDpG2LEErwgTSFrbWnvOH9+0RLD0zAhf0FD9WokLXZe3BRHCME 3Wqrh6QUTXgu6d8+ksy6maXUdoTLobMZh+3miE2VXNRE6yu47bpbDhvubJ/9Y8zMcV0b p1ZGXSOb7dUqhg/ZjcFgFiQVDkQisvIpeOoOjjKXW2ZbEJAarhB2ONpVbW8JiSkP7OXt 5y7YYB0N6q+83k8SNtJ0drGVJqJk/MgqB/3r6DiE7ILnTFu6cmsQ/S5Z1+xRHH7ViHb8 JQ9w== X-Forwarded-Encrypted: i=1; AJvYcCXIsh2HF3c6qQzyXAt7+vinVNilLH6zX+pZ1KV0YK6WN00KGdGvn5SVZHKwo8eKb2oWnJRsvTAzPVImPMKK1pQj64U= X-Gm-Message-State: AOJu0YxHlZi9Ncvp1kcE5Xe1wiwPktqeI+uOsXZDyZ+G+yFQrbs2tLeV BQMrAq0JRtzRhn67eAfn4Ln0F8tg6cuJPFPcKnzCKBd7VI47MgTp5F/XfLMrP4cObh7L8C3sZ/W 1nNU38NKWTl7jTmHbKzUv2RgkAMo= X-Google-Smtp-Source: AGHT+IEjnTYxfirewIQ1lX56xzcoZLgRN10ewnue+kjoxb7KArMYmYjXx7GE732BsCGyKbWCp/wFaYPAj3bW4A8q/no= X-Received: by 2002:a0d:d68c:0:b0:607:6fe9:f55a with SMTP id y134-20020a0dd68c000000b006076fe9f55amr1451239ywd.35.1709022228290; Tue, 27 Feb 2024 00:23:48 -0800 (PST) MIME-Version: 1.0 References: <20240227070418.62292-1-ioworker0@gmail.com> <90471b2f-826e-4275-a9a3-ec673c3e6af8@redhat.com> In-Reply-To: <90471b2f-826e-4275-a9a3-ec673c3e6af8@redhat.com> From: Lance Yang Date: Tue, 27 Feb 2024 16:23:36 +0800 Message-ID: Subject: Re: [PATCH 1/1] mm/memory: Fix boundary check for next PFN in folio_pte_batch() To: David Hildenbrand Cc: akpm@linux-foundation.org, ryan.roberts@arm.com, 21cnbao@gmail.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 3C29020005 X-Stat-Signature: aztebkurni9ou7zdy1311wzswtrbi5wb X-HE-Tag: 1709022229-598824 X-HE-Meta: U2FsdGVkX1+lTmAy6DBHacOyfxDNb5lA5AQL1oWtoqIsJ/qYUIU0J0GoAVWoI7WOrTRKgz0+byPzpk4ppKZnDErgyQggFRWsune+fP8P8oE+gJL3/0ey+C/3V2tYCybEYkuHI08776H4fX5v7x5/HHZbZHT3y7d3a+U0L6bQgWqBtJUBqIfF7GTgWOQdNjhK5eP7dXP87nUJdhGcrwO95nQqPLq5vLJud2R1mt5u8srVe43GeLWCV22lk9Zu6kHOBWqJmT64+hASs5O43yDptMPYJ9y3fcN61y9am9okXBGHeXC3UL2+37FOxGZZx7Tzgxh0C2lrZkHzohl8Fe4dWtaHz6VfTtG+MNb2bSfQ1TO/FK/BiypuQBhjqFPgteXd8ctn0twDRHH7ItAk+9OIjaZIFL0uamclp1kGXNHeGjwQhwQMApL4aaEYOAXARk3zHb+lR/Bo/Y3uuK/naIh3Gf8heYm0xC9vqtm7egkDBZO+I7EDsE2Z/5sFJUuy8T48prhf2z+lxf6CiH39E6QRU/RKJyPT02hrRjn+AKDfU4cL0s+oLOwUyVIoNSYFn+Z+6/9I7Bir/nhK8oDujIKHBabcY75pA9kmEc3ztr2ZKNtQ4w6b3hnKxUZAcoq14lWd3EUmWArcUWD+eiut1ZxrT1kpl6Ev0il7NCxretRRRe98wrDgZLMgBDRILqQPMqTCAdwV600jzgjiKMeKjtb5fKfP4h1t1pjWIMZJUccp3GQdSadqqTKId8KxubiuJ/y4XjgfwMLWtXbskrzoiyLcX5Yz82YVeGBLXryVk+boK7FY+x/xdcFR2BO4ikEtQjsPSZqc5HWH14r/6FyDxBa+mrRLBLTLup/ndrOt7mxDT/iZeWOebnkyByMe7Ns3XsAHFxwKDhB/WJSuHQ71k6D3EpC4ANKD+Y3QckYLDojmA03+1MTQIAB3kIkiR+6D0MJeBlcNT5svYhPLb0HCJVg Om6U+U23 AxoKGiC74jfAOzTKUYFTqmwfVQepaG20LZ3hpXAc6ZGLxsGbXTbXqerMfjTvVbh2rlJSc1P69d4rLZduoHbpTWlKfDV0d2Gb5tLoDtEcSUmWSIEnOGHLoW7FSln7aeJy+KKkOS4gWNUS+tK8U65msE9xbh+srXtAhldb07TVXbmClstAq4GLzvTN/MeQurQaenf5PJ05r1oISHlxQTs1Isp76DQvJEpnTnfx1E2edKqQjZtAoz8z35RRPPfzlN6s5qjRtQecdYlVpw3fpypX2lIgg9s0PBEzF/kx75JoSFhSyAqTN21G4nraSEXhqMgzhN/fRd7FUWce5xZ23oCALUvqZcLgE6viX3BwyzlS3Uk0C7ThmRBTxi5RGSQkw4SesSA06foR2fIK0vzwCYJkj7Nnw+AuiR2uRkUhtkUy34e7m5Io8MN9bOaCp/CncMmDXYrlP X-Bogosity: Ham, tests=bogofilter, spamicity=0.000108, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: Hey David, Thanks for taking time to review! On Tue, Feb 27, 2024 at 3:30=E2=80=AFPM David Hildenbrand wrote: > > On 27.02.24 08:04, Lance Yang wrote: > > Previously, in folio_pte_batch(), only the upper boundary of the > > folio was checked using '>=3D' for comparison. This led to > > incorrect behavior when the next PFN exceeded the lower boundary > > of the folio, especially in corner cases where the next PFN might > > fall into a different folio. > > Which commit does this fix? > > The introducing commit (f8d937761d65c87e9987b88ea7beb7bddc333a0e) is > already in mm-stable, so we would need a Fixes: tag. Unless, Ryan's > changes introduced a problem. > > BUT > > I don't see what is broken. :) > > Can you please give an example/reproducer? For example1: PTE0 is present for large folio1. PTE1 is present for large folio1. PTE2 is present for large folio1. PTE3 is present for large folio1. folio_nr_pages(folio1) is 4. folio_nr_pages(folio2) is 4. pte =3D *start_ptep =3D PTE0; max_nr =3D folio_nr_pages(folio2); If folio_pfn(folio1) < folio_pfn(folio2), the return value of folio_pte_batch(folio2, start_ptep, pte, max_nr) will be 4(Actually it should be 0). For example2: PTE0 is present for large folio2. PTE1 is present for large folio1. PTE2 is present for large folio1. PTE3 is present for large folio1. folio_nr_pages(folio1) is 4. folio_nr_pages(folio2) is 4. pte =3D *start_ptep =3D PTE0; max_nr =3D folio_nr_pages(folio1); If max_nr=3D4, the return value of folio_pte_batch(folio1, start_ptep, pte, max_nr) will be 1(Actually it should be 0). folio_pte_batch() will soon be exported, and IMO, these corner cases may ne= ed to be handled. Thanks, Lance > > We know that the first PTE maps the folio. By incrementing the PFN using > pte_next_pfn/pte_advance_pfn, we cannot suddenly get a lower PFN. > > So how would pte_advance_pfn(folio_start_pfn + X) suddenly give us a PFN > lower than folio_start_pfn? > > Note that we are not really concerned about any kind of > pte_advance_pfn() overflow that could generate PFN=3D0. I convinces mysel= f > that that that is something we don't have to worry about. > > > [I also thought about getting rid of the pte_pfn(pte) >=3D folio_end_pfn > and instead limiting end_ptep. But that requires more work before the > loop and feels more like a micro-optimization.] > > > > > Signed-off-by: Lance Yang > > --- > > mm/memory.c | 7 +++++-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/mm/memory.c b/mm/memory.c > > index 642b4f2be523..e5291d1e8c37 100644 > > --- a/mm/memory.c > > +++ b/mm/memory.c > > @@ -986,12 +986,15 @@ static inline int folio_pte_batch(struct folio *f= olio, unsigned long addr, > > pte_t *start_ptep, pte_t pte, int max_nr, fpb_t flags, > > bool *any_writable) > > { > > - unsigned long folio_end_pfn =3D folio_pfn(folio) + folio_nr_pages= (folio); > > + unsigned long folio_start_pfn, folio_end_pfn; > > const pte_t *end_ptep =3D start_ptep + max_nr; > > pte_t expected_pte, *ptep; > > bool writable; > > int nr; > > > > + folio_start_pfn =3D folio_pfn(folio); > > + folio_end_pfn =3D folio_start_pfn + folio_nr_pages(folio); > > + > > if (any_writable) > > *any_writable =3D false; > > > > @@ -1015,7 +1018,7 @@ static inline int folio_pte_batch(struct folio *f= olio, unsigned long addr, > > * corner cases the next PFN might fall into a different > > * folio. > > */ > > - if (pte_pfn(pte) >=3D folio_end_pfn) > > + if (pte_pfn(pte) >=3D folio_end_pfn || pte_pfn(pte) < fol= io_start_pfn) > > break; > > > > if (any_writable) > > -- > Cheers, > > David / dhildenb >