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 EBB18C32771 for ; Mon, 26 Sep 2022 18:06:55 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3B1358E0076; Mon, 26 Sep 2022 14:06:55 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 35F698E0066; Mon, 26 Sep 2022 14:06:55 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1DA6D8E0076; Mon, 26 Sep 2022 14:06:55 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 0E85A8E0066 for ; Mon, 26 Sep 2022 14:06:55 -0400 (EDT) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id D65221C5C14 for ; Mon, 26 Sep 2022 18:06:54 +0000 (UTC) X-FDA: 79955017548.28.26C010C Received: from mail-pj1-f53.google.com (mail-pj1-f53.google.com [209.85.216.53]) by imf31.hostedemail.com (Postfix) with ESMTP id 9002820002 for ; Mon, 26 Sep 2022 18:06:54 +0000 (UTC) Received: by mail-pj1-f53.google.com with SMTP id p1-20020a17090a2d8100b0020040a3f75eso7680987pjd.4 for ; Mon, 26 Sep 2022 11:06:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date; bh=uKupFgjivPCVWYH7YpnPdpMxsUheL0Hiz5eQQ4/hkjA=; b=c35pan48SDotyfrbvsIlNtjdZWZ7qLKSNIpOasMZwUDnsmINpbIzpuk9DguJWu30uw FGWkZ3hOBslZc9JxxhYY543StabsnFaDe0BH87TdFh8rbob/cJ9+lp2LS1EqqIKWlgID wAG6+jyb2dgQMDgEm+ggbpk1phX/M1G82j4e1Ggk4/NetOayNz6XfBGwJ8ao7j1OpaZE 8ccp3VwHakbLJHEuMQk+ZO0dNY0pkS//YARMEpfg4NgdEWEw7s11HMrKiuWxseJjfVpo GaP6/USmHGWBGNsTuMd7825pPTc/ADYI3mmcirQUXB1eSLFhiXX7hX45xHztWFHELPb0 zQBQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date; bh=uKupFgjivPCVWYH7YpnPdpMxsUheL0Hiz5eQQ4/hkjA=; b=bESOSSZ00wLeDkvWxv8kM/19Hkvb5BzZz2DFlzpahqxXkxkIWcvUvfuEmUPHhkFURY prxRPWlND7YTj9D/l1DqRp4lYoEuarJd8UO1KowX5J+/7bfz3u5jYWKq9i+b8YoG3u7r wUKBdrSHb/Uy+eRes0wavn7Ei1/Bmb/xcEgUNdSr+PXzamdW4FRseFpV9KgDeOAp4BiO Iz0deaAboSOYperk+VPu2Ba2VF7EamM8k3zMZ/ohYzpmgh7+Kc2HyDXN8TSTnzg+hxZU BJ40ES2A/wJEhmE3IBTHwNnTd1MchzKMfoMQmx4DznBT+OrTtaswSH7tegbZuVmUh8Uz wZuw== X-Gm-Message-State: ACrzQf1E2WwnhBCtEZ4TYdTHnrbKs1fHAwh8/eJ5C0J0MjJ8DTU14z24 WHzUlPNeUpYcZhHLf36NDYM8KVKbwm6z87SSD7yySP+o X-Google-Smtp-Source: AMsMyM51ScmXFvsbluB/k0FvYYHesp+KgMnMHzKA6R6rwPkiymnOPuzVdaWHCxM/Q7O9vPk84Scvi+Hyb9uMiBfownw= X-Received: by 2002:a17:902:d508:b0:178:b7b1:beb3 with SMTP id b8-20020a170902d50800b00178b7b1beb3mr23611672plg.102.1664215613527; Mon, 26 Sep 2022 11:06:53 -0700 (PDT) MIME-Version: 1.0 References: <20220921060616.73086-1-ying.huang@intel.com> <20220921060616.73086-3-ying.huang@intel.com> <87o7v2lbn4.fsf@nvdebian.thelocal> In-Reply-To: <87o7v2lbn4.fsf@nvdebian.thelocal> From: Yang Shi Date: Mon, 26 Sep 2022 11:06:40 -0700 Message-ID: Subject: Re: [RFC 2/6] mm/migrate_pages: split unmap_and_move() to _unmap() and _move() To: Alistair Popple Cc: Huang Ying , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrew Morton , Zi Yan , Baolin Wang , Oscar Salvador , Matthew Wilcox Content-Type: text/plain; charset="UTF-8" ARC-Authentication-Results: i=1; imf31.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=c35pan48; spf=pass (imf31.hostedemail.com: domain of shy828301@gmail.com designates 209.85.216.53 as permitted sender) smtp.mailfrom=shy828301@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1664215614; a=rsa-sha256; cv=none; b=aEbvog+jc8TrsNMtr4B7hvFT+qPuNLAQpr+ZVOW/HJvrkmaAq6SduOhVcYB3ncGLhBXwJA Q3SGdl5kal36HQuVhdjP/EBgVpYUCZTZtyPhBXBwl9K3oUDoY1qRpPPonBZazIFL6IPNDG da6pELzrQXkJXrTOFSNJraNFvWZS4Fk= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1664215614; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=uKupFgjivPCVWYH7YpnPdpMxsUheL0Hiz5eQQ4/hkjA=; b=vaOF+XtOHeORS3iYGQTFauuwGL8oVd9uByAo/f38Pqj5nSy3FHZjwUGhT/BfuAhV5MHoWV x+mEdJrLazs7u3DyBHWB3kS8tbqovVNw7uJon47RY59bLmGzlBOXLCs2HzL8tkua1qdtBb zj/aMR1hI9QStWunegjn9jwn7evp64Q= X-Rspamd-Server: rspam11 X-Rspamd-Queue-Id: 9002820002 X-Rspam-User: Authentication-Results: imf31.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=c35pan48; spf=pass (imf31.hostedemail.com: domain of shy828301@gmail.com designates 209.85.216.53 as permitted sender) smtp.mailfrom=shy828301@gmail.com; dmarc=pass (policy=none) header.from=gmail.com X-Stat-Signature: fdj44ah9f4zbs7hum6yi1gxwr1ccfjn8 X-HE-Tag: 1664215614-300374 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Mon, Sep 26, 2022 at 2:37 AM Alistair Popple wrote: > > > Huang Ying writes: > > > This is a preparation patch to batch the page unmapping and moving > > for the normal pages and THP. > > > > In this patch, unmap_and_move() is split to migrate_page_unmap() and > > migrate_page_move(). So, we can batch _unmap() and _move() in > > different loops later. To pass some information between unmap and > > move, the original unused newpage->mapping and newpage->private are > > used. > > This looks like it could cause a deadlock between two threads migrating > the same pages if force == true && mode != MIGRATE_ASYNC as > migrate_page_unmap() will call lock_page() while holding the lock on > other pages in the list. Therefore the two threads could deadlock if the > pages are in a different order. It seems unlikely to me since the page has to be isolated from lru before migration. The isolating from lru is atomic, so the two threads unlikely see the same pages on both lists. But there might be other cases which may incur deadlock, for example, filesystem writeback IIUC. Some filesystems may lock a bunch of pages then write them back in a batch. The same pages may be on the migration list and they are also dirty and seen by writeback. I'm not sure whether I miss something that could prevent such a deadlock from happening. > > > Signed-off-by: "Huang, Ying" > > Cc: Zi Yan > > Cc: Yang Shi > > Cc: Baolin Wang > > Cc: Oscar Salvador > > Cc: Matthew Wilcox > > --- > > mm/migrate.c | 164 ++++++++++++++++++++++++++++++++++++++------------- > > 1 file changed, 122 insertions(+), 42 deletions(-) > > > > diff --git a/mm/migrate.c b/mm/migrate.c > > index 117134f1c6dc..4a81e0bfdbcd 100644 > > --- a/mm/migrate.c > > +++ b/mm/migrate.c > > @@ -976,13 +976,32 @@ static int move_to_new_folio(struct folio *dst, struct folio *src, > > return rc; > > } > > > > -static int __unmap_and_move(struct page *page, struct page *newpage, > > +static void __migrate_page_record(struct page *newpage, > > + int page_was_mapped, > > + struct anon_vma *anon_vma) > > +{ > > + newpage->mapping = (struct address_space *)anon_vma; > > + newpage->private = page_was_mapped; > > +} > > + > > +static void __migrate_page_extract(struct page *newpage, > > + int *page_was_mappedp, > > + struct anon_vma **anon_vmap) > > +{ > > + *anon_vmap = (struct anon_vma *)newpage->mapping; > > + *page_was_mappedp = newpage->private; > > + newpage->mapping = NULL; > > + newpage->private = 0; > > +} > > + > > +#define MIGRATEPAGE_UNMAP 1 > > + > > +static int __migrate_page_unmap(struct page *page, struct page *newpage, > > int force, enum migrate_mode mode) > > { > > struct folio *folio = page_folio(page); > > - struct folio *dst = page_folio(newpage); > > int rc = -EAGAIN; > > - bool page_was_mapped = false; > > + int page_was_mapped = 0; > > struct anon_vma *anon_vma = NULL; > > bool is_lru = !__PageMovable(page); > > > > @@ -1058,8 +1077,8 @@ static int __unmap_and_move(struct page *page, struct page *newpage, > > goto out_unlock; > > > > if (unlikely(!is_lru)) { > > - rc = move_to_new_folio(dst, folio, mode); > > - goto out_unlock_both; > > + __migrate_page_record(newpage, page_was_mapped, anon_vma); > > + return MIGRATEPAGE_UNMAP; > > } > > > > /* > > @@ -1085,11 +1104,41 @@ static int __unmap_and_move(struct page *page, struct page *newpage, > > VM_BUG_ON_PAGE(PageAnon(page) && !PageKsm(page) && !anon_vma, > > page); > > try_to_migrate(folio, 0); > > - page_was_mapped = true; > > + page_was_mapped = 1; > > + } > > + > > + if (!page_mapped(page)) { > > + __migrate_page_record(newpage, page_was_mapped, anon_vma); > > + return MIGRATEPAGE_UNMAP; > > } > > > > - if (!page_mapped(page)) > > - rc = move_to_new_folio(dst, folio, mode); > > + if (page_was_mapped) > > + remove_migration_ptes(folio, folio, false); > > + > > +out_unlock_both: > > + unlock_page(newpage); > > +out_unlock: > > + /* Drop an anon_vma reference if we took one */ > > + if (anon_vma) > > + put_anon_vma(anon_vma); > > + unlock_page(page); > > +out: > > + > > + return rc; > > +} > > + > > +static int __migrate_page_move(struct page *page, struct page *newpage, > > + enum migrate_mode mode) > > +{ > > + struct folio *folio = page_folio(page); > > + struct folio *dst = page_folio(newpage); > > + int rc; > > + int page_was_mapped = 0; > > + struct anon_vma *anon_vma = NULL; > > + > > + __migrate_page_extract(newpage, &page_was_mapped, &anon_vma); > > + > > + rc = move_to_new_folio(dst, folio, mode); > > > > /* > > * When successful, push newpage to LRU immediately: so that if it > > @@ -1110,14 +1159,11 @@ static int __unmap_and_move(struct page *page, struct page *newpage, > > remove_migration_ptes(folio, > > rc == MIGRATEPAGE_SUCCESS ? dst : folio, false); > > > > -out_unlock_both: > > unlock_page(newpage); > > -out_unlock: > > /* Drop an anon_vma reference if we took one */ > > if (anon_vma) > > put_anon_vma(anon_vma); > > unlock_page(page); > > -out: > > /* > > * If migration is successful, decrease refcount of the newpage, > > * which will not free the page because new page owner increased > > @@ -1129,18 +1175,31 @@ static int __unmap_and_move(struct page *page, struct page *newpage, > > return rc; > > } > > > > -/* > > - * Obtain the lock on page, remove all ptes and migrate the page > > - * to the newly allocated page in newpage. > > - */ > > -static int unmap_and_move(new_page_t get_new_page, > > - free_page_t put_new_page, > > - unsigned long private, struct page *page, > > - int force, enum migrate_mode mode, > > - enum migrate_reason reason, > > - struct list_head *ret) > > +static void migrate_page_done(struct page *page, > > + enum migrate_reason reason) > > +{ > > + /* > > + * Compaction can migrate also non-LRU pages which are > > + * not accounted to NR_ISOLATED_*. They can be recognized > > + * as __PageMovable > > + */ > > + if (likely(!__PageMovable(page))) > > + mod_node_page_state(page_pgdat(page), NR_ISOLATED_ANON + > > + page_is_file_lru(page), -thp_nr_pages(page)); > > + > > + if (reason != MR_MEMORY_FAILURE) > > + /* We release the page in page_handle_poison. */ > > + put_page(page); > > +} > > + > > +/* Obtain the lock on page, remove all ptes. */ > > +static int migrate_page_unmap(new_page_t get_new_page, free_page_t put_new_page, > > + unsigned long private, struct page *page, > > + struct page **newpagep, int force, > > + enum migrate_mode mode, enum migrate_reason reason, > > + struct list_head *ret) > > { > > - int rc = MIGRATEPAGE_SUCCESS; > > + int rc = MIGRATEPAGE_UNMAP; > > struct page *newpage = NULL; > > > > if (!thp_migration_supported() && PageTransHuge(page)) > > @@ -1151,19 +1210,48 @@ static int unmap_and_move(new_page_t get_new_page, > > ClearPageActive(page); > > ClearPageUnevictable(page); > > /* free_pages_prepare() will clear PG_isolated. */ > > - goto out; > > + list_del(&page->lru); > > + migrate_page_done(page, reason); > > + return MIGRATEPAGE_SUCCESS; > > } > > > > newpage = get_new_page(page, private); > > if (!newpage) > > return -ENOMEM; > > + *newpagep = newpage; > > > > - newpage->private = 0; > > - rc = __unmap_and_move(page, newpage, force, mode); > > + rc = __migrate_page_unmap(page, newpage, force, mode); > > + if (rc == MIGRATEPAGE_UNMAP) > > + return rc; > > + > > + /* > > + * A page that has not been migrated will have kept its > > + * references and be restored. > > + */ > > + /* restore the page to right list. */ > > + if (rc != -EAGAIN) > > + list_move_tail(&page->lru, ret); > > + > > + if (put_new_page) > > + put_new_page(newpage, private); > > + else > > + put_page(newpage); > > + > > + return rc; > > +} > > + > > +/* Migrate the page to the newly allocated page in newpage. */ > > +static int migrate_page_move(free_page_t put_new_page, unsigned long private, > > + struct page *page, struct page *newpage, > > + enum migrate_mode mode, enum migrate_reason reason, > > + struct list_head *ret) > > +{ > > + int rc; > > + > > + rc = __migrate_page_move(page, newpage, mode); > > if (rc == MIGRATEPAGE_SUCCESS) > > set_page_owner_migrate_reason(newpage, reason); > > > > -out: > > if (rc != -EAGAIN) { > > /* > > * A page that has been migrated has all references > > @@ -1179,20 +1267,7 @@ static int unmap_and_move(new_page_t get_new_page, > > * we want to retry. > > */ > > if (rc == MIGRATEPAGE_SUCCESS) { > > - /* > > - * Compaction can migrate also non-LRU pages which are > > - * not accounted to NR_ISOLATED_*. They can be recognized > > - * as __PageMovable > > - */ > > - if (likely(!__PageMovable(page))) > > - mod_node_page_state(page_pgdat(page), NR_ISOLATED_ANON + > > - page_is_file_lru(page), -thp_nr_pages(page)); > > - > > - if (reason != MR_MEMORY_FAILURE) > > - /* > > - * We release the page in page_handle_poison. > > - */ > > - put_page(page); > > + migrate_page_done(page, reason); > > } else { > > if (rc != -EAGAIN) > > list_add_tail(&page->lru, ret); > > @@ -1405,6 +1480,7 @@ int migrate_pages(struct list_head *from, new_page_t get_new_page, > > int pass = 0; > > bool is_thp = false; > > struct page *page; > > + struct page *newpage = NULL; > > struct page *page2; > > int rc, nr_subpages; > > LIST_HEAD(ret_pages); > > @@ -1493,9 +1569,13 @@ int migrate_pages(struct list_head *from, new_page_t get_new_page, > > if (PageHuge(page)) > > continue; > > > > - rc = unmap_and_move(get_new_page, put_new_page, > > - private, page, pass > 2, mode, > > + rc = migrate_page_unmap(get_new_page, put_new_page, private, > > + page, &newpage, pass > 2, mode, > > reason, &ret_pages); > > + if (rc == MIGRATEPAGE_UNMAP) > > + rc = migrate_page_move(put_new_page, private, > > + page, newpage, mode, > > + reason, &ret_pages); > > /* > > * The rules are: > > * Success: page will be freed