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

Tejun Heo
2021-12-10T18:38:07Z

Hello,

On Fri, Dec 10, 2021 at 09:53:41AM -0800, Linus Torvalds wrote:
> 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.

Yeah, I was initially doing ctx->procs_it and ->psi_trigger which looked
kinda silly so then nested structs and then why not the union which is kinda
logical in semantic sense.

> 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..

It's a bit of bikeshedding but I wanna explicitly denote who at currently
uses the fields, so how about nested structs w/ embedded iterator? If other
files ever want to share fields, we can shift those fields to the common
area together with ->ns.

Thanks.

--
tejun