cgroup: rstat: Cleanup flushing functions and locking

Now that the rstat lock is being re-acquired on every CPU iteration in
cgroup_rstat_flush_locked(), having the initially acquire the lock is
unnecessary and unclear.

Inline cgroup_rstat_flush_locked() into cgroup_rstat_flush() and move
the lock/unlock calls to the beginning and ending of the loop body to
make the critical section obvious.

cgroup_rstat_flush_hold/release() do not make much sense with the lock
being dropped and reacquired internally. Since it has no external
callers, remove it and explicitly acquire the lock in
cgroup_base_stat_cputime_show() instead.

This leaves the code with a single flushing function,
cgroup_rstat_flush().

Signed-off-by: Yosry Ahmed <yosry.ahmed@linux.dev>
Signed-off-by: Tejun Heo <tj@kernel.org>

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 8e7415c64ed1d5d6240fcacc3b8ab54f539e3a01..28e999f2c64213fa013fa338547b6ddfae0777d0 100644 (file)
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -690,8 +690,6 @@ static inline void cgroup_path_from_kernfs_id(u64 id, char *buf, size_t buflen)
  */
 void cgroup_rstat_updated(struct cgroup *cgrp, int cpu);
 void cgroup_rstat_flush(struct cgroup *cgrp);
-void cgroup_rstat_flush_hold(struct cgroup *cgrp);
-void cgroup_rstat_flush_release(struct cgroup *cgrp);
 
 /*
  * Basic resource stats.

diff --git a/kernel/cgroup/rstat.c b/kernel/cgroup/rstat.c
index 2c1053e83945e12d8406f37566b016b723d6dcae..7831978a26bb96796be62f6851d59b2dfeb8dcc9 100644 (file)
--- a/kernel/cgroup/rstat.c
+++ b/kernel/cgroup/rstat.c
@@ -299,17 +299,29 @@ static inline void __cgroup_rstat_unlock(struct cgroup *cgrp, int cpu_in_loop)
        spin_unlock_irq(&cgroup_rstat_lock);
 }
 
-/* see cgroup_rstat_flush() */
-static void cgroup_rstat_flush_locked(struct cgroup *cgrp)
-       __releases(&cgroup_rstat_lock) __acquires(&cgroup_rstat_lock)
+/**
+ * cgroup_rstat_flush - flush stats in @cgrp's subtree
+ * @cgrp: target cgroup
+ *
+ * Collect all per-cpu stats in @cgrp's subtree into the global counters
+ * and propagate them upwards.  After this function returns, all cgroups in
+ * the subtree have up-to-date ->stat.
+ *
+ * This also gets all cgroups in the subtree including @cgrp off the
+ * ->updated_children lists.
+ *
+ * This function may block.
+ */
+__bpf_kfunc void cgroup_rstat_flush(struct cgroup *cgrp)
 {
        int cpu;
 
-       lockdep_assert_held(&cgroup_rstat_lock);
-
+       might_sleep();
        for_each_possible_cpu(cpu) {
                struct cgroup *pos = cgroup_rstat_updated_list(cgrp, cpu);
 
+               /* Reacquire for each CPU to avoid disabling IRQs too long */
+               __cgroup_rstat_lock(cgrp, cpu);
                for (; pos; pos = pos->rstat_flush_next) {
                        struct cgroup_subsys_state *css;
 
@@ -322,64 +334,12 @@ static void cgroup_rstat_flush_locked(struct cgroup *cgrp)
                                css->ss->css_rstat_flush(css, cpu);
                        rcu_read_unlock();
                }
-
-               /* play nice and avoid disabling interrupts for a long time */
                __cgroup_rstat_unlock(cgrp, cpu);
                if (!cond_resched())
                        cpu_relax();
-               __cgroup_rstat_lock(cgrp, cpu);
        }
 }
 
-/**
- * cgroup_rstat_flush - flush stats in @cgrp's subtree
- * @cgrp: target cgroup
- *
- * Collect all per-cpu stats in @cgrp's subtree into the global counters
- * and propagate them upwards.  After this function returns, all cgroups in
- * the subtree have up-to-date ->stat.
- *
- * This also gets all cgroups in the subtree including @cgrp off the
- * ->updated_children lists.
- *
- * This function may block.
- */
-__bpf_kfunc void cgroup_rstat_flush(struct cgroup *cgrp)
-{
-       might_sleep();
-
-       __cgroup_rstat_lock(cgrp, -1);
-       cgroup_rstat_flush_locked(cgrp);
-       __cgroup_rstat_unlock(cgrp, -1);
-}
-
-/**
- * cgroup_rstat_flush_hold - flush stats in @cgrp's subtree and hold
- * @cgrp: target cgroup
- *
- * Flush stats in @cgrp's subtree and prevent further flushes.  Must be
- * paired with cgroup_rstat_flush_release().
- *
- * This function may block.
- */
-void cgroup_rstat_flush_hold(struct cgroup *cgrp)
-       __acquires(&cgroup_rstat_lock)
-{
-       might_sleep();
-       __cgroup_rstat_lock(cgrp, -1);
-       cgroup_rstat_flush_locked(cgrp);
-}
-
-/**
- * cgroup_rstat_flush_release - release cgroup_rstat_flush_hold()
- * @cgrp: cgroup used by tracepoint
- */
-void cgroup_rstat_flush_release(struct cgroup *cgrp)
-       __releases(&cgroup_rstat_lock)
-{
-       __cgroup_rstat_unlock(cgrp, -1);
-}
-
 int cgroup_rstat_init(struct cgroup *cgrp)
 {
        int cpu;
@@ -614,11 +574,12 @@ void cgroup_base_stat_cputime_show(struct seq_file *seq)
        struct cgroup_base_stat bstat;
 
        if (cgroup_parent(cgrp)) {
-               cgroup_rstat_flush_hold(cgrp);
+               cgroup_rstat_flush(cgrp);
+               __cgroup_rstat_lock(cgrp, -1);
                bstat = cgrp->bstat;
                cputime_adjust(&cgrp->bstat.cputime, &cgrp->prev_cputime,
                               &bstat.cputime.utime, &bstat.cputime.stime);
-               cgroup_rstat_flush_release(cgrp);
+               __cgroup_rstat_unlock(cgrp, -1);
        } else {
                root_cgroup_cputime(&bstat);
        }