archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <>
To: Tejun Heo <>
Cc: "Eric W. Biederman" <>,
	Michal Koutny <>, Jens Axboe <>,
	Kees Cook <>,
	Oleg Nesterov <>,
	Peter Zijlstra <>,
	Thomas Gleixner <>,
	Jim Newsome <>,
	Alexey Gladkov <>,
	Andy Lutomirski <>,
	Jann Horn <>,
	Linux Kernel Mailing List <>,
	Security Officers <>,
	Kernel Team <>
Subject: Re: [PATCH 2/6] cgroup: Allocate cgroup_file_ctx for kernfs_open_file->priv
Date: Fri, 10 Dec 2021 09:53:41 -0800	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Thu, Dec 9, 2021 at 1:47 PM Tejun Heo <> 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

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

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


  reply	other threads:[~2021-12-10 17:54 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-09 21:47 [PATCHSET cgroup/for-5.16-fixes] cgroup: Use open-time creds and namespace for migration perm checks Tejun Heo
2021-12-09 21:47 ` [PATCH 1/6] cgroup: Use open-time credentials for process migraton " Tejun Heo
2021-12-10 17:41   ` Linus Torvalds
2021-12-09 21:47 ` [PATCH 2/6] cgroup: Allocate cgroup_file_ctx for kernfs_open_file->priv Tejun Heo
2021-12-10 17:53   ` Linus Torvalds [this message]
2021-12-10 18:38     ` Tejun Heo
2021-12-10 18:45       ` Linus Torvalds
2021-12-10 19:06         ` Tejun Heo
2021-12-10 19:14           ` Linus Torvalds
2021-12-09 21:47 ` [PATCH 3/6] cgroup: Use open-time cgroup namespace for process migration perm checks Tejun Heo
2021-12-09 21:47 ` [PATCH 4/6] selftests: cgroup: Make cg_create() use 0755 for permission instead of 0644 Tejun Heo
2021-12-09 21:47 ` [PATCH 5/6] selftests: cgroup: Test open-time credential usage for migration checks Tejun Heo
2021-12-09 21:47 ` [PATCH 6/6] selftests: cgroup: Test open-time cgroup namespace " Tejun Heo
2021-12-13 19:18 [PATCHSET v2 cgroup/for-5.16-fixes] cgroup: Use open-time creds and namespace for migration perm checks Tejun Heo
2021-12-13 19:18 ` [PATCH 2/6] cgroup: Allocate cgroup_file_ctx for kernfs_open_file->priv Tejun Heo
2021-12-13 19:29   ` Linus Torvalds
2021-12-13 19:56     ` Tejun Heo
2021-12-14 17:03   ` Michal Koutný

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:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='' \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \

* 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).