Re: [PATCH 1/2] kernfs: add kernfs_ops.free operation to free resources tied to the file

Suren Baghdasaryan
2023-06-26T20:21:07Z

On Mon, Jun 26, 2023 at 1:17 PM Suren Baghdasaryan <surenb@google.com> wrote:
>
> kernfs_ops.release operation can be called from kernfs_drain_open_files
> which is not tied to the file's real lifecycle. Introduce a new kernfs_ops
> free operation which is called only when the last fput() of the file is
> performed and therefore is strictly tied to the file's lifecycle. This
> operation will be used for freeing resources tied to the file, like
> waitqueues used for polling the file.

While this patchset touches kernfs, cgroups and psi areas (3 different
maintainers), I think cgroups are the most relevant area for it, so
IMHO Tejun's tree would be the right one to get them once reviewed and
acknowledged. Thanks!

>
> Signed-off-by: Suren Baghdasaryan <surenb@google.com>
> ---
> fs/kernfs/file.c | 8 +++++---
> include/linux/kernfs.h | 5 +++++
> 2 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
> index 40c4661f15b7..acc52d23d8f6 100644
> --- a/fs/kernfs/file.c
> +++ b/fs/kernfs/file.c
> @@ -766,7 +766,7 @@ static int kernfs_fop_open(struct inode *inode, struct file *file)
>
> /* used from release/drain to ensure that ->release() is called exactly once */
> static void kernfs_release_file(struct kernfs_node *kn,
> - struct kernfs_open_file *of)
> + struct kernfs_open_file *of, bool final)
> {
> /*
> * @of is guaranteed to have no other file operations in flight and
> @@ -787,6 +787,8 @@ static void kernfs_release_file(struct kernfs_node *kn,
> of->released = true;
> of_on(of)->nr_to_release--;
> }
> + if (final && kn->attr.ops->free)
> + kn->attr.ops->free(of);
> }
>
> static int kernfs_fop_release(struct inode *inode, struct file *filp)
> @@ -798,7 +800,7 @@ static int kernfs_fop_release(struct inode *inode, struct file *filp)
> struct mutex *mutex;
>
> mutex = kernfs_open_file_mutex_lock(kn);
> - kernfs_release_file(kn, of);
> + kernfs_release_file(kn, of, true);
> mutex_unlock(mutex);
> }
>
> @@ -852,7 +854,7 @@ void kernfs_drain_open_files(struct kernfs_node *kn)
> }
>
> if (kn->flags & KERNFS_HAS_RELEASE)
> - kernfs_release_file(kn, of);
> + kernfs_release_file(kn, of, false);
> }
>
> WARN_ON_ONCE(on->nr_mmapped || on->nr_to_release);
> 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);
>
> /*
> * Read is handled by either seq_file or raw_read().
> --
> 2.41.0.162.gfafddb0af9-goog
>