Messages in this thread | | | Date | Fri, 10 Dec 2021 08:38:02 -1000 | From | Tejun Heo <> | Subject | Re: [PATCH 2/6] cgroup: Allocate cgroup_file_ctx for kernfs_open_file->priv |
| |
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
| |