On 3/9/22 9:52 AM, T.J. Mercier wrote:

Does this test inteded to be built on all architectures? Is arch
check necessary here?

Also does this test require root previleges - I see mount and
unmount operations in the test. If so add root check and skip
if non-root user runs the test.
  

Does umount require root previleges? Same commment as above about
non-root user running test.

+}
+
+struct binderfs_ctx create_binderfs(const char *name)
+{
+	int fd, ret, saved_errno;
+	struct binderfs_device device = { 0 };
+	struct binderfs_ctx ctx = { 0 };
+
+	/*
+	 * P_tmpdir is set to "/tmp/" on Android platforms where Binder is most
+	 * commonly used, but this path does not actually exist on Android. We
+	 * will first try using "/data/local/tmp" and fallback to P_tmpdir if
+	 * that fails for non-Android platforms.
+	 */
+	static const char tmpdir[] = "/data/local/tmp";
+	static const size_t MAX_TMPDIR_SIZE =
+		sizeof(tmpdir) > sizeof(P_tmpdir) ?
+		sizeof(tmpdir) : sizeof(P_tmpdir);
+	static const char template[] = "/binderfs_XXXXXX";
+
+	char *mkdtemp_result;
+	char binderfs_mntpt[MAX_TMPDIR_SIZE + sizeof(template)];
+	char device_path[MAX_TMPDIR_SIZE + sizeof(template) + BINDERFS_MAX_NAME];
+
+	snprintf(binderfs_mntpt, sizeof(binderfs_mntpt), "%s%s", tmpdir, template);
+
+	mkdtemp_result = mkdtemp(binderfs_mntpt);
+	if (mkdtemp_result == NULL) {
+		fprintf(stderr, "Failed to create binderfs mountpoint at %s: %s.\n",
+			binderfs_mntpt, strerror(errno));
+		fprintf(stderr, "Trying fallback mountpoint...\n");
+		snprintf(binderfs_mntpt, sizeof(binderfs_mntpt), "%s%s", P_tmpdir, template);
+		if (mkdtemp(binderfs_mntpt) == NULL) {
+			fprintf(stderr, "Failed to create binderfs mountpoint at %s: %s\n",
+			binderfs_mntpt, strerror(errno));
+			return ctx;
+		}
+	}
+	fprintf(stderr, "Binderfs mountpoint created at %s\n", binderfs_mntpt);

Does mount require root previleges? Same commment as above about
non-root user running test.

+
+	if (mount(NULL, binderfs_mntpt, "binder", 0, 0)) {
+		perror("Could not mount binderfs");
+		rmdir(binderfs_mntpt);
+		return ctx;
+	}
+	fprintf(stderr, "Binderfs mounted at %s\n", binderfs_mntpt);
+
+	strncpy(device.name, name, sizeof(device.name));
+	snprintf(device_path, sizeof(device_path), "%s/binder-control", binderfs_mntpt);
+	fd = open(device_path, O_RDONLY | O_CLOEXEC);
+	if (!fd) {
+		perror("Failed to open binder-control device");
+		binderfs_unmount(binderfs_mntpt);
+		return ctx;
+	}
+
+	ret = ioctl(fd, BINDER_CTL_ADD, &device);
+	saved_errno = errno;
+	close(fd);
+	errno = saved_errno;
+	if (ret) {
+		perror("Failed to allocate new binder device");
+		binderfs_unmount(binderfs_mntpt);
+		return ctx;
+	}
+
+	fprintf(stderr, "Allocated new binder device with major %d, minor %d, and name %s at %s\n",
+		device.major, device.minor, device.name, binderfs_mntpt);
+
+	ctx.name = strdup(name);
+	ctx.mountpoint = strdup(binderfs_mntpt);
+	return ctx;
+}
+
+void destroy_binderfs(struct binderfs_ctx *ctx)
+{
+	char path[PATH_MAX];
+
+	snprintf(path, sizeof(path), "%s/%s", ctx->mountpoint, ctx->name);
+
+	if (unlink(path))
+		fprintf(stderr, "Failed to unlink binder device %s: %s\n", path, strerror(errno));
+	else
+		fprintf(stderr, "Destroyed binder %s at %s\n", ctx->name, ctx->mountpoint);
+
+	binderfs_unmount(ctx->mountpoint);
+
+	free(ctx->name);
+	free(ctx->mountpoint);
+}
+
+struct binder_ctx open_binder(struct binderfs_ctx *bfs_ctx)
+{
+	struct binder_ctx ctx = {.fd = -1, .memory = NULL};
+	char path[PATH_MAX];
+
+	snprintf(path, sizeof(path), "%s/%s", bfs_ctx->mountpoint, bfs_ctx->name);
+	ctx.fd = open(path, O_RDWR | O_NONBLOCK | O_CLOEXEC);
+	if (ctx.fd < 0) {
+		fprintf(stderr, "Error opening binder device %s: %s\n", path, strerror(errno));

Does this require root previleges?

Same question about root preveliges?

+
+	if (dmabuf_heap_alloc(heap_fd, bytes, &dmabuf_fd))
+		TH_LOG("dmabuf allocation failed! - %s", strerror(errno));
+	close(heap_fd);
+
+	return dmabuf_fd;
+}
+
+int binder_request_dmabuf(int binder_fd)
+{
+	int ret;
+
+	/*
+	 * We just send an empty binder_buffer_object to initiate a transaction
+	 * with the context manager, who should respond with a single dmabuf
+	 * inside a binder_fd_array_object.
+	 */
+
+	struct binder_buffer_object bbo = {
+		.hdr.type = BINDER_TYPE_PTR,
+		.flags = 0,
+		.buffer = 0,
+		.length = 0,
+		.parent = 0, /* No parent */
+		.parent_offset = 0 /* No parent */
+	};
+
+	binder_size_t offsets[] = {0};
+
+	struct {
+		int32_t cmd;
+		struct binder_transaction_data btd;
+	} __attribute__((packed)) bc = {
+		.cmd = BC_TRANSACTION,
+		.btd = {
+			.target = { 0 },
+			.cookie = 0,
+			.code = BINDER_CODE,
+			.flags = TF_ACCEPT_FDS, /* We expect a FDA in the reply */
+			.data_size = sizeof(bbo),
+			.offsets_size = sizeof(offsets),
+			.data.ptr = {
+				(binder_uintptr_t)&bbo,
+				(binder_uintptr_t)offsets
+			}
+		},
+	};
+
+	struct {
+		int32_t reply_noop;
+	} __attribute__((packed)) br;
+
+	ret = do_binder_write_read(binder_fd, &bc, sizeof(bc), &br, sizeof(br));
+	if (ret >= sizeof(br) && expect_binder_reply(br.reply_noop, BR_NOOP)) {
+		return -1;
+	} else if (ret < sizeof(br)) {
+		fprintf(stderr, "Not enough bytes in binder reply %d\n", ret);
+		return -1;
+	}
+	return 0;
+}
+
+int send_dmabuf_reply(int binder_fd, struct binder_transaction_data *tr, int dmabuf_fd)
+{
+	int ret;
+	/*
+	 * The trailing 0 is to achieve the necessary alignment for the binder
+	 * buffer_size.
+	 */
+	int fdarray[] = { dmabuf_fd, 0 };
+
+	struct binder_buffer_object bbo = {
+		.hdr.type = BINDER_TYPE_PTR,
+		.flags = BINDER_BUFFER_FLAG_SENDER_NO_NEED,
+		.buffer = (binder_uintptr_t)fdarray,
+		.length = sizeof(fdarray),
+		.parent = 0, /* No parent */
+		.parent_offset = 0 /* No parent */
+	};
+
+	struct binder_fd_array_object bfdao = {
+		.hdr.type = BINDER_TYPE_FDA,
+		.num_fds = 1,
+		.parent = 0, /* The binder_buffer_object */
+		.parent_offset = 0 /* FDs follow immediately */
+	};
+
+	uint64_t sz = sizeof(fdarray);
+	uint8_t data[sizeof(sz) + sizeof(bbo) + sizeof(bfdao)];
+	binder_size_t offsets[] = {sizeof(sz), sizeof(sz)+sizeof(bbo)};
+
+	memcpy(data,                            &sz, sizeof(sz));
+	memcpy(data + sizeof(sz),               &bbo, sizeof(bbo));
+	memcpy(data + sizeof(sz) + sizeof(bbo), &bfdao, sizeof(bfdao));
+
+	struct {
+		int32_t cmd;
+		struct binder_transaction_data_sg btd;
+	} __attribute__((packed)) bc = {
+		.cmd = BC_REPLY_SG,
+		.btd.transaction_data = {
+			.target = { tr->target.handle },
+			.cookie = tr->cookie,
+			.code = BINDER_CODE,
+			.flags = 0,
+			.data_size = sizeof(data),
+			.offsets_size = sizeof(offsets),
+			.data.ptr = {
+				(binder_uintptr_t)data,
+				(binder_uintptr_t)offsets
+			}
+		},
+		.btd.buffers_size = sizeof(fdarray)
+	};
+
+	struct {
+		int32_t reply_noop;
+	} __attribute__((packed)) br;
+
+	ret = do_binder_write_read(binder_fd, &bc, sizeof(bc), &br, sizeof(br));
+	if (ret >= sizeof(br) && expect_binder_reply(br.reply_noop, BR_NOOP)) {
+		return -1;
+	} else if (ret < sizeof(br)) {
+		fprintf(stderr, "Not enough bytes in binder reply %d\n", ret);
+		return -1;
+	}
+	return 0;
+}
+
+struct binder_transaction_data *binder_wait_for_transaction(int binder_fd,
+							    uint32_t *readbuf,
+							    size_t readsize)
+{
+	static const int MAX_EVENTS = 1, EPOLL_WAIT_TIME_MS = 3 * 1000;
+	struct binder_reply {
+		int32_t reply0;
+		int32_t reply1;
+		struct binder_transaction_data btd;
+	} *br;
+	struct binder_transaction_data *ret = NULL;
+	struct epoll_event events[MAX_EVENTS];
+	int epoll_fd, num_events, readcount;
+	uint32_t bc[] = { BC_ENTER_LOOPER };
+
+	do_binder_write_read(binder_fd, &bc, sizeof(bc), NULL, 0);
+
+	epoll_fd = epoll_create1(EPOLL_CLOEXEC);
+	if (epoll_fd == -1) {
+		perror("epoll_create");
+		return NULL;
+	}
+
+	events[0].events = EPOLLIN;
+	if (epoll_ctl(epoll_fd, EPOLL_CTL_ADD, binder_fd, &events[0])) {
+		perror("epoll_ctl add");
+		goto err_close;
+	}
+
+	num_events = epoll_wait(epoll_fd, events, MAX_EVENTS, EPOLL_WAIT_TIME_MS);
+	if (num_events < 0) {
+		perror("epoll_wait");
+		goto err_ctl;
+	} else if (num_events == 0) {
+		fprintf(stderr, "No events\n");
+		goto err_ctl;
+	}
+
+	readcount = do_binder_write_read(binder_fd, NULL, 0, readbuf, readsize);
+	fprintf(stderr, "Read %d bytes from binder\n", readcount);
+
+	if (readcount < (int)sizeof(struct binder_reply)) {
+		fprintf(stderr, "read_consumed not large enough\n");
+		goto err_ctl;
+	}
+
+	br = (struct binder_reply *)readbuf;
+	if (expect_binder_reply(br->reply0, BR_NOOP))
+		goto err_ctl;
+
+	if (br->reply1 == BR_TRANSACTION) {
+		if (br->btd.code == BINDER_CODE)
+			ret = &br->btd;
+		else
+			fprintf(stderr, "Received transaction with unexpected code: %u\n",
+				br->btd.code);
+	} else {
+		expect_binder_reply(br->reply1, BR_TRANSACTION_COMPLETE);
+	}
+
+err_ctl:
+	if (epoll_ctl(epoll_fd, EPOLL_CTL_DEL, binder_fd, NULL))
+		perror("epoll_ctl del");
+err_close:
+	close(epoll_fd);
+	return ret;
+}
+
+static int child_request_dmabuf_transfer(const char *cgroup, void *arg)
+{
+	UNUSED(cgroup);
+	int ret = -1;
+	uint32_t readbuf[32];
+	struct binderfs_ctx bfs_ctx = *(struct binderfs_ctx *)arg;
+	struct binder_ctx b_ctx;
+
+	fprintf(stderr, "Child PID: %d\n", getpid());
+
+	b_ctx = open_binder(&bfs_ctx);
+	if (b_ctx.fd < 0) {
+		fprintf(stderr, "Child unable to open binder\n");
+		return -1;
+	}
+
+	if (binder_request_dmabuf(b_ctx.fd))
+		goto err;
+
+	/* The child must stay alive until the binder reply is received */
+	if (binder_wait_for_transaction(b_ctx.fd, readbuf, sizeof(readbuf)) == NULL)
+		ret = 0;
+
+	/*
+	 * We don't close the received dmabuf here so that the parent can
+	 * inspect the cgroup gpu memory charges to verify the charge transfer
+	 * completed successfully.
+	 */
+err:
+	close_binder(&b_ctx);
+	fprintf(stderr, "Child done\n");
+	return ret;
+}
+
+TEST(gpu_cgroup_dmabuf_transfer)
+{
+	static const char * const GPUMEM_FILENAME = "gpu.memory.current";
+	static const size_t ONE_MiB = 1024 * 1024;
+
+	int ret, dmabuf_fd;
+	uint32_t readbuf[32];
+	long memsize;
+	pid_t child_pid;
+	struct binderfs_ctx bfs_ctx;
+	struct binder_ctx b_ctx;
+	struct cgroup_ctx cg_ctx;
+	struct binder_transaction_data *tr;
+	struct flat_binder_object *fbo;
+	struct binder_buffer_object *bbo;
+

If root previges is necessary - pls add check here and skip.

+	bfs_ctx = create_binderfs("testbinder");
+	if (bfs_ctx.name == NULL)
+		ksft_exit_skip("The Android binderfs filesystem is not available\n");
+
+	cg_ctx = create_cgroups(_metadata);
+	if (cg_ctx.root == NULL) {
+		destroy_binderfs(&bfs_ctx);
+		ksft_exit_skip("cgroup v2 isn't mounted\n");
+	}
+
+	ASSERT_EQ(cg_enter_current(cg_ctx.source), 0) {
+		TH_LOG("Could not move parent to cgroup: %s", cg_ctx.source);
+		goto err_cg;
+	}
+
+	dmabuf_fd = alloc_dmabuf_from_system_heap(_metadata, ONE_MiB);
+	ASSERT_GE(dmabuf_fd, 0) {
+		goto err_cg;
+	}
+	TH_LOG("Allocated dmabuf");
+
+	memsize = cg_read_key_long(cg_ctx.source, GPUMEM_FILENAME, "system");
+	ASSERT_EQ(memsize, ONE_MiB) {
+		TH_LOG("GPU memory used after allocation: %ld but it should be %lu",
+		       memsize, (unsigned long)ONE_MiB);
+		goto err_dmabuf;
+	}
+
+	b_ctx = open_binder(&bfs_ctx);
+	ASSERT_GE(b_ctx.fd, 0) {
+		TH_LOG("Parent unable to open binder");
+		goto err_dmabuf;
+	}
+	TH_LOG("Opened binder at %s/%s", bfs_ctx.mountpoint, bfs_ctx.name);
+
+	ASSERT_EQ(become_binder_context_manager(b_ctx.fd), 0) {
+		TH_LOG("Cannot become context manager: %s", strerror(errno));
+		goto err_binder;
+	}
+
+	child_pid = cg_run_nowait(cg_ctx.dest, child_request_dmabuf_transfer, &bfs_ctx);
+	ASSERT_GT(child_pid, 0) {
+		TH_LOG("Error forking: %s", strerror(errno));
+		goto err_binder;
+	}
+
+	tr = binder_wait_for_transaction(b_ctx.fd, readbuf, sizeof(readbuf));
+	ASSERT_NE(tr, NULL) {
+		TH_LOG("Error receiving transaction request from child");
+		goto err_child;
+	}
+	fbo = (struct flat_binder_object *)tr->data.ptr.buffer;
+	ASSERT_EQ(fbo->hdr.type, BINDER_TYPE_PTR) {
+		TH_LOG("Did not receive a buffer object from child");
+		goto err_child;
+	}
+	bbo = (struct binder_buffer_object *)fbo;
+	ASSERT_EQ(bbo->length, 0) {
+		TH_LOG("Did not receive an empty buffer object from child");
+		goto err_child;
+	}
+
+	TH_LOG("Received transaction from child");
+	send_dmabuf_reply(b_ctx.fd, tr, dmabuf_fd);
+
+	ASSERT_EQ(cg_read_key_long(cg_ctx.dest, GPUMEM_FILENAME, "system"), ONE_MiB) {
+		TH_LOG("Destination cgroup does not have system charge!");
+		goto err_child;
+	}
+	ASSERT_EQ(cg_read_key_long(cg_ctx.source, GPUMEM_FILENAME, "system"), 0) {
+		TH_LOG("Source cgroup still has system charge!");
+		goto err_child;
+	}
+	TH_LOG("Charge transfer succeeded!");
+
+err_child:
+	waitpid(child_pid, &ret, 0);
+	if (WIFEXITED(ret))
+		TH_LOG("Child %d terminated with %d", child_pid, WEXITSTATUS(ret));
+	else
+		TH_LOG("Child terminated abnormally");

What does this mean? What are the conditions that could cause this?
Pls include more info. in the message.

+err_binder:
+	close_binder(&b_ctx);
+err_dmabuf:
+	close(dmabuf_fd);
+err_cg:
+	destroy_cgroups(_metadata, &cg_ctx);
+	destroy_binderfs(&bfs_ctx);
+}
+
+TEST_HARNESS_MAIN

thanks,
-- Shuah