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 07243C7618E for ; Wed, 26 Apr 2023 20:47:21 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3CB166B012B; Wed, 26 Apr 2023 16:47:21 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 37B776B012C; Wed, 26 Apr 2023 16:47:21 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1CEBE6B012D; Wed, 26 Apr 2023 16:47:21 -0400 (EDT) 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 095666B012B for ; Wed, 26 Apr 2023 16:47:21 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id C1232803FA for ; Wed, 26 Apr 2023 20:47:20 +0000 (UTC) X-FDA: 80724727440.24.2455742 Received: from mail-io1-f48.google.com (mail-io1-f48.google.com [209.85.166.48]) by imf23.hostedemail.com (Postfix) with ESMTP id B869814001D for ; Wed, 26 Apr 2023 20:47:18 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=koJsYM57; spf=pass (imf23.hostedemail.com: domain of dianders@chromium.org designates 209.85.166.48 as permitted sender) smtp.mailfrom=dianders@chromium.org; dmarc=pass (policy=none) header.from=chromium.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1682542038; 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=qieP3ULrKIRmkqJSGHHaaVXipgOCBnCPp/roS3/SzsQ=; b=SZ0s8TbVEKsflFfo6wlVeFd62I/N87vrroGwzuvxrLG/yDpuKQHKOGAhF0VbLufhp9b6MK vUbFKQWE3Yb0BVN+QeCQwEv7U2c7XHWGhdLb4MIImXJFFMccXadYGRTdzou/AFQqE5PWmu PvzlcnIXi28/pOx6f2sUkauz+j9LtEU= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=koJsYM57; spf=pass (imf23.hostedemail.com: domain of dianders@chromium.org designates 209.85.166.48 as permitted sender) smtp.mailfrom=dianders@chromium.org; dmarc=pass (policy=none) header.from=chromium.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1682542038; a=rsa-sha256; cv=none; b=0Z5jr954SbIIJu+txzOkjD1rauD/9NXFINJMNerwN/38Y5+fMDowksfA0XOeW/uTgak2ZJ RopQ1XqfGJZ0XQG0sspDGZ8+TdSnCfLf1j0V22P8iL3Ywi0BdhRiJqdNl32HMSXTrHjyoM cdSbmf4a3uus4pr8+JHYD8nPJt2tQ+Y= Received: by mail-io1-f48.google.com with SMTP id ca18e2360f4ac-76655852953so21693039f.0 for ; Wed, 26 Apr 2023 13:47:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1682542036; x=1685134036; 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=qieP3ULrKIRmkqJSGHHaaVXipgOCBnCPp/roS3/SzsQ=; b=koJsYM57b8enPH1rMWWeP8K7efF/SZ/aBEp22WGtyB1+jaBDWbSU+5XegVQAyFaQmA eskg+TgJLSU1mGh/83zAWLxx+kSP/ifI7EkkiCB7CViZSMnneFI8pJxigrLsQASvlQAF SHWBgS/XxcJzT/5Et3Bg9RcIP+AxvsksGr2Wo= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682542036; x=1685134036; 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=qieP3ULrKIRmkqJSGHHaaVXipgOCBnCPp/roS3/SzsQ=; b=cfNQIRH9sUr6bNxMd1fYPTQC6UWDp+/QWntbe4ZjqWLMZguwLpXAgoGNaFiTK0kZs6 /nONV++PoekMTq+kdyJKPj8Ib0jwBNx45TwjDqo8lGa2fing+VaFmjW7dKpxc/jYZwm9 v9C+q+OK1ptfCTD00QYFVCLMQ9MP9MmxmGJAr43eFua9rNQDfeo4wCBj7W2yHfeGPj2d CV1VBO43ZwAlSFLFasNMWfN8NkULw+Q5pX7QVJnd2Rm0Y32vgNlduVeM37k+Yj2HotWn xLcA10CpD6ZyE3rMW5dnKu75DkLwf3k+tWqgQSwL4even0QlXYK0TTL8W2HP+glissqi TaqA== X-Gm-Message-State: AAQBX9eZfuRPZP0UVp2gEN/3DmEolN1Q6dctJdWV77m14KH1LS48IUai wS/YfKa/ovMB0ZcJTfxxm4dHsVZ05nBP1caXOkw= X-Google-Smtp-Source: AKy350bDowZanm8zL3xIYfkeW8jn9injuoqwz9gbwtD7OMNHYtCPA/goCVW6ccR75fB94QVAmtEGYg== X-Received: by 2002:a5e:df45:0:b0:763:bb4d:a415 with SMTP id g5-20020a5edf45000000b00763bb4da415mr10637649ioq.18.1682542035727; Wed, 26 Apr 2023 13:47:15 -0700 (PDT) Received: from mail-il1-f181.google.com (mail-il1-f181.google.com. [209.85.166.181]) by smtp.gmail.com with ESMTPSA id m2-20020a056638260200b00411930674c4sm4480963jat.162.2023.04.26.13.47.14 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 26 Apr 2023 13:47:14 -0700 (PDT) Received: by mail-il1-f181.google.com with SMTP id e9e14a558f8ab-329577952c5so325135ab.1 for ; Wed, 26 Apr 2023 13:47:14 -0700 (PDT) X-Received: by 2002:a05:6e02:1d94:b0:329:3f69:539e with SMTP id h20-20020a056e021d9400b003293f69539emr77336ila.2.1682542033757; Wed, 26 Apr 2023 13:47:13 -0700 (PDT) MIME-Version: 1.0 References: <20230421221249.1616168-1-dianders@chromium.org> <20230421151135.v2.1.I2b71e11264c5c214bc59744b9e13e4c353bc5714@changeid> <20230422051858.1696-1-hdanton@sina.com> <20230425010917.1984-1-hdanton@sina.com> <20230426100918.ku32k6mqoogsnijn@techsingularity.net> In-Reply-To: From: Doug Anderson Date: Wed, 26 Apr 2023 13:46:58 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v2 1/4] mm/filemap: Add folio_lock_timeout() To: Matthew Wilcox Cc: Mel Gorman , Hillf Danton , Andrew Morton , Alexander Viro , Christian Brauner , Linus Torvalds , linux-kernel@vger.kernel.org, linux-mm@kvack.org, Yu Zhao Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam03 X-Stat-Signature: x8u46tw75qnnry7m8gp5544yuiizc9ko X-Rspamd-Queue-Id: B869814001D X-HE-Tag: 1682542038-951222 X-HE-Meta: U2FsdGVkX19C/zmShs7X5VHRTnQWFb76ze6Vp3fU+5vmZo6bq3JLEnBFXBQ52F+wGz50JVjQOSuujiMfYCPvZ/dFE/rMBZnYmB3WPdZDwWzPGjb2BUAexQupw93eYPUItisJxhmJEqRoEoKPv7Iw7acuSD7GQ/F76sxkHqk4UXGSRqq6NwN0qLV6DWbfD2NT4dCmmhq6DzJJV5O8UnI1EeFgEIWDSzyVOO36EqCeqbQ56zDd+zOccD7tyGfiEspSae1JKEy6+iuo4WZUgrkDD+BZQd30er2ckiQxdRePsLEVEDTNs8rGrFzNIKi3Zb5ucBxRXVW7BoQ5MZF4NcFbhY9VliEvbQWgnBp21Lve6NzHUiytKbqIg9zSx7uJtQBIskA5/Ck+eB7LCDvn1T6r6Jqjj3n+w2H9H+PtbS7jRMg1F/TG9DY7KBUCn08NSZDEt6urJwwxk0YEj6iLjMv/NrKcmq66zIdk86efqwPAou47tsflzuFhS3UjAfJhQqULM31T9LNhmhjQ902hXnVCakR5scslvPR+px6os+CRn7TMaCL0xzqx8OIRjJhnfF3SLo1xCvPNDi7X5DCR+ZwnVG2Pc2x1BM74gkZ5LmpI0kKSV5+HpNO/0BkjlwXAb94QVHPURNWBmPUQGZ+QX9x6bCsoZ7GSC50IgXPxEVU0HVa+dudViswQ0hJDk166vFJvEtTea8UYLYhgTPOe2yP/NMdF2+3W4fZ7xZpYgOq9oJNV8aO5lR+2wjSonekWbK0oW5n/hiEPglC0djcsVgzm+qDH8F+sB81CBGXolcPsPipzib/yJcdz9ebQOiWOEjDxqrgSO1RaJlxPXddm3MfLW5ytxAHTIn427PpdbTb0cFboLD9ga/HcdHCEj+/Di+Y6roKe6aSpr2toYkEnrCMDh8iLFmXuERU0EksVq74UbwvrEHb754DaBwJvB3e6xGjXa3o8UVMKzaT2ZKGgxOZ vkNG5RQu EtVCL6P62+zW5Qt8gblUUxqa3I3iynUrWo7lfuNxPuobP5Gd57vzJJ/6a7CAzPkaNeMEUPucoBl7d0/lQA8mQUBL8fOqfKuzIvXVPR7wcYiIvS1sXBlYq37DynThk8tP/nUjmM+LAcRrPHFuQOad2gn4ZsybDyDyFHJqp0TvMTBAqsUOup+i+ReOu73ehJbrEWzvxzoQKThKaY26u/zrRmM8I0b9KrpuxZmO6AH9cSb2Kdbb7ULdhvCdBIBGPDrq/VRDKqsQ5Agmy/FOvVYghKwXVHTpEOawlXIRYL9OxGLXiSpcq4Gi3EmRLcsl0uZvIUAwC4mlEmiNHO5Z6vl/Xs69r1U+g50d0v8+UoSiKlkS96hbxv8+hOHSc/W6o2J0098nFv/eq6QkLL9yppnIBMcH6eTpucqhQbpZImXYY7jdmTDhuow4NBemMePPh0yAEMS4IHas+pmHJd3ELpfmjQoFvEg== 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: Hi, On Wed, Apr 26, 2023 at 8:14=E2=80=AFAM Matthew Wilcox wrote: > > I'm not generally a fan of lock-with-timeout approaches. I think the > rationale for this one makes sense, but we're going to see some people > try to use this for situations where it doesn't make sense. Although it won't completely prevent the issue, I could add a comment to the function (and the similar lock_buffer_timeout() added in patch #2 [1] at least warning people that it's discouraged to use the function without very careful consideration. [1] https://lore.kernel.org/r/20230421151135.v2.2.Ie146eec4d41480ebeb15f0cf= dfb3bc9095e4ebd9@changeid/ > I almost > wonder if we shouldn't spin rather than sleep on this lock, since the > window of time we're willing to wait is so short. It doesn't feel like spinning is the right move in this particular case. While we want to enable kcompactd to move forward, it's not so urgent that it needs to take up lots of CPU cycles doing so. ...and, in fact, the cases I've seen us be blocked is when we're under memory pressure and spinning would be counterproductive to getting out of that pressure. > I'm certainly not > willing to NAK this patch since it's clearly fixing a real problem. > > Hm. If the problem is that we want to wait for the lock unless the > lock is being held for I/O, we can actually tell that in the caller. > > if (folio_test_uptodate(folio)) > folio_lock(folio); > else > folio_trylock(folio); > > (the folio lock isn't held for writeback, just taken and released; > if the folio is uptodate, the folio lock should only be taken for a > short time; if it's !uptodate then it's probably being read) The current place in patch #3 where I'm using folio_lock_timeout() only calls it if a folio_trylock() already failed [2]. So I guess the idea would be that if the trylock failed and folio_test_uptodate() returns 0 then we immediately fail, otherwise we call the unbounded folio_trylock()? I put some traces in and ran my test and it turns out that in every case (except one) where the tre initial folio_trylock() failed I saw folio_test_uptodate() return 0. Assuming my test case is typical, I think that means that coding it with folio_test_uptodate() is roughly the same as just never waiting at all for the folio lock in the SYNC_LIGHT case. In the original discussion of my v1 patch people didn't like that idea. ...so I think that for now I'm going to keep it with the timeout flow. -- After all of the discussion and continued digging into the code that I've done, I'm still of the opinion that this patch series is worthwhile and in the spirit of the SYNC_LIGHT mode of migration, but I also don't believe it's going to be a panacea for any particular case. Presumably even if kcompactd gets blocked for a second or two under a lot of memory pressure it won't be the absolute end of the world. In that spirit, I'll plan to post v3 in a day or two, but I'll also continue to say that if someone tells me that they really hate it that I can put it on the back burner. [2] https://lore.kernel.org/r/20230421151135.v2.3.Ia86ccac02a303154a0b8bc60= 567e7a95d34c96d3@changeid/