From: Christian Brauner <brauner-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
To: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Suren Baghdasaryan
<surenb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org,
peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org,
lujialin4-hv44wF8Li93QT0dZR+AlfA@public.gmane.org,
lizefan.x-EC8Uxl6Npydl57MIdRCFDg@public.gmane.org,
hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org,
mingo-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
ebiggers-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org,
viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org,
juri.lelli-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
vincent.guittot-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
dietmar.eggemann-5wv7dgnIgG8@public.gmane.org,
rostedt-nx8X9YLhiw1AfugRpC6u6w@public.gmane.org,
bsegall-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org,
mgorman-l3A5Bk7waGM@public.gmane.org,
bristot-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
vschneid-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
kernel-team-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org
Subject: Re: [PATCH 1/2] kernfs: add kernfs_ops.free operation to free resources tied to the file
Date: Tue, 27 Jun 2023 10:24:15 +0200 [thread overview]
Message-ID: <20230627-kanon-hievt-bfdb583ddaa6@brauner> (raw)
In-Reply-To: <ZJn1tQDgfmcE7mNG-NiLfg/pYEd1N0TnZuCh8vA@public.gmane.org>
On Mon, Jun 26, 2023 at 10:31:49AM -1000, Tejun Heo wrote:
> On Mon, Jun 26, 2023 at 01:17:12PM -0700, Suren Baghdasaryan wrote:
> > diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
> > index 73f5c120def8..a7e404ff31bb 100644
> > --- a/include/linux/kernfs.h
> > +++ b/include/linux/kernfs.h
> > @@ -273,6 +273,11 @@ struct kernfs_ops {
> > */
> > int (*open)(struct kernfs_open_file *of);
> > void (*release)(struct kernfs_open_file *of);
> > + /*
> > + * Free resources tied to the lifecycle of the file, like a
> > + * waitqueue used for polling.
> > + */
> > + void (*free)(struct kernfs_open_file *of);
>
> I think this can use a bit more commenting - ie. explain that release may be
> called earlier than the actual freeing of the file and how that can lead to
> problems. Othre than that, looks fine to me.
It seems the more natural thing to do would be to introduce a ->drain()
operation and order it before ->release(), no?
WARNING: multiple messages have this Message-ID (diff)
From: Christian Brauner <brauner@kernel.org>
To: Tejun Heo <tj@kernel.org>
Cc: Suren Baghdasaryan <surenb@google.com>,
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
Subject: Re: [PATCH 1/2] kernfs: add kernfs_ops.free operation to free resources tied to the file
Date: Tue, 27 Jun 2023 10:24:15 +0200 [thread overview]
Message-ID: <20230627-kanon-hievt-bfdb583ddaa6@brauner> (raw)
In-Reply-To: <ZJn1tQDgfmcE7mNG@slm.duckdns.org>
On Mon, Jun 26, 2023 at 10:31:49AM -1000, Tejun Heo wrote:
> On Mon, Jun 26, 2023 at 01:17:12PM -0700, Suren Baghdasaryan wrote:
> > diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
> > index 73f5c120def8..a7e404ff31bb 100644
> > --- a/include/linux/kernfs.h
> > +++ b/include/linux/kernfs.h
> > @@ -273,6 +273,11 @@ struct kernfs_ops {
> > */
> > int (*open)(struct kernfs_open_file *of);
> > void (*release)(struct kernfs_open_file *of);
> > + /*
> > + * Free resources tied to the lifecycle of the file, like a
> > + * waitqueue used for polling.
> > + */
> > + void (*free)(struct kernfs_open_file *of);
>
> I think this can use a bit more commenting - ie. explain that release may be
> called earlier than the actual freeing of the file and how that can lead to
> problems. Othre than that, looks fine to me.
It seems the more natural thing to do would be to introduce a ->drain()
operation and order it before ->release(), no?
next prev parent reply other threads:[~2023-06-27 8:24 UTC|newest]
Thread overview: 62+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-26 20:17 [PATCH 1/2] kernfs: add kernfs_ops.free operation to free resources tied to the file Suren Baghdasaryan
2023-06-26 20:17 ` Suren Baghdasaryan
[not found] ` <20230626201713.1204982-1-surenb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2023-06-26 20:17 ` [PATCH 2/2] sched/psi: tie psi trigger destruction with file's lifecycle Suren Baghdasaryan
2023-06-26 20:17 ` Suren Baghdasaryan
2023-06-26 20:31 ` [PATCH 1/2] kernfs: add kernfs_ops.free operation to free resources tied to the file Tejun Heo
2023-06-26 20:31 ` Tejun Heo
2023-06-26 20:39 ` Suren Baghdasaryan
2023-06-26 20:39 ` Suren Baghdasaryan
[not found] ` <ZJn1tQDgfmcE7mNG-NiLfg/pYEd1N0TnZuCh8vA@public.gmane.org>
2023-06-27 8:24 ` Christian Brauner [this message]
2023-06-27 8:24 ` Christian Brauner
2023-06-27 17:09 ` Suren Baghdasaryan
2023-06-27 17:09 ` Suren Baghdasaryan
[not found] ` <CAJuCfpECKqYiekDK6Zw58w10n1T4Q3R+2nymfHX2ZGfQVDC3VQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2023-06-27 17:30 ` Christian Brauner
2023-06-27 17:30 ` Christian Brauner
2023-06-27 17:36 ` Suren Baghdasaryan
2023-06-27 17:36 ` Suren Baghdasaryan
2023-06-27 18:42 ` Tejun Heo
[not found] ` <ZJstlHU4Y3ZtiWJe-NiLfg/pYEd1N0TnZuCh8vA@public.gmane.org>
2023-06-27 20:09 ` Suren Baghdasaryan
2023-06-27 20:09 ` Suren Baghdasaryan
[not found] ` <CAJuCfpFUrPGVSnZ9+CmMz31GjRNN+tNf6nUmiCgx0Cs5ygD64A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2023-06-27 21:43 ` Suren Baghdasaryan
2023-06-27 21:43 ` Suren Baghdasaryan
[not found] ` <CAJuCfpFe2OdBjZkwHW5UCFUbnQh7hbNeqs7B99PXMXdFNjKb5Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2023-06-27 21:58 ` Suren Baghdasaryan
2023-06-27 21:58 ` Suren Baghdasaryan
2023-06-28 1:54 ` Tejun Heo
[not found] ` <ZJuSzlHfbLj3OjvM-NiLfg/pYEd1N0TnZuCh8vA@public.gmane.org>
2023-06-28 3:09 ` Suren Baghdasaryan
2023-06-28 3:09 ` Suren Baghdasaryan
2023-06-28 7:26 ` Christian Brauner
2023-06-28 7:26 ` Christian Brauner
2023-06-28 7:46 ` Suren Baghdasaryan
[not found] ` <CAJuCfpHqZ=5a_2k==FsdBbwDCF7+s7Ji3aZ37LBqUgyXLMz7gA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2023-06-28 8:41 ` Christian Brauner
2023-06-28 8:41 ` Christian Brauner
2023-06-28 16:28 ` Suren Baghdasaryan
2023-06-28 16:28 ` Suren Baghdasaryan
2023-06-28 17:35 ` Christian Brauner
2023-06-28 18:02 ` Tejun Heo
2023-06-28 18:02 ` Tejun Heo
2023-06-28 18:18 ` Suren Baghdasaryan
2023-06-28 18:18 ` Suren Baghdasaryan
[not found] ` <CAJuCfpEFo6WowJ_4XPXH+=D4acFvFqEa4Fuc=+qF8=Jkhn=3pA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2023-06-28 18:42 ` Greg KH
2023-06-28 18:42 ` Greg KH
2023-06-28 20:12 ` Suren Baghdasaryan
2023-06-28 20:12 ` Suren Baghdasaryan
2023-06-28 20:34 ` Tejun Heo
2023-06-28 21:50 ` Suren Baghdasaryan
2023-06-28 21:50 ` Suren Baghdasaryan
[not found] ` <CAJuCfpFKjhmti8k6OHoDHAu6dPvqP0jn8FFdSDPqmRfH97bkiQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2023-06-30 0:59 ` Suren Baghdasaryan
2023-06-30 0:59 ` Suren Baghdasaryan
[not found] ` <CAJuCfpH3JcwADEYPBhzUcunj0dcgYNRo+0sODocdhbuXQsbsUQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2023-06-30 8:21 ` Christian Brauner
2023-06-30 8:21 ` Christian Brauner
2023-07-10 20:38 ` Tejun Heo
2023-07-10 20:38 ` Tejun Heo
2023-06-28 17:58 ` Tejun Heo
2023-06-27 6:25 ` Greg KH
2023-06-27 6:25 ` Greg KH
2023-06-27 17:03 ` Suren Baghdasaryan
2023-06-27 17:03 ` Suren Baghdasaryan
[not found] ` <CAJuCfpGUTMP2FTzzx+bq9_5KZjo1r_qspHYZXK2Ors-yU3XhqQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2023-06-27 17:23 ` Christian Brauner
2023-06-27 17:23 ` Christian Brauner
2023-06-27 17:36 ` Matthew Wilcox
2023-06-27 17:36 ` Matthew Wilcox
2023-06-26 20:21 ` Suren Baghdasaryan
2023-06-26 20:21 ` Suren Baghdasaryan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230627-kanon-hievt-bfdb583ddaa6@brauner \
--to=brauner-dgejt+ai2ygdnm+yrofe0a@public.gmane.org \
--cc=akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org \
--cc=bristot-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
--cc=bsegall-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org \
--cc=cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=dietmar.eggemann-5wv7dgnIgG8@public.gmane.org \
--cc=ebiggers-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
--cc=gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org \
--cc=hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org \
--cc=juri.lelli-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
--cc=kernel-team-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org \
--cc=linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=lizefan.x-EC8Uxl6Npydl57MIdRCFDg@public.gmane.org \
--cc=lujialin4-hv44wF8Li93QT0dZR+AlfA@public.gmane.org \
--cc=mgorman-l3A5Bk7waGM@public.gmane.org \
--cc=mingo-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
--cc=oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
--cc=peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org \
--cc=rostedt-nx8X9YLhiw1AfugRpC6u6w@public.gmane.org \
--cc=surenb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org \
--cc=tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
--cc=vincent.guittot-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
--cc=viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org \
--cc=vschneid-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.