On 7/9/24 20:07, Oleg Nesterov wrote:
Hi Tejun,
Thanks for looking at this, can you review this V2 patch from Pavel?
To me it makes sense even without 1/2 which I didn't even bother to
read. At least as a simple workaround for now.
They are kind of separate but without 1/2 this patch creates
another infinite loop, even though it's harder to hit and
is io_uring specific.
On 07/09, Tejun Heo wrote:
Hello,
On Tue, Jul 09, 2024 at 03:05:21PM +0100, Pavel Begunkov wrote:
-----------------------------------------------------------------------
Either way I have no idea whether a cgroup_task_frozen() task should
react to task_work_add(TWA_SIGNAL) or not.
Documentation/admin-guide/cgroup-v2.rst says
Writing "1" to the file causes freezing of the cgroup and all
descendant cgroups. This means that all belonging processes will
be stopped and will not run until the cgroup will be explicitly
unfrozen.
AFAICS this is not accurate, they can run but can't return to user-mode.
So I guess task_work_run() is fine.
IIUC it's a user facing doc, so maybe it's accurate enough from that
perspective. But I do agree that the semantics around task_work is
not exactly clear.
A good correctness test for cgroup freezer is whether it'd be safe to
snapshot and restore the tasks in the cgroup while frozen.
Well, I don't really understand what can snapshot/restore actually mean...
CRIU, I assume. I'll try it ...
I forgot everything about cgroup freezer and I am already sleeping, but even
if we forget about task_work_add/TIF_NOTIFY_SIGNAL/etc, afaics ptrace can
change the state of cgroup_task_frozen() task between snapshot and restore ?
... but I'm inclined to think the patch makes sense regardless,
we're replacing an infinite loop with wait-wake-execute-wait.
--
Pavel Begunkov
