On 7/6/26 06:49, Mirian Shilakadze wrote:
Thanks, and fair point.

First the bound: the walk in proc_perms_start is over the registered proc tree
(the proc_dir_entry nodes under proc_root), not the per task /proc/<pid> dirs, 
so
it does not grow with the number of processes. Per node it is cheap too,
proc_perms_shown returns right away when the node has no map, which is the 
common
case, and kmapset_lookup only runs on nodes that have one and walks just that
node's own entries, sorted so it stops early. It is not a scan of all keys.

Agree, also all process entries are skipped and not checked at all, so it's 
fine.


I did not expect a container to read ve.proc_permissions at all, these are host
side cgroup control files, so I expected only the host to trigger this walk and
thought the atomic hold was fine. But while digging into it I discovered that a
container can read its own ve.proc_permissions and trigger the same walk, and 
the
existing ve.sysfs_permissions is the same. There is already a task to hide 
these ve
cgroup files inside containers, 
https://virtuozzo.atlassian.net/browse/VSTOR-120574,
which would take care of it. I discussed this with Pavel and we agreed to note 
it
as a known limitation for now.

If you think the atomic section is a problem even when only the host can reach 
it, I
am happy to rewrite proc_perms_start and proc_perms_next to drop and retake
proc_subdir_lock on each step, the way proc_readdir_de does, so the held 
section is
one node at a time.


It might be a problem - I checked what is happening in proc_perms_start and it 
does
iterate over all /proc entries (including all CT PIDs) trying to find the first
entry added in a map. This might be time-consuming for containers with many 
processes.

--
Best regards, Riabchun Vladimir
Linux Kernel Developer, Virtuozzo

_______________________________________________
Devel mailing list
[email protected]
https://lists.openvz.org/mailman/listinfo/devel

Reply via email to