[PATCH 0/7] memcg: account the remaining uncharged kernel allocations

[PATCH 0/7] memcg: account the remaining uncharged kernel allocations

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

Kernel memory that scales with user controlled input is supposed to be
charged to the memcg of the task that induces it, so that memory.max
contains the damage and the accounting is honest.  Sweeping the
interfaces that let unprivileged or delegated users drive kernel
allocations turned up seven more places where the charge is missing.
Every fix is one flag; the interesting part is who can pull the lever
and how much a pull buys.

An open sg fd reserves up to max_sectors worth of buffer, SG_MAX_QUEUE
(16) commands can be in flight per fd, and the fd count is bounded only
by RLIMIT_NOFILE.  udmabuf metadata costs about 4MB of SUnreclaim per
GB of buffer with size_limit_mb defaulting to INT_MAX.  vhost holds
roughly 150KB of resident kernel memory per SET_OWNER, and
/dev/vhost-net is handed to containers by device plugins.  A ftrace
instance allocates two full sets of per-cpu ring buffers the moment it
is created, about 1.375MB per cpu per set, gated by tracefs permissions
alone.  iommufd's hw_queue contiguity check allocates an array sized by
a u64 from userspace, frees it when the check fails, and can be
repeated in a tight loop.  io_uring still leaves the ring context, the
SQPOLL thread data, io-wq, the alloc cache arrays, the kbuf iovecs and
the registered eventfds uncharged.  The rsrc and tctx surfaces are
already charged by commit 075d5837bafc ("io_uring/rsrc: charge buffer
arrays and nodes to memcg") and commit 0b34e7b20a48 ("io_uring: charge
tctx nodes and personalities"); patches 6-7 pick up what those did not.

Verified on next-20261104 under qemu, same config with and without the
series, everything builtin: eight sg fds with 1MB reserved buffers
charge 8MB with the series and nothing without; resizing the global
trace buffer to 16MB per cpu charges 67MB against 0 before; four 512MB
udmabuf buffers charge around 8MB of metadata; sixteen vhost SET_OWNER
calls charge 4MB; 1000 io_uring rings charge 7.6MB on top of what the
in-tree charging already accounts.  All slab allocations in the series
are served by kmalloc caches, so they keep charging under the
SLAB_MAY_ACCOUNT opt-in rule queued in slab/for-next; the tracing patch
accounts page allocations, which are not affected by it.

Two in-flight series overlap but do not collide.  The dma-heap explicit
memcg charging series moves dma-buf data page charging to an explicit
path and removes __GFP_ACCOUNT from the system heap, while patch 2 here
accounts udmabuf metadata only, which that series does not see; if it
lands first, the system heap reference in patch 2 wants dropping.  The
io_uring patches sit on top of the charging Mason and Jens already
merged, and cite those commits as the in-tree predecessors.

I am sending these in one thread so the class is visible in one place.
If any of you would rather take your piece through your own tree, an
ack is all I need and I will split the series.

Tao Cui (7):
  scsi: sg and st: account the command buffer memory to memcg
  dma-buf/udmabuf: account the page array metadata to memcg
  vhost: account the per-device kernel allocations to memcg
  tracing: account ring buffer pages and trace_array metadata to memcg
  iommufd: account the hw_queue page array to memcg
  io_uring: account the ring context and SQPOLL thread data to memcg
  io_uring: account the remaining per-object allocations to memcg

 drivers/dma-buf/udmabuf.c      |  7 ++++---
 drivers/iommu/iommufd/viommu.c |  3 ++-
 drivers/scsi/sg.c              | 23 ++++++++++++++++++++---
 drivers/scsi/st.c              |  7 ++++---
 drivers/vhost/net.c            | 14 ++++++++------
 drivers/vhost/vhost.c          | 16 ++++++++++------
 drivers/vhost/vsock.c          |  5 +++--
 io_uring/alloc_cache.c         |  3 ++-
 io_uring/eventfd.c             |  2 +-
 io_uring/io-wq.c               |  4 ++--
 io_uring/io_uring.c            |  2 +-
 io_uring/kbuf.c                |  2 +-
 io_uring/rsrc.c                |  3 ++-
 io_uring/sqpoll.c              |  2 +-
 io_uring/tctx.c                |  4 ++--
 kernel/trace/ring_buffer.c     |  8 +++++++-
 kernel/trace/trace.c           |  6 +++---
 17 files changed, 73 insertions(+), 38 deletions(-)

-- 
2.43.0

[PATCH 1/7] scsi: sg and st: account the command buffer memory to memcg

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

The sg data buffers are unaccounted: the per-fd reserved buffer, the
scatter-gather page array, and the order-0..N pages behind it, plus the
per-device tape buffer in st.  All of it is user controllable.  The
reserved size is capped only by the device max_sectors, SG_MAX_QUEUE
(16) commands can be in flight per fd, and the fd count is bounded by
RLIMIT_NOFILE.  Nothing is charged to the memcg of the process driving
it.  The September 2026 fixes in sg_build_indirect reshaped the element
sizing and left the accounting alone.

Mark these allocations __GFP_ACCOUNT so the driving process pays for
the memory it induces.

Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 drivers/scsi/sg.c | 23 ++++++++++++++++++++---
 drivers/scsi/st.c |  7 ++++---
 2 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 6551f1284d44..9c1414479b21 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -911,6 +911,19 @@ static int put_compat_request_table(struct compat_sg_req_info __user *o,
 }
 #endif
 
+/*
+ * The sg passthrough of the blktrace ioctls has the same exposure as the
+ * block device path: holding the node is the only requirement today, and
+ * a created trace outlives the fd.  Must match the gate in
+ * blk_trace_ioctl().
+ */
+static bool sg_is_blktrace_cmd(unsigned int cmd)
+{
+	return cmd == BLKTRACESETUP || cmd == BLKTRACESETUP2 ||
+	       cmd == BLKTRACESTART || cmd == BLKTRACESTOP ||
+	       cmd == BLKTRACETEARDOWN;
+}
+
 static long
 sg_ioctl_common(struct file *filp, Sg_device *sdp, Sg_fd *sfp,
 		unsigned int cmd_in, void __user *p)
@@ -920,6 +933,9 @@ sg_ioctl_common(struct file *filp, Sg_device *sdp, Sg_fd *sfp,
 	Sg_request *srp;
 	unsigned long iflags;
 
+	if (sg_is_blktrace_cmd(cmd_in) && !capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
 	SCSI_LOG_TIMEOUT(3, sg_printk(KERN_INFO, sdp,
 				   "sg_ioctl: cmd=0x%x\n", (int) cmd_in));
 	read_only = (O_RDWR != (filp->f_flags & O_ACCMODE));
@@ -1829,7 +1845,7 @@ static int
 sg_build_sgat(Sg_scatter_hold * schp, const Sg_fd * sfp, int tablesize)
 {
 	int sg_bufflen = tablesize * sizeof(struct page *);
-	gfp_t gfp_flags = GFP_ATOMIC | __GFP_NOWARN;
+	gfp_t gfp_flags = GFP_ATOMIC | __GFP_NOWARN | __GFP_ACCOUNT;
 
 	schp->pages = kzalloc(sg_bufflen, gfp_flags);
 	if (!schp->pages)
@@ -1844,7 +1860,8 @@ sg_build_indirect(Sg_scatter_hold * schp, Sg_fd * sfp, int buff_size)
 	int ret_sz = 0, i, k, rem_sz, num, mx_sc_elems;
 	int sg_tablesize = sfp->parentdp->sg_tablesize;
 	int blk_size = buff_size, order;
-	gfp_t gfp_mask = GFP_ATOMIC | __GFP_COMP | __GFP_NOWARN | __GFP_ZERO;
+	gfp_t gfp_mask = GFP_ATOMIC | __GFP_COMP | __GFP_NOWARN |
+			 __GFP_ACCOUNT | __GFP_ZERO;
 
 	if (blk_size < 0)
 		return -EFAULT;
@@ -2134,7 +2151,7 @@ sg_add_sfp(Sg_device * sdp)
 	unsigned long iflags;
 	int bufflen;
 
-	sfp = kzalloc_obj(*sfp, GFP_ATOMIC | __GFP_NOWARN);
+	sfp = kzalloc_obj(*sfp, GFP_ATOMIC | __GFP_NOWARN | __GFP_ACCOUNT);
 	if (!sfp)
 		return ERR_PTR(-ENOMEM);
 
diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index e9d41e3cc2dd..b92633ab2757 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -4074,7 +4074,7 @@ static struct st_buffer *new_tape_buffer(int max_sg)
 {
 	struct st_buffer *tb;
 
-	tb = kzalloc_obj(struct st_buffer);
+	tb = kzalloc_obj(struct st_buffer, GFP_KERNEL_ACCOUNT);
 	if (!tb) {
 		printk(KERN_NOTICE "st: Can't allocate new tape buffer.\n");
 		return NULL;
@@ -4083,7 +4083,8 @@ static struct st_buffer *new_tape_buffer(int max_sg)
 	tb->use_sg = max_sg;
 	tb->buffer_size = 0;
 
-	tb->reserved_pages = kzalloc_objs(struct page *, max_sg);
+	tb->reserved_pages = kzalloc_objs(struct page *, max_sg,
+					  GFP_KERNEL_ACCOUNT);
 	if (!tb->reserved_pages) {
 		kfree(tb);
 		return NULL;
@@ -4109,7 +4110,7 @@ static int enlarge_buffer(struct st_buffer * STbuffer, int new_size)
 
 	max_segs = STbuffer->use_sg;
 
-	priority = GFP_KERNEL | __GFP_NOWARN;
+	priority = GFP_KERNEL | __GFP_NOWARN | __GFP_ACCOUNT;
 
 	if (STbuffer->cleared)
 		priority |= __GFP_ZERO;
-- 
2.43.0

[PATCH 2/7] dma-buf/udmabuf: account the page array metadata to memcg

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

udmabuf pins the pages of a memfd and hands them out as a dma-buf.  The
data pages keep the accounting of the memfd they were instantiated in,
but the bookkeeping around them does not: the page pointer array and
the pinned_folios array, both sized by the buffer page count, plus the
transient folios array sized by the memfd folio count, are plain
GFP_KERNEL.  A 1GB buffer costs about 4MB of uncharged SUnreclaim
memory, and size_limit_mb defaults to INT_MAX.

Charge the three kvmalloc allocations so the creator pays for the
metadata it induces.  The system heap in the same subsystem already
accounts its buffer allocation; the data pages here stay with the
memfd.

Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 drivers/dma-buf/udmabuf.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
index df6dd0046242..40182c82df6b 100644
--- a/drivers/dma-buf/udmabuf.c
+++ b/drivers/dma-buf/udmabuf.c
@@ -190,11 +190,12 @@ static void unpin_all_folios(struct udmabuf *ubuf)
 
 static __always_inline int init_udmabuf(struct udmabuf *ubuf, pgoff_t pgcnt)
 {
-	ubuf->pages = kvmalloc_objs(*ubuf->pages, pgcnt);
+	ubuf->pages = kvmalloc_objs(*ubuf->pages, pgcnt, GFP_KERNEL_ACCOUNT);
 	if (!ubuf->pages)
 		return -ENOMEM;
 
-	ubuf->pinned_folios = kvmalloc_objs(*ubuf->pinned_folios, pgcnt);
+	ubuf->pinned_folios = kvmalloc_objs(*ubuf->pinned_folios, pgcnt,
+					    GFP_KERNEL_ACCOUNT);
 	if (!ubuf->pinned_folios)
 		return -ENOMEM;
 
@@ -387,7 +388,7 @@ static long udmabuf_create(struct miscdevice *device,
 	if (ret)
 		goto err;
 
-	folios = kvmalloc_objs(*folios, max_nr_folios);
+	folios = kvmalloc_objs(*folios, max_nr_folios, GFP_KERNEL_ACCOUNT);
 	if (!folios) {
 		ret = -ENOMEM;
 		goto err;
-- 
2.43.0

[PATCH 3/7] vhost: account the per-device kernel allocations to memcg

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

Everything except the worker struct is allocated with plain GFP_KERNEL:
the per-vq iovec and log arrays at VHOST_SET_OWNER, the memory table at
VHOST_SET_MEM_TABLE whose size follows the user supplied region count,
the pending message nodes, and the vhost_net/vhost_vsock device structs
with their fallback to vmalloc.  The worker has been accounted since the
vhost_task conversion.

A container that can open /dev/vhost-net or /dev/vhost-vsock, which
device plugins hand out with passthrough, holds roughly 150KB of
resident kernel memory per device after SET_OWNER.  Charge the rest the
same way the worker already is.

Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 drivers/vhost/net.c   | 14 ++++++++------
 drivers/vhost/vhost.c | 16 ++++++++++------
 drivers/vhost/vsock.c |  5 +++--
 3 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 4570a4ffbab2..bab6597c696d 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -249,7 +249,7 @@ vhost_net_ubuf_alloc(struct vhost_virtqueue *vq, bool zcopy)
 	/* No zero copy backend? Nothing to count. */
 	if (!zcopy)
 		return NULL;
-	ubufs = kmalloc_obj(*ubufs);
+	ubufs = kmalloc_obj(*ubufs, GFP_KERNEL_ACCOUNT);
 	if (!ubufs)
 		return ERR_PTR(-ENOMEM);
 	atomic_set(&ubufs->refcount, 1);
@@ -302,7 +302,8 @@ static int vhost_net_set_ubuf_info(struct vhost_net *n)
 		if (!zcopy)
 			continue;
 		n->vqs[i].ubuf_info =
-			kmalloc_objs(*n->vqs[i].ubuf_info, UIO_MAXIOV);
+			kmalloc_objs(*n->vqs[i].ubuf_info, UIO_MAXIOV,
+				     GFP_KERNEL_ACCOUNT);
 		if  (!n->vqs[i].ubuf_info)
 			goto err;
 	}
@@ -1344,17 +1345,18 @@ static int vhost_net_open(struct inode *inode, struct file *f)
 	struct xdp_buff *xdp;
 	int i;
 
-	n = kvmalloc_obj(*n, GFP_KERNEL | __GFP_RETRY_MAYFAIL);
+	n = kvmalloc_obj(*n, GFP_KERNEL | __GFP_RETRY_MAYFAIL |
+			 __GFP_ACCOUNT);
 	if (!n)
 		return -ENOMEM;
-	vqs = kmalloc_objs(*vqs, VHOST_NET_VQ_MAX);
+	vqs = kmalloc_objs(*vqs, VHOST_NET_VQ_MAX, GFP_KERNEL_ACCOUNT);
 	if (!vqs) {
 		kvfree(n);
 		return -ENOMEM;
 	}
 
 	queue = kmalloc_array(VHOST_NET_BATCH, sizeof(void *),
-			      GFP_KERNEL);
+			      GFP_KERNEL_ACCOUNT);
 	if (!queue) {
 		kfree(vqs);
 		kvfree(n);
@@ -1362,7 +1364,7 @@ static int vhost_net_open(struct inode *inode, struct file *f)
 	}
 	n->vqs[VHOST_NET_VQ_RX].rxq.queue = queue;
 
-	xdp = kmalloc_objs(*xdp, VHOST_NET_BATCH);
+	xdp = kmalloc_objs(*xdp, VHOST_NET_BATCH, GFP_KERNEL_ACCOUNT);
 	if (!xdp) {
 		kfree(vqs);
 		kvfree(n);
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index ea127af58ada..d1274106ad5d 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -526,11 +526,14 @@ static long vhost_dev_alloc_iovecs(struct vhost_dev *dev)
 
 	for (i = 0; i < dev->nvqs; ++i) {
 		vq = dev->vqs[i];
-		vq->indirect = kmalloc_objs(*vq->indirect, UIO_MAXIOV);
-		vq->log = kmalloc_objs(*vq->log, dev->iov_limit);
-		vq->heads = kmalloc_objs(*vq->heads, dev->iov_limit);
+		vq->indirect = kmalloc_objs(*vq->indirect, UIO_MAXIOV,
+					    GFP_KERNEL_ACCOUNT);
+		vq->log = kmalloc_objs(*vq->log, dev->iov_limit,
+				       GFP_KERNEL_ACCOUNT);
+		vq->heads = kmalloc_objs(*vq->heads, dev->iov_limit,
+					 GFP_KERNEL_ACCOUNT);
 		vq->nheads = kmalloc_array(dev->iov_limit, sizeof(*vq->nheads),
-					   GFP_KERNEL);
+					   GFP_KERNEL_ACCOUNT);
 		if (!vq->indirect || !vq->log || !vq->heads || !vq->nheads)
 			goto err_nomem;
 	}
@@ -2053,7 +2056,8 @@ static long vhost_set_memory(struct vhost_dev *d, struct vhost_memory __user *m)
 		return -E2BIG;
 	if (max_iotlb_entries <= 0)
 		return -EINVAL;
-	newmem = kvzalloc_flex(*newmem, regions, mem.nregions);
+	newmem = kvzalloc_flex(*newmem, regions, mem.nregions,
+			       GFP_KERNEL_ACCOUNT);
 	if (!newmem)
 		return -ENOMEM;
 
@@ -3405,7 +3409,7 @@ EXPORT_SYMBOL_GPL(vhost_disable_notify);
 struct vhost_msg_node *vhost_new_msg(struct vhost_virtqueue *vq, int type)
 {
 	/* Make sure all padding within the structure is initialized. */
-	struct vhost_msg_node *node = kzalloc_obj(*node);
+	struct vhost_msg_node *node = kzalloc_obj(*node, GFP_KERNEL_ACCOUNT);
 	if (!node)
 		return NULL;
 
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index 0cb87971d749..600daeb05e7b 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -721,11 +721,12 @@ static int vhost_vsock_dev_open(struct inode *inode, struct file *file)
 	/* This struct is large and allocation could fail, fall back to vmalloc
 	 * if there is no other way.
 	 */
-	vsock = kvmalloc_obj(*vsock, GFP_KERNEL | __GFP_RETRY_MAYFAIL);
+	vsock = kvmalloc_obj(*vsock, GFP_KERNEL | __GFP_RETRY_MAYFAIL |
+			     __GFP_ACCOUNT);
 	if (!vsock)
 		return -ENOMEM;
 
-	vqs = kmalloc_objs(*vqs, ARRAY_SIZE(vsock->vqs));
+	vqs = kmalloc_objs(*vqs, ARRAY_SIZE(vsock->vqs), GFP_KERNEL_ACCOUNT);
 	if (!vqs) {
 		ret = -ENOMEM;
 		goto out;
-- 
2.43.0

[PATCH 4/7] tracing: account ring buffer pages and trace_array metadata to memcg

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

Creating an instance immediately allocates two full sets of per-cpu
ring buffers, array buffer plus snapshot, about 1.375MB per cpu per set
on the default config, plus the trace_array and its name.  None of it
is charged: neither trace.c nor ring_buffer.c passes __GFP_ACCOUNT,
while user_events in the same directory accounts everything it
allocates.

Instance creation is currently gated by tracefs file permissions
alone, so a
rootful container with the host tracefs bound in can create instances
until the host runs out of memory.  Charge the ring buffer pages and
the trace_array metadata to the creating cgroup.

Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 kernel/trace/ring_buffer.c | 8 +++++++-
 kernel/trace/trace.c       | 6 +++---
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 1fd28a72e6ee..676c059c112c 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -441,7 +441,13 @@ static struct buffer_data_page *alloc_cpu_data(int cpu, int order)
 	 * gracefully without invoking oom-killer and the system is not
 	 * destabilized.
 	 */
-	mflags = GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_COMP | __GFP_ZERO;
+	/*
+	 * The subbuffers are charged to the memcg of whoever caused the
+	 * allocation: the creator of a trace instance, or root at boot for
+	 * the global trace.
+	 */
+	mflags = GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_COMP | __GFP_ZERO |
+		  __GFP_ACCOUNT;
 
 	page = alloc_pages_node(cpu_to_node(cpu), mflags, order);
 	if (!page)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index e8c4efa8107b..04b60b9cc4f5 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -8693,11 +8693,11 @@ trace_array_create_systems(const char *name, const char *systems,
 	int ret;
 
 	ret = -ENOMEM;
-	tr = kzalloc_obj(*tr);
+	tr = kzalloc_obj(*tr, GFP_KERNEL_ACCOUNT);
 	if (!tr)
 		return ERR_PTR(ret);
 
-	tr->name = kstrdup(name, GFP_KERNEL);
+	tr->name = kstrdup(name, GFP_KERNEL_ACCOUNT);
 	if (!tr->name)
 		goto out_free_tr;
 
@@ -8708,7 +8708,7 @@ trace_array_create_systems(const char *name, const char *systems,
 		goto out_free_tr;
 
 	if (systems) {
-		tr->system_names = kstrdup_const(systems, GFP_KERNEL);
+		tr->system_names = kstrdup_const(systems, GFP_KERNEL_ACCOUNT);
 		if (!tr->system_names)
 			goto out_free_tr;
 	}
-- 
2.43.0

[PATCH 5/7] iommufd: account the hw_queue page array to memcg

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

IOMMUFD_CMD_HW_QUEUE_ALLOC pins up to max_npages pages just to check
that a user supplied queue region is physically contiguous, and the
page array for that check is plain GFP_KERNEL.  The size comes from
cmd->length, a u64 that is only overflow checked, and the array is
freed again as soon as the contiguity check fails, so the allocation
can be repeated concurrently: a 1GB region costs about 2MB of
uncharged memory per attempt.

Every other iommufd allocation that scales with user input is already
charged, and the pinned pages charge the mapper at map time.  Charge
this array like the rest.

Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 drivers/iommu/iommufd/viommu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/iommufd/viommu.c b/drivers/iommu/iommufd/viommu.c
index f7951057a1e5..ab17e190da49 100644
--- a/drivers/iommu/iommufd/viommu.c
+++ b/drivers/iommu/iommufd/viommu.c
@@ -330,7 +330,8 @@ iommufd_hw_queue_alloc_phys(struct iommu_hw_queue_alloc *cmd,
 	 * Use kvcalloc() to avoid memory fragmentation for a large page array.
 	 * Set __GFP_NOWARN to avoid syzkaller blowups
 	 */
-	pages = kvzalloc_objs(*pages, max_npages, GFP_KERNEL | __GFP_NOWARN);
+	pages = kvzalloc_objs(*pages, max_npages,
+			      GFP_KERNEL | __GFP_NOWARN | __GFP_ACCOUNT);
 	if (!pages)
 		return ERR_PTR(-ENOMEM);
 
-- 
2.43.0

[PATCH 6/7] io_uring: account the ring context and SQPOLL thread data to memcg

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

The io_ring_ctx and the SQPOLL io_sqd are the last per-ring allocations
made with plain GFP_KERNEL.  The larger surfaces around them are
already charged: io_mapped_ubuf and io_rsrc_node since commit
075d5837bafc ("io_uring/rsrc: charge buffer arrays and nodes to memcg"),
the tctx node and its XArrays since commit 0b34e7b20a48 ("io_uring:
charge tctx nodes and personalities").

Charge the context and the SQPOLL thread data the same way.

Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 io_uring/io_uring.c | 2 +-
 io_uring/rsrc.c     | 3 ++-
 io_uring/sqpoll.c   | 2 +-
 io_uring/tctx.c     | 4 ++--
 4 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 03724419c4e1..d39cdde49a9f 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -229,7 +229,7 @@ static __cold struct io_ring_ctx *io_ring_ctx_alloc(struct io_uring_params *p)
 	int hash_bits;
 	bool ret;
 
-	ctx = kzalloc_obj(*ctx);
+	ctx = kzalloc_obj(*ctx, GFP_KERNEL_ACCOUNT);
 	if (!ctx)
 		return NULL;
 
diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
index e0345c3e764e..9a971c983686 100644
--- a/io_uring/rsrc.c
+++ b/io_uring/rsrc.c
@@ -825,7 +825,8 @@ static bool io_coalesce_buffer(struct page ***pages, int *nr_pages,
 	unsigned i, j;
 
 	/* Store head pages only*/
-	new_array = kvmalloc_objs(struct page *, nr_folios);
+	new_array = kvmalloc_objs(struct page *, nr_folios,
+				  GFP_KERNEL_ACCOUNT);
 	if (!new_array)
 		return false;
 
diff --git a/io_uring/sqpoll.c b/io_uring/sqpoll.c
index d387488db2de..b82d3a285337 100644
--- a/io_uring/sqpoll.c
+++ b/io_uring/sqpoll.c
@@ -172,7 +172,7 @@ static struct io_sq_data *io_get_sq_data(struct io_uring_params *p,
 			return sqd;
 	}
 
-	sqd = kzalloc_obj(*sqd);
+	sqd = kzalloc_obj(*sqd, GFP_KERNEL_ACCOUNT);
 	if (!sqd)
 		return ERR_PTR(-ENOMEM);
 
diff --git a/io_uring/tctx.c b/io_uring/tctx.c
index b4fecea3ea47..6673393100ec 100644
--- a/io_uring/tctx.c
+++ b/io_uring/tctx.c
@@ -23,7 +23,7 @@ static struct io_wq *io_init_wq_offload(struct io_ring_ctx *ctx,
 	mutex_lock(&ctx->uring_lock);
 	hash = ctx->hash_map;
 	if (!hash) {
-		hash = kzalloc_obj(*hash);
+		hash = kzalloc_obj(*hash, GFP_KERNEL_ACCOUNT);
 		if (!hash) {
 			mutex_unlock(&ctx->uring_lock);
 			return ERR_PTR(-ENOMEM);
@@ -85,7 +85,7 @@ __cold struct io_uring_task *io_uring_alloc_task_context(struct task_struct *tas
 	struct io_uring_task *tctx;
 	int ret;
 
-	tctx = kzalloc_obj(*tctx);
+	tctx = kzalloc_obj(*tctx, GFP_KERNEL_ACCOUNT);
 	if (unlikely(!tctx))
 		return ERR_PTR(-ENOMEM);
 
-- 
2.43.0

[PATCH 7/7] io_uring: account the remaining per-object allocations to memcg

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

A handful of io_uring allocations still scale with user controlled
counts on plain GFP_KERNEL: the per-ring io-wq and its workers, the
alloc cache entry arrays, the provided buffer peek and import iovec
arrays, and the registered eventfd contexts.  The tctx side is covered
by commit 0b34e7b20a48 ("io_uring: charge tctx nodes and
personalities").

Charge what is left.

Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 io_uring/alloc_cache.c | 3 ++-
 io_uring/eventfd.c     | 2 +-
 io_uring/io-wq.c       | 4 ++--
 io_uring/kbuf.c        | 2 +-
 4 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/io_uring/alloc_cache.c b/io_uring/alloc_cache.c
index 58423888b736..f41df1878633 100644
--- a/io_uring/alloc_cache.c
+++ b/io_uring/alloc_cache.c
@@ -22,7 +22,8 @@ bool io_alloc_cache_init(struct io_alloc_cache *cache,
 			 unsigned max_nr, unsigned int size,
 			 unsigned int init_bytes)
 {
-	cache->entries = kvmalloc_array(max_nr, sizeof(void *), GFP_KERNEL);
+	cache->entries = kvmalloc_array(max_nr, sizeof(void *),
+				      GFP_KERNEL_ACCOUNT);
 	if (!cache->entries)
 		return true;
 
diff --git a/io_uring/eventfd.c b/io_uring/eventfd.c
index 63fe6e5d79ba..52f5d5b85f30 100644
--- a/io_uring/eventfd.c
+++ b/io_uring/eventfd.c
@@ -132,7 +132,7 @@ int io_eventfd_register(struct io_ring_ctx *ctx, void __user *arg,
 	if (copy_from_user(&fd, fds, sizeof(*fds)))
 		return -EFAULT;
 
-	ev_fd = kmalloc_obj(*ev_fd);
+	ev_fd = kmalloc_obj(*ev_fd, GFP_KERNEL_ACCOUNT);
 	if (!ev_fd)
 		return -ENOMEM;
 
diff --git a/io_uring/io-wq.c b/io_uring/io-wq.c
index 92df54c6f50c..98f3aa378baf 100644
--- a/io_uring/io-wq.c
+++ b/io_uring/io-wq.c
@@ -981,7 +981,7 @@ static bool create_io_worker(struct io_wq *wq, struct io_wq_acct *acct)
 
 	__set_current_state(TASK_RUNNING);
 
-	worker = kzalloc_obj(*worker);
+	worker = kzalloc_obj(*worker, GFP_KERNEL_ACCOUNT);
 	if (!worker) {
 fail:
 		atomic_dec(&acct->nr_running);
@@ -1358,7 +1358,7 @@ struct io_wq *io_wq_create(unsigned bounded, struct io_wq_data *data)
 	if (WARN_ON_ONCE(!bounded))
 		return ERR_PTR(-EINVAL);
 
-	wq = kzalloc_obj(struct io_wq);
+	wq = kzalloc_obj(struct io_wq, GFP_KERNEL_ACCOUNT);
 	if (!wq)
 		return ERR_PTR(-ENOMEM);
 
diff --git a/io_uring/kbuf.c b/io_uring/kbuf.c
index abf44f5dba26..bc6e552a91dd 100644
--- a/io_uring/kbuf.c
+++ b/io_uring/kbuf.c
@@ -288,7 +288,7 @@ static int io_ring_buffers_peek(struct io_kiocb *req, struct buf_sel_arg *arg,
 	 * a speculative peek operation.
 	 */
 	if (arg->mode & KBUF_MODE_EXPAND && nr_avail > nr_iovs && arg->max_len) {
-		iov = kmalloc_objs(struct iovec, nr_avail);
+		iov = kmalloc_objs(struct iovec, nr_avail, GFP_KERNEL_ACCOUNT);
 		if (unlikely(!iov))
 			return -ENOMEM;
 		arg->iovs = iov;
-- 
2.43.0

Re: [PATCH 2/7] dma-buf/udmabuf: account the page array metadata to memcg

[Christian König]

On 11/6/26 14:18, Tao Cui wrote:
> From: Tao Cui <cuitao@kylinos.cn>
> 
> udmabuf pins the pages of a memfd and hands them out as a dma-buf.  The
> data pages keep the accounting of the memfd they were instantiated in,
> but the bookkeeping around them does not: the page pointer array and
> the pinned_folios array, both sized by the buffer page count, plus the
> transient folios array sized by the memfd folio count, are plain
> GFP_KERNEL.  A 1GB buffer costs about 4MB of uncharged SUnreclaim
> memory, and size_limit_mb defaults to INT_MAX.
> 
> Charge the three kvmalloc allocations so the creator pays for the
> metadata it induces.  The system heap in the same subsystem already
> accounts its buffer allocation; the data pages here stay with the
> memfd.

T.J. needs to take a look here, but I fear that this might break uAPI.

A lot of the DMA-buf resources are intentionally not charged against memcg.

Regards,
Christian.

> 
> Signed-off-by: Tao Cui <cuitao@kylinos.cn>
> ---
>  drivers/dma-buf/udmabuf.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
> index df6dd0046242..40182c82df6b 100644
> --- a/drivers/dma-buf/udmabuf.c
> +++ b/drivers/dma-buf/udmabuf.c
> @@ -190,11 +190,12 @@ static void unpin_all_folios(struct udmabuf *ubuf)
> 
>  static __always_inline int init_udmabuf(struct udmabuf *ubuf, pgoff_t pgcnt)
>  {
> -       ubuf->pages = kvmalloc_objs(*ubuf->pages, pgcnt);
> +       ubuf->pages = kvmalloc_objs(*ubuf->pages, pgcnt, GFP_KERNEL_ACCOUNT);
>         if (!ubuf->pages)
>                 return -ENOMEM;
> 
> -       ubuf->pinned_folios = kvmalloc_objs(*ubuf->pinned_folios, pgcnt);
> +       ubuf->pinned_folios = kvmalloc_objs(*ubuf->pinned_folios, pgcnt,
> +                                           GFP_KERNEL_ACCOUNT);
>         if (!ubuf->pinned_folios)
>                 return -ENOMEM;
> 
> @@ -387,7 +388,7 @@ static long udmabuf_create(struct miscdevice *device,
>         if (ret)
>                 goto err;
> 
> -       folios = kvmalloc_objs(*folios, max_nr_folios);
> +       folios = kvmalloc_objs(*folios, max_nr_folios, GFP_KERNEL_ACCOUNT);
>         if (!folios) {
>                 ret = -ENOMEM;
>                 goto err;
> --
> 2.43.0
> 

Re: [PATCH 2/7] dma-buf/udmabuf: account the page array metadata to memcg

[T.J. Mercier]

On Fri, Nov 6, 2026 at 5:23 AM Christian König <christian.koenig@amd.com> wrote:
>
> On 11/6/26 14:18, Tao Cui wrote:
> > From: Tao Cui <cuitao@kylinos.cn>
> >
> > udmabuf pins the pages of a memfd and hands them out as a dma-buf.  The
> > data pages keep the accounting of the memfd they were instantiated in,
> > but the bookkeeping around them does not: the page pointer array and
> > the pinned_folios array, both sized by the buffer page count, plus the
> > transient folios array sized by the memfd folio count, are plain
> > GFP_KERNEL.  A 1GB buffer costs about 4MB of uncharged SUnreclaim
> > memory, and size_limit_mb defaults to INT_MAX.
> >
> > Charge the three kvmalloc allocations so the creator pays for the
> > metadata it induces.  The system heap in the same subsystem already
> > accounts its buffer allocation;

Only if the mem_accounting param is set to true. We (Android)
currently depend on it being disabled because system heap buffers
don't have a single owner for their entire lifetime (ownership often
changes immediately after they are allocated), and the kernel doesn't
have the ability to switch who is charged for the buffer memory quite
yet. Albert is working hard on it at the moment.

Could you update the message to note the mem_accounting param pls?

> > the data pages here stay with the
> > memfd.

memfd / udmabuf is a little special because it uses shmem for the
backing store, which is already tracked by memcg under all conditions
(except hugetlb). We don't currently use udmabuf, so the lack of memcg
charge transfer for udmabuf is not an issue. But once we get charge
transfer working for the system heap it'd be good to follow up with
udmabuf.

> T.J. needs to take a look here, but I fear that this might break uAPI.

Thanks for looping me in Christian. The buffer allocations cost
gigabytes, but these metadata arrays are only 4MB per 1GB of buffer,
so I don't expect charging them to break anyone. Leaving them
uncharged is an accounting gap though, since someone could spam many
udmabufs from the same memfd and only pay for the memfd pages once. So
this change is ok with me.

> A lot of the DMA-buf resources are intentionally not charged against memcg.
>
> Regards,
> Christian.
>
> >
> > Signed-off-by: Tao Cui <cuitao@kylinos.cn>

With the commit message updated to mention the mem_accounting param:
Reviewed-by: T.J. Mercier <tjmercier@google.com>

Re: [PATCH 1/7] scsi: sg and st: account the command buffer memory to memcg

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential
issues to consider:
- [High] scsi: sg: GFP_ATOMIC | __GFP_ACCOUNT in sg_build_indirect()
  bypasses memcg max
- [Medium] scsi: sg: unrelated and broken blktrace CAP_SYS_ADMIN gate in
  sg_ioctl_common()
- [Medium] scsi: st: misattributed GFP_KERNEL_ACCOUNT probe charges in
  new_tape_buffer()
- [Low] scsi: sg: commit message cites non-existent history in
  sg_build_indirect()
It also noticed 1 related pre-existing issue this patch does not
introduce:
- [High] scsi: sg: shift-out-of-bounds in sg_build_indirect() [1]
--

commit ec96b2b0cdd1df3575b4521cd9f81da9824bc2b2
Author: Tao Cui <cuitao@kylinos.cn>

scsi: sg and st: account the command buffer memory to memcg

This commit adds __GFP_ACCOUNT to buffer and descriptor allocations in the
sg and st drivers to charge them to the calling process's memory cgroup, and
adds a CAP_SYS_ADMIN check for blktrace ioctls in sg_ioctl_common().

> The sg data buffers are unaccounted: the per-fd reserved buffer, the
> scatter-gather page array, and the order-0..N pages behind it, plus the
> per-device tape buffer in st.  All of it is user controllable.  The
> reserved size is capped only by the device max_sectors, SG_MAX_QUEUE
> (16) commands can be in flight per fd, and the fd count is bounded by
> RLIMIT_NOFILE.  Nothing is charged to the memcg of the process driving
> it.  The September 2026 fixes in sg_build_indirect reshaped the element
> sizing and left the accounting alone.

[Severity: Low]
Is the commit message accurate about the history of sg_build_indirect() and
the reserved buffer cap?

In git history, sg_build_indirect() has not been modified since
aaff5ebaa269 ("scsi: remove the unchecked_isa_dma flag") in 2021, and its
element sizing has not changed since 10db10d144c0 ("sg: convert the indirect
IO path to use the block layer") in 2008.  In addition, the initial per-fd
reserved buffer in sg_add_sfp() is capped by min_t(int, def_reserved_size,
max_sectors_bytes(sdp->device->request_queue)), while only resizing via
SG_SET_RESERVED_SIZE is capped solely by max_sectors_bytes().

> diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
> index 5408f002e6c01..80d919fa5cf91 100644
> --- a/drivers/scsi/sg.c
> +++ b/drivers/scsi/sg.c
> @@ -910,6 +910,19 @@ static int put_compat_request_table(struct compat_sg_req_info __user *o,
>  }
>  #endif
>  
> +/*
> + * The sg passthrough of the blktrace ioctls has the same exposure as the
> + * block device path: holding the node is the only requirement today, and
> + * a created trace outlives the fd.  Must match the gate in
> + * blk_trace_ioctl().
> + */
> +static bool sg_is_blktrace_cmd(unsigned int cmd)
> +{
> +	return cmd == BLKTRACESETUP || cmd == BLKTRACESETUP2 ||
> +	       cmd == BLKTRACESTART || cmd == BLKTRACESTOP ||
> +	       cmd == BLKTRACETEARDOWN;
> +}
> +
>  static long
>  sg_ioctl_common(struct file *filp, Sg_device *sdp, Sg_fd *sfp,
>  		unsigned int cmd_in, void __user *p)
> @@ -919,6 +932,9 @@ sg_ioctl_common(struct file *filp, Sg_device *sdp, Sg_fd *sfp,
>  	Sg_request *srp;
>  	unsigned long iflags;
>  
> +	if (sg_is_blktrace_cmd(cmd_in) && !capable(CAP_SYS_ADMIN))
> +		return -EPERM;
> +

[Severity: Medium]
Does this capability check belong in a memcg accounting commit, and does it
cause a UAPI regression for unprivileged users with access to /dev/sg*?

Despite the comment, blk_trace_ioctl() in kernel/trace/blktrace.c does not
check capable(CAP_SYS_ADMIN); block devices gate these ioctls only on access
to the device node.  Moreover, sg_is_blktrace_cmd() matches BLKTRACESETUP2
even though sg_ioctl_common() only implements BLKTRACESETUP, BLKTRACESTART,
BLKTRACESTOP, and BLKTRACETEARDOWN, so unprivileged callers now get -EPERM
instead of -ENOTTY for BLKTRACESETUP2.

> @@ -1841,7 +1857,7 @@ static int
>  sg_build_sgat(Sg_scatter_hold * schp, const Sg_fd * sfp, int tablesize)
>  {
>  	int sg_bufflen = tablesize * sizeof(struct page *);
> -	gfp_t gfp_flags = GFP_ATOMIC | __GFP_NOWARN;
> +	gfp_t gfp_flags = GFP_ATOMIC | __GFP_NOWARN | __GFP_ACCOUNT;
>  
>  	schp->pages = kzalloc(sg_bufflen, gfp_flags);
>  	if (!schp->pages)
> @@ -1856,7 +1872,8 @@ sg_build_indirect(Sg_scatter_hold * schp, Sg_fd * sfp, int buff_size)
>  	int ret_sz = 0, i, k, rem_sz, num, mx_sc_elems;
>  	int sg_tablesize = sfp->parentdp->sg_tablesize;
>  	int blk_size = buff_size, order;
> -	gfp_t gfp_mask = GFP_ATOMIC | __GFP_COMP | __GFP_NOWARN | __GFP_ZERO;
> +	gfp_t gfp_mask = GFP_ATOMIC | __GFP_COMP | __GFP_NOWARN |
> +			 __GFP_ACCOUNT | __GFP_ZERO;
>  
>  	if (blk_size < 0)
>  		return -EFAULT;

[Severity: High]
This problem wasn't introduced by this patch, but can the ret_sz shift in
sg_build_indirect() trigger a signed shift-out-of-bounds [1]?

drivers/scsi/sg.c:sg_build_indirect() {
	...
	num = scatter_elem_sz;
	...
	order = get_order(num);
retry:
	ret_sz = 1 << (PAGE_SHIFT + order);
	...
}

On 4 KB page architectures, setting the scatter_elem_sz module parameter
near INT_MAX gives order == 19, so 1 << (PAGE_SHIFT + order) evaluates the
signed 32-bit shift 1 << 31, triggering UBSAN or yielding a negative ret_sz
that corrupts the allocation loop.

> @@ -2146,7 +2163,7 @@ sg_add_sfp(Sg_device * sdp)
>  	unsigned long iflags;
>  	int bufflen;
>  
> -	sfp = kzalloc_obj(*sfp, GFP_ATOMIC | __GFP_NOWARN);
> +	sfp = kzalloc_obj(*sfp, GFP_ATOMIC | __GFP_NOWARN | __GFP_ACCOUNT);
>  	if (!sfp)
>  		return ERR_PTR(-ENOMEM);

[Severity: High]
Does combining GFP_ATOMIC with __GFP_ACCOUNT in sg_build_sgat(),
sg_build_indirect(), and sg_add_sfp() allow processes to bypass their memcg
memory.max limit?

GFP_ATOMIC sets __GFP_HIGH and lacks __GFP_DIRECT_RECLAIM.  When a memcg is
at memory.max, try_charge_memcg() skips reclaim because the allocation
cannot block and then force-charges past memory.max because __GFP_HIGH is
set:

mm/memcontrol.c:try_charge_memcg() {
	...
nomem:
	if (!(gfp_mask & (__GFP_NOFAIL | __GFP_HIGH)))
		return -ENOMEM;
force:
	page_counter_charge(&memcg->memory, nr_pages);
	...
}

All callers of sg_add_sfp(), sg_build_sgat(), and sg_build_indirect()
(sg_open(), SG_SET_RESERVED_SIZE in sg_ioctl_common(), and sg_start_req())
run in sleepable process context holding only mutexes, so using GFP_KERNEL
would allow memcg reclaim and enforce memory.max.

> diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
> index f1c3c49466379..7083d37622c08 100644
> --- a/drivers/scsi/st.c
> +++ b/drivers/scsi/st.c
> @@ -3973,7 +3973,7 @@ static struct st_buffer *new_tape_buffer(int max_sg)
>  {
>  	struct st_buffer *tb;
>  
> -	tb = kzalloc_obj(struct st_buffer);
> +	tb = kzalloc_obj(struct st_buffer, GFP_KERNEL_ACCOUNT);
>  	if (!tb) {
>  		printk(KERN_NOTICE "st: Can't allocate new tape buffer.\n");
>  		return NULL;
> @@ -3982,7 +3982,8 @@ static struct st_buffer *new_tape_buffer(int max_sg)
>  	tb->use_sg = max_sg;
>  	tb->buffer_size = 0;
>  
> -	tb->reserved_pages = kzalloc_objs(struct page *, max_sg);
> +	tb->reserved_pages = kzalloc_objs(struct page *, max_sg,
> +					  GFP_KERNEL_ACCOUNT);
>  	if (!tb->reserved_pages) {
>  		kfree(tb);
>  		return NULL;

[Severity: Medium]
Does using GFP_KERNEL_ACCOUNT in new_tape_buffer() charge the device probe
context instead of the process driving tape I/O?

new_tape_buffer() is only called from st_probe() when a tape device is
attached, and both tb and tb->reserved_pages live until scsi_tape_release(),
so they are never charged to the process performing I/O.  Meanwhile, the
per-I/O page array allocated in the driving process's context during direct
I/O in sgl_map_user_pages() (pages = kmalloc_objs(*pages, max_pages))
remains unaccounted.

[1] https://sashiko.dev/#/bug/linux-ccac9556-cc9e-4b08-b44f-5f7ffb342827

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106131829.3228926-1-cui.tao@linux.dev?part=1

Re: [PATCH 4/7] tracing: account ring buffer pages and trace_array metadata to memcg

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [Medium] tracing: alloc_cpu_data() mischarges global_trace and
  read-page swaps
- [Medium] tracing: alloc_cpu_data() memcg limit triggers
  trace_rb_cpu_prepare() WARN
- [Low] tracing: inaccurate snapshot size claim and inconsistent
  GFP_KERNEL_ACCOUNT use
It also noticed 1 related pre-existing issue this patch does not
introduce:
- [Medium] tracing: memory leak in trace_array_create_systems() [1]
--

commit a11da3138f9fda850c1b05bae82b0be7e738b994
Author: Tao Cui <cuitao@kylinos.cn>

tracing: account ring buffer pages and trace_array metadata to memcg

This commit adds __GFP_ACCOUNT to ring buffer subbuffer allocations in
alloc_cpu_data() and switches trace_array allocations in
trace_array_create_systems() to GFP_KERNEL_ACCOUNT.

> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index e760d2fe386fb..f925936378639 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -441,7 +441,13 @@ static struct buffer_data_page *alloc_cpu_data(int cpu, int order)
>  	 * gracefully without invoking oom-killer and the system is not
>  	 * destabilized.
>  	 */
> -	mflags = GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_COMP | __GFP_ZERO;
> +	/*
> +	 * The subbuffers are charged to the memcg of whoever caused the
> +	 * allocation: the creator of a trace instance, or root at boot for
> +	 * the global trace.
> +	 */
> +	mflags = GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_COMP | __GFP_ZERO |
> +		  __GFP_ACCOUNT;
>  
>  	page = alloc_pages_node(cpu_to_node(cpu), mflags, order);
>  	if (!page)

[Severity: Medium]
Does charging every alloc_cpu_data() call to current's memcg misattribute
global_trace subbuffers and swapped read pages?

At boot, tracer_alloc_buffers() keeps global_trace at its minimum size
(ring_buf_size = 1) when ring_buffer_expanded is false.  global_trace is
only expanded to trace_buf_size at runtime in tracing_update_buffers() or
tracing_set_tracer() when a task first enables an event, sets a tracer, or
loads a module using trace_printk().  With __GFP_ACCOUNT in
alloc_cpu_data(), those global_trace pages are charged to whichever cgroup
triggers the lazy expansion and stay pinned to that memcg because
global_trace is never freed.

In addition, ring_buffer_alloc_read_page() calls alloc_cpu_data() when
reading or splicing from trace_pipe_raw, and ring_buffer_read_page() swaps
that reader-allocated page directly into reader->page while handing the old
ring buffer page back to the reader to be freed:

kernel/trace/ring_buffer.c:ring_buffer_read_page() {
    ...
		dpage = reader->page;
		reader->page = data_page->data;
		local_set(&reader->write, 0);
		local_set(&reader->entries, 0);
		reader->read = 0;
		data_page->data = dpage;
    ...
}

A host process reading trace_pipe_raw from a container's instance will
replace the container's charged pages with uncharged pages and uncharge the
container as the old pages are freed, while a cgroup task reading
global_trace will swap pages charged to its own memcg into global_trace.
Similarly, trace_rb_cpu_prepare() during runtime CPU hotplug allocates
per-CPU buffers for all existing instances in the context of the task
onlining the CPU.

[Severity: Medium]
Can hitting the memcg limit in alloc_cpu_data() during instance creation
trigger the WARN(1) in trace_rb_cpu_prepare()?

When alloc_buffer() creates a ring buffer, it allocates the first CPU's
buffer directly and then calls
cpuhp_state_add_instance(CPUHP_TRACE_RB_PREPARE, &buffer->node), which runs
trace_rb_cpu_prepare() for each remaining present CPU:

kernel/trace/ring_buffer.c:trace_rb_cpu_prepare() {
    ...
	buffer->buffers[cpu] =
		rb_allocate_cpu_buffer(buffer, nr_pages, cpu);
	if (!buffer->buffers[cpu]) {
		WARN(1, "failed to allocate ring buffer on CPU %u\n",
		     cpu);
		return -ENOMEM;
	}
    ...
}

Because __rb_allocate_pages() only checks si_mem_available() against global
memory, a cgroup with enough headroom for the first CPU (~1.375 MB) that
hits its memcg limit on any subsequent CPU in trace_rb_cpu_prepare() will
trigger this WARN(1) and crash the host if panic_on_warn is set.

> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 08614af8fad1c..fd1cfd1f66fbe 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -8677,11 +8677,11 @@ trace_array_create_systems(const char *name, const char *systems,
>  	int ret;
>  
>  	ret = -ENOMEM;
> -	tr = kzalloc_obj(*tr);
> +	tr = kzalloc_obj(*tr, GFP_KERNEL_ACCOUNT);
>  	if (!tr)
>  		return ERR_PTR(ret);
>  
> -	tr->name = kstrdup(name, GFP_KERNEL);
> +	tr->name = kstrdup(name, GFP_KERNEL_ACCOUNT);
>  	if (!tr->name)
>  		goto out_free_tr;
>  
> @@ -8692,7 +8692,7 @@ trace_array_create_systems(const char *name, const char *systems,
>  		goto out_free_tr;
>  
>  	if (systems) {
> -		tr->system_names = kstrdup_const(systems, GFP_KERNEL);
> +		tr->system_names = kstrdup_const(systems, GFP_KERNEL_ACCOUNT);
>  		if (!tr->system_names)
>  			goto out_free_tr;
>  	}

[Severity: Low]
Should the alloc_cpumask_var() and zalloc_cpumask_var() calls right between
tr->name and tr->system_names, as well as alloc_cpu_page() and
alloc_cpu_buffer() in ring_buffer.c, also use GFP_KERNEL_ACCOUNT?

Also, the commit message says creating an instance immediately allocates a
full ~1.375 MB per-CPU snapshot buffer, but trace_allocate_snapshot() passes
size = 1 when allocate_snapshot is false (which is always the case after
boot), so snapshot_buffer only allocates the minimum pages until a snapshot
is armed.

[Severity: Medium]
This problem wasn't introduced by this patch, but in
trace_array_create_systems() [1], does the out_free_tr error path leak
tr->tracers when trace_array_create_dir() fails?

trace_array_create_dir() calls __update_tracer() -> add_tracer(), which
allocates struct tracers and struct tracer_flags entries on tr->tracers.  If
a later step in trace_array_create_dir() fails and jumps to out_free_tr,
nothing calls free_tracers(tr), leaking the entries already added to
tr->tracers.

[1] https://sashiko.dev/#/bug/linux-9449d18e-1233-4181-9199-09eaead7b576

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106131829.3228926-1-cui.tao@linux.dev?part=4

Re: [PATCH 6/7] io_uring: account the ring context and SQPOLL thread data to memcg

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] io_uring: commit message omits rsrc and tctx changes and
  uncharged allocs
--

commit 3d47975c4208930279f98824279734ca99b5e9bd
Author: Tao Cui <cuitao@kylinos.cn>

> io_uring: account the ring context and SQPOLL thread data to memcg
>
> The io_ring_ctx and the SQPOLL io_sqd are the last per-ring allocations
> made with plain GFP_KERNEL.  The larger surfaces around them are
> already charged: io_mapped_ubuf and io_rsrc_node since commit
> 075d5837bafc ("io_uring/rsrc: charge buffer arrays and nodes to memcg"),
> the tctx node and its XArrays since commit 0b34e7b20a48 ("io_uring:
> charge tctx nodes and personalities").
>
> Charge the context and the SQPOLL thread data the same way.
>
> Signed-off-by: Tao Cui <cuitao@kylinos.cn>

[Severity: Low]
Does the commit message match the actual changes and current tree state?

Alongside io_ring_ctx and io_sq_data, the patch also switches new_array in
io_coalesce_buffer(), ctx->hash_map in io_init_wq_offload(), and
io_uring_task in io_uring_alloc_task_context() to GFP_KERNEL_ACCOUNT without
mentioning them in the subject or changelog.

The changelog also says these are the last per-ring GFP_KERNEL allocations.
While commit "io_uring: account the remaining per-object allocations to
memcg" later in the series converts io_alloc_cache_init() and io_wq_create(),
percpu_ref_init(&ctx->refs, ...) in io_ring_ctx_alloc(),
percpu_counter_init(&tctx->inflight, ...) in io_uring_alloc_task_context(),
and alloc_cpumask_var(&wq->cpu_mask, ...) in io_wq_create() still use plain
GFP_KERNEL.

Finally, the cited commits 075d5837bafc ("io_uring/rsrc: charge buffer
arrays and nodes to memcg") and 0b34e7b20a48 ("io_uring: charge tctx nodes
and personalities") are not present in the base tree, so io_alloc_imu(),
io_rsrc_node_alloc(), io_tctx_install_node(), tctx->xa, and
ctx->personalities remain uncharged.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106131829.3228926-1-cui.tao@linux.dev?part=6