Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <CAJuCfpFUrPGVSnZ9+CmMz31GjRNN+tNf6nUmiCgx0Cs5ygD64A@mail.gmail.com>
 <CAJuCfpFe2OdBjZkwHW5UCFUbnQh7hbNeqs7B99PXMXdFNjKb5Q@mail.gmail.com>
 <CAJuCfpG2_trH2DuudX_E0CWfMxyTKfPWqJU14zjVxpTk6kPiWQ@mail.gmail.com>
 <ZJuSzlHfbLj3OjvM@slm.duckdns.org> <CAJuCfpGoNbLOLm08LWKPOgn05+FB1GEqeMTUSJUZpRmDYQSjpA@mail.gmail.com>
 <20230628-meisennest-redlich-c09e79fde7f7@brauner> <CAJuCfpHqZ=5a_2k==FsdBbwDCF7+s7Ji3aZ37LBqUgyXLMz7gA@mail.gmail.com>
 <20230628-faden-qualvoll-6c33b570f54c@brauner> <CAJuCfpF=DjwpWuhugJkVzet2diLkf8eagqxjR8iad39odKdeYQ@mail.gmail.com>
 <20230628-spotten-anzweifeln-e494d16de48a@brauner> <ZJx1nkqbQRVCaKgF@slm.duckdns.org>
In-Reply-To: <ZJx1nkqbQRVCaKgF@slm.duckdns.org>
From:   Suren Baghdasaryan <surenb@google.com>
Date:   Wed, 28 Jun 2023 11:18:20 -0700
Message-ID: <CAJuCfpEFo6WowJ_4XPXH+=D4acFvFqEa4Fuc=+qF8=Jkhn=3pA@mail.gmail.com>
Subject: Re: [PATCH 1/2] kernfs: add kernfs_ops.free operation to free
 resources tied to the file
To:     Tejun Heo <tj@kernel.org>
Cc:     Christian Brauner <brauner@kernel.org>, gregkh@linuxfoundation.org,
        peterz@infradead.org, lujialin4@huawei.com,
        lizefan.x@bytedance.com, hannes@cmpxchg.org, mingo@redhat.com,
        ebiggers@kernel.org, oleg@redhat.com, akpm@linux-foundation.org,
        viro@zeniv.linux.org.uk, juri.lelli@redhat.com,
        vincent.guittot@linaro.org, dietmar.eggemann@arm.com,
        rostedt@goodmis.org, bsegall@google.com, mgorman@suse.de,
        bristot@redhat.com, vschneid@redhat.com,
        linux-kernel@vger.kernel.org, cgroups@vger.kernel.org,
        linux-fsdevel@vger.kernel.org, kernel-team@android.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, Jun 28, 2023 at 11:02=E2=80=AFAM Tejun Heo <tj@kernel.org> wrote:
>
> On Wed, Jun 28, 2023 at 07:35:20PM +0200, Christian Brauner wrote:
> > > To summarize my understanding of your proposal, you suggest adding ne=
w
> > > kernfs_ops for the case you marked (1) and change ->release() to do
> > > only (2). Please correct me if I misunderstood. Greg, Tejun, WDYT?
> >
> > Yes. I can't claim to know all the intricate implementation details of
> > kernfs ofc but this seems sane to me.
>
> This is going to be massively confusing for vast majority of kernfs users=
.
> The contract kernfs provides is that you can tell kernfs that you want ou=
t
> and then you can do so synchronously in a finite amount of time (you stil=
l
> have to wait for in-flight operations to finish but that's under your
> control). Adding an operation which outlives that contract as something
> usual to use is guaranteed to lead to obscure future crnashes. For a
> temporary fix, it's fine as long as it's marked clearly but please don't
> make it something seemingly widely useable.
>
> We have a long history of modules causing crashes because of this. The
> severing semantics is not there just for fun.

I'm sure there are reasons things are working as they do today. Sounds
like we can't change the ->release() logic from what it is today...
Then the question is how do we fix this case needing to release a
resource which can be released only when there are no users of the
file? My original suggestion was to add a kernfs_ops operation which
would indicate there are no more users but that seems to be confusing.
Are there better ways to fix this issue?
Thanks,
Suren.

>
> Thanks.
>
> --
> tejun