Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 20 Jul 2022 14:02:13 GMT
From:      Mark Johnston <markj@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: d57ddc1d2804 - stable/13 - aio: Improve UMA usage
Message-ID:  <202207201402.26KE2Dp5010035@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=d57ddc1d2804654c4bc6015d9f17704912ac2153

commit d57ddc1d2804654c4bc6015d9f17704912ac2153
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2022-06-20 16:17:11 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2022-07-20 13:53:48 +0000

    aio: Improve UMA usage
    
    - Remove the AIO proc zone.  This zone gets one allocation per AIO
      daemon process, which isn't enough to warrant a dedicated zone.  Plus,
      unlike other AIO structures, aiops are small (32 bytes with LP64), so
      UMA doesn't provide better space efficiency than malloc(9).  Change
      one of the malloc types in vfs_aio.c to make it more general.
    
    - Don't set the NOFREE flag on the other AIO zones.  This flag means
      that memory allocated to the AIO subsystem is never freed back to the
      VM, so it's always preferable to avoid using it when possible.  NOFREE
      was set without explanation when AIO was converted to use UMA 20 years
      ago, but it does not appear to be required; all of the structures
      allocated from UMA (per-process kaioinfo, kaiocb, and aioliojob) keep
      track of references and get freed only when none exist.  Plus, these
      structures will contain dangling pointer after they're freed (e.g.,
      the "cred", "fd_file" and "uiop" fields of struct kaiocb), so
      use-after-frees are dangerous even when the structures themselves are
      type-stable.
    
    Reviewed by:    asomers
    Sponsored by:   The FreeBSD Foundation
    
    (cherry picked from commit 9553bc89dbc38a861a750ddc605cec0381788c5d)
---
 sys/kern/vfs_aio.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/sys/kern/vfs_aio.c b/sys/kern/vfs_aio.c
index 32a31ec80cfb..90c49d4818b3 100644
--- a/sys/kern/vfs_aio.c
+++ b/sys/kern/vfs_aio.c
@@ -104,7 +104,7 @@ FEATURE(aio, "Asynchronous I/O");
 SYSCTL_DECL(_p1003_1b);
 
 static MALLOC_DEFINE(M_LIO, "lio", "listio aio control block list");
-static MALLOC_DEFINE(M_AIOS, "aios", "aio_suspend aio control block list");
+static MALLOC_DEFINE(M_AIO, "aio", "structures for asynchronous I/O");
 
 static SYSCTL_NODE(_vfs, OID_AUTO, aio, CTLFLAG_RW | CTLFLAG_MPSAFE, 0,
     "Async IO management");
@@ -339,11 +339,10 @@ static int	filt_lio(struct knote *kn, long hint);
 /*
  * Zones for:
  * 	kaio	Per process async io info
- *	aiop	async io process data
  *	aiocb	async io jobs
  *	aiolio	list io jobs
  */
-static uma_zone_t kaio_zone, aiop_zone, aiocb_zone, aiolio_zone;
+static uma_zone_t kaio_zone, aiocb_zone, aiolio_zone;
 
 /* kqueue filters for aio */
 static struct filterops aio_filtops = {
@@ -412,13 +411,11 @@ aio_onceonly(void)
 	TAILQ_INIT(&aio_jobs);
 	aiod_unr = new_unrhdr(1, INT_MAX, NULL);
 	kaio_zone = uma_zcreate("AIO", sizeof(struct kaioinfo), NULL, NULL,
-	    NULL, NULL, UMA_ALIGN_PTR, UMA_ZONE_NOFREE);
-	aiop_zone = uma_zcreate("AIOP", sizeof(struct aioproc), NULL,
-	    NULL, NULL, NULL, UMA_ALIGN_PTR, UMA_ZONE_NOFREE);
+	    NULL, NULL, UMA_ALIGN_PTR, 0);
 	aiocb_zone = uma_zcreate("AIOCB", sizeof(struct kaiocb), NULL, NULL,
-	    NULL, NULL, UMA_ALIGN_PTR, UMA_ZONE_NOFREE);
+	    NULL, NULL, UMA_ALIGN_PTR, 0);
 	aiolio_zone = uma_zcreate("AIOLIO", sizeof(struct aioliojob), NULL,
-	    NULL, NULL, NULL, UMA_ALIGN_PTR, UMA_ZONE_NOFREE);
+	    NULL, NULL, NULL, UMA_ALIGN_PTR, 0);
 	aiod_lifetime = AIOD_LIFETIME_DEFAULT;
 	jobrefid = 1;
 	p31b_setcfg(CTL_P1003_1B_ASYNCHRONOUS_IO, _POSIX_ASYNCHRONOUS_IO);
@@ -1093,7 +1090,7 @@ aio_daemon(void *_id)
 	 * Allocate and ready the aio control info.  There is one aiop structure
 	 * per daemon.
 	 */
-	aiop = uma_zalloc(aiop_zone, M_WAITOK);
+	aiop = malloc(sizeof(*aiop), M_AIO, M_WAITOK);
 	aiop->aioproc = p;
 	aiop->aioprocflags = 0;
 
@@ -1159,7 +1156,7 @@ aio_daemon(void *_id)
 	TAILQ_REMOVE(&aio_freeproc, aiop, list);
 	num_aio_procs--;
 	mtx_unlock(&aio_job_mtx);
-	uma_zfree(aiop_zone, aiop);
+	free(aiop, M_AIO);
 	free_unr(aiod_unr, id);
 	vmspace_free(myvm);
 
@@ -2032,11 +2029,11 @@ sys_aio_suspend(struct thread *td, struct aio_suspend_args *uap)
 	} else
 		tsp = NULL;
 
-	ujoblist = malloc(uap->nent * sizeof(ujoblist[0]), M_AIOS, M_WAITOK);
+	ujoblist = malloc(uap->nent * sizeof(ujoblist[0]), M_AIO, M_WAITOK);
 	error = copyin(uap->aiocbp, ujoblist, uap->nent * sizeof(ujoblist[0]));
 	if (error == 0)
 		error = kern_aio_suspend(td, uap->nent, ujoblist, tsp);
-	free(ujoblist, M_AIOS);
+	free(ujoblist, M_AIO);
 	return (error);
 }
 
@@ -2954,7 +2951,7 @@ freebsd32_aio_suspend(struct thread *td, struct freebsd32_aio_suspend_args *uap)
 	} else
 		tsp = NULL;
 
-	ujoblist = malloc(uap->nent * sizeof(ujoblist[0]), M_AIOS, M_WAITOK);
+	ujoblist = malloc(uap->nent * sizeof(ujoblist[0]), M_AIO, M_WAITOK);
 	ujoblist32 = (uint32_t *)ujoblist;
 	error = copyin(uap->aiocbp, ujoblist32, uap->nent *
 	    sizeof(ujoblist32[0]));
@@ -2964,7 +2961,7 @@ freebsd32_aio_suspend(struct thread *td, struct freebsd32_aio_suspend_args *uap)
 
 		error = kern_aio_suspend(td, uap->nent, ujoblist, tsp);
 	}
-	free(ujoblist, M_AIOS);
+	free(ujoblist, M_AIO);
 	return (error);
 }
 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202207201402.26KE2Dp5010035>