Re: [PATCH v7 2/2] nvmet: add cgroup_id to charge namespace I/O to a cgroup

[Tejun Heo]

Hello, Peng.

On Tue, Sep 29, 2026 at 04:24:11PM -0700, Peng Yu wrote:
> Implementation:
> * Add a `cgroup_id` attribute under the nvmet namespace folder, e.g.:
>   /sys/kernel/config/nvmet/subsystems/nqn.2026-09.io.test01:bdev/namespaces/1/cgroup_id
> * We can write a cgroup id to it, then that cgroup will control
>   the IOs used by the namespace.

This doesn't follow the usual description format or content. Can you
write it as prose explaining why the change is needed and what it does,
along the lines of the scenario section in the cover letter?

> +CONFIGFS_ATTR(nvmet_ns_, cgroup_id);

Documentation/ABI/stable/configfs-nvmet has an entry for each namespace
attribute. Can you add one for cgroup_id? It should say that it takes a
cgroup2 ID, that 0 clears it, that it can only be changed while the
namespace is disabled, and that the namespace's I/Os are charged to the
cgroup's effective io css.

> +static inline void nvmet_blkcg_set_bio(struct nvmet_ns *ns, struct bio *bio)
> +{
> +	struct cgroup_subsys_state *css;
> +
> +	if (!ns->cgrp)
> +		return;
> +
> +	css = cgroup_get_e_css(ns->cgrp, &io_cgrp_subsys);
> +	bio_associate_blkg_from_css(bio, css);
> +	css_put(css);
> +}

bio_init() and bio_alloc() already associate the bio with the current
kthread's blkcg, so each of these bios gets associated twice. If the rw
and zone append paths wrapped their bio allocations in
nvmet_blkcg_begin()/end() like the discard and flush paths do, this
function wouldn't be needed and chained bios would be covered
automatically. Not a blocker either way.

Otherwise, from the cgroup side, this looks fine to me.

Thanks.

--
tejun