Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 15 Feb 2023 21:33:14 GMT
From:      John Baldwin <jhb@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 98844e99d40a - main - aio: Fix more synchronization issues in aio_biowakeup.
Message-ID:  <202302152133.31FLXEep021521@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by jhb:

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

commit 98844e99d40a90ae89d84762e07150af3a8f89bd
Author:     John Baldwin <jhb@FreeBSD.org>
AuthorDate: 2023-02-15 21:32:52 +0000
Commit:     John Baldwin <jhb@FreeBSD.org>
CommitDate: 2023-02-15 21:32:52 +0000

    aio: Fix more synchronization issues in aio_biowakeup.
    
    - Use atomic_store to set job->error.  atomic_set does an or
      operation, not assignment.
    
    - Use refcount_* to manage job->nbio.
    
      This ensures proper memory barriers are present so that the last bio
      won't see a possibly stale value of job->error.
    
    - Don't re-read job->error after reading it via atomic_load.
    
    Reported by:    markj (1)
    Reviewed by:    mjg, markj
    Differential Revision:  https://reviews.freebsd.org/D38611
---
 sys/kern/vfs_aio.c | 11 ++++++-----
 sys/sys/aio.h      |  2 +-
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/sys/kern/vfs_aio.c b/sys/kern/vfs_aio.c
index 9868b8770747..d32e6fea15ca 100644
--- a/sys/kern/vfs_aio.c
+++ b/sys/kern/vfs_aio.c
@@ -1281,7 +1281,7 @@ aio_qbio(struct proc *p, struct kaiocb *job)
 	}
 
 	bios = malloc(sizeof(struct bio *) * iovcnt, M_TEMP, M_WAITOK);
-	atomic_store_int(&job->nbio, iovcnt);
+	refcount_init(&job->nbio, iovcnt);
 	for (i = 0; i < iovcnt; i++) {
 		struct vm_page** pages;
 		struct bio *bp;
@@ -2489,15 +2489,16 @@ aio_biowakeup(struct bio *bp)
 	 * error of whichever failed bio completed last.
 	 */
 	if (flags & BIO_ERROR)
-		atomic_set_int(&job->error, bio_error);
+		atomic_store_int(&job->error, bio_error);
 	if (opcode & LIO_WRITE)
 		atomic_add_int(&job->outblock, nblks);
 	else
 		atomic_add_int(&job->inblock, nblks);
 
-	if (atomic_fetchadd_int(&job->nbio, -1) == 1) {
-		if (atomic_load_int(&job->error))
-			aio_complete(job, -1, job->error);
+	if (refcount_release(&job->nbio)) {
+		bio_error = atomic_load_int(&job->error);
+		if (bio_error != 0)
+			aio_complete(job, -1, bio_error);
 		else
 			aio_complete(job, atomic_load_long(&job->nbytes), 0);
 	}
diff --git a/sys/sys/aio.h b/sys/sys/aio.h
index 7b931d04febc..272132acf91e 100644
--- a/sys/sys/aio.h
+++ b/sys/sys/aio.h
@@ -151,7 +151,7 @@ struct kaiocb {
 	aio_handle_fn_t *handle_fn;	/* (c) backend handle function */
 	union {				/* Backend-specific data fields */
 		struct {		/* BIO backend */
-			int	nbio;	/* Number of remaining bios */
+			volatile u_int nbio; /* Number of remaining bios */
 			int	error;	/* Worst error of all bios */
 			long	nbytes;	/* Bytes completed so far */
 		};



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