Re: [PATCH v2 0/4] cgroup/rdma: add rdma.peak and rdma.events[.local]

[Tejun Heo <tj@kernel.org>]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1511 bytes --]

Hello,

v1 points are fully addressed.  A few more on v2:

* In rdmacg_resource_set_max(), the new "rpool has peak/events" check
  uses `goto dev_err` to skip free_cg_rpool_locked().  It works
  because ret is still 0 at that point, but dev_err is the error
  label and this isn't an error path.  Restructure so the free is
  guarded by an if, or rename the label.

* By the end of patch 3, the rpool-keep predicate is five lines
  duplicated in uncharge_cg_locked() and rdmacg_resource_set_max().
  Worth extracting into a rpool_has_persistent_state() helper — a
  sixth counter later then changes one site, not two.

* Switching rdmacg_event_locked() from get_ to find_ avoids the
  spurious-rpool problem I raised in v1, but it also means
  ancestors of over_cg without a prior rpool for this device
  silently drop the hierarchical event.  Now that the rpool-keep
  check covers event counters, get_ + keep-alive would give full
  hierarchical coverage without the issue from v1 (rpools getting
  freed on the next uncharge).  The struct is small and rpool
  presence isn't user-observable.  Worth reconsidering — or, if you
  keep find_, note the caveat in the rdma.events documentation.

* Patch 3 also extends rdma.events with hierarchical alloc_fail
  but the commit message only describes rdma.events.local.  Mention
  the rdma.events change.

* In rdmacg_events_show() / rdmacg_events_local_show(), the
  `(s64)READ_ONCE(u64) ... %lld` pattern can drop the cast and use
  %llu.

Thanks.

--
tejun