Changeset View
Standalone View
sys/kern/vfs_aio.c
Show First 20 Lines • Show All 562 Lines • ▼ Show 20 Lines | aio_free_entry(struct kaiocb *job) | ||||
if (job->uiop != &job->uio) | if (job->uiop != &job->uio) | ||||
free(job->uiop, M_IOV); | free(job->uiop, M_IOV); | ||||
uma_zfree(aiocb_zone, job); | uma_zfree(aiocb_zone, job); | ||||
AIO_LOCK(ki); | AIO_LOCK(ki); | ||||
return (0); | return (0); | ||||
} | } | ||||
void | |||||
aio_thread_exit(struct thread *td) | |||||
{ | |||||
struct kaioinfo *ki; | |||||
struct proc *p; | |||||
/* | |||||
* Common case. It's OK if we see a slightly out of date version as | |||||
* we'll recheck under lock if it's non-zero. | |||||
*/ | |||||
if (atomic_load_int(&td->td_aio_count) == 0) | |||||
asomers: This is racy unless you use acquire/release semantics. You should use `atomic_load_acq_int`… | |||||
return; | |||||
p = td->td_proc; | |||||
ki = p->p_aioinfo; | |||||
/* If we've ever submitted an IO, p_aioinfo must be set. */ | |||||
MPASS(ki != NULL); | |||||
/* | |||||
* Wait until there are no unfinished jobs that were submitted by this | |||||
* thread. | |||||
*/ | |||||
AIO_LOCK(ki); | |||||
while (atomic_load_int(&td->td_aio_count) > 0) { | |||||
asomersUnsubmitted Not Done Inline ActionsIf unsafe AIO is enabled, then this loop could block indefinitely. That's not very good. Is there a way to allow a thread to exit even in this case? What about if it it nulls out the td field for every active job of this thread, and then aio_complete checks whether that field is null before adjusting its counters? For that matter, AIO's resource usage isn't really associated with any one thread. The I/O itself doesn't happen in a thread's context. So it would be reasonable to account for it not in the submitting thread's td_ru structure but in the process's p_ru structure. asomers: If unsafe AIO is enabled, then this loop could block indefinitely. That's not very good. Is… | |||||
ki->kaio_flags |= KAIO_WAKEUP; | |||||
msleep(&p->p_aioinfo, AIO_MTX(ki), PRIBIO, "aiotde", hz); | |||||
} | |||||
AIO_UNLOCK(ki); | |||||
} | |||||
static void | static void | ||||
aio_proc_rundown_exec(void *arg, struct proc *p, | aio_proc_rundown_exec(void *arg, struct proc *p, | ||||
struct image_params *imgp __unused) | struct image_params *imgp __unused) | ||||
{ | { | ||||
aio_proc_rundown(arg, p); | aio_proc_rundown(arg, p); | ||||
} | } | ||||
static int | static int | ||||
▲ Show 20 Lines • Show All 354 Lines • ▼ Show 20 Lines | TAILQ_FOREACH_SAFE(sjob, &ki->kaio_syncqueue, list, sjobn) { | ||||
continue; | continue; | ||||
TAILQ_INSERT_TAIL(&ki->kaio_syncready, sjob, list); | TAILQ_INSERT_TAIL(&ki->kaio_syncready, sjob, list); | ||||
schedule_fsync = true; | schedule_fsync = true; | ||||
} | } | ||||
if (schedule_fsync) | if (schedule_fsync) | ||||
taskqueue_enqueue(taskqueue_aiod_kick, | taskqueue_enqueue(taskqueue_aiod_kick, | ||||
&ki->kaio_sync_task); | &ki->kaio_sync_task); | ||||
} | } | ||||
/* | |||||
* Drop our reference to the submitting thread. This allows it to | |||||
* exit. | |||||
*/ | |||||
atomic_subtract_int(&job->td->td_aio_count, 1); | |||||
job->td = NULL; | |||||
if (ki->kaio_flags & KAIO_WAKEUP) { | if (ki->kaio_flags & KAIO_WAKEUP) { | ||||
ki->kaio_flags &= ~KAIO_WAKEUP; | ki->kaio_flags &= ~KAIO_WAKEUP; | ||||
wakeup(&userp->p_aioinfo); | wakeup(&userp->p_aioinfo); | ||||
} | } | ||||
} | } | ||||
static void | static void | ||||
aio_schedule_fsync(void *context, int pending) | aio_schedule_fsync(void *context, int pending) | ||||
▲ Show 20 Lines • Show All 81 Lines • ▼ Show 20 Lines | |||||
aio_complete(struct kaiocb *job, long status, int error) | aio_complete(struct kaiocb *job, long status, int error) | ||||
{ | { | ||||
struct kaioinfo *ki; | struct kaioinfo *ki; | ||||
struct proc *userp; | struct proc *userp; | ||||
job->uaiocb._aiocb_private.error = error; | job->uaiocb._aiocb_private.error = error; | ||||
job->uaiocb._aiocb_private.status = status; | job->uaiocb._aiocb_private.status = status; | ||||
/* | |||||
* Transfer the resource usage delta to the submitting thread's | |||||
* counters. | |||||
*/ | |||||
if (job->outblock) | |||||
asomersUnsubmitted Not Done Inline ActionsIs it safe to acquire job->td's thread lock here? If so you could eliminate the atomic ops, and you could also eliminate the wait for all I/O to complete in aio_thread_exit. asomers: Is it safe to acquire `job->td`'s thread lock here? If so you could eliminate the atomic ops… | |||||
RU_ATOMIC_ADD(job->td->td_ru.ru_oublock, job->outblock); | |||||
if (job->inblock) | |||||
RU_ATOMIC_ADD(job->td->td_ru.ru_inblock, job->inblock); | |||||
if (job->msgsnd) | |||||
RU_ATOMIC_ADD(job->td->td_ru.ru_msgsnd, job->msgsnd); | |||||
if (job->msgrcv) | |||||
RU_ATOMIC_ADD(job->td->td_ru.ru_msgrcv, job->msgrcv); | |||||
userp = job->userproc; | userp = job->userproc; | ||||
ki = userp->p_aioinfo; | ki = userp->p_aioinfo; | ||||
AIO_LOCK(ki); | AIO_LOCK(ki); | ||||
KASSERT(!(job->jobflags & KAIOCB_FINISHED), | KASSERT(!(job->jobflags & KAIOCB_FINISHED), | ||||
("duplicate aio_complete")); | ("duplicate aio_complete")); | ||||
job->jobflags |= KAIOCB_FINISHED; | job->jobflags |= KAIOCB_FINISHED; | ||||
if ((job->jobflags & (KAIOCB_QUEUEING | KAIOCB_CANCELLING)) == 0) { | if ((job->jobflags & (KAIOCB_QUEUEING | KAIOCB_CANCELLING)) == 0) { | ||||
▲ Show 20 Lines • Show All 652 Lines • ▼ Show 20 Lines | case LIO_READ: | ||||
break; | break; | ||||
case LIO_WRITE: | case LIO_WRITE: | ||||
job->uiop->uio_rw = UIO_WRITE; | job->uiop->uio_rw = UIO_WRITE; | ||||
break; | break; | ||||
} | } | ||||
job->uiop->uio_offset = job->uaiocb.aio_offset; | job->uiop->uio_offset = job->uaiocb.aio_offset; | ||||
job->uiop->uio_td = td; | job->uiop->uio_td = td; | ||||
/* | |||||
* Take a reference to the submitting thread, so that worker daemons | |||||
* can update this thread's resource usage counters. | |||||
*/ | |||||
job->td = td; | |||||
atomic_add_int(&td->td_aio_count, 1); | |||||
if (opcode == LIO_MLOCK) { | if (opcode == LIO_MLOCK) { | ||||
aio_schedule(job, aio_process_mlock); | aio_schedule(job, aio_process_mlock); | ||||
error = 0; | error = 0; | ||||
} else if (fp->f_ops->fo_aio_queue == NULL) | } else if (fp->f_ops->fo_aio_queue == NULL) | ||||
error = aio_queue_file(fp, job); | error = aio_queue_file(fp, job); | ||||
else | else | ||||
error = fo_aio_queue(fp, job); | error = fo_aio_queue(fp, job); | ||||
if (error) | if (error) | ||||
▲ Show 20 Lines • Show All 214 Lines • ▼ Show 20 Lines | TAILQ_FOREACH(job, &ki->kaio_done, plist) { | ||||
if (job->ujob == ujob) | if (job->ujob == ujob) | ||||
break; | break; | ||||
} | } | ||||
if (job != NULL) { | if (job != NULL) { | ||||
MPASS(job->jobflags & KAIOCB_FINISHED); | MPASS(job->jobflags & KAIOCB_FINISHED); | ||||
status = job->uaiocb._aiocb_private.status; | status = job->uaiocb._aiocb_private.status; | ||||
error = job->uaiocb._aiocb_private.error; | error = job->uaiocb._aiocb_private.error; | ||||
td->td_retval[0] = status; | td->td_retval[0] = status; | ||||
td->td_ru.ru_oublock += job->outblock; | |||||
td->td_ru.ru_inblock += job->inblock; | |||||
td->td_ru.ru_msgsnd += job->msgsnd; | |||||
td->td_ru.ru_msgrcv += job->msgrcv; | |||||
aio_free_entry(job); | aio_free_entry(job); | ||||
AIO_UNLOCK(ki); | AIO_UNLOCK(ki); | ||||
ops->store_error(ujob, error); | ops->store_error(ujob, error); | ||||
ops->store_status(ujob, status); | ops->store_status(ujob, status); | ||||
} else { | } else { | ||||
error = EINVAL; | error = EINVAL; | ||||
AIO_UNLOCK(ki); | AIO_UNLOCK(ki); | ||||
} | } | ||||
▲ Show 20 Lines • Show All 611 Lines • ▼ Show 20 Lines | kern_aio_waitcomplete(struct thread *td, struct aiocb **ujobp, | ||||
} | } | ||||
if (job != NULL) { | if (job != NULL) { | ||||
MPASS(job->jobflags & KAIOCB_FINISHED); | MPASS(job->jobflags & KAIOCB_FINISHED); | ||||
ujob = job->ujob; | ujob = job->ujob; | ||||
status = job->uaiocb._aiocb_private.status; | status = job->uaiocb._aiocb_private.status; | ||||
error = job->uaiocb._aiocb_private.error; | error = job->uaiocb._aiocb_private.error; | ||||
td->td_retval[0] = status; | td->td_retval[0] = status; | ||||
td->td_ru.ru_oublock += job->outblock; | |||||
td->td_ru.ru_inblock += job->inblock; | |||||
td->td_ru.ru_msgsnd += job->msgsnd; | |||||
td->td_ru.ru_msgrcv += job->msgrcv; | |||||
aio_free_entry(job); | aio_free_entry(job); | ||||
AIO_UNLOCK(ki); | AIO_UNLOCK(ki); | ||||
ops->store_aiocb(ujobp, ujob); | ops->store_aiocb(ujobp, ujob); | ||||
ops->store_error(ujob, error); | ops->store_error(ujob, error); | ||||
ops->store_status(ujob, status); | ops->store_status(ujob, status); | ||||
} else | } else | ||||
AIO_UNLOCK(ki); | AIO_UNLOCK(ki); | ||||
▲ Show 20 Lines • Show All 589 Lines • Show Last 20 Lines |
This is racy unless you use acquire/release semantics. You should use atomic_load_acq_int here and below and everywhere this variable is modified must use atomic_store_rel_int or similar.