Date: Tue, 26 May 2026 21:23:01 +0000 From: Alex Richardson <arichardson@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 675382f12b67 - main - p9fs: Refactor buffer allocations to avoid zeroing large payloads Message-ID: <6a160f35.227fd.152a77c4@gitrepo.freebsd.org>
index | next in thread | raw e-mail
The branch main has been updated by arichardson: URL: https://cgit.FreeBSD.org/src/commit/?id=675382f12b67e8b7b2f35bd1f8dfd96b8d8e4aae commit 675382f12b67e8b7b2f35bd1f8dfd96b8d8e4aae Author: Alex Richardson <arichardson@FreeBSD.org> AuthorDate: 2026-05-26 19:37:39 +0000 Commit: Alex Richardson <arichardson@FreeBSD.org> CommitDate: 2026-05-26 19:37:39 +0000 p9fs: Refactor buffer allocations to avoid zeroing large payloads Allocating large buffers with M_ZERO adds unnecessary overhead since the data is immediately overwritten. This change embeds the tc and rc p9_buffer structs directly into p9_req_t so we only zero the small metadata headers. The actual data payload is allocated with M_NOWAIT. Embedding the metadata headers by value also allows the p9fs_buf_zone UMA items to be sized exactly to P9FS_MTU, ensuring they are nicely aligned. This also adds proper error handling to p9_get_request() to handle UMA allocation failures. Reviewed by: markj MFC after: 1 week Differential Revision: https://reviews.freebsd.org/D56495 --- sys/dev/virtio/p9fs/virtio_p9fs.c | 10 ++--- sys/fs/p9fs/p9_client.c | 95 +++++++++++++++++---------------------- sys/fs/p9fs/p9_client.h | 4 +- 3 files changed, 49 insertions(+), 60 deletions(-) diff --git a/sys/dev/virtio/p9fs/virtio_p9fs.c b/sys/dev/virtio/p9fs/virtio_p9fs.c index 19a32fea458e..c347458b4f8e 100644 --- a/sys/dev/virtio/p9fs/virtio_p9fs.c +++ b/sys/dev/virtio/p9fs/virtio_p9fs.c @@ -112,7 +112,7 @@ SYSCTL_UINT(_vfs_9p, OID_AUTO, ackmaxidle, CTLFLAG_RW, &vt9p_ackmaxidle, 0, static int vt9p_req_wait(struct vt9p_softc *chan, struct p9_req_t *req) { - KASSERT(req->tc->tag != req->rc->tag, + KASSERT(req->tc.tag != req->rc.tag, ("%s: request %p already completed", __func__, req)); if (msleep(req, VT9P_MTX(chan), 0, "chan lock", vt9p_ackmaxidle * hz)) { @@ -124,7 +124,7 @@ vt9p_req_wait(struct vt9p_softc *chan, struct p9_req_t *req) "for an ack from host\n", vt9p_ackmaxidle); return (EIO); } - KASSERT(req->tc->tag == req->rc->tag, + KASSERT(req->tc.tag == req->rc.tag, ("%s spurious event on request %p", __func__, req)); return (0); } @@ -157,7 +157,7 @@ vt9p_request(void *handle, struct p9_req_t *req) req_retry: sglist_reset(sg); /* Handle out VirtIO ring buffers */ - error = sglist_append(sg, req->tc->sdata, req->tc->size); + error = sglist_append(sg, req->tc.sdata, req->tc.size); if (error != 0) { P9_DEBUG(ERROR, "%s: sglist append failed\n", __func__); VT9P_UNLOCK(chan); @@ -165,7 +165,7 @@ req_retry: } readable = sg->sg_nseg; - error = sglist_append(sg, req->rc->sdata, req->rc->capacity); + error = sglist_append(sg, req->rc.sdata, req->rc.capacity); if (error != 0) { P9_DEBUG(ERROR, "%s: sglist append failed\n", __func__); VT9P_UNLOCK(chan); @@ -226,7 +226,7 @@ vt9p_intr_complete(void *xsc) VT9P_LOCK(chan); again: while ((curreq = virtqueue_dequeue(vq, NULL)) != NULL) { - curreq->rc->tag = curreq->tc->tag; + curreq->rc.tag = curreq->tc.tag; wakeup_one(curreq); } if (virtqueue_enable_intr(vq) != 0) { diff --git a/sys/fs/p9fs/p9_client.c b/sys/fs/p9fs/p9_client.c index 547de98c4c03..27a6c1eb366d 100644 --- a/sys/fs/p9fs/p9_client.c +++ b/sys/fs/p9fs/p9_client.c @@ -104,43 +104,33 @@ p9_parse_opts(struct mount *mp, struct p9_client *clnt) } /* Allocate buffer for sending request and getting responses */ -static struct p9_buffer * -p9_buffer_alloc(int alloc_msize) +static void +p9_buffer_alloc(struct p9_buffer *fc, int alloc_msize) { - struct p9_buffer *fc; - - fc = uma_zalloc(p9fs_buf_zone, M_WAITOK | M_ZERO); + bzero(fc, sizeof(*fc)); fc->capacity = alloc_msize; - fc->offset = 0; - fc->size = 0; - fc->sdata = (char *)fc + sizeof(struct p9_buffer); - - return (fc); + fc->sdata = uma_zalloc(p9fs_buf_zone, M_WAITOK); } /* Free memory used by request and response buffers */ static void -p9_buffer_free(struct p9_buffer **buf) +p9_buffer_free(struct p9_buffer *buf) { - - /* Free the sdata buffers first, then the whole structure*/ - uma_zfree(p9fs_buf_zone, *buf); - *buf = NULL; + uma_zfree(p9fs_buf_zone, buf->sdata); + buf->sdata = NULL; } /* Free the request */ static void p9_free_req(struct p9_client *clnt, struct p9_req_t *req) { + if (req == NULL) + return; - if (req->tc != NULL) { - if (req->tc->tag != P9_NOTAG) - p9_tag_destroy(clnt, req->tc->tag); - p9_buffer_free(&req->tc); - } - - if (req->rc != NULL) - p9_buffer_free(&req->rc); + if (req->tc.tag != P9_NOTAG) + p9_tag_destroy(clnt, req->tc.tag); + p9_buffer_free(&req->tc); + p9_buffer_free(&req->rc); uma_zfree(p9fs_req_zone, req); } @@ -156,17 +146,17 @@ p9_get_request(struct p9_client *clnt, int *error) alloc_msize = P9FS_MTU; req = uma_zalloc(p9fs_req_zone, M_WAITOK | M_ZERO); - req->tc = p9_buffer_alloc(alloc_msize); - req->rc = p9_buffer_alloc(alloc_msize); + p9_buffer_alloc(&req->tc, alloc_msize); + p9_buffer_alloc(&req->rc, alloc_msize); tag = p9_tag_create(clnt); if (tag == P9_NOTAG) { *error = EAGAIN; - req->tc->tag = P9_NOTAG; + req->tc.tag = P9_NOTAG; p9_free_req(clnt, req); return (NULL); } - req->tc->tag = tag; + req->tc.tag = tag; return (req); } @@ -208,7 +198,7 @@ p9_client_check_return(struct p9_client *c, struct p9_req_t *req) char *ename; /* Check what we have in the receive bufer .*/ - error = p9_parse_receive(req->rc, c); + error = p9_parse_receive(&req->rc, c); if (error != 0) goto out; @@ -216,17 +206,17 @@ p9_client_check_return(struct p9_client *c, struct p9_req_t *req) * No error, We are done with the preprocessing. Return to the caller * and process the actual data. */ - if (req->rc->id != P9PROTO_RERROR && req->rc->id != P9PROTO_RLERROR) + if (req->rc.id != P9PROTO_RERROR && req->rc.id != P9PROTO_RLERROR) return (0); /* * Interpreting the error is done in different ways for Linux and * Unix version. Make sure you interpret it right. */ - if (req->rc->id == P9PROTO_RERROR) { - error = p9_buf_readf(req->rc, c->proto_version, "s?d", &ename, &ecode); - } else if (req->rc->id == P9PROTO_RLERROR) { - error = p9_buf_readf(req->rc, c->proto_version, "d", &ecode); + if (req->rc.id == P9PROTO_RERROR) { + error = p9_buf_readf(&req->rc, c->proto_version, "s?d", &ename, &ecode); + } else if (req->rc.id == P9PROTO_RLERROR) { + error = p9_buf_readf(&req->rc, c->proto_version, "d", &ecode); } else { goto out; } @@ -241,15 +231,15 @@ p9_client_check_return(struct p9_client *c, struct p9_req_t *req) * not present can hit this and return. Hence it is made a debug print. */ if (error != 0) { - if (req->rc->id == P9PROTO_RERROR) { + if (req->rc.id == P9PROTO_RERROR) { P9_DEBUG(PROTO, "RERROR error %d ename %s\n", error, ename); - } else if (req->rc->id == P9PROTO_RLERROR) { + } else if (req->rc.id == P9PROTO_RLERROR) { P9_DEBUG(PROTO, "RLERROR error %d\n", error); } } - if (req->rc->id == P9PROTO_RERROR) { + if (req->rc.id == P9PROTO_RERROR) { free(ename, M_TEMP); } return (error); @@ -308,21 +298,21 @@ p9_client_prepare_req(struct p9_client *c, int8_t type, } /* Marshall the data according to QEMU standards */ - *error = p9_buf_prepare(req->tc, type); + *error = p9_buf_prepare(&req->tc, type); if (*error != 0) { P9_DEBUG(ERROR, "%s: p9_buf_prepare failed: %d\n", __func__, *error); goto out; } - *error = p9_buf_vwritef(req->tc, c->proto_version, fmt, ap); + *error = p9_buf_vwritef(&req->tc, c->proto_version, fmt, ap); if (*error != 0) { P9_DEBUG(ERROR, "%s: p9_buf_vwrite failed: %d\n", __func__, *error); goto out; } - *error = p9_buf_finalize(c, req->tc); + *error = p9_buf_finalize(c, &req->tc); if (*error != 0) { P9_DEBUG(ERROR, "%s: p9_buf_finalize failed: %d \n", __func__, *error); @@ -474,7 +464,7 @@ p9_client_version(struct p9_client *c) if (error != 0) return (error); - error = p9_buf_readf(req->rc, c->proto_version, "ds", &msize, &version); + error = p9_buf_readf(&req->rc, c->proto_version, "ds", &msize, &version); if (error != 0) { P9_DEBUG(ERROR, "%s: version error: %d\n", __func__, error); goto out; @@ -519,8 +509,7 @@ p9_init_zones(void) /* Create the buffer zone */ p9fs_buf_zone = uma_zcreate("p9fs buf zone", - sizeof(struct p9_buffer) + P9FS_MTU, NULL, NULL, - NULL, NULL, UMA_ALIGN_PTR, 0); + P9FS_MTU, NULL, NULL, NULL, NULL, UMA_ALIGN_PTR, 0); } void @@ -623,7 +612,7 @@ p9_client_attach(struct p9_client *clnt, struct p9_fid *afid, if (*error != 0) goto out; - *error = p9_buf_readf(req->rc, clnt->proto_version, "Q", &qid); + *error = p9_buf_readf(&req->rc, clnt->proto_version, "Q", &qid); if (*error != 0) { P9_DEBUG(ERROR, "%s: p9_buf_readf failed: %d \n", __func__, *error); @@ -777,7 +766,7 @@ p9_client_walk(struct p9_fid *oldfid, uint16_t nwnames, char **wnames, return (NULL); } - *error = p9_buf_readf(req->rc, clnt->proto_version, "R", &nwqids, + *error = p9_buf_readf(&req->rc, clnt->proto_version, "R", &nwqids, &wqids); if (*error != 0) goto out; @@ -842,7 +831,7 @@ p9_client_open(struct p9_fid *fid, int mode) if (error != 0) return (error); - error = p9_buf_readf(req->rc, clnt->proto_version, "Qd", &fid->qid, + error = p9_buf_readf(&req->rc, clnt->proto_version, "Qd", &fid->qid, &mtu); if (error != 0) goto out; @@ -892,7 +881,7 @@ p9_client_readdir(struct p9_fid *fid, char *data, uint64_t offset, return (-error); } - error = p9_buf_readf(req->rc, clnt->proto_version, "D", &count, + error = p9_buf_readf(&req->rc, clnt->proto_version, "D", &count, &dataptr); if (error != 0) { P9_DEBUG(ERROR, "%s: p0_buf_readf failed: %d\n", @@ -945,7 +934,7 @@ p9_client_read(struct p9_fid *fid, uint64_t offset, uint32_t count, char *data) return (-error); } - error = p9_buf_readf(req->rc, clnt->proto_version, "D", &count, + error = p9_buf_readf(&req->rc, clnt->proto_version, "D", &count, &dataptr); if (error != 0) { P9_DEBUG(ERROR, "%s: p9_buf_readf failed: %d\n", @@ -1017,7 +1006,7 @@ p9_client_write(struct p9_fid *fid, uint64_t offset, uint32_t count, char *data) return (-error); } - error = p9_buf_readf(req->rc, clnt->proto_version, "d", &ret); + error = p9_buf_readf(&req->rc, clnt->proto_version, "d", &ret); if (error != 0) { P9_DEBUG(ERROR, "%s: p9_buf_readf error: %d\n", __func__, error); @@ -1069,7 +1058,7 @@ p9_client_file_create(struct p9_fid *fid, char *name, uint32_t perm, int mode, if (error != 0) return (error); - error = p9_buf_readf(req->rc, clnt->proto_version, "Qd", &qid, &mtu); + error = p9_buf_readf(&req->rc, clnt->proto_version, "Qd", &qid, &mtu); if (error != 0) goto out; @@ -1101,7 +1090,7 @@ p9_client_statfs(struct p9_fid *fid, struct p9_statfs *stat) return (error); } - error = p9_buf_readf(req->rc, clnt->proto_version, "ddqqqqqqd", + error = p9_buf_readf(&req->rc, clnt->proto_version, "ddqqqqqqd", &stat->type, &stat->bsize, &stat->blocks, &stat->bfree, &stat->bavail, &stat->files, &stat->ffree, &stat->fsid, &stat->namelen); @@ -1173,7 +1162,7 @@ p9_create_symlink(struct p9_fid *fid, char *name, char *symtgt, gid_t gid) if (error != 0) return (error); - error = p9_buf_readf(req->rc, clnt->proto_version, "Q", &qid); + error = p9_buf_readf(&req->rc, clnt->proto_version, "Q", &qid); if (error != 0) { P9_DEBUG(ERROR, "%s: buf_readf failed %d\n", __func__, error); return (error); @@ -1226,7 +1215,7 @@ p9_readlink(struct p9_fid *fid, char **target) if (error != 0) return (error); - error = p9_buf_readf(req->rc, clnt->proto_version, "s", target); + error = p9_buf_readf(&req->rc, clnt->proto_version, "s", target); if (error != 0) { P9_DEBUG(ERROR, "%s: buf_readf failed %d\n", __func__, error); return (error); @@ -1260,7 +1249,7 @@ p9_client_getattr(struct p9_fid *fid, struct p9_stat_dotl *stat_dotl, goto error; } - err = p9_buf_readf(req->rc, clnt->proto_version, "A", stat_dotl); + err = p9_buf_readf(&req->rc, clnt->proto_version, "A", stat_dotl); if (err != 0) { P9_DEBUG(ERROR, "%s: buf_readf failed %d\n", __func__, err); goto error; diff --git a/sys/fs/p9fs/p9_client.h b/sys/fs/p9fs/p9_client.h index 4eb82c0232f4..5db46d97c704 100644 --- a/sys/fs/p9fs/p9_client.h +++ b/sys/fs/p9fs/p9_client.h @@ -54,8 +54,8 @@ enum p9_proto_versions { /* P9 Request exchanged between Host and Guest */ struct p9_req_t { - struct p9_buffer *tc; /* request buffer */ - struct p9_buffer *rc; /* response buffer */ + struct p9_buffer tc; /* request buffer */ + struct p9_buffer rc; /* response buffer */ }; /* 9P transport status */home | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6a160f35.227fd.152a77c4>