Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.36 as permitted sender) client-ip=23.128.96.36;
MIME-Version: 1.0
References: <20230926162228.68666-1-mjguzik@gmail.com> <CAHk-=wjUCLfuKks-VGTG9hrFAORb5cuzqyC0gRXptYGGgL=YYg@mail.gmail.com>
 <CAGudoHGej+gmmv0OOoep2ENkf7hMBib-KL44Fu=Ym46j=r6VEA@mail.gmail.com> <20230927-kosmetik-babypuppen-75bee530b9f0@brauner>
In-Reply-To: <20230927-kosmetik-babypuppen-75bee530b9f0@brauner>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Wed, 27 Sep 2023 10:48:20 -0700
Message-ID: <CAHk-=whLadznjNKZPYUjxVzAyCH-rRhb24_KaGegKT9E6A86Kg@mail.gmail.com>
Subject: Re: [PATCH v2] vfs: shave work on failed file open
To:     Christian Brauner <brauner@kernel.org>
Cc:     Mateusz Guzik <mjguzik@gmail.com>, viro@zeniv.linux.org.uk,
        linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
Content-Type: multipart/mixed; boundary="0000000000000dc77506065acdeb"
Precedence: bulk

--0000000000000dc77506065acdeb
Content-Type: text/plain; charset="UTF-8"

On Wed, 27 Sept 2023 at 07:10, Christian Brauner <brauner@kernel.org> wrote:
>
> No need to resend I can massage this well enough in-tree.

Hmm. Please do, but here's some more food for thought for at least the
commit message.

Because there's more than the "__fput_sync()" issue at hand, we have
another delayed thing that this patch ends up short-circuiting, which
wasn't obvious from the original description.

I'm talking about the fact that our existing "file_free()" ends up
doing the actual release with

        call_rcu(&f->f_rcuhead, file_free_rcu);

and the patch under discussion avoids that part too.

And I actually like that it avoids it, I just think it should be
mentioned explicitly, because it wasn't obvious to me until I actually
looked at the old __fput() path. Particularly since it means that the
f_creds are free'd synchronously now.

I do think that's fine, although I forget what path it was that
required that rcu-delayed cred freeing. Worth mentioning, and maybe
worth thinking about.

However, when I *did* look at it, it strikes me that we could do this
differently.

Something like this (ENTIRELY UNTESTED) patch, which just moves this
logic into fput() itself.

Again: ENTIRELY UNTESTED, and I might easily have screwed up. But it
looks simpler and more straightforward to me. But again: that may be
because I missed something.

             Linus

--0000000000000dc77506065acdeb
Content-Type: text/x-patch; charset="US-ASCII"; name="patch.diff"
Content-Disposition: attachment; filename="patch.diff"
Content-Transfer-Encoding: base64
Content-ID: <f_ln21i4ht0>
X-Attachment-Id: f_ln21i4ht0

IGZzL2ZpbGVfdGFibGUuYyB8IDIyICsrKysrKysrKysrKysrKysrKysrKysKIDEgZmlsZSBjaGFu
Z2VkLCAyMiBpbnNlcnRpb25zKCspCgpkaWZmIC0tZ2l0IGEvZnMvZmlsZV90YWJsZS5jIGIvZnMv
ZmlsZV90YWJsZS5jCmluZGV4IGVlMjFiM2RhOWQwOC4uNGZiODdhMDM4MmQ5IDEwMDY0NAotLS0g
YS9mcy9maWxlX3RhYmxlLmMKKysrIGIvZnMvZmlsZV90YWJsZS5jCkBAIC00MzAsMTEgKzQzMCwz
MyBAQCBFWFBPUlRfU1lNQk9MX0dQTChmbHVzaF9kZWxheWVkX2ZwdXQpOwogCiBzdGF0aWMgREVD
TEFSRV9ERUxBWUVEX1dPUksoZGVsYXllZF9mcHV0X3dvcmssIGRlbGF5ZWRfZnB1dCk7CiAKKy8q
CisgKiBDYWxsZWQgZm9yIGZpbGVzIHRoYXQgd2VyZSBuZXZlciBmdWxseSBvcGVuZWQsIGFuZAor
ICogZG9uJ3QgbmVlZCB0aGUgUkNVLWRlbGF5ZWQgZnJlZWluZzogdGhleSBoYXZlIG5ldmVyCisg
KiBiZWVuIGFjY2Vzc2VkIGluIGFueSBvdGhlciBjb250ZXh0LgorICovCitzdGF0aWMgdm9pZCBm
cHV0X2ltbWVkaWF0ZShzdHJ1Y3QgZmlsZSAqZikKK3sKKwlzZWN1cml0eV9maWxlX2ZyZWUoZik7
CisJcHV0X2NyZWQoZi0+Zl9jcmVkKTsKKwlpZiAobGlrZWx5KCEoZi0+Zl9tb2RlICYgRk1PREVf
Tk9BQ0NPVU5UKSkpCisJCXBlcmNwdV9jb3VudGVyX2RlYygmbnJfZmlsZXMpOworCWlmICh1bmxp
a2VseShmLT5mX21vZGUgJiBGTU9ERV9CQUNLSU5HKSkgeworCQlwYXRoX3B1dChiYWNraW5nX2Zp
bGVfcmVhbF9wYXRoKGYpKTsKKwkJa2ZyZWUoYmFja2luZ19maWxlKGYpKTsKKwl9IGVsc2Ugewor
CQlrbWVtX2NhY2hlX2ZyZWUoZmlscF9jYWNoZXAsIGYpOworCX0KK30KKwogdm9pZCBmcHV0KHN0
cnVjdCBmaWxlICpmaWxlKQogewogCWlmIChhdG9taWNfbG9uZ19kZWNfYW5kX3Rlc3QoJmZpbGUt
PmZfY291bnQpKSB7CiAJCXN0cnVjdCB0YXNrX3N0cnVjdCAqdGFzayA9IGN1cnJlbnQ7CiAKKwkJ
aWYgKHVubGlrZWx5KCEoZmlsZS0+Zl9tb2RlICYgRk1PREVfT1BFTkVEKSkpCisJCQlyZXR1cm4g
ZnB1dF9pbW1lZGlhdGUoZmlsZSk7CisKIAkJaWYgKGxpa2VseSghaW5faW50ZXJydXB0KCkgJiYg
ISh0YXNrLT5mbGFncyAmIFBGX0tUSFJFQUQpKSkgewogCQkJaW5pdF90YXNrX3dvcmsoJmZpbGUt
PmZfcmN1aGVhZCwgX19fX2ZwdXQpOwogCQkJaWYgKCF0YXNrX3dvcmtfYWRkKHRhc2ssICZmaWxl
LT5mX3JjdWhlYWQsIFRXQV9SRVNVTUUpKQo=
--0000000000000dc77506065acdeb--