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 38A3BC7618E for ; Wed, 26 Apr 2023 21:40:19 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B80356B007B; Wed, 26 Apr 2023 17:40:18 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B30766B00CA; Wed, 26 Apr 2023 17:40:18 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9F8D36B0102; Wed, 26 Apr 2023 17:40:18 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 8EBD96B007B for ; Wed, 26 Apr 2023 17:40:18 -0400 (EDT) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 62E77A03FE for ; Wed, 26 Apr 2023 21:40:18 +0000 (UTC) X-FDA: 80724860916.04.C4BC022 Received: from mail-ej1-f47.google.com (mail-ej1-f47.google.com [209.85.218.47]) by imf21.hostedemail.com (Postfix) with ESMTP id 3845A1C0009 for ; Wed, 26 Apr 2023 21:40:15 +0000 (UTC) Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=JyMXLMER; spf=pass (imf21.hostedemail.com: domain of dianders@chromium.org designates 209.85.218.47 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=1682545216; a=rsa-sha256; cv=none; b=sw1bnGbj/bHbQ30KPYwE3kRkiVDfN4AzGM4guBY7h5hy1HQwXgLUzzuK7YmzYeXFPbLUUl xr34Y/lBL5MsGFCyQ3I3/Y6sbSBcd1/IN8zV6ENlHZsLoM1pzDoc5n0HlozU0ShPYIdZn8 0o5JDh/buYvXfHIYjuFnZwxgKsMd3eE= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=JyMXLMER; spf=pass (imf21.hostedemail.com: domain of dianders@chromium.org designates 209.85.218.47 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=1682545216; 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=jFtlN+f5KTeNyVfIo3GYR/FbjWj+5XTUcjpUzFHTqqw=; b=UXFwvivm5E0tixGUJ+68gIAfeGheTYMjqMGHqZ79aB7wb9juGvRSAvriibTKSV/tYDMXqT 2k9taxFH1wEgl/OvmD93+AOON/HQbEHn44Xl98iZU0QWtx1fs7tnHb2Kt2HZCQyGvn8hjL wZv0eh0QOwLB9a/4QlAYoJR38Q91+kY= Received: by mail-ej1-f47.google.com with SMTP id a640c23a62f3a-958bb7731a9so971135566b.0 for ; Wed, 26 Apr 2023 14:40:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1682545213; x=1685137213; 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=jFtlN+f5KTeNyVfIo3GYR/FbjWj+5XTUcjpUzFHTqqw=; b=JyMXLMERhmv2f/gWWRKfNXHRq7ZBcxlvt41D7yM1ueseRZYoBLK671HCWV0/SkikJ/ V4bVhg+z130kzHG6QVwri3Pk0Wp1ZwVubjazj53SKZGdJcqhLA6jHkI0T48mBgivTqBS OeDjJoJ8KQ7XQUgilvbG37XxeqJnPgRbkvtic= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682545213; x=1685137213; 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=jFtlN+f5KTeNyVfIo3GYR/FbjWj+5XTUcjpUzFHTqqw=; b=QjN70KRRwT7ARbaqv++hCfDVvL1c2Yfx7cmmJ1yyD+JN9PNhc1oj5jHTJo1UZXKvqO 3PrAysVpRCMZIw2j12ShCAkjTXrCIscqbBtWdh6qk/zR46Z5xyykBqdsHSJ/rXK7nCJs IgQa4f8DxnHCbhYDAEqC0QXt2pduLG9gEJaM03Dlo+isajL9NJu1x4ppXozKK6GwXjkX iKNHHWQ/n2f1LBwGmWJd/yxyFgSgATh0cz0g3wcYbnoKYzVfh62hzZeYPheqHEpenld6 Ck0trGSHL8Xnp11EqhjzjvRDIJrAa3r9bI4FZK3QcU4N295bs380kaFFsf4bvBK+ALXU /4iQ== X-Gm-Message-State: AAQBX9eYdvt1VZzX27zNyJ8iJ4S8FO+mDbyA6iyuQaByA0W5jOFQKMz9 RBUXlDhOJTPntatN0qkaF02w2zgv5ScCspmq++cXuQ== X-Google-Smtp-Source: AKy350bdsRfgqwQlYWGCWa33ygAXU92NCnW+pVRN7go3kbx71wMkqpVEKs7qCiraIq7joTqfx9vecQ== X-Received: by 2002:a17:906:39c4:b0:94e:dbf7:2dfe with SMTP id i4-20020a17090639c400b0094edbf72dfemr17046699eje.11.1682545213618; Wed, 26 Apr 2023 14:40:13 -0700 (PDT) Received: from mail-wm1-f43.google.com (mail-wm1-f43.google.com. [209.85.128.43]) by smtp.gmail.com with ESMTPSA id 10-20020a170906328a00b009599c3a019fsm4561950ejw.60.2023.04.26.14.40.12 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 26 Apr 2023 14:40:12 -0700 (PDT) Received: by mail-wm1-f43.google.com with SMTP id 5b1f17b1804b1-3f256b84f14so127005e9.0 for ; Wed, 26 Apr 2023 14:40:12 -0700 (PDT) X-Received: by 2002:a05:600c:1c12:b0:3f1:926e:c941 with SMTP id j18-20020a05600c1c1200b003f1926ec941mr297wms.2.1682545212312; Wed, 26 Apr 2023 14:40:12 -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 14:39:56 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v2 1/4] mm/filemap: Add folio_lock_timeout() To: Matthew Wilcox , Mel Gorman Cc: 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-Queue-Id: 3845A1C0009 X-Rspamd-Server: rspam01 X-Stat-Signature: uuq9eyjyc4tqabqmoqdtu8mguen4e9dw X-HE-Tag: 1682545215-651193 X-HE-Meta: U2FsdGVkX1+orkBoWHi8ipzFTw1dZZakVXz97m4sKiuZ7zwCyLYfVLFKJ3uVMluupcZFwou68tnxkzoci0oC71GH+yz7xogmy1p+qNx8ewA9VlsuaTQqS7bqOWBm8WIkED/g2uIWRqcUy75Gs/zbsan4n0srbDzx3taph+nt6UoOHBeB35SNKkQQjmJwHshKGFjyZXEtfQqwvHMB41WXIPqxIQYa3XuKm2KmBE5dlj/Q6eBHcG0vh8/K2f2yiH6GRDHP+YhE1E374tfbnM6DatcdiVi529/7Ex51Ak6y8JQaJ8TvFwYnTYH9Riao9+9ICxA+YAOW/5zGDtrYRSWervGx+DBVyEPWUL5NnQcSMZ9nVSJL3w+Zcpm/P+KPfxRwN3vek1hIyVGJEIXUJhvewf7XhpBFUjOwOJLwY3fr08iV/74EY1vg8gdA80JquSaY7tvW87OAUaPWXTEQBFLGzCZFCnyutv5a1XiU8pt5FYJ4uQThP5Fa6ad93P1jQi4O6vwHARHlz/CbFHdutTUqHhA88/Su0anxMsM1LYg2bcPd7ddqBLMUa7XsfihBhxNXrT9jIUPJFlx5VU2Nxgy5ED55b+0gGgZUJQYJVRExU7zSy1N13K0KPS4pL3JIfRYAJjG8012neC+1PaSY7J9xon6k5vpY47lflIwtGDbc9LId+NT7wYrzn0FtLx/t0qIPZDrc90gRGPVGifhIB2NGQqMpNTsl5dK0aSYjPXAcfChkr0vHX/ALrOHHme+zklMXJRVjMscGdeH66CYSfWa76TtM3fYRnNmLWUAdIRnJ/A3Z/mKgzWwOpbcgtKVStA2k7FuqevsF3muR9qjG5OIj7wB4b+AHv9cd1L+wCheXGCmw1n5H5KMMD2KzzdgYD/kJWyWlHjRi/w02AxAcpqy42WVO3vKBoNuZODDgQPxg9l6N0EaZ2X5hlX33WsTVUmHs5MZZfDzBiUX3bfZHwqn AkjitIGm KhMKc36J5Qc1PX5WTV+aBBZv1tCe4NVawBSW2KTdFk56DhvuxErknapmw4WGnOa8YDbgwnSF+S9bJyTwDhxpbzkzySH4wzZk/TT1AmggpqV8Eo8l/J3UvJJ5WyVT5zhZf9vYf6gb/oO02cYv8yEo1uc5+VW6FHF2U+XiIzOadCbJwP7AzIJ4t78jhwgRc8g6oQgdB3saLSNpMcZz/N0KVrREKUNuweu8yrE7rOVpkowdMS/i/DmRoxGYfrXSYN+6crfpkEmyg2nhyaJrWa/wi6rYAXnDJxzfMGmYZ/U/483HbDY5i2Tn7N4E6QwN09yACA08DNCvpl49UVHoB3vb7T/qUV128wg2s7DnxDB3g+7niTxZ/NQhNnmc344uJANq27xFJD9cidvQtF7VmBOuHNeN6GL88Df+oN419gOjOB2JH68O2rFydN4S2qq+irjtL5TT2VfSkdzhU57CoJWLcV/bmRX4twozCq9j98i2L+0kvdx/ut5V0IXj1Z/4KtMtJO/gvcTQxJcpKuhHf+incfTcCfA== 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 2:27=E2=80=AFPM Matthew Wilcox wrote: > > On Wed, Apr 26, 2023 at 01:46:58PM -0700, Doug Anderson wrote: > > 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 peopl= e > > > 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 > > People don't read comments. Agreed, it's just better than nothing... > > > 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()? > > Looking at the actual code, here's what I'd do: > > +++ b/mm/migrate.c > @@ -1156,6 +1156,14 @@ static int migrate_folio_unmap(new_page_t get_new_= page, free_page_t put_new_page > if (current->flags & PF_MEMALLOC) > goto out; > > + /* > + * In "light" mode, we can wait for transient locks (eg > + * inserting a page into the page table), but it's not > + * worth waiting for I/O. > + */ > + if (mode =3D=3D MIGRATE_SYNC_LIGHT && !folio_test_uptodat= e(folio)) > + goto out; > + > folio_lock(src); > } > locked =3D true; > > > 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. > > I think that means that your specific test is generally going to > exercise the case where the lock is held because we're waiting for I/O. > That's exactly what you set it up to produce, after all! But it won't > affect the cases where the folio lock is being held for other reasons, > which your testcase is incredibly unlikely to produce. Sure, I'm happy to do it like you say. Do you have any suggestions for the similar lock_buffer() case, or are you OK w/ the timeout there? Mel: do you have any comments? In your previous response [1] you seemed to indicate that you thought that short waits for read were a good idea. [1] https://lore.kernel.org/r/20230420102304.7wdquge2b7r3xerj@techsingulari= ty.net -Doug