[PATCH v6 0/3] nvmet: add cgroup_id to charge namespace I/O to a cgroup
Tejun Heo
tj at kernel.org
Mon Sep 28 11:24:41 PDT 2026
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
More information about the Linux-nvme
mailing list