On Wed, Jul 27, 2022 at 11:56 PM Yonghong Song [off-list ref] wrote:

On 7/22/22 1:33 PM, Hao Luo wrote:

quoted

On Fri, Jul 22, 2022 at 11:36 AM Kumar Kartikeya Dwivedi
[off-list ref] wrote:

quoted

On Fri, 22 Jul 2022 at 19:52, Yosry Ahmed [off-list ref] wrote:

<...>

quoted

quoted

quoted

+
+static int __cgroup_iter_seq_show(struct seq_file *seq,
+                                 struct cgroup_subsys_state *css, int in_stop);
+
+static void cgroup_iter_seq_stop(struct seq_file *seq, void *v)
+{
+       /* pass NULL to the prog for post-processing */
+       if (!v)
+               __cgroup_iter_seq_show(seq, NULL, true);
+       mutex_unlock(&cgroup_mutex);

I'm just curious, but would it be a good optimization (maybe in a
follow up) to move this mutex_unlock before the check on v? That
allows you to store/buffer some info you want to print as a compressed
struct in a map, then write the full text to the seq_file outside the
cgroup_mutex lock in the post-processing invocation.

It probably also allows you to walk the whole hierarchy, if one
doesn't want to run into seq_file buffer limit (or it can decide what
to print within the limit in the post processing invocation), or it
can use some out of band way (ringbuf, hashmap, etc.) to send the data
to userspace. But all of this can happen without holding cgroup_mutex
lock.

Thanks Kumar.

It sounds like an idea, but the key thing is not about moving
cgroup_mutex unlock before the check IMHO. The user can achieve
compression using the current infra. Compression could actually be
done in the bpf program. user can define and output binary content and
implement a userspace library to parse/decompress when reading out the
data.

Right mutex_unlock() can be moved to the beginning of the
function since the cgroup is not used in
   __cgroup_iter_seq_show(seq, NULL, true)

Ok. Will do.