Re: [PATCH v6 0/3] nvmet: add cgroup_id to charge namespace I/O to a cgroup

[Tejun Heo <tj@kernel.org>]

Hello, Peng.

The following is a Claude-generated review.

On Sun, Sep 27, 2026 at 11:14:14PM -0700, Peng Yu wrote:
> Scenario:
> * Create multiple nvmet subsystems/namespaces.
> * The namespaces are backed by different LVM logical volumes.
> * Some of the logical volumes share the same physical volumes.
> * The subsystems are exported to different users.
> * We should provide each user a specific iops/bps quota, thus a noisy
>   neighbor won't impact the performance of other logical volumes.

- 1/3: Can you explain why in the description? Per-IO users such as nvmet
  need the lookup to be O(1) instead of walking up the hierarchy.

  rebind_subsystems() moves root csses between hierarchies without
  updating e_css[]. This is fine as a root cgroup's e_css[] always points
  to init_css_set.subsys[], but that isn't obvious. Maybe note it in a
  comment in init_cgroup_root()?

  Can you also add a comment on the new e_css[] field like the fields
  around it? It should say what it points to (the css of the nearest
  ancestor including self which has the subsystem enabled) and that it's
  updated under cgroup_mutex and read under RCU.

- 2/3: This isn't needed with the change suggested for 3/3 below.
  Otherwise, cgroup_e_css() now has the same shape as cgroup_css() and
  could be a static inline in include/linux/cgroup.h instead of an export.

- 3/3: nvmet_blkcg_set_bio() calls bio_associate_blkg_from_css() under
  rcu_read_lock(). If the blkg doesn't exist yet, it grabs queue_lock
  inside the RCU section. f928145cbcb5 ("mm/page_io: don't nest queue_lock
  under rcu in bio_associate_blkg_from_page()") removed the same nesting
  from mm/page_io.c to prepare for protecting blkcg with blkcg_mutex
  instead of queue_lock. How about the following instead?

	css = cgroup_get_e_css(ns->cgrp, &io_cgrp_subsys);
	bio_associate_blkg_from_css(bio, css);
	css_put(css);

  With 1/3, this is O(1) too and it skips csses which are going offline
  the same way nvmet_blkcg_begin() does. It also only uses symbols which
  are already exported.

Thanks.

--
tejun