Re: [PATCH 2/6] cgroup: Allocate cgroup_file_ctx for kernfs_open_file->priv

[Linus Torvalds <torvalds@linuxfoundation.org>]

On Thu, Dec 9, 2021 at 1:47 PM Tejun Heo <tj@kernel.org> wrote:
>
> of->priv is currently used by each interface file implementation to store
> private information. This patch collects the current two private data usages
> into struct cgroup_file_ctx which is allocated and freed by the common path.
> This allows generic private data which applies to multiple files, which will
> be used to in the following patch.

I'm not sure if it's worth it having that union just to make the
struct be 8 bytes instead of 16 (and later 16 bytes instead of 24),
when the real cost is that dynamic allocation overhead, and there's
likely only one or two active actual allocations at a time.

IOW, I'm not convinced there's any real memory savings, and making it
a union means that now you have to be very careful about who does
what..

And yes, people historically had to be very careful _anyway_, because
the "union" was kind of implicit in how there was just a shared 'void
*priv' thing that was either that iterator pointer or that psi trigger
pointer.

So in that sense this is a semantically minimal patch, but I think
that practically speaking we'd be better off without that possible
source of confusion, and just always have that cgroup proc file have a
full structure.

In fact, I think it means we could just make the thing *contain* the
iterator, instead of having a pointer to an iterator, and getting rid
of the now redundant dynamic alloc/free of the iterator pointer.

Wouldn't that simplify things? And might there not be some cgroup
pressure user that also wants to use the iterator interfaces? Maybe
not, my point is more that once we have an explicit struct allocation
for cgroup proc files, we might as well clarify and simplify the
code..

           Linus