Changeset View
Changeset View
Standalone View
Standalone View
sys/kern/vfs_aio.c
Context not available. | |||||
/* | /* | ||||
* Move all data to a permanent storage device. This code | * Move all data to a permanent storage device. This code | ||||
* simulates the fsync syscall. | * simulates the fsync and fdatasync syscalls. | ||||
*/ | */ | ||||
static int | static int | ||||
aio_fsync_vnode(struct thread *td, struct vnode *vp) | aio_fsync_vnode(struct thread *td, struct vnode *vp, int op) | ||||
{ | { | ||||
struct mount *mp; | struct mount *mp; | ||||
int error; | int error; | ||||
Context not available. | |||||
vm_object_page_clean(vp->v_object, 0, 0, 0); | vm_object_page_clean(vp->v_object, 0, 0, 0); | ||||
VM_OBJECT_WUNLOCK(vp->v_object); | VM_OBJECT_WUNLOCK(vp->v_object); | ||||
} | } | ||||
error = VOP_FSYNC(vp, MNT_WAIT, td); | if (op == LIO_DSYNC) | ||||
error = VOP_FDATASYNC(vp, td); | |||||
else | |||||
error = VOP_FSYNC(vp, MNT_WAIT, td); | |||||
VOP_UNLOCK(vp); | VOP_UNLOCK(vp); | ||||
vn_finished_write(mp); | vn_finished_write(mp); | ||||
Context not available. | |||||
struct file *fp = job->fd_file; | struct file *fp = job->fd_file; | ||||
int error = 0; | int error = 0; | ||||
KASSERT(job->uaiocb.aio_lio_opcode == LIO_SYNC, | KASSERT(job->uaiocb.aio_lio_opcode == LIO_SYNC || | ||||
job->uaiocb.aio_lio_opcode == LIO_DSYNC, | |||||
("%s: opcode %d", __func__, job->uaiocb.aio_lio_opcode)); | ("%s: opcode %d", __func__, job->uaiocb.aio_lio_opcode)); | ||||
kib: This line should have 4-spaces indent. | |||||
td->td_ucred = job->cred; | td->td_ucred = job->cred; | ||||
if (fp->f_vnode != NULL) | if (fp->f_vnode != NULL) { | ||||
error = aio_fsync_vnode(td, fp->f_vnode); | error = aio_fsync_vnode(td, fp->f_vnode, | ||||
job->uaiocb.aio_lio_opcode); | |||||
} | |||||
td->td_ucred = td_savedcred; | td->td_ucred = td_savedcred; | ||||
Done Inline ActionsYou could just pass the opcode through. It might be slightly easier to read to do the 'opcode == LIO_SYNC' comparison in aio_fsync_vnode() directly rather than here in the caller, but that's really minor. jhb: You could just pass the opcode through. It might be slightly easier to read to do the 'opcode… | |||||
if (error) | if (error) | ||||
aio_complete(job, -1, error); | aio_complete(job, -1, error); | ||||
Context not available. | |||||
error = fget_read(td, fd, &cap_pread_rights, &fp); | error = fget_read(td, fd, &cap_pread_rights, &fp); | ||||
break; | break; | ||||
case LIO_SYNC: | case LIO_SYNC: | ||||
case LIO_DSYNC: | |||||
error = fget(td, fd, &cap_fsync_rights, &fp); | error = fget(td, fd, &cap_fsync_rights, &fp); | ||||
break; | break; | ||||
case LIO_MLOCK: | case LIO_MLOCK: | ||||
Context not available. | |||||
if (error) | if (error) | ||||
goto err3; | goto err3; | ||||
if (opcode == LIO_SYNC && fp->f_vnode == NULL) { | if ((opcode == LIO_SYNC || opcode == LIO_DSYNC) && fp->f_vnode == NULL) { | ||||
error = EINVAL; | error = EINVAL; | ||||
goto err3; | goto err3; | ||||
} | } | ||||
Done Inline ActionsA few lines below here is a check job2->uaiocb.aio_lio_opcode != LIO_SYNC. Why didn't you modify that to check for LIO_DSYNC too? Pro tip: if you generate the diff with svn diff -D9999 Phabricator will intelligently show more context. Or, install arcanist-php72 and create the review with arc diff --create. asomers: A few lines below here is a check `job2->uaiocb.aio_lio_opcode != LIO_SYNC`. Why didn't you… | |||||
Done Inline ActionsYeah, without that check it would set the KAIOCB_CHECKSYNC flag on other LIO_DSYNC requests queued earlier, which wasn't intentional. Fixed. tmunro: Yeah, without that check it would set the KAIOCB_CHECKSYNC flag on other LIO_DSYNC requests… | |||||
Context not available. | |||||
error = 0; | error = 0; | ||||
break; | break; | ||||
case LIO_SYNC: | case LIO_SYNC: | ||||
case LIO_DSYNC: | |||||
AIO_LOCK(ki); | AIO_LOCK(ki); | ||||
TAILQ_FOREACH(job2, &ki->kaio_jobqueue, plist) { | TAILQ_FOREACH(job2, &ki->kaio_jobqueue, plist) { | ||||
if (job2->fd_file == job->fd_file && | if (job2->fd_file == job->fd_file && | ||||
job2->uaiocb.aio_lio_opcode != LIO_SYNC && | job2->uaiocb.aio_lio_opcode != LIO_SYNC && | ||||
job2->uaiocb.aio_lio_opcode != LIO_DSYNC && | |||||
job2->seqno < job->seqno) { | job2->seqno < job->seqno) { | ||||
job2->jobflags |= KAIOCB_CHECKSYNC; | job2->jobflags |= KAIOCB_CHECKSYNC; | ||||
job->pending++; | job->pending++; | ||||
Done Inline ActionsI would perhaps consolidate the checks: switch (op) { case O_SYNC: listop = LIO_SYNC; break; case O_DYSNC: listop = LIO_DSYNC; break; default: return (EINVAL); } jhb: I would perhaps consolidate the checks:
```
switch (op) {
case O_SYNC:
listop… | |||||
Context not available. | |||||
kern_aio_fsync(struct thread *td, int op, struct aiocb *ujob, | kern_aio_fsync(struct thread *td, int op, struct aiocb *ujob, | ||||
struct aiocb_ops *ops) | struct aiocb_ops *ops) | ||||
{ | { | ||||
int listop; | |||||
if (op != O_SYNC) /* XXX lack of O_DSYNC */ | switch (op) { | ||||
case O_SYNC: | |||||
listop = LIO_SYNC; | |||||
break; | |||||
case O_DSYNC: | |||||
listop = LIO_DSYNC; | |||||
break; | |||||
default: | |||||
return (EINVAL); | return (EINVAL); | ||||
return (aio_aqueue(td, ujob, NULL, LIO_SYNC, ops)); | } | ||||
return (aio_aqueue(td, ujob, NULL, listop, ops)); | |||||
} | } | ||||
int | int | ||||
Context not available. |
This line should have 4-spaces indent.