Re: [PATCH V8 1/2] cgroup/rstat: Avoid flushing if there is an ongoing overlapping flush

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 30/07/2024 20.54, Yosry Ahmed wrote:
[..]
+static inline void __cgroup_rstat_lock(struct cgroup *cgrp, int cpu_in_loop,
+                                      bool already_contended)
          __acquires(&cgroup_rstat_lock)
   {
-       bool contended;
+       bool locked = false;

-       contended = !spin_trylock_irq(&cgroup_rstat_lock);
-       if (contended) {
-               trace_cgroup_rstat_lock_contended(cgrp, cpu_in_loop, contended);
+       if (already_contended) /* Skip trylock if already contended */
+               locked = __cgroup_rstat_trylock(cgrp, cpu_in_loop);
Should this be the other way around?

I think it is correct, but I used it wrong in once place, in
cgroup_rstat_flush_hold(), as cgroup_rstat_trylock_flusher() returning
false doesn't mean it was already_contended, but that ongoing flusher
"skipped" (and waited for) a flush.  I need to correct this.

Something isn't adding up here as well. The comment says skip trylock
if already contended, then if already_contended is true we do a
trylock. Am I confusing myself here? 🙂

Your are correct. Thanks you for spelling it out for me!
I will send a V9 tomorrow, then deploy it to my prod experiment hosts,
and retest as I think my mistake here affects the prod results, as some
of the tracepoints gets skipped due to this.

Again thanks for catching this!!!
--Jesper




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]     [Monitors]

  Powered by Linux