From: Tejun Heo <tj@kernel.org>
To: Linus Torvalds <torvalds@linuxfoundation.org>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>,
Michal Koutny <mkoutny@suse.com>, Jens Axboe <axboe@kernel.dk>,
Kees Cook <keescook@chromium.org>,
Oleg Nesterov <oleg@redhat.com>,
Peter Zijlstra <peterz@infradead.org>,
Thomas Gleixner <tglx@linutronix.de>,
Jim Newsome <jnewsome@torproject.org>,
Alexey Gladkov <legion@kernel.org>,
Andy Lutomirski <luto@amacapital.net>,
Jann Horn <jannh@google.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Security Officers <security@kernel.org>,
Kernel Team <kernel-team@fb.com>
Subject: Re: [PATCH 2/6] cgroup: Allocate cgroup_file_ctx for kernfs_open_file->priv
Date: Mon, 13 Dec 2021 09:56:26 -1000 [thread overview]
Message-ID: <Ybelaqt8uiyN+DkW@slm.duckdns.org> (raw)
In-Reply-To: <CAHk-=wgQQhQakEgKeCcDp==wk__pu32=k-1m63prWx+XjzyhHg@mail.gmail.com>
Hello,
On Mon, Dec 13, 2021 at 11:29:36AM -0800, Linus Torvalds wrote:
> Looking at the patch, I mostly like the end result, although it
> highlights how odd that __cgroup_procs_start() thing is.
>
> I wonder if that 'started' flag could be removed, and the
> css_task_iter_start() thing could be done at allocation time?
>
> But I may just be confused. This is not a NAK, just a "that pattern
> looks slightly odd to me".
Yeah, it's a bit odd. So...
css_task_iters are stateful and a bit expensive to initialize, so we don't
want to do it from generic open path. However, we can add an open method for
cgroup_files and then do it from there.
However, while these files don't allow random seeking, they do allow seeking
to 0 to re-read the whole content, so the iteration start path needs to
handle both the initial read and subsequent repeating reads. While this can
be handled by starting iter on open and then always restarting it on
seq_start, that isn't ideal given the prior point. So, if we want to handle
it in the seq_start path, we need to distinguish the initial read and
re-reads anyway.
This may be neater if we start the iter from open and restart from seek so
that the seq_start method doesn't have to distinguish the two. However,
kernfs doesn't forward the seek event downwards (yet anyway).
So, there's a bit of awkwardness but it at least isn't gratuitous given the
surroundings.
Thanks.
--
tejun
next prev parent reply other threads:[~2021-12-13 19:56 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
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 1/6] cgroup: Use open-time credentials for process migraton " Tejun Heo
2021-12-14 17:03 ` Michal Koutný
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 [this message]
2021-12-14 17:03 ` Michal Koutný
2021-12-14 19:44 ` [PATCH v3 " Tejun Heo
2021-12-15 7:37 ` Michal Koutný
2021-12-16 9:22 ` [cgroup] 27183b4e07: WARNING:at_mm/slab.c:#___cache_free kernel test robot
2021-12-16 9:43 ` Michal Koutný
2021-12-13 19:18 ` [PATCH 3/6] cgroup: Use open-time cgroup namespace for process migration perm checks Tejun Heo
2021-12-14 17:04 ` Michal Koutný
2021-12-13 19:18 ` [PATCH 4/6] selftests: cgroup: Make cg_create() use 0755 for permission instead of 0644 Tejun Heo
2021-12-14 17:04 ` Michal Koutný
2021-12-13 19:18 ` [PATCH 5/6] selftests: cgroup: Test open-time credential usage for migration checks Tejun Heo
2021-12-14 17:04 ` Michal Koutný
2021-12-13 19:18 ` [PATCH 6/6] selftests: cgroup: Test open-time cgroup namespace " Tejun Heo
2021-12-14 17:04 ` Michal Koutný
2022-01-06 21:05 ` [PATCHSET v2 cgroup/for-5.16-fixes] cgroup: Use open-time creds and namespace for migration perm checks Tejun Heo
-- strict thread matches above, loose matches on Subject: below --
2021-12-09 21:47 [PATCHSET " Tejun Heo
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
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
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:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Ybelaqt8uiyN+DkW@slm.duckdns.org \
--to=tj@kernel.org \
--cc=axboe@kernel.dk \
--cc=ebiederm@xmission.com \
--cc=jannh@google.com \
--cc=jnewsome@torproject.org \
--cc=keescook@chromium.org \
--cc=kernel-team@fb.com \
--cc=legion@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@amacapital.net \
--cc=mkoutny@suse.com \
--cc=oleg@redhat.com \
--cc=peterz@infradead.org \
--cc=security@kernel.org \
--cc=tglx@linutronix.de \
--cc=torvalds@linuxfoundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* 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
all inboxes | Powered by JetHome®