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 50AFFC52D7B for ; Tue, 13 Aug 2024 05:01:12 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C5EA06B00A1; Tue, 13 Aug 2024 01:01:11 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C0E8E6B00A2; Tue, 13 Aug 2024 01:01:11 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AFD0F6B00A3; Tue, 13 Aug 2024 01:01:11 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 91DE86B00A1 for ; Tue, 13 Aug 2024 01:01:11 -0400 (EDT) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 395DC12060C for ; Tue, 13 Aug 2024 05:01:11 +0000 (UTC) X-FDA: 82446023142.14.76A1BA4 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf17.hostedemail.com (Postfix) with ESMTP id 642274001B for ; Tue, 13 Aug 2024 05:01:08 +0000 (UTC) Authentication-Results: imf17.hostedemail.com; dkim=none; spf=pass (imf17.hostedemail.com: domain of dev.jain@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=dev.jain@arm.com; dmarc=pass (policy=none) header.from=arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1723525233; a=rsa-sha256; cv=none; b=1DA0zFR2Flj9nqiPliQZYDsLzmMSTUOTAz4nhRSOXqAH3b22st5OhJaQWa/wRevY5GCaY4 NtXzOvKp48arl3A9F2YZ5ATQCqRZbb9D7Zx0TsG+MLfPxrQSQbBCraIsd5Hyta1NSWcmpU xPeJ5XELcwqLp3G5SBhwOt/vvo8zsKU= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=none; spf=pass (imf17.hostedemail.com: domain of dev.jain@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=dev.jain@arm.com; dmarc=pass (policy=none) header.from=arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1723525233; 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; bh=xJH2xVk4tfBUB2V4lgP4caF6VIBkpK68fjinBk0aNzY=; b=IPtXvKnV8wTcLuYvUYLL3WsSfWGNPT97WGf7SPcdJ/yQ8nXzF838ZR6njGDtJydaetXjcW gIKn3Byz6Os3yTcPhO8UK7MP3cnqqpzHM9cGPGZtmFdJpsDyrTcswlBX1/gx+DFDVKQ4Ru /1zDq60yetnrSa/bSwo0WR4Lhgy9z8I= Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id C98B4FEC; Mon, 12 Aug 2024 22:01:32 -0700 (PDT) Received: from [10.162.42.21] (e116581.arm.com [10.162.42.21]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 389E43F40C; Mon, 12 Aug 2024 22:00:56 -0700 (PDT) Message-ID: Date: Tue, 13 Aug 2024 10:30:50 +0530 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 1/2] mm: Retry migration earlier upon refcount mismatch From: Dev Jain To: "Huang, Ying" Cc: akpm@linux-foundation.org, shuah@kernel.org, david@redhat.com, willy@infradead.org, ryan.roberts@arm.com, anshuman.khandual@arm.com, catalin.marinas@arm.com, cl@gentwo.org, vbabka@suse.cz, mhocko@suse.com, apopple@nvidia.com, osalvador@suse.de, baolin.wang@linux.alibaba.com, dave.hansen@linux.intel.com, will@kernel.org, baohua@kernel.org, ioworker0@gmail.com, gshan@redhat.com, mark.rutland@arm.com, kirill.shutemov@linux.intel.com, hughd@google.com, aneesh.kumar@kernel.org, yang@os.amperecomputing.com, peterx@redhat.com, broonie@kernel.org, mgorman@techsingularity.net, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-kselftest@vger.kernel.org References: <20240809103129.365029-1-dev.jain@arm.com> <20240809103129.365029-2-dev.jain@arm.com> <87frrauwwv.fsf@yhuang6-desk2.ccr.corp.intel.com> <15dbe4ac-a036-4029-ba08-e12a236f448a@arm.com> <87bk1yuuzu.fsf@yhuang6-desk2.ccr.corp.intel.com> <95b72817-5444-4ced-998a-1cb90f42bf49@arm.com> <8734naurhm.fsf@yhuang6-desk2.ccr.corp.intel.com> <9d84e4e8-ac54-4eb1-a113-3f32aea864c9@arm.com> Content-Language: en-US In-Reply-To: <9d84e4e8-ac54-4eb1-a113-3f32aea864c9@arm.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Stat-Signature: msepxeud43x95pt95zu9abfzofm5sk9s X-Rspamd-Queue-Id: 642274001B X-Rspam-User: X-Rspamd-Server: rspam10 X-HE-Tag: 1723525268-939881 X-HE-Meta: U2FsdGVkX1+EDfY4ItJW13vaTBmB/P+dZ/QBTNf49Z2e0q4QgefqFmbiTDTXdtG29rl0f/7XfgqQBpj/PmDiuBWnzh/FhfyBZTYGgf2JH4R5KiA0efyRi6YAS6t+ejgy54Hs1TclYlrrNDjIKRE2oJkHh4Xm/j0l9Qsy5Vj3Kqz+tHk306HfcwGMAVlhJLo9B6OG/sAMVBl4FtD7+Ff3tatTIQMte8MnyNvM6WDJvE6QxU/qczC6ouVpusYpmsriwWvNgalU+qPLI+mPJRwUkD5qd2jLo5PZU7lqszT8t3UGyFHS7fXbn9QedWcJ+iCkgJ14rG7sdT897ox5eaG8dIJ9tQ2PPfjEugzlVMzsmQz2+XcLjkzK2zUDQlwUr/AYn6c7EVcmehOJYtIXysKtoO70i75ya8jfXKbuVKlWR2gk+MMyJIS7SNGVXnCEXpYNIFco/Ak9+42MSi+VpGO8a4tRPke2+97voB7Y5cOEsPshAEG6pTlNrR7O+RQ7GqkzyeoDOGr4HRJgTbloMN/0lrJj1DwtF82neDooitaE4GinHTIoD7DXISgyPeoSD78s3qmGQPbA+0F2RAWVEgRMmPyLMejjMK3JRYhN6HeEOiVztzuNwPCb6LubGitKi3fVLxFnBKH3u+TR26wfh8l94Un9KQLmfQh1/Y1DmbqRhuprnVeEx9lGxtBmcMdMhOueDMHXC9wZiDCvtPr3O1of3ytcDZcA1Diuzk/0LN2Nsh2AYjXL6X3tNBxgLKKMsnw7gtQdpONVMdlvEI6ehEN4PK/NEFYe1GBv0zFeeCWNGaxT/PJdEt0Z7Gmh08+ewRZY9Gro91nkLOjw3/VURHpNnqGqg5hTjR0VZ3U7Qdk2fpsLNmqIWXDTRDNjDz6u21bsqE/P13HO8m5hUT8krXp60VxXiLv4MV7IQO6jDiWgjGqutQUXyIKmJUkbZgIGUoLFZNzKqUILOl0u7EgeH5M 8XOOigki tLRwO+lX5YzgbLMa6OrkYLNoM0i3WAcOPQ3aolpHiL1tamEtCgayVMT/1lRrYAFC4hbocsEkzoLfnsf0mHxgIX8iOzElBimtbPVriuWrmIvP0G5HrljBUrkm1Qecoqf/Ul5aWirLOx2XJZ6Jm/nZbVbFX+qOD2odWFOtSQ+/es7GJinGgSBmoVu9mu/vM9r8Gz1Nh1AkKNpEZT5noj55ezu6P7SEi00/pc+1PdgRb5m+M64s4D9BjaPUCzxIIiDkLSk60uyYF7U+vqdSmj62LNe/zwhQPHyuh23zz0VEtfhyk13QQnFWu0Pe1ts/qmxT5CE0lkFD1uerTEDc90AxEHfwZyZQgKIrfA+QOq1XbS6AHE/NElfsinXehq7NVbgOtbFBX4yLC9D0EF4n3AJ6nw7VciA== 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: List-Subscribe: List-Unsubscribe: On 8/12/24 17:38, Dev Jain wrote: > > On 8/12/24 13:01, Huang, Ying wrote: >> Dev Jain writes: >> >>> On 8/12/24 11:45, Huang, Ying wrote: >>>> Dev Jain writes: >>>> >>>>> On 8/12/24 11:04, Huang, Ying wrote: >>>>>> Hi, Dev, >>>>>> >>>>>> Dev Jain writes: >>>>>> >>>>>>> As already being done in __migrate_folio(), wherein we backoff >>>>>>> if the >>>>>>> folio refcount is wrong, make this check during the unmapping >>>>>>> phase, upon >>>>>>> the failure of which, the original state of the PTEs will be >>>>>>> restored and >>>>>>> the folio lock will be dropped via migrate_folio_undo_src(), any >>>>>>> racing >>>>>>> thread will make progress and migration will be retried. >>>>>>> >>>>>>> Signed-off-by: Dev Jain >>>>>>> --- >>>>>>>     mm/migrate.c | 9 +++++++++ >>>>>>>     1 file changed, 9 insertions(+) >>>>>>> >>>>>>> diff --git a/mm/migrate.c b/mm/migrate.c >>>>>>> index e7296c0fb5d5..477acf996951 100644 >>>>>>> --- a/mm/migrate.c >>>>>>> +++ b/mm/migrate.c >>>>>>> @@ -1250,6 +1250,15 @@ static int >>>>>>> migrate_folio_unmap(new_folio_t get_new_folio, >>>>>>>         } >>>>>>>           if (!folio_mapped(src)) { >>>>>>> +        /* >>>>>>> +         * Someone may have changed the refcount and maybe >>>>>>> sleeping >>>>>>> +         * on the folio lock. In case of refcount mismatch, >>>>>>> bail out, >>>>>>> +         * let the system make progress and retry. >>>>>>> +         */ >>>>>>> +        struct address_space *mapping = folio_mapping(src); >>>>>>> + >>>>>>> +        if (folio_ref_count(src) != >>>>>>> folio_expected_refs(mapping, src)) >>>>>>> +            goto out; >>>>>>>             __migrate_folio_record(dst, old_page_state, anon_vma); >>>>>>>             return MIGRATEPAGE_UNMAP; >>>>>>>         } >>>>>> Do you have some test results for this?  For example, after >>>>>> applying the >>>>>> patch, the migration success rate increased XX%, etc. >>>>> I'll get back to you on this. >>>>> >>>>>> My understanding for this issue is that the migration success >>>>>> rate can >>>>>> increase if we undo all changes before retrying.  This is the >>>>>> current >>>>>> behavior for sync migration, but not for async migration.  If so, >>>>>> we can >>>>>> use migrate_pages_sync() for async migration too to increase success >>>>>> rate?  Of course, we need to change the function name and comments. >>>>> As per my understanding, this is not the current behaviour for sync >>>>> migration. After successful unmapping, we fail in >>>>> migrate_folio_move() >>>>> with -EAGAIN, we do not call undo src+dst (rendering the loop around >>>>> migrate_folio_move() futile), we do not push the failed folio onto >>>>> the >>>>> ret_folios list, therefore, in _sync(), _batch() is never tried >>>>> again. >>>> In migrate_pages_sync(), migrate_pages_batch(,MIGRATE_ASYNC) will be >>>> called first, if failed, the folio will be restored to the original >>>> state (unlocked).  Then migrate_pages_batch(,_SYNC*) is called again. >>>> So, we unlock once.  If it's necessary, we can unlock more times via >>>> another level of loop. >>> Yes, that's my point. We need to undo src+dst and retry. >> For sync migration, we undo src+dst and retry now, but only once.  You >> have shown that more retrying increases success rate. >> >>> We will have >>> to decide where we want this retrying to be; do we want to change the >>> return value, end up in the while loop wrapped around _sync(), and >>> retry >>> there by adding another level of loop, or do we want to make use of the >>> existing retry loops, one of which is wrapped around _unmap(); the >>> latter >>> is my approach. The utility I see for the former approach is that, >>> in case >>> of a large number of page migrations (which should usually be the >>> case), >>> we are giving more time for the folio to get retried. The latter >>> does not >>> give much time and discards the folio if it did not succeed under 7 >>> times. >> Because it's a race, I guess that most folios will be migrated >> successfully in the first pass. >> >> My concerns of your method are that it deal with just one case >> specially.  While retrying after undoing all appears more general. > > > Makes sense. Also, please ignore my "change the return value" > thing, I got confused between unmap_folios, ret_folios, etc. > Now I think I understood what the lists are doing :) > >> >> If it's really important to retry after undoing all, we can either >> convert two retying loops of migrate_pages_batch() into one loop, or >> remove retry loop in migrate_pages_batch() and retry in its caller >> instead. > > And if I implemented this correctly, the following makes the test > pass always: > https://www.codedump.xyz/diff/Zrn7EdxzNXmXyNXe Okay, I did mess up with the implementation, leading to a false positive. Let me try again :) > > > >> >> -- >> Best Regards, >> Huang, Ying >