From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f175.google.com (mail-pl1-f175.google.com [209.85.214.175]) (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 0B375B673 for ; Tue, 14 Jan 2025 06:01:09 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.175 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736834471; cv=none; b=jQyTGFCWX6YXhzawX8x5o6LDVbrZ+syvsuz061OWhzwENQ5pMzzbXbkns+3bCAd/tFgD7dUyyq4yeY92oLmS5twmANkXRUpWg+Q1Zszzg7vwqYgz/dMp7E6Jef5+CFi19ZEgoZkn0lm670wPrGDCUWEV5drdFpSmHiFnwoWzaaU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736834471; c=relaxed/simple; bh=bDzMi862KqE6AiaD0Ofm3/CvA6dmp3pFtk0BMwFuxq8=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version:Content-Type; b=ZgaU9jOR6hrpn2j7rdmToQtY/RhsfDeP1CaTrleKHupLh/taWekaCTuSRHWwHxsZmjpFMtFSUdQZs3AZ7TyKxhCNJClt0y8qB0wqVsRdcIn2qGRGTXyqKl3UEKmQvpVfHXR8AfHIORxbvIXr/azBYdj084156prrF6dqXl1Cf94= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=Ys1x8DtP; arc=none smtp.client-ip=209.85.214.175 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="Ys1x8DtP" Received: by mail-pl1-f175.google.com with SMTP id d9443c01a7336-2165cb60719so90865505ad.0 for ; Mon, 13 Jan 2025 22:01:09 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1736834469; x=1737439269; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=P1D6gYEoM2XnA7p/mZdXiYm6Q1A06yMI0y9WsArZB/I=; b=Ys1x8DtPBZLd/sc/u7t+p81/+xg5oi561OWxvqgZ9KnP9VBHxMbKXndzz1Gi0/ealR cJxIZv+LqrgUxq5weeU+sZh5zOphJVQdMT8mB1TIc4i9I0JtpBwavuAug0HXw52Rk5xD JU8XnKNZe51vNuVmo6fIs10MgWfFZxZW52QtKfY+kuZ9g1LH4+9yebl+SKHKhHfAebEG LxfISTQZoKgdhZibYvKME2QB9exJTURAAWgXTCZ5Xgd9zO0X6seRed8zK9F4hAwjK4fo Qvcwt38DKd4qMah7oX8VBaqx3paTzVgtVxhrLyjfWhvNd74o+byAIa0pAb2QXyT1ZApk Kpkw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736834469; x=1737439269; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=P1D6gYEoM2XnA7p/mZdXiYm6Q1A06yMI0y9WsArZB/I=; b=CM2g4tskjo8k1ljGf/6Is93jWPXZo+dfTzO64PG6gxyPY+/MzjVmACKjqXQHqjcBLD 5A2mQ+AApK4PEPDfkfW5PWEupGOV5LDAQBDBOy8Dp1rZBMyJCDqTZznIZaqiHZCGwIJU 07yIaTcm6/Wov8A4Xs9MrhwldjuT9oqOIJOTIjUvkwmrph/h8TG8i3S3KF7MWWcTIpVm DOyld5zWTnUq/kqnwsH0ajhy9/2Q2llL7X3vONC7d1kddWk1KqPdKNyxGKQo769fFkYZ QvQ+Udpd6xJlgmRNb1vqwSD8fIevngH0tfU71S9lLIi5K94apsEpFs/aYXwbUBk/Cre+ geBw== X-Forwarded-Encrypted: i=1; AJvYcCVE+tlnJFGdUCtrX/xpUSEeviEKqp++gixfVNGWn7jQ9txLNsx6cP2RKEOygdM149e83ERpcaZgcv4wdKQ=@vger.kernel.org X-Gm-Message-State: AOJu0YzvBXc+xOlDdoi5f6AxuA1KZGOG5fKaIHi9GfJ07kHLltMNTHvm fv+tDl0yeruuJakMT71H7SSTkOefwa4Az397DLxK5J7tRLx7iZ+/ X-Gm-Gg: ASbGnctSXmB+lvwqxQIgTUrWVqy4oHQWyydb8WUSG38jvzegSQogRQtMxd9bopTBEBL midV1eGunxwF3+RopkEyiDfDpbFDK40WH8NMMOr+Edh1oPTRshMUYF+a5K8yC8r86cfdpwEsmtC u/I7Li2ESBBSYgUn9fgac7SoWdnJ/IPHpDSv61uCkpx2KUruSCoKBQnSXu+cSaA5/45JRaBKHeF huyUkyKDrkv+EH+E9YLwYRVp44c+8rTv0r2F51M761zbg9LdHOFKg9y47W22IQ+HJve5zCDmovm 1FEDCUUZ X-Google-Smtp-Source: AGHT+IEDHRmGK1uA7EjxC0SWT2vjWc7ySfaqXzxlqfNbOvSfJNPnY2oCVxBO6d5hzH1KmB/6COb9uA== X-Received: by 2002:a17:903:41c5:b0:215:19ae:77bf with SMTP id d9443c01a7336-21a83f4ea67mr365135525ad.19.1736834469125; Mon, 13 Jan 2025 22:01:09 -0800 (PST) Received: from Barrys-MBP.hub ([2407:7000:af65:8200:39b5:3f0b:acf3:9158]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-2f54a26acadsm11013436a91.3.2025.01.13.22.01.03 (version=TLS1_3 cipher=TLS_CHACHA20_POLY1305_SHA256 bits=256/256); Mon, 13 Jan 2025 22:01:08 -0800 (PST) From: Barry Song <21cnbao@gmail.com> To: baolin.wang@linux.alibaba.com Cc: 21cnbao@gmail.com, akpm@linux-foundation.org, chrisl@kernel.org, david@redhat.com, ioworker0@gmail.com, kasong@tencent.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-riscv@lists.infradead.org, lorenzo.stoakes@oracle.com, ryan.roberts@arm.com, v-songbaohua@oppo.com, x86@kernel.org, ying.huang@intel.com, zhengtangquan@oppo.com Subject: Re: [PATCH v2 4/4] mm: Avoid splitting pmd for lazyfree pmd-mapped THP in try_to_unmap Date: Tue, 14 Jan 2025 19:00:59 +1300 Message-Id: <20250114060059.14058-1-21cnbao@gmail.com> X-Mailer: git-send-email 2.39.3 (Apple Git-146) In-Reply-To: <20250114040914.9986-1-21cnbao@gmail.com> References: <20250114040914.9986-1-21cnbao@gmail.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit > > >               if (!pvmw.pte) { > > > +                     lazyfree = folio_test_anon(folio) && !folio_test_swapbacked(folio); > > > > You've checked lazyfree here, so can we remove the duplicate check in > > unmap_huge_pmd_locked()? Then the code should be: > > > >                 if (lazyfree && unmap_huge_pmd_locked(...)) > >                         goto walk_done; > > > right. it seems unmap_huge_pmd_locked() only handles lazyfree pmd-mapped > thp. so i guess the code could be: > > diff --git a/mm/huge_memory.c b/mm/huge_memory.c > index aea49f7125f1..c4c3a7896de4 100644 > --- a/mm/huge_memory.c > +++ b/mm/huge_memory.c > @@ -3131,11 +3131,10 @@ bool unmap_huge_pmd_locked(struct vm_area_struct *vma, unsigned long addr, >         VM_WARN_ON_FOLIO(!folio_test_pmd_mappable(folio), folio); >         VM_WARN_ON_FOLIO(!folio_test_locked(folio), folio); >         VM_WARN_ON_ONCE(!IS_ALIGNED(addr, HPAGE_PMD_SIZE)); > +       VM_WARN_ON_FOLIO(!folio_test_anon(folio), folio); > +       VM_WARN_ON_FOLIO(folio_test_swapbacked(folio), folio); > > -       if (folio_test_anon(folio) && !folio_test_swapbacked(folio)) > -               return __discard_anon_folio_pmd_locked(vma, addr, pmdp, folio); > - > -       return false; > +       return __discard_anon_folio_pmd_locked(vma, addr, pmdp, folio); >  } > >  static void remap_page(struct folio *folio, unsigned long nr, int flags) > diff --git a/mm/rmap.c b/mm/rmap.c > index 02c4e4b2cd7b..72907eb1b8fe 100644 > --- a/mm/rmap.c > +++ b/mm/rmap.c > @@ -1671,7 +1671,7 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma, >         DEFINE_FOLIO_VMA_WALK(pvmw, folio, vma, address, 0); >         pte_t pteval; >         struct page *subpage; > -       bool anon_exclusive, lazyfree, ret = true; > +       bool anon_exclusive, ret = true; >         struct mmu_notifier_range range; >         enum ttu_flags flags = (enum ttu_flags)(long)arg; >         int nr_pages = 1; > @@ -1724,18 +1724,16 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma, >                 } > >                 if (!pvmw.pte) { > -                       lazyfree = folio_test_anon(folio) && !folio_test_swapbacked(folio); > - > -                       if (unmap_huge_pmd_locked(vma, pvmw.address, pvmw.pmd, > -                                                 folio)) > -                               goto walk_done; > -                       /* > -                        * unmap_huge_pmd_locked has either already marked > -                        * the folio as swap-backed or decided to retain it > -                        * due to GUP or speculative references. > -                        */ > -                       if (lazyfree) > +                       if (folio_test_anon(folio) && !folio_test_swapbacked(folio)) { > +                               if (unmap_huge_pmd_locked(vma, pvmw.address, pvmw.pmd, folio)) > +                                       goto walk_done; > +                               /* > +                                * unmap_huge_pmd_locked has either already marked > +                                * the folio as swap-backed or decided to retain it > +                                * due to GUP or speculative references. > +                                */ >                                 goto walk_abort; > +                       } > >                         if (flags & TTU_SPLIT_HUGE_PMD) { >                                 /* > > > > > >                       if (unmap_huge_pmd_locked(vma, pvmw.address, pvmw.pmd, > > >                                                 folio)) > > >                               goto walk_done; > > > +                     /* > > > +                      * unmap_huge_pmd_locked has either already marked > > > +                      * the folio as swap-backed or decided to retain it > > > +                      * due to GUP or speculative references. > > > +                      */ > > > +                     if (lazyfree) > > > +                             goto walk_abort; > > > > > >                       if (flags & TTU_SPLIT_HUGE_PMD) { > > >                               /* The final diff is as follows. Baolin, do you have any additional comments before I send out v3? diff --git a/mm/huge_memory.c b/mm/huge_memory.c index 3d3ebdc002d5..47cc8c3f8f80 100644 --- a/mm/huge_memory.c +++ b/mm/huge_memory.c @@ -3070,8 +3070,12 @@ static bool __discard_anon_folio_pmd_locked(struct vm_area_struct *vma, int ref_count, map_count; pmd_t orig_pmd = *pmdp; - if (folio_test_dirty(folio) || pmd_dirty(orig_pmd)) + if (pmd_dirty(orig_pmd)) + folio_set_dirty(folio); + if (folio_test_dirty(folio) && !(vma->vm_flags & VM_DROPPABLE)) { + folio_set_swapbacked(folio); return false; + } orig_pmd = pmdp_huge_clear_flush(vma, addr, pmdp); @@ -3098,8 +3102,15 @@ static bool __discard_anon_folio_pmd_locked(struct vm_area_struct *vma, * * The only folio refs must be one from isolation plus the rmap(s). */ - if (folio_test_dirty(folio) || pmd_dirty(orig_pmd) || - ref_count != map_count + 1) { + if (pmd_dirty(orig_pmd)) + folio_set_dirty(folio); + if (folio_test_dirty(folio) && !(vma->vm_flags & VM_DROPPABLE)) { + folio_set_swapbacked(folio); + set_pmd_at(mm, addr, pmdp, orig_pmd); + return false; + } + + if (ref_count != map_count + 1) { set_pmd_at(mm, addr, pmdp, orig_pmd); return false; } @@ -3119,12 +3130,11 @@ bool unmap_huge_pmd_locked(struct vm_area_struct *vma, unsigned long addr, { VM_WARN_ON_FOLIO(!folio_test_pmd_mappable(folio), folio); VM_WARN_ON_FOLIO(!folio_test_locked(folio), folio); + VM_WARN_ON_FOLIO(!folio_test_anon(folio), folio); + VM_WARN_ON_FOLIO(folio_test_swapbacked(folio), folio); VM_WARN_ON_ONCE(!IS_ALIGNED(addr, HPAGE_PMD_SIZE)); - if (folio_test_anon(folio) && !folio_test_swapbacked(folio)) - return __discard_anon_folio_pmd_locked(vma, addr, pmdp, folio); - - return false; + return __discard_anon_folio_pmd_locked(vma, addr, pmdp, folio); } static void remap_page(struct folio *folio, unsigned long nr, int flags) diff --git a/mm/rmap.c b/mm/rmap.c index 3ef659310797..72907eb1b8fe 100644 --- a/mm/rmap.c +++ b/mm/rmap.c @@ -1724,9 +1724,16 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma, } if (!pvmw.pte) { - if (unmap_huge_pmd_locked(vma, pvmw.address, pvmw.pmd, - folio)) - goto walk_done; + if (folio_test_anon(folio) && !folio_test_swapbacked(folio)) { + if (unmap_huge_pmd_locked(vma, pvmw.address, pvmw.pmd, folio)) + goto walk_done; + /* + * unmap_huge_pmd_locked has either already marked + * the folio as swap-backed or decided to retain it + * due to GUP or speculative references. + */ + goto walk_abort; + } if (flags & TTU_SPLIT_HUGE_PMD) { /* -- 2.39.3 (Apple Git-146)