On Thu, Aug 22, 2019 at 06:58:23AM -0700, Tejun Heo <tj@xxxxxxxxxx> wrote: > Nack for the whole series. Please stop mixing interface and locking > changes. A bit naïve question -- why to keep unused functions that may come handy, in the future, possibly? (I reckon there is no clear general answer, the two cases below illustrate that.) The reasons for ->css_rstat_flush() presence that I see: - it's the required extension point of rstat mechanism, - together with cgroup_rstat_lock it keeps a guarantee for potential users that they don't need to synchronize on their own when aggregating stats. I'm not so sure about cgroup_rstat_flush_irqsafe() though. That function is currently not used. Would anyone possibly need doing stats aggregation in IRQ context? I don't see the reason. Then the cgroup_rstat_lock could be taken without IRQs disabled as proposed, no? [1] The "[PATCH 4/4] cgroup: Acquire cgroup_rstat_lock with enabled interrupts" also moves the IRQs disabling to the per-cpu cgroup_rstat_cpu_lock. Honestly, I don't understand the reasoning. (Although, disabling IRQs with this lock may see some use if stat updates happened from within IRQ handlers.) Michal [1] Or do we need IRQs disabled because of cputime_adjust() obtaining consistent data (in cgroup_base_stat_cputime_show())? Then cgroup_rstat_flush_hold() shouldn't pass may_sleep = true.
Attachment:
signature.asc
Description: Digital signature
