From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from invmail4.hynix.com (exvmail4.skhynix.com [166.125.252.92]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 541662361C9 for ; Wed, 15 Jan 2025 10:23:21 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=166.125.252.92 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736936609; cv=none; b=dpGJyE1Qq1MeLXVxiedUE2fHHPfuiWsvZwrr40jg1UcsR6AV+9WEFtSxf/EnMIyuCQPKnoNGEv8mC5KRIMli2Egzf62AtYNs/uhrcjMCeQVU7wSULjBgpJHtyZf7jq2KpsxE9mgxemik8z5KmA2Ptw0sEofE4RAzXsBwDtnw7Dk= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736936609; c=relaxed/simple; bh=QNKz8hwKzvbT5OM5B/u8VwtXZo6bO8OjVLvhejhjBnc=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=N0VAONwuwbMwJINnE5MDJCIBvbS12Xoe9TO1uZnrTV0CZb3RCGGBObR/Jc8SafslTfVeKFtIfnQ6aK2GbZL+mv06G4wOUFCmzibnCFIqYk4IocQuaeTxJmINOvrJgkKiq9GCS4K/ak9TpMFRDfilc81XO/Cm6oXX7En0Q4281w0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=sk.com; spf=pass smtp.mailfrom=sk.com; arc=none smtp.client-ip=166.125.252.92 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=sk.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=sk.com X-AuditID: a67dfc5b-3e1ff7000001d7ae-ce-67878c98f706 Date: Wed, 15 Jan 2025 19:23:15 +0900 From: Byungchul Park To: Shivank Garg Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, kernel_team@skhynix.com, akpm@linux-foundation.org, ziy@nvidia.com Subject: Re: [PATCH] mm: separate move/undo parts from migrate_pages_batch() Message-ID: <20250115102315.GA4090@system.software.com> References: <20250115095343.46390-1-byungchul@sk.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=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.9.4 (2018-02-28) X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrFLMWRmVeSWpSXmKPExsXC9ZZnoe6MnvZ0g84V5hZz1q9hs7i8aw6b xb01/1ktzq35zG4x++g9dgdWj9ZLf9k8Nn2axO5xYsZvFo/e5ndsHp83yQWwRnHZpKTmZJal FunbJXBl3L3/ibXgqV7FpOsf2RsYXyp3MXJwSAiYSHQvCepi5AQz215cYAcJswioSryfbAkS ZhNQl7hx4ycziC0ioCzROGMPC4jNLFAlsXr9TFYQW1jAR2LO901MIDavgLnErROPwWwhgTSJ jZdWMEPEBSVOznwC1aslcePfSyaQVcwC0hLL/3GAhDkFrCX+fT4BViIKtOrAtuNAJVxAl81g k9j0eD0zxJmSEgdX3GCZwCgwC8nYWUjGzkIYu4CReRWjUGZeWW5iZo6JXkZlXmaFXnJ+7iZG YAgvq/0TvYPx04XgQ4wCHIxKPLwX4tvShVgTy4orcw8xSnAwK4nwLmFrTRfiTUmsrEotyo8v Ks1JLT7EKM3BoiTOa/StPEVIID2xJDU7NbUgtQgmy8TBKdXAmB7090dI93mJpGPXqiz2Jexp PCop3XApojLKtqfy0cPHTikHDOrV9iy1PnX0Q/KLv2UJD63U/H7cvrLC9p3uyj/qPl6X3i6u i/vueUqoZA7rQqm6g3YuO3nkYvcI1un7aujMtkoIffPBpfubMk+e47GqE2KdPJ/C386eL80s Wrt2Y22u0F9WJZbijERDLeai4kQA03AIxV0CAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrELMWRmVeSWpSXmKPExsXC5WfdrDujpz3doPu7usWc9WvYLA7PPclq cXnXHDaLe2v+s1qcW/OZ3WL20XvsDmwerZf+snls+jSJ3ePEjN8sHr3N79g8Fr/4wOTxeZNc AFsUl01Kak5mWWqRvl0CV8bd+59YC57qVUy6/pG9gfGlchcjJ4eEgIlE24sL7F2MHBwsAqoS 7ydbgoTZBNQlbtz4yQxiiwgoSzTO2MMCYjMLVEmsXj+TFcQWFvCRmPN9ExOIzStgLnHrxGMw W0ggTWLjpRXMEHFBiZMzn0D1aknc+PeSCWQVs4C0xPJ/HCBhTgFriX+fT4CViAKtOrDtONME Rt5ZSLpnIemehdC9gJF5FaNIZl5ZbmJmjqlecXZGZV5mhV5yfu4mRmBILqv9M3EH45fL7ocY BTgYlXh4T0S0pQuxJpYVV+YeYpTgYFYS4V3C1pouxJuSWFmVWpQfX1Sak1p8iFGag0VJnNcr PDVBSCA9sSQ1OzW1ILUIJsvEwSnVwMjqfs9O3L3wgt6kl+Ez1k89KVcU73Xb5Ei628c1Mulm SdNE4iNY4q8mL5aUVfrcxlf987/TkXptPUYNXfc7d87/M/lwx9nkl35OuNqncoGJSQtki1iu 7O6ddXM++8lj8wPe3vbQW8Jeyv42yTCn13hnn21YuP7mzFu1FeZWQcs5GizeHwmsVGIpzkg0 1GIuKk4EAPfnddNFAgAA X-CFilter-Loop: Reflected On Wed, Jan 15, 2025 at 03:37:15PM +0530, Shivank Garg wrote: > Hi Byungchul, > > On 1/15/2025 3:23 PM, Byungchul Park wrote: > > Hi, > > > > This is a part of luf(lazy unmap flush) patchset and also referred > > several times by e.g. Shivank Garg and Zi Yan who are currently working > > on page migration optimization. Why don't we take this first so that > > such a migration optimization tries can use it. > > > > Byungchul > > > > --->8--- > > From a65a6e4975962707bf87171e317f005c6276887e Mon Sep 17 00:00:00 2001 > > From: Byungchul Park > > Date: Thu, 8 Aug 2024 15:53:58 +0900 > > Subject: [PATCH] mm: separate move/undo parts from migrate_pages_batch() > > > > Functionally, no change. This is a preparation for migration > > optimization tries that require to use separated folio lists for its own > > handling during migration. Refactored migrate_pages_batch() so as to > > separate move/undo parts from migrate_pages_batch(). > > > > Signed-off-by: Byungchul Park > > --- > > mm/migrate.c | 134 +++++++++++++++++++++++++++++++-------------------- > > 1 file changed, 83 insertions(+), 51 deletions(-) > > [...] > > > + */ > > + switch(rc) { > ^^^ > minor nit here - space after 'switch' before the parentheses. > This code style issue was present in the original code and was carried > over during refactoring. I should've taken more care of it. Thank you. Byungchul > With this fix, please add- > Reviewed-by: Shivank Garg > > > + case -EAGAIN: > > + *retry += 1; > > + *thp_retry += is_thp; > > + *nr_retry_pages += nr_pages; > > + break; > > + case MIGRATEPAGE_SUCCESS: > > + stats->nr_succeeded += nr_pages; > > + stats->nr_thp_succeeded += is_thp; > > + break; > > + default: > > + *nr_failed += 1; > > + stats->nr_thp_failed += is_thp; > > + stats->nr_failed_pages += nr_pages; > > + break; > > + } > > + dst = dst2; > > + dst2 = list_next_entry(dst, lru); > > + } > > +} > > + > > +static void migrate_folios_undo(struct list_head *src_folios, > > + struct list_head *dst_folios, > > + free_folio_t put_new_folio, unsigned long private, > > + struct list_head *ret_folios) > > +{ > > + struct folio *folio, *folio2, *dst, *dst2; > > + > > + dst = list_first_entry(dst_folios, struct folio, lru); > > + dst2 = list_next_entry(dst, lru); > > + list_for_each_entry_safe(folio, folio2, src_folios, lru) { > > + int old_page_state = 0; > > + struct anon_vma *anon_vma = NULL; > > + > > + __migrate_folio_extract(dst, &old_page_state, &anon_vma); > > + migrate_folio_undo_src(folio, old_page_state & PAGE_WAS_MAPPED, > > + anon_vma, true, ret_folios); > > + list_del(&dst->lru); > > + migrate_folio_undo_dst(dst, true, put_new_folio, private); > > + dst = dst2; > > + dst2 = list_next_entry(dst, lru); > > + } > > +} > > + > > /* > > * migrate_pages_batch() first unmaps folios in the from list as many as > > * possible, then move the unmapped folios. > > @@ -1717,7 +1792,7 @@ static int migrate_pages_batch(struct list_head *from, > > int pass = 0; > > bool is_thp = false; > > bool is_large = false; > > - struct folio *folio, *folio2, *dst = NULL, *dst2; > > + struct folio *folio, *folio2, *dst = NULL; > > int rc, rc_saved = 0, nr_pages; > > LIST_HEAD(unmap_folios); > > LIST_HEAD(dst_folios); > > @@ -1888,42 +1963,11 @@ static int migrate_pages_batch(struct list_head *from, > > thp_retry = 0; > > nr_retry_pages = 0; > > > > - dst = list_first_entry(&dst_folios, struct folio, lru); > > - dst2 = list_next_entry(dst, lru); > > - list_for_each_entry_safe(folio, folio2, &unmap_folios, lru) { > > - is_thp = folio_test_large(folio) && folio_test_pmd_mappable(folio); > > - nr_pages = folio_nr_pages(folio); > > - > > - cond_resched(); > > - > > - rc = migrate_folio_move(put_new_folio, private, > > - folio, dst, mode, > > - reason, ret_folios); > > - /* > > - * The rules are: > > - * Success: folio will be freed > > - * -EAGAIN: stay on the unmap_folios list > > - * Other errno: put on ret_folios list > > - */ > > - switch(rc) { > > - case -EAGAIN: > > - retry++; > > - thp_retry += is_thp; > > - nr_retry_pages += nr_pages; > > - break; > > - case MIGRATEPAGE_SUCCESS: > > - stats->nr_succeeded += nr_pages; > > - stats->nr_thp_succeeded += is_thp; > > - break; > > - default: > > - nr_failed++; > > - stats->nr_thp_failed += is_thp; > > - stats->nr_failed_pages += nr_pages; > > - break; > > - } > > - dst = dst2; > > - dst2 = list_next_entry(dst, lru); > > - } > > + /* Move the unmapped folios */ > > + migrate_folios_move(&unmap_folios, &dst_folios, > > + put_new_folio, private, mode, reason, > > + ret_folios, stats, &retry, &thp_retry, > > + &nr_failed, &nr_retry_pages); > > } > > nr_failed += retry; > > stats->nr_thp_failed += thp_retry; > > @@ -1932,20 +1976,8 @@ static int migrate_pages_batch(struct list_head *from, > > rc = rc_saved ? : nr_failed; > > out: > > /* Cleanup remaining folios */ > > - dst = list_first_entry(&dst_folios, struct folio, lru); > > - dst2 = list_next_entry(dst, lru); > > - list_for_each_entry_safe(folio, folio2, &unmap_folios, lru) { > > - int old_page_state = 0; > > - struct anon_vma *anon_vma = NULL; > > - > > - __migrate_folio_extract(dst, &old_page_state, &anon_vma); > > - migrate_folio_undo_src(folio, old_page_state & PAGE_WAS_MAPPED, > > - anon_vma, true, ret_folios); > > - list_del(&dst->lru); > > - migrate_folio_undo_dst(dst, true, put_new_folio, private); > > - dst = dst2; > > - dst2 = list_next_entry(dst, lru); > > - } > > + migrate_folios_undo(&unmap_folios, &dst_folios, > > + put_new_folio, private, ret_folios); > > > > return rc; > > }