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 D6B51C7EE22 for ; Wed, 10 May 2023 21:33:50 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D3FCD6B0072; Wed, 10 May 2023 17:33:49 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id CEFFC6B0074; Wed, 10 May 2023 17:33:49 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BB8516B0075; Wed, 10 May 2023 17:33:49 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id AC7CD6B0072 for ; Wed, 10 May 2023 17:33:49 -0400 (EDT) Received: from smtpin21.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 75D4DADDE0 for ; Wed, 10 May 2023 21:33:49 +0000 (UTC) X-FDA: 80775647778.21.809660B Received: from mail-ed1-f43.google.com (mail-ed1-f43.google.com [209.85.208.43]) by imf13.hostedemail.com (Postfix) with ESMTP id 6142220012 for ; Wed, 10 May 2023 21:33:47 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=linux-foundation.org header.s=google header.b=FZEJN0tl; spf=pass (imf13.hostedemail.com: domain of torvalds@linuxfoundation.org designates 209.85.208.43 as permitted sender) smtp.mailfrom=torvalds@linuxfoundation.org; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1683754427; 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=zexEus7JqShogX6gLLW2/DY+uGniLGr8wmolClBo1cs=; b=BDiZ7JcSh80CxoiWcGiYcABiPIL7lklxO9gnoydXncyXgVgnthJegtU1dzAGpFodXehVxV CSjCrt8eJRGlG8M4wgEACSewjcSCehmAOO1NiuDy3AJLI4jTP3ZrMfOqFu9E6Sco9YrhRD fMM91WNTaf9vvQpZuPl+w6ZoIjH48Tk= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1683754427; a=rsa-sha256; cv=none; b=mMwkU4jxa6OjX+J9SA9rqKdzyytCgas0/EOGnYGPYZqGmsGrWUBiaozhnq1ajPjRxaoDhi jUKjmHQG+ODF7nt1hxLhJzhTPOJlxnBf9eZ4hfaaW39Xvy4dYRhN3/zV3JjfYOV/cHOn3+ x6FkWyRNramzJjuBqY3X2ykBlj9LYOQ= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=linux-foundation.org header.s=google header.b=FZEJN0tl; spf=pass (imf13.hostedemail.com: domain of torvalds@linuxfoundation.org designates 209.85.208.43 as permitted sender) smtp.mailfrom=torvalds@linuxfoundation.org; dmarc=none Received: by mail-ed1-f43.google.com with SMTP id 4fb4d7f45d1cf-50bc4ba28cbso13946395a12.0 for ; Wed, 10 May 2023 14:33:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; t=1683754425; x=1686346425; 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=zexEus7JqShogX6gLLW2/DY+uGniLGr8wmolClBo1cs=; b=FZEJN0tlmIRTYPCxSWZ/878RHq68aEheF7B3kfBa5GI2+6nb/FfUAAJ1lhoPZdnVcd j4qcMix1N1b4IwAy8JtzoE7jL8dhMpem0QgOTISyKLWZvEAuc406cgjQroXfcfoffGf/ po2dPRJDUmI2U02Ej/SQSbaZmIgfTQf7TjRxU= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1683754425; x=1686346425; 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=zexEus7JqShogX6gLLW2/DY+uGniLGr8wmolClBo1cs=; b=Tmpw7wCMjmXW2NwrAcZDLHBPQKrxtIQ2wWgLhwklWFLjAhWsOKokaZ35aoWpYH5OUk w5T7glpwb2sYzGotPUR+u9ckK2STJoctkN+fQOMEqEc7eqPbsrSSI6pTRd6zKrdGE0W7 3459MiBkAAYtcZm1QsiXreRkEmuMZRkYSEFwDgQptRPLQmUiRlhx/FC0nChpAZMP0yaO rlveDIqqrtkud91MKQFHQa0ii/EjL9kkFMybpnzY5MufbTxeIxuxGRQIimZk6TSF2lxL CJX6Ve0v/MeFla7XPqpeNrO4B7so1HvPnKU3/A52s1GqkGlkJl1Iab25CdL2MF2EDTKQ TaqQ== X-Gm-Message-State: AC+VfDxnk/2iiL5IMGV+NcWpn50G41cQK3f+VlHrTP/EgEYiYSF7Qr5Z TRvLnpfYw/Voetw/ct57DxhuZ4ldfjpwpZsCj9PSQg== X-Google-Smtp-Source: ACHHUZ58LMXTb7T92zt+cmpiRkmV6J0oRqg7yFs3myZcUHORf3W+qjUjXBhTxOlWYZKuaduJ0rJ6ig== X-Received: by 2002:a17:907:d1c:b0:966:5912:c4b with SMTP id gn28-20020a1709070d1c00b0096659120c4bmr12243334ejc.76.1683754425370; Wed, 10 May 2023 14:33:45 -0700 (PDT) Received: from mail-ej1-f42.google.com (mail-ej1-f42.google.com. [209.85.218.42]) by smtp.gmail.com with ESMTPSA id fe18-20020a1709072a5200b00968242f8c37sm3151782ejc.50.2023.05.10.14.33.44 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 10 May 2023 14:33:44 -0700 (PDT) Received: by mail-ej1-f42.google.com with SMTP id a640c23a62f3a-956ff2399b1so1465403366b.3 for ; Wed, 10 May 2023 14:33:44 -0700 (PDT) X-Received: by 2002:a17:907:7b9b:b0:957:db05:a35d with SMTP id ne27-20020a1709077b9b00b00957db05a35dmr20319319ejc.48.1683754424213; Wed, 10 May 2023 14:33:44 -0700 (PDT) MIME-Version: 1.0 References: <20230506160415.2992089-1-willy@infradead.org> <20230509191918.GB18828@cmpxchg.org> In-Reply-To: From: Linus Torvalds Date: Wed, 10 May 2023 16:33:26 -0500 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH] filemap: Handle error return from __filemap_get_folio() To: Peter Xu , Andrew Lutomirski Cc: Johannes Weiner , "Matthew Wilcox (Oracle)" , Josef Bacik , Andrew Morton , linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, Dan Carpenter , syzbot+48011b86c8ea329af1b9@syzkaller.appspotmail.com, Christoph Hellwig Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: rseybr1j6nek4ezfqk8dqmid48rjupay X-Rspamd-Server: rspam03 X-Rspam-User: X-Rspamd-Queue-Id: 6142220012 X-HE-Tag: 1683754427-5665 X-HE-Meta: U2FsdGVkX19qMAIMPRtjWKkG5eGVXdH3OlXecFIx9qVV/d1vURrMz+26B33Hvy/gChCNFVZF+AC+O0SP3R+FC8010SMILRJk827I+t0Xy4ud1J9k5LYzi2uj9HiwhqAXee7N3QZPqOByr0LYsyt9d7+Lv3hMLlyrz7EDPljtcv2K3pC/Ppxy+5nalpdCOifjQTcNo+QcxzIuS8PEKtsY9I4gvy23uR1tiwlIIcviYJyCVjKY2+rUjg9i7tNsUJOCJw9kICJgWFljStJJxm8xYphRksKzx9JTCDHjWP5fnrdyosBb39hCWOE4j95i8MTLPBONJb0vgrW118eW/kQIfhWbvNhwF2BVymZP25svcLQpiXKoOS2Cdp44h5UOJG7ROBovCuc/K2uz1GYh1ZdPaG3QWJeOEt8L1VEXCqRgDNJ8lhh2qB6PUseCx4iQxOj8d9dH1dAz/StH4wh6QNcztPwkDWLuRzRKPeyMfSPxXEyGSKpg1n4slaNEx2S5DQAThEpMB2aHaG+/OUJbOr8NHSPuA/l3dFOvLC6lrxsydpXZXFGR83tz0LIGNDwMrh4/ZDOx4UkjU0bjS12KJMdh+lzYzF/gfnWHhCy//ufwbCTcwOaxq15TsGQch/NjyGmM1ZAQ4zenZgM+/NZ4OmJpsvDYhxnE1MKAdSGfxtdrDRL2SsieI3JTdQNO6q1DDTjGtck34ww6YtZ5mHy7ObE86c+XB2vzrcIAkpQtjeGg9u3ZzQKeC7nCgOKP2o8xp1g3x8rMEgcUqtqm6TebRhyrEURTVw4FbvvAHOWrJ89Y+MbhMt9Xy52LDJtoqGIToxNQ3dPC4rADvzakiepTa0LqytnGKAd/zlPos1AYMFuNYhfGjtDLLFoo8v+3TNLfVCPU3Xf0qMSW/XrLczwuuR+pvMOzFL9s0mWmJ8UYkilcwWn5rq7yQoA933gS3czmVORPDAdost63NkA1YUkuI4F 2EP/RTKH 0DtlzgsrhpTaEUWIKSHh5bilBZWmY4v4VYlp/oJE6lrBFf8Fcqry2VVaTQR2SKWXhSkhcG9VIfrZS6HJRyKQYvw6lHvsnYz4oVQbruSARabPH2KV/q/DmyTQuP+IBHFDaS5otiW25wP6rzFdL+xG9tKHtv3HdjP6/CmSyCMPimUUT1wI8Po8KBmDa+xdyp9NitUGpuqIqUhAcTY0wOT81dkXgtD/MGFyRLQFu0NJdAIoRc1osmbJ5pr5CY32lVJhpULvdjJeM354B+/ErvbXb94pjcSikGlnGJtgOqwRBUVCz0WZQE7AHJqwmJ06XmXbqyJpvgUJ+bxwb7cymg2OhbSfRxPCtqIoDB6MdsEpEHyxFFdY/uJIJcaJOmFDtHR25k79TnPn0Bzo3I0FHkFB08C7e4+dffErcogrG59YuISs6nhooU2wEc2mjS7kFrjxh/ILDIE2SI1itE4SCdnlHHlUUTlzzidAcqzO0Mz0zPoNpeqleSMlbU2bLcuCxwuMvs7/unJThyCs6svWA3qPBOV9RqpAKK7q+n6gYoROXo3DcTEEJSO3ptt4mOoyhNw9WMHtPcadbO3HGKXIbYfeeYRHpBVAteWn0UquIZOO84Wzo/Yr8Lvc/3p0f67qWJSxiwscuhH7KQ6gAAXNCRtZULccnf0+UCdQAa/zV 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 Wed, May 10, 2023 at 3:27=E2=80=AFPM Peter Xu wrote: > > The change looks all right to me. I hate how this path has turned very architecture-specific again, and this patch makes it worse. For a short while we shared a lot of the code between architectures thanks to the new fault helpers (ie fault_signal_pending() and friends). Now CONFIG_PER_VMA_LOCK has made a mockery of that, and this patch makes it worse by just fixing x86. As is historically always the case. And I think that patch is actually buggy. Because it does that ret =3D VM_FAULT_SIGBUS; if (fpin) goto out_retry; in the fault path, and now returns VM_FAULT_SIGBUS | VM_FAULT_RETRY as a re= sult. And there are a lot of users that will be *very* unhappy about that combina= tion. Look at mm/gup.c, for example. It will do if (ret & VM_FAULT_ERROR) { int err =3D vm_fault_to_errno(ret, *flags); if (err) return err; ... and not react to VM_FAULT_RETRY being set at all - so it won't clear the "locked" variable, for example. And that all just makes it very clear how much of a painpoint that conditional lock release is. It's horrendous. That's always a huge pain in general, and it really complicates how things get handled. It has very real and obvious historical reasons ("we never released the lock" being converted one case at a time to "we release the lock in this situation and set the bit to tell you"), but I wonder if we should just say "handle_mm_fault() _always_ releases the lock". We'd still keep the RETRY bit as a "this did not complete, you need to retry", but at least the whole secondary meaning of "oh, and if it isn't set, you need to release the lock you took" would go away. Because RETRY has really annoying semantics today. Again - I know exactly _why_ it has those horrendous semantics, and I'm not blaming anybody, but it makes it really painful to deal with 15 different architectures that then have to deal with those things. How painful would it be to try to take baby steps and remove those kinds of things and slowly move towards a situation where RETRY isn't such a magically special bit? Peter Xu, you did a lot of the helper function cleanups, and moved some of the accounting code into generic code. It's a few years ago, but maybe you still remember this area.. And Luto, I'm adding you to the participants because you've touched that code more than most. Could we make the rule be that handle_mm_fault() just *always* releases the lock? How painful would that be? For many of the architectures, I *think* it would mean just removing the mmap_read_unlock(mm); in the fault handling path (after the RETRY test). But there's a couple of other uses of handle_mm_fault() too, notably GUP. Am I just dreaming? Or could we try to simplify this area first? Because I think Johannes' patch right now is quite buggy, and only converted one caller, when a lot of other callers will currently find it very problematic to see both VM_FAULT_RETRY and one of the error bits.. It's also possible that I'm misreading things. But I really think the lock handling would be a lot easier if the rule was "it is locked on entry, it's always unlocked on exit". Of course, with the vma locking, the "it" above is nontrivial too. That sure didn't simplify thinking about this all.... Linus